Skip to content
Snippets Groups Projects

Corpus upload modal with graphQL and upload progress

Merged - requested to merge 342433-corpus-upload-progress into master
All threads resolved!

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:

#342583 (closed)

#342581 (closed)

#342580 (closed)

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

Screen_Shot_2021-11-06_at_5.39.30_PMScreen_Shot_2021-11-06_at_5.41.23_PMScreen_Shot_2021-11-06_at_5.41.30_PMScreen_Shot_2021-11-06_at_5.41.35_PM

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.

Related to #342433 (closed)

Edited by -

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • - changed milestone to %14.5

    changed milestone to %14.5

  • - assigned to @farias-gl

    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:

    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 🚫 Danger

  • Bundle size analysis [beta]

    This compares changes in bundle size for entry points between the commits f526246b and c9468212

    Special assets

    Entrypoint / 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: 12

    Expand
    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: 3

    Expand
    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 the bundle-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 🚫 Danger

  • - changed title from Change corpus upload modal to use prop instead of graphQL to Corpus upload modal with graphQL and upload progress

    changed title from Change corpus upload modal to use prop instead of graphQL to Corpus upload modal with graphQL and upload progress

  • Allure report

    allure-report-publisher generated test report for c9468212!

    review-qa-smoke: 📝 test report

  • - added 1115 commits

    added 1115 commits

    Compare with previous version

  • - added 1 commit

    added 1 commit

    • 6a78972e - Implement corpus upload functionality

    Compare with previous version

  • - added 187 commits

    added 187 commits

    Compare with previous version

  • - changed the description

    changed the description

  • - changed the description

    changed the description

  • - requested review from @pburdette

    requested review from @pburdette

  • -
  • removed workflowin dev label

  • Payton Burdette
  • Payton Burdette
  • Payton Burdette
  • Payton Burdette removed review request for @pburdette

    removed review request for @pburdette

  • - added 1103 commits

    added 1103 commits

    Compare with previous version

  • - requested review from @pburdette

    requested review from @pburdette

  • Author Contributor

    @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 -
  • Payton Burdette approved this merge request

    approved this merge request

  • 👋 @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:

  • Payton Burdette requested review from @ealcantara and removed review request for @pburdette

    requested review from @ealcantara and removed review request for @pburdette

  • - requested review from @peterhegman

    requested review from @peterhegman

  • - removed review request for @ealcantara

    removed review request for @ealcantara

  • Peter Hegman removed review request for @peterhegman

    removed review request for @peterhegman

  • removed workflowin review label

  • - added 861 commits

    added 861 commits

    Compare with previous version

  • - mentioned in issue #345985

    mentioned in issue #345985

  • - requested review from @peterhegman

    requested review from @peterhegman

  • - requested review from @mfangman

    requested review from @mfangman

  • Natalia Tepluhina
  • Peter Hegman changed milestone to %14.6

    changed milestone to %14.6

  • Peter Hegman approved this merge request

    approved this merge request

  • Peter Hegman resolved all threads

    resolved all threads

  • Peter Hegman enabled an automatic merge when the pipeline for 9bc166eb succeeds

    enabled an automatic merge when the pipeline for 9bc166eb succeeds

  • added UX label

  • Michael Fangman approved this merge request

    approved this merge request

  • Michael Fangman removed review request for @mfangman

    removed review request for @mfangman

  • merged

  • Peter Hegman mentioned in commit 97c1d0d8

    mentioned in commit 97c1d0d8

  • added workflowcanary label and removed workflowstaging label

  • added workflowproduction label and removed workflowcanary label

  • - mentioned in merge request !73893 (merged)

    mentioned in merge request !73893 (merged)

  • Please register or sign in to reply
    Loading