Add feature flag for CI variables drawer
What does this MR do and why?
For #410414 (closed)
The drawer will replace the modal form for creating, editing, and deleting CI Variables.
This MR is purely for the rendering the drawer under the feature flag. Interactions with the UI (such as validations and submitting the form) will be implemented in future iterations.
The MR is broken down into different commits for: 1) showing the drawer under a feature flag; 2) scaffolding the component with UI elements. The feature flag will be rolled out on a user actor basis because the CI variables setting exists on the instance-level, group-level, and project-level. The drawer/modal component is shared in all three contexts.
No changelog entry yet since this will be developed under the ci_variable_drawer
feature flag.
Implementation
Iteration | MR |
---|---|
Setup drawer with feature flag |
![]() |
Add form validations | ![]() |
Ensure that all mutations are working and update QA specs | ![]() |
Rollout | #418005 (closed) |
Screenshots or screen recordings
Before | After |
---|---|
![]() |
![]() |
How to set up and validate locally
Numbered steps to set up and validate the change are strongly suggested.
- Enable the
ci_variable_drawer
feature flag. - Go to Settings > CI/CD > Variables.
- Click on the Add variable button or edit a variable.
- The drawer should show up instead of the modal.
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.
Merge request reports
Activity
changed milestone to %16.2
assigned to @mgandres
- A deleted user
added backend feature flag labels
- Resolved by Mireya Andres
4 Warnings This merge request is quite big (527 lines changed), please consider splitting it into multiple merge requests. 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 contains lines with QA selectors. Please ensure e2e:package-and-test
job is run.This merge request adds a new rule to app/assets/stylesheets/framework/common.scss or app/assets/stylesheets/utilities.scss. 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.
QA Selectors
The following changed lines in this MR contain QA selectors:
- file
app/assets/javascripts/ci/ci_variable_list/components/ci_variable_drawer.vue
: -
-
data-qa-selector="ci_variable_key_field"
-
-
-
data-qa-selector="ci_variable_value_field"
-
Please ensure
e2e:package-and-test
job is run and the tests are passing.For the list of known failures please refer to the latest pipeline triage issue.
If your changes are under a feature flag, please check our Testing with feature flags documentation for instructions.
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 Sylvester Chin (
@schin1
) (UTC+8, same timezone as@mgandres
)Heinrich Lee Yu (
@engwan
) (UTC+8, same timezone as@mgandres
)frontend Thomas Randolph (
@thomasrandolph
) (UTC-6, 14 hours behind@mgandres
)Frédéric Caplette (
@f_caplette
) (UTC-4, 12 hours behind@mgandres
)test for spec/features/*
Sylvester Chin (
@schin1
) (UTC+8, same timezone as@mgandres
)Maintainer review is optional for test for spec/features/*
UX Kevin Comoli (
@kcomoli
) (UTC+2, 6 hours behind@mgandres
)Maintainer review is optional for UX 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.
Changes to utility SCSS files
Addition to
app/assets/stylesheets/utilities.scss
You have added a new rule to
app/assets/stylesheets/utilities.scss
. Are you sure you need this rule?If it is a component class shared across items, could it be added to the component as a utility class or to the component's stylesheet? If not, consider adding it to
app/assets/stylesheets/framework/common.scss
If it is a new utility class, is there another class that shares the same values in either this file or in
app/assets/stylesheets/utilities.scss
? If not, please be sure this addition follows the Gitlab UI naming style so it may be removed when these rules are included. See Include gitlab-ui utility-class library for more about this project.If needed, you can retry the
danger-review
job that generated this comment.Generated by
Dangermentioned in commit gitlab-org-sandbox/gitlab-jh-validation@330c713a
- Resolved by 🤖 GitLab Bot 🤖
Proper labels assigned to this merge request. Please ignore me.
@mgandres - please see the following guidance and update this merge request.1 Error Please add typebug typefeature, or typemaintenance label to this merge request. Edited by 🤖 GitLab Bot 🤖
Bundle size analysis [beta]
This compares changes in bundle size for entry points between the commits 2d3cdde2 and cd275b15
Special assetsEntrypoint / Name Size before Size after Diff Diff in percent average 4.15 MB 4.15 MB - 0.0 % mainChunk 3.01 MB 3.01 MB - 0.0 % Significant Growth: 8Expand
Entrypoint / Name Size before Size after Diff Diff in percent pages.admin.application_settings 663.14 KB 677.36 KB +14.22 KB 2.1 % pages.admin.application_settings.advanced_search 678.63 KB 692.85 KB +14.22 KB 2.1 % pages.admin.application_settings.integrations 676.6 KB 690.82 KB +14.22 KB 2.1 % pages.admin.application_settings.metrics_and_profiling 667.02 KB 681.24 KB +14.22 KB 2.1 % pages.admin.application_settings.network 664.3 KB 678.52 KB +14.22 KB 2.1 % pages.admin.application_settings.repository 676.19 KB 690.41 KB +14.22 KB 2.1 % pages.admin.application_settings.service_usage_data 666.98 KB 681.2 KB +14.22 KB 2.1 % pages.admin.application_settings.templates 687.49 KB 701.71 KB +14.22 KB 2.1 %
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 (
@leipert
,@markrian
,@ohoral
or@pgascouvaillancourt
) for review, if you are unsure about the size increase.Note: We do not have exact data for 2d3cdde2. So we have used data from: 372909ea.
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!e2e-test-on-gdk:
test report for cd275b15expand test summary
+------------------------------------------------------------------+ | suites summary | +-------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +-------------+--------+--------+---------+-------+-------+--------+ | Create | 19 | 0 | 0 | 18 | 19 | ❗ | | Data Stores | 20 | 0 | 0 | 15 | 20 | ❗ | | Plan | 47 | 0 | 0 | 40 | 47 | ❗ | | Govern | 19 | 0 | 0 | 18 | 19 | ❗ | | Manage | 12 | 0 | 1 | 12 | 13 | ❗ | | Verify | 8 | 0 | 0 | 8 | 8 | ❗ | +-------------+--------+--------+---------+-------+-------+--------+ | Total | 125 | 0 | 1 | 111 | 126 | ❗ | +-------------+--------+--------+---------+-------+-------+--------+
e2e-package-and-test:
test report for cd275b15expand test summary
+-------------------------------------------------------------+ | suites summary | +--------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +--------+--------+--------+---------+-------+-------+--------+ | Verify | 95 | 1 | 10 | 2 | 106 | ❌ | +--------+--------+--------+---------+-------+-------+--------+ | Total | 95 | 1 | 10 | 2 | 106 | ❌ | +--------+--------+--------+---------+-------+-------+--------+
e2e-review-qa:
test report for cd275b15expand test summary
+-----------------------------------------------------------------------+ | suites summary | +------------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +------------------+--------+--------+---------+-------+-------+--------+ | Plan | 3 | 0 | 1 | 0 | 4 | ✅ | | Verify | 8 | 0 | 0 | 0 | 8 | ✅ | | Create | 8 | 0 | 1 | 0 | 9 | ✅ | | Monitor | 4 | 0 | 0 | 0 | 4 | ✅ | | Framework sanity | 0 | 0 | 1 | 0 | 1 | ➖ | | Manage | 1 | 0 | 0 | 0 | 1 | ✅ | | Govern | 2 | 0 | 0 | 0 | 2 | ✅ | | Data Stores | 2 | 0 | 0 | 0 | 2 | ✅ | +------------------+--------+--------+---------+-------+-------+--------+ | Total | 28 | 0 | 3 | 0 | 31 | ✅ | +------------------+--------+--------+---------+-------+-------+--------+
mentioned in merge request gitlab-ui!3569 (merged)
added 316 commits
-
bd75154e...c753c53d - 314 commits from branch
master
- 00a52ecd - Add feature flag for CI variables drawer
- 059372e3 - Add other form elements
-
bd75154e...c753c53d - 314 commits from branch
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@a7a42896
marked the checklist item I have evaluated the MR acceptance checklist for this MR. as completed
added 188 commits
-
059372e3...95b1d341 - 186 commits from branch
master
- af418c8a - Add feature flag for CI variables drawer
- cf10121f - Add other form elements
-
059372e3...95b1d341 - 186 commits from branch
added featureenhancement typefeature labels
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@76250df3
added 2 commits
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@c7a3cfbc
- Resolved by Ezekiel Kigbo
@rutgerwessels Could you give this a backend test review?
I plan to update the specs in a future iteration when the feature is more complete, so for now this MR just makes sure that the tests for modal are passing.
- Resolved by Ezekiel Kigbo
- Resolved by Ezekiel Kigbo
requested review from @veethika, @djadmin, and @rutgerwessels
requested review from @jessieay and removed review request for @rutgerwessels
@rutgerwessels
, thanks for approving this merge request.This is the first time the merge request is approved. To ensure full test coverage, a new pipeline will be started shortly.
For more info, please refer to the following links:
added pipeline:mr-approved label
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@c6402fb6
added UX label
removed review request for @veethika
- Resolved by Mireya Andres
removed review request for @jessieay
- Resolved by Dheeraj Joshi
- Resolved by Ezekiel Kigbo
- Resolved by Mireya Andres
- Resolved by Dheeraj Joshi
question: have we considered re-using the same form underneath the modal component for the drawer component?
ci_variable_drawer.vue
ci_variable_modal.vue
This way we won't have to maintain two separate components with the same form elements.
added 277 commits
-
cf52c267...1115a83f - 273 commits from branch
master
- 39f4e95d - Add feature flag for CI variables drawer
- 3e559492 - Add other form elements
- 1d2b4672 - Update MR based on frontend/UX review
- e104715e - Add tests when ci_variable_drawer is enabled
Toggle commit list-
cf52c267...1115a83f - 273 commits from branch
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@da09221a
added 164 commits
-
e104715e...596a4863 - 160 commits from branch
master
- 8bb491c4 - Add feature flag for CI variables drawer
- 0ebd3a53 - Add other form elements
- 0d166b77 - Update MR based on frontend/UX review
- 3afb5083 - Add tests when ci_variable_drawer is enabled
Toggle commit list-
e104715e...596a4863 - 160 commits from branch
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@6b871a41
added 6 commits
-
3afb5083...db08fbba - 2 commits from branch
master
- f2485866 - Add feature flag for CI variables drawer
- 9161319e - Add other form elements
- 4f96c74d - Update MR based on frontend/UX review
- 24a07d55 - Add tests when ci_variable_drawer is enabled
Toggle commit list-
3afb5083...db08fbba - 2 commits from branch
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@a6082af5