Corpus upload modal with graphQL and upload progress
What does this MR do and why?
This MR is a happy path MR to get part 1 of 2 of corpus upload working. It does not handle all edge cases and error validations.
Follow up issues after this MR:
Notes for reviewer.
Clicking the add
button is not covered in the scope of this MR. That is the second part of the 2 part upload. part 1 (this MR) is the upload. The second part is "commit/confirm" the upload which is part of #342580 (closed)
This MR is behind a feature flag, and there are about 4 more issues after this one before we release next milestone, so this MR has happy path only.
Describe in detail what your merge request does and why.
Screenshots or screen recordings
How to set up and validate locally
- From the gitlab repo open a rails console and enable the :corpus_management feature flag
bundle exec rails c
then
[4] pry(main)> Feature.enable(:corpus_management)
Navigate to:
http://172.16.123.1:3000/<group>/<project>/-/security/configuration/corpus_management
Click on New Corpus
, upload any zip
file of a large size. 500 MB > (or in chrome debugger, throttle network traffic so we can see upload progress)
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Related to #342433 (closed)
Merge request reports
Activity
changed milestone to %14.5
assigned to @farias-gl
2 Warnings ⚠ 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.
⚠ This merge request changed files with disabled eslint rules. Please consider fixing them. 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.
Disabled eslint rules
The following files have disabled
eslint
rules. Please consider fixing them:ee/app/assets/javascripts/security_configuration/corpus_management/graphql/resolvers/resolvers.js
Run the following command for more details
node_modules/.bin/eslint --report-unused-disable-directives --no-inline-config \ 'ee/app/assets/javascripts/security_configuration/corpus_management/graphql/resolvers/resolvers.js'
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 frontend Justin Ho ( @justin_ho
) (UTC+7, 13 hours ahead of@farias-gl
)Phil Hughes ( @iamphill
) (UTC+0, 6 hours ahead of@farias-gl
)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 f526246b and c9468212
✨ Special assetsEntrypoint / Name Size before Size after Diff Diff in percent average 3.58 MB 3.58 MB -5.16 KB -0.1 % mainChunk 2.27 MB 2.26 MB -4.51 KB -0.2 % 😨 Significant Growth: 12Expand
Entrypoint / Name Size before Size after Diff Diff in percent pages.ldap.omniauth_callbacks 125.66 KB 128.35 KB +2.69 KB 2.1 % pages.omniauth_callbacks 125.66 KB 128.35 KB +2.69 KB 2.1 % pages.sessions 125.69 KB 128.38 KB +2.69 KB 2.1 % pages.trials 125.69 KB 128.38 KB +2.69 KB 2.1 % pages.trials.apply 126.61 KB 129.3 KB +2.69 KB 2.1 % pages.trials.create_lead 127.12 KB 129.81 KB +2.69 KB 2.1 % pages.trials.new 126.59 KB 129.28 KB +2.69 KB 2.1 % pages.trials.select 126.08 KB 128.77 KB +2.69 KB 2.1 % pages.groups.wikis 90.78 KB 93.02 KB +2.24 KB 2.5 % pages.groups.wikis.diff 99.24 KB 101.49 KB +2.24 KB 2.3 % The table above is limited to 10 entries. Please look at the full report for more details
🎉 Significant Reduction: 3Expand
Entrypoint / Name Size before Size after Diff Diff in percent pages.projects.security.policies.index 1.87 MB 1.68 MB -190.78 KB -10.0 % pages.projects.security.policies.edit 1.63 MB 1.44 MB -189.11 KB -11.3 % pages.projects.security.policies.new 1.63 MB 1.44 MB -189.11 KB -11.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.Note: We do not have exact data for f526246b. So we have used data from: 7c455246.
The target commit was too new, so we used the latest commit from master we have info on.
It might help to rerun thebundle-size-review
job
This might mean that you have a few false positives in this report. If something unrelated to your code changes is reported, you can check this comparison in order to see if they caused this change.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 c9468212!review-qa-smoke:
📝 test reportadded 1115 commits
-
f650fcd0...c1953919 - 1112 commits from branch
master
- a74c0042 - Implement corpus upload functionality
- 93df6e96 - Cleanup
- 56221626 - Working cancel
Toggle commit list-
f650fcd0...c1953919 - 1112 commits from branch
added 187 commits
-
6a78972e...ae1b7af2 - 186 commits from branch
master
- 3f32d73d - Implement corpus upload functionality
-
6a78972e...ae1b7af2 - 186 commits from branch
- Resolved by Peter Hegman
@pburdette mind doing the review?
requested review from @pburdette
- Resolved by -
removed workflowin dev label
- Resolved by Payton Burdette
- Resolved by -
- Resolved by -
removed review request for @pburdette
added 1103 commits
-
3f32d73d...e71ca745 - 1101 commits from branch
master
- f996ff96 - Implement corpus upload functionality
- 54c18408 - Code review changes
-
3f32d73d...e71ca745 - 1101 commits from branch
requested review from @pburdette
@pburdette should be good for another review. If there is any more code feedback please don't hesitate to ping me on slack so I can address as soon as you leave the feedback.
Edited by -👋 @pburdette
, thanks for approving this merge request.This is the first time the merge request is approved. To ensure full test coverage, a new pipeline has been started.
For more info, please refer to the following links:
requested review from @ealcantara and removed review request for @pburdette
- Resolved by Peter Hegman
@peterhegman would you mind doing the maintainer review? I was looking the MR dashboard looks like Enrique has 4 MRs already in his queue.
requested review from @peterhegman
removed review request for @ealcantara
removed review request for @peterhegman
added workflowin review label
removed workflowin review label
added 861 commits
-
54c18408...37a81dd0 - 858 commits from branch
master
- 755489b5 - Implement corpus upload functionality
- 4978e8a8 - Code review changes
- c9468212 - Add API unit tests
Toggle commit list-
54c18408...37a81dd0 - 858 commits from branch
mentioned in issue #345985
requested review from @peterhegman
requested review from @mfangman
- Resolved by Michael Fangman
@mfangman for UX review. Please see screenshots in issue description.
- Resolved by Peter Hegman
changed milestone to %14.6
enabled an automatic merge when the pipeline for 9bc166eb succeeds
added UX label
removed review request for @mfangman
mentioned in commit 97c1d0d8
added workflowstaging-ref label
added workflowstaging-canary label and removed workflowstaging-ref label
added workflowstaging label and removed workflowstaging-canary label
added workflowcanary label and removed workflowstaging label
added workflowproduction label and removed workflowcanary label
mentioned in merge request !73893 (merged)
added releasedcandidate label
added releasedpublished label and removed releasedcandidate label