Add initial project storage ui
What does this MR do?
Relates to #334885, #334886, #334887
This MR adds the initial UI for the Project Storage, being it the headline (#334885) and table (#334886, #334887)
There were a lot of discussions in the initial design and in the epic, and some decisions were made that don't reflect what you see in the design, so I'll summarize them here
- In this first iteration, we will simplify the design to condense the storage by category into one bar and one table of data, because Repository Storage is just 1 item in the storage statistics and cannot be broken down into subitems as the design suggests
- GraphQL field reference: ProjectStatistics
- Since we have 1 table and 1 bar now, we haven't decided on what to put in the headline (title + subtitle + learn more link + total storage used). So in this implementation I just copied what we have in the
Group usage quotas
. This decision was based on the epic description which states "We should re-use the view as-is for the first iteration and then build on that to improve the storage view in both the group level and the project level." - Icons or not: "Let's keep it minimal since there isn't a clear direction yet and proceed without icons. We can add them as a follow-on iteration shortly after." https://gitlab.com/gitlab-org/gitlab/-/issues/321429/designs/initial.png#note_599268344
- Description for storage type: https://gitlab.com/groups/gitlab-org/-/epics/6264#note_615221560
- Where should the link for the storage type go: We haven't decided yet (https://gitlab.com/gitlab-org/gitlab/-/issues/321429/designs/initial.png#note_609925769) And we don't know that we have a "how to reduce the size" topic for each of these storage types (https://gitlab.com/groups/gitlab-org/-/epics/6264#note_615221549) "So in the design, I think these can probably just be question marks next to each storage type."
Screenshots or Screencasts (strongly suggested)
Group Usage Quotas (inspiration page) | Project Usage Quotas |
---|---|
![]() |
![]() |
How to setup and validate locally (strongly suggested)
- Enable
project_storage_ui
feature flag. Go torails console
and execute the following:Feature.enable(:project_storage_ui)
- Visit
/<namespace>/<project>/-/usage_quotas
on a project you havemaintainer
permission
Does this MR meet the acceptance criteria?
Conformity
-
I have included changelog trailers, or none are needed. (Does this MR need a changelog?) -
I have added/updated documentation, or it's not needed. (Is documentation required?) -
I have properly separated EE content from FOSS, or this MR is FOSS only. (Where should EE code go?) -
I have added information for database reviewers in the MR description, or it's not needed. (Does this MR have database related changes?) -
I have self-reviewed this MR per code review guidelines. -
This MR does not harm performance, or I have asked a reviewer to help assess the performance impact. (Merge request performance guidelines) -
I have followed the style guides. -
This change is backwards compatible across updates, or this does not apply.
Availability and Testing
-
I have added/updated tests following the Testing Guide, or it's not needed. (Consider all test levels. See the Test Planning Process.) -
I have tested this MR in all supported browsers, or it's not needed. -
I have informed the Infrastructure department of a default or new setting change per definition of done, or it's not needed.
Merge request reports
Activity
assigned to @sheldonled
- A deleted user
added frontend label
3 Warnings This merge request is quite big (781 lines changed), please consider splitting it into multiple merge requests. af4d71b5: Commits that change 30 or more lines across at least 3 files should describe these changes in the commit body. For more information, take a look at our Commit message guidelines. featureaddition and featureenhancement merge requests normally have a documentation change. Consider adding a documentation update or confirming the documentation plan with the Technical Writer counterpart.
For more information, see:
- The Handbook page on merge request types.
- The definition of done documentation.
1 Message CHANGELOG missing: If you want to create a changelog entry for GitLab FOSS, add the
Changelog
trailer to the commit message you want to add to the changelog.If you want to create a changelog entry for GitLab EE, also add the
EE: true
trailer to your commit message.If this merge request doesn't need a CHANGELOG entry, feel free to ignore this message.
Reviewer roulette
Changes that require review have been detected!
Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:
Category Reviewer Maintainer backend Ross Fuhrman ( @rossfuhrman
) (UTC-5, 6 hours behind@sheldonled
)Heinrich Lee Yu ( @engwan
) (UTC+8, 7 hours ahead of@sheldonled
)frontend Samantha Ming ( @sming-gitlab
) (UTC-7, 8 hours behind@sheldonled
)Vitaly Slobodin ( @vitallium
) (UTC+3, 2 hours ahead of@sheldonled
)To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.
To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.
Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.
If needed, you can retry the
danger-review
job that generated this comment.Generated by
DangerBundle size analysis [beta]
This compares changes in bundle size for entry points between the commits 2653948a and af4d71b5
Special assetsEntrypoint / Name Size before Size after Diff Diff in percent average 3.15 MB 3.15 MB -1.04 KB -0.0 % mainChunk 1.91 MB 1.91 MB - -0.0 % Significant Growth: 1Expand
Entrypoint / Name Size before Size after Diff Diff in percent pages.projects.usage_quotas 807.66 KB 947.93 KB +140.27 KB 17.4 % Significant Reduction: 3Expand
Entrypoint / Name Size before Size after Diff Diff in percent pages.projects.jobs.index 1.16 MB 1.02 MB -150.54 KB -12.6 % pages.groups.wikis 1.29 MB 1.24 MB -48.82 KB -3.7 % pages.projects.compare.show 267.59 KB 258.86 KB -8.73 KB -3.3 %
Your MR has at least one entrypoint growing significantly (more > 1 KB or 2%). If you write new or extend existing features, this is expected and there is nothing to worry about.
Please consider pinging someone from the FE Foundations (
@dmishunov
,@justin_ho
,@mikegreiling
or@nmezzopera
) for review, if you are unsure about the size increase.Please look at the full report for more details
Read more about how this report works.
Generated by
DangerAllure report
allure-report-publisher
generated test report for af4d71b5!review-qa-smoke:
test reportadded featureaddition typefeature labels
changed milestone to %14.3
added 110 commits
-
97d60343...8bf70117 - 108 commits from branch
led/334884-fetch-project-capacity-usage
- 84764d89 - Address initial suggestions from MR review
- 575478c2 - Add initial project storage UI
-
97d60343...8bf70117 - 108 commits from branch
added 126 commits
-
8b0be2fe...52dab4f6 - 125 commits from branch
led/334884-fetch-project-capacity-usage
- 26e98b18 - Add initial project storage UI
-
8b0be2fe...52dab4f6 - 125 commits from branch
- A deleted user
added backend label
removed backend label
marked the checklist item I have included changelog trailers, or none are needed. (Does this MR need a changelog?) as completed
marked the checklist item I have added/updated documentation, or it's not needed. (Is documentation required?) as completed
marked the checklist item I have properly separated EE content from FOSS, or this MR is FOSS only. (Where should EE code go?) as completed
marked the checklist item I have added information for database reviewers in the MR description, or it's not needed. (Does this MR have database related changes?) as completed
marked the checklist item I have self-reviewed this MR per code review guidelines. as completed
marked the checklist item This MR does not harm performance, or I have asked a reviewer to help assess the performance impact. (Merge request performance guidelines) as completed
marked the checklist item I have followed the style guides. as completed
marked the checklist item This change is backwards compatible across updates, or this does not apply. as completed
marked the checklist item I have tested this MR in all supported browsers, or it's not needed. as completed
marked the checklist item I have informed the Infrastructure department of a default or new setting change per definition of done, or it's not needed. as completed
- Resolved by Rémy Coutable
requested review from @vshumilo
requested review from @mlunoe
@mnearents can you take a look at this MR from a UX point of view? There were a few discussions and decisions don't match the initial design. Since you are the one assigned to the issue I thought of tagging you here
(I tried my best at summarizing the decisions on this MR description)
@sheldonled Thanks for pinging me. I have a couple comments/questions about the screenshots
- When I originally did the design, it was for 2 skus. Now that we have one, I see the usage breakdown, but it's not apparent whether I'm close to my overall storage quota.
- Saying "You used X MiB" and then showing a full bar below it looks like I've used all my storage if I am glancing at it. It's hard to tell it's a breakdown. It's missing the "Usage breakdown" title which separates this breakdown bar from the "You've used" section
- Do we really need two decimal precision?
- The original design you're referencing is the project-level storage overview. This was the group-level storage design: https://gitlab.com/gitlab-org/gitlab/-/issues/321430/designs/no_repo_over_limit_simplified.png In this design, you can see there is a column called "Repo usage". Now that there's one sku we won't have multiple columns, but I still liked the XX/XX GB to show that a project is nearing a quota. I'm not caught up on this effort enough to know where the quota is anymore (just got back from parental leave), but wherever it is (namespace or project level), we should show some indication of how near they are using the XX/XX GB format (if applicable).
- Does the MVC not include a purchase button? Looking through some discussion, I saw one here, but maybe I'm missing some comment somewhere that says it's not MVC.
- For the project-level screenshot, I have the same comment as above that the numbers don't really mean anything unless we can show how they affect my quotas. In the original design, we were able to show how much project-level repo storage was being used. Now, we might need something like "this project is using X% of your overall storage". Whether that's text, or a graph, or whatever. Might not be MVC, but something that came to my mind.
@mnearents Thanks for bringing these points up.
- The reason for having just 1 SKU is that the second one (Repository Storage) doesn't have a breakdown, so we can't build a table for it or it would have just 1 item. This was discovered and mentioned here.
- The title wasn't added because in the design the titles refer to Repository+Other Storage. I can add a generic "Usage Breakdown" title to this one
I didn't before because I was unsure on what to use as a title - 2 decimal precision is just the standard output of our code, I will look into having just 1 decimal precision
- The cards above the table illustrated in the link you mentioned were not part of this initial issue breakdown because the effort was based on this design. Maybe @amandarueda can help us decide on whether to include the Object Storage / Repository Storage cards here.
- The only conversation I found related to
purchase
button was here. Again, the design we based the development in doesn't include one, so we haven't developed one. This purchase button you saw is for Namespace Storage. We can add it here too, WDYT @amandarueda ?- The purchase button in Namespace storage is part of the Usage Bar Graph, this is being worked out by @aalakkad in !68866 (merged) So he'll be the best to know if we include the button as part of the same issue or create a new one (should we decide to include the button)
- Yeah, a "this project is using X% of your overall storage" piece of UI makes sense here.
An overall note, this MR relates to project storage only. The first screenshot is indeed from group storage, but this is because that UI was used for inspiration, since so many decisions and changes made along the discussions do not reflect what we see in the initial design, quoting from the epic:
We should re-use the view as-is for the first iteration and then build on that to improve the storage view in both the group level and the project level.
@amandarueda can you help us on what are the next steps here?
Thanks for the tag @mnearents @sheldonled before jumping in, I'd like to highlight that using numbered bullets is much more convenient and easier to reference than non-numbered bullets (see point 13 in writing guidelines.
- We are moving forward with 1 sku, this is irrespective of the difficulties mentioned here.
- We have some confusion here.
- The bar on the group page should reflect a total used (not a breakdown of consumption by type) as per the group design
- I don't think this needs a title but defer to @mnearents here
- The bar on the project page is the breakdown of consumption by type as per the project design
- This title should be "Usage breakdown" or "Usage breakout" - @mnearents do we have this language in the app somewhere else which we can mimic?
- The bar on the group page should reflect a total used (not a breakdown of consumption by type) as per the group design
- Yes, please change to 1 decimal.
- For now, we don't need cards on the project page. For the group page, we should not change the cards which are currently in production (image below)
- For now, let's only provide purchase buttons on group pages, I think the complication of displaying to appropriate users on project pages isn't worth the effort atm.
- @mnearents please create a follow-up UX design for this, we will address in a subsequent issue.
@sheldonled nice work, comments to the bullets above. In general, please opt to release in iterations and break this thing down as small as you can. Anything that is nice to have can be addressed in follow-on issues. LMK if there are any unanswered questions.
Product view of Group Usage Quotas Page
Thanks for the input @amandarueda I updated this MR accordingly and updated the screenshots as well
@mnearents Can you have another look at this so we can get the MR merged?
@amandarueda @mnearents I've rebased the changes and updated the screenshots to include the usage graph (added in previous MR).
@sheldonled @aalakkad @amandarueda The only thing I would change is moving the "Usage Breakdown" title and description above the graph. But, I don't care if we do it now or as a follow up, so I'll leave that all up to you all.
@sheldonled It's been a while since I've used rails console or gdk. Is there a way to get this working in the review app? So far I've had no luck because none of the projects show the usage quotas settings for me. Or GitPod. It's been over a year since GDK worked for me so I don't think I'll be able to get that going anytime soon. Or send me a video walkthrough so I can see it at desktop and mobile sizes?
@mnearents Here are 2 videos showing desktop then mobile sizes for both group's usage and project's usage, if that's not what you meant please let me know what should I record.
If you prefer to have a sync meeting we can schedule one.
Group's usage quota Project's usage quota (this MR) Screen_Recording_2021-09-09_at_8.41.12_AM Screen_Recording_2021-09-09_at_8.41.46_AM I'll ask the maintainer to move forward with this MR since there are no more changes to the MR changes, if there are any further enhancements (whether related to the mobile view or something else) we can do a follow-up before we roll out the feature flag to users (I'll be sure to ping you and Amanda before we roll out, so we can test it in staging).
/cc @amandarueda
@aalakkad perfect, thanks. LGTM!
@mnearents I feel like there should be a green circle by your name as Reviewer?
@amandarueda yeah I was looking for an option to "approve additionally" or whatever, for the UX department to track MR approvals, but don't see a way to do that. @jackib
@mvanremmerden Do you know why there isn't a green circle? Is it because @mnearents can't merge?
@jackib I'm pretty sure @mnearents is spot on with the fact that it would show up if he pushed "Approve additionally", but the reason why it's not there anymore is that the MR is already merged. Before it was merged, it should have looked like this:
We can test it out like this: @mnearents do you see the "Approve additionally" button in this MR?
@mvanremmerden If you're talking about this one that's already merged, I do not. If you meant to link to another MR, I don't see your link.
If you meant to link to another MR, I don't see your link. If you meant to link to another MR, I don't see your link.
I did mean another MR, but forgot to paste it
♂️ Here we go again: !69803 (closed)
requested review from @mnearents
- Resolved by Michael Lunøe
added 76 commits
-
26e98b18...5c89b2f0 - 75 commits from branch
led/334884-fetch-project-capacity-usage
- 6b36487e - Add initial project storage UI
-
26e98b18...5c89b2f0 - 75 commits from branch
- A deleted user
added backend label
marked the checklist item I have added/updated tests following the Testing Guide, or it's not needed. (Consider all test levels. See the Test Planning Process.) as completed
- Resolved by Sheldon Led
- Resolved by Ammar Alakkad
@sheldonled Taking a look at this for the initial review. Pardon the partial review here. Something screwed up with the comments, so they suddenly weren't visible to me anymore!
This generally seems good. I added a few points for you to consider.
Praise: Thank you for the thorough description and references to different decisions!
Suggestion (non-blocking): We probably shouldn't make this close #334885, #334886, and #334887, but rather just relate them. We can close them once they have been verified in staging (being behind a feature flag). Also, it seems that there is still some work to do with adding an asterisk/bold font/warning icon to the artifact row: https://gitlab.com/gitlab-org/gitlab/-/issues/334887#note_654360517.
Notice: You seem to have a test failing: https://gitlab.com/gitlab-org/gitlab/-/jobs/1536672558
Edited by Michael Lunøe
- Resolved by Ammar Alakkad
- Resolved by Michael Lunøe
- Resolved by Michael Lunøe
- Resolved by Michael Lunøe
- Resolved by Michael Lunøe