diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-02-04 06:09:22 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-02-04 06:09:22 +0000 |
commit | a0806c73204d0fe3c3986b5c8140c582283f83c8 (patch) | |
tree | e54aeca8607f2d150decfc6f7624d6a5f22e6cf8 | |
parent | 112200c9a9a19061aa5c74394bbc2409041c4c20 (diff) | |
download | gitlab-ce-a0806c73204d0fe3c3986b5c8140c582283f83c8.tar.gz |
Add latest changes from gitlab-org/gitlab@master
37 files changed, 399 insertions, 126 deletions
diff --git a/.gitlab/ci/rails.gitlab-ci.yml b/.gitlab/ci/rails.gitlab-ci.yml index 7b3168fbbd7..fc6c005a13a 100644 --- a/.gitlab/ci/rails.gitlab-ci.yml +++ b/.gitlab/ci/rails.gitlab-ci.yml @@ -502,7 +502,6 @@ rspec:feature-flags: - .coverage-base - .rails:rules:rspec-feature-flags stage: post-test - allow_failure: true # We cannot use needs since it would mean needing 84 jobs (since most are parallelized) # so we use `dependencies` here. dependencies: @@ -522,7 +521,11 @@ rspec:feature-flags: - memory-on-boot script: - run_timed_command "bundle install --jobs=$(nproc) --path=vendor --retry=3 --quiet --without default development test production puma unicorn kerberos metrics omnibus ed25519" - - 'run_timed_command "bundle exec scripts/used-feature-flags" || (scripts/slack master-broken "☠️ \`${CI_JOB_NAME}\` failed! ☠️ See ${CI_JOB_URL}" ci_failing "GitLab Bot" && exit 1)' + - if [ "$CI_COMMIT_BRANCH" == "$CI_DEFAULT_BRANCH" ]; then + run_timed_command "bundle exec scripts/used-feature-flags" || (scripts/slack master-broken "☠️ \`${CI_JOB_NAME}\` failed! ☠️ See ${CI_JOB_URL}" ci_failing "GitLab Bot" && exit 1); + else + run_timed_command "bundle exec scripts/used-feature-flags"; + fi # EE/FOSS: default refs (MRs, master, schedules) jobs # ####################################################### diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index 875a4557d42..4d84f9747ad 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -879,6 +879,7 @@ - <<: *if-not-ee when: never - <<: *if-master-schedule-2-hourly + allow_failure: true - <<: *if-merge-request-title-run-all-rspec .rails:rules:master-schedule-nightly--code-backstage: diff --git a/app/assets/javascripts/jobs/components/job_app.vue b/app/assets/javascripts/jobs/components/job_app.vue index 8f0470c9e4a..52b60a1effd 100644 --- a/app/assets/javascripts/jobs/components/job_app.vue +++ b/app/assets/javascripts/jobs/components/job_app.vue @@ -54,11 +54,6 @@ export default { required: false, default: null, }, - runnerHelpUrl: { - type: String, - required: false, - default: null, - }, deploymentHelpUrl: { type: String, required: false, @@ -250,7 +245,6 @@ export default { v-if="shouldRenderSharedRunnerLimitWarning" :quota-used="job.runners.quota.used" :quota-limit="job.runners.quota.limit" - :runners-path="runnerHelpUrl" :project-path="projectPath" :subscriptions-more-minutes-url="subscriptionsMoreMinutesUrl" /> @@ -330,7 +324,6 @@ export default { 'right-sidebar-collapsed': !isSidebarOpen, }" :artifact-help-url="artifactHelpUrl" - :runner-help-url="runnerHelpUrl" data-testid="job-sidebar" /> </div> diff --git a/app/assets/javascripts/jobs/components/sidebar.vue b/app/assets/javascripts/jobs/components/sidebar.vue index 6ff2f7d199a..83eddc232a1 100644 --- a/app/assets/javascripts/jobs/components/sidebar.vue +++ b/app/assets/javascripts/jobs/components/sidebar.vue @@ -41,11 +41,6 @@ export default { required: false, default: '', }, - runnerHelpUrl: { - type: String, - required: false, - default: '', - }, }, computed: { ...mapGetters(['hasForwardDeploymentFailure']), @@ -135,7 +130,7 @@ export default { <gl-icon :size="14" name="external-link" /> </gl-link> </div> - <job-sidebar-details-container :runner-help-url="runnerHelpUrl" /> + <job-sidebar-details-container /> <artifacts-block v-if="hasArtifact" :artifact="job.artifact" :help-url="artifactHelpUrl" /> <trigger-block v-if="hasTriggers" :trigger="job.trigger" /> <commit-block diff --git a/app/assets/javascripts/jobs/components/sidebar_job_details_container.vue b/app/assets/javascripts/jobs/components/sidebar_job_details_container.vue index e909ad0241e..84ce6674104 100644 --- a/app/assets/javascripts/jobs/components/sidebar_job_details_container.vue +++ b/app/assets/javascripts/jobs/components/sidebar_job_details_container.vue @@ -3,6 +3,7 @@ import { mapState } from 'vuex'; import { __, sprintf } from '~/locale'; import timeagoMixin from '~/vue_shared/mixins/timeago'; import { timeIntervalInWords } from '~/lib/utils/datetime_utility'; +import { helpPagePath } from '~/helpers/help_page_helper'; import DetailRow from './sidebar_detail_row.vue'; export default { @@ -11,13 +12,6 @@ export default { DetailRow, }, mixins: [timeagoMixin], - props: { - runnerHelpUrl: { - type: String, - required: false, - default: '', - }, - }, computed: { ...mapState(['job']), coverage() { @@ -51,6 +45,11 @@ export default { queued() { return timeIntervalInWords(this.job.queued); }, + runnerHelpUrl() { + return helpPagePath('ci/runners/README.html', { + anchor: 'set-maximum-job-timeout-for-a-runner', + }); + }, runnerId() { return `${this.job.runner.description} (#${this.job.runner.id})`; }, diff --git a/app/assets/javascripts/jobs/index.js b/app/assets/javascripts/jobs/index.js index 1ad6292a030..3e00056ee81 100644 --- a/app/assets/javascripts/jobs/index.js +++ b/app/assets/javascripts/jobs/index.js @@ -13,7 +13,6 @@ export default () => { const { artifactHelpUrl, deploymentHelpUrl, - runnerHelpUrl, runnerSettingsUrl, variablesSettingsUrl, subscriptionsMoreMinutesUrl, @@ -39,7 +38,6 @@ export default () => { props: { artifactHelpUrl, deploymentHelpUrl, - runnerHelpUrl, runnerSettingsUrl, variablesSettingsUrl, subscriptionsMoreMinutesUrl, diff --git a/app/assets/javascripts/sidebar/components/reviewers/reviewer_avatar_link.vue b/app/assets/javascripts/sidebar/components/reviewers/reviewer_avatar_link.vue index b1b04564a62..87780888c2f 100644 --- a/app/assets/javascripts/sidebar/components/reviewers/reviewer_avatar_link.vue +++ b/app/assets/javascripts/sidebar/components/reviewers/reviewer_avatar_link.vue @@ -76,8 +76,8 @@ export default { class="d-inline-block" > <!-- use d-flex so that slot can be appropriately styled --> - <span class="d-flex"> - <reviewer-avatar :user="user" :img-size="32" :issuable-type="issuableType" /> + <span class="gl-display-flex gl-align-items-center"> + <reviewer-avatar :user="user" :img-size="24" :issuable-type="issuableType" /> <slot :user="user"></slot> </span> </gl-link> diff --git a/app/assets/javascripts/sidebar/components/reviewers/reviewers.vue b/app/assets/javascripts/sidebar/components/reviewers/reviewers.vue index cd62fe5be0f..00a2456c6b6 100644 --- a/app/assets/javascripts/sidebar/components/reviewers/reviewers.vue +++ b/app/assets/javascripts/sidebar/components/reviewers/reviewers.vue @@ -46,6 +46,9 @@ export default { assignSelf() { this.$emit('assign-self'); }, + requestReview(data) { + this.$emit('request-review', data); + }, }, }; </script> @@ -66,6 +69,7 @@ export default { :users="sortedReviewers" :root-path="rootPath" :issuable-type="issuableType" + @request-review="requestReview" /> </div> </div> diff --git a/app/assets/javascripts/sidebar/components/reviewers/sidebar_reviewers.vue b/app/assets/javascripts/sidebar/components/reviewers/sidebar_reviewers.vue index 03aa8732170..4bfae2dfffa 100644 --- a/app/assets/javascripts/sidebar/components/reviewers/sidebar_reviewers.vue +++ b/app/assets/javascripts/sidebar/components/reviewers/sidebar_reviewers.vue @@ -83,6 +83,9 @@ export default { return new Flash(__('Error occurred when saving reviewers')); }); }, + requestReview(data) { + this.mediator.requestReview(data); + }, }, }; </script> @@ -101,6 +104,7 @@ export default { :editable="store.editable" :issuable-type="issuableType" class="value" + @request-review="requestReview" /> </div> </template> diff --git a/app/assets/javascripts/sidebar/components/reviewers/uncollapsed_reviewer_list.vue b/app/assets/javascripts/sidebar/components/reviewers/uncollapsed_reviewer_list.vue index e82a271d007..be5fd93f77c 100644 --- a/app/assets/javascripts/sidebar/components/reviewers/uncollapsed_reviewer_list.vue +++ b/app/assets/javascripts/sidebar/components/reviewers/uncollapsed_reviewer_list.vue @@ -1,6 +1,7 @@ <script> // NOTE! For the first iteration, we are simply copying the implementation of Assignees // It will soon be overhauled in Issue https://gitlab.com/gitlab-org/gitlab/-/issues/233736 +import { GlButton, GlTooltipDirective, GlIcon } from '@gitlab/ui'; import { __, sprintf } from '~/locale'; import ReviewerAvatarLink from './reviewer_avatar_link.vue'; @@ -8,8 +9,13 @@ const DEFAULT_RENDER_COUNT = 5; export default { components: { + GlButton, + GlIcon, ReviewerAvatarLink, }, + directives: { + GlTooltip: GlTooltipDirective, + }, props: { users: { type: Array, @@ -28,6 +34,8 @@ export default { data() { return { showLess: true, + loading: false, + requestedReviewSuccess: false, }; }, computed: { @@ -61,43 +69,53 @@ export default { toggleShowLess() { this.showLess = !this.showLess; }, + reRequestReview(userId) { + this.loading = true; + this.$emit('request-review', { userId, callback: this.requestReviewComplete }); + }, + requestReviewComplete(success) { + if (success) { + this.requestedReviewSuccess = true; + + setTimeout(() => { + this.requestedReviewSuccess = false; + }, 1500); + } + + this.loading = false; + }, }, }; </script> <template> - <reviewer-avatar-link - v-if="hasOneUser" - #default="{ user }" - tooltip-placement="left" - :tooltip-has-name="false" - :user="firstUser" - :root-path="rootPath" - :issuable-type="issuableType" - > - <div class="gl-ml-3 gl-line-height-normal"> - <div class="author">{{ user.name }}</div> - <div class="username">{{ username }}</div> - </div> - </reviewer-avatar-link> - <div v-else> - <div class="user-list"> - <div v-for="user in uncollapsedUsers" :key="user.id" class="user-item"> - <reviewer-avatar-link :user="user" :root-path="rootPath" :issuable-type="issuableType" /> - </div> - </div> - <div v-if="renderShowMoreSection" class="user-list-more"> - <button - type="button" - class="btn-link" - data-qa-selector="more_reviewers_link" - @click="toggleShowLess" - > - <template v-if="showLess"> - {{ hiddenReviewersLabel }} - </template> - <template v-else>{{ __('- show less') }}</template> - </button> + <div> + <div + v-for="(user, index) in users" + :key="user.id" + :class="{ 'gl-mb-3': index !== users.length - 1 }" + data-testid="reviewer" + > + <reviewer-avatar-link :user="user" :root-path="rootPath" :issuable-type="issuableType"> + <div class="gl-ml-3">@{{ user.username }}</div> + </reviewer-avatar-link> + <gl-icon + v-if="requestedReviewSuccess" + :size="24" + name="check" + class="float-right gl-text-green-500" + /> + <gl-button + v-else-if="user.can_update_merge_request && user.reviewed" + v-gl-tooltip.left + :title="__('Re-request review')" + :loading="loading" + class="float-right gl-text-gray-500!" + size="small" + icon="redo" + variant="link" + @click="reRequestReview(user.id)" + /> </div> </div> </template> diff --git a/app/assets/javascripts/sidebar/components/todo_toggle/todo.vue b/app/assets/javascripts/sidebar/components/todo_toggle/todo.vue index 5d53b8a1b1d..f589e7555b3 100644 --- a/app/assets/javascripts/sidebar/components/todo_toggle/todo.vue +++ b/app/assets/javascripts/sidebar/components/todo_toggle/todo.vue @@ -42,7 +42,7 @@ export default { buttonClasses() { return this.collapsed ? 'btn-blank btn-todo sidebar-collapsed-icon dont-change-state' - : 'btn btn-default btn-todo issuable-header-btn float-right'; + : 'gl-button btn btn-default btn-todo issuable-header-btn float-right'; }, buttonLabel() { return this.isTodo ? MARK_TEXT : TODO_TEXT; diff --git a/app/assets/javascripts/sidebar/queries/reviewer_rereview.mutation.graphql b/app/assets/javascripts/sidebar/queries/reviewer_rereview.mutation.graphql new file mode 100644 index 00000000000..73765e7d77b --- /dev/null +++ b/app/assets/javascripts/sidebar/queries/reviewer_rereview.mutation.graphql @@ -0,0 +1,5 @@ +mutation mergeRequestRequestRereview($projectPath: ID!, $iid: String!, $userId: ID!) { + mergeRequestReviewerRereview(input: { projectPath: $projectPath, iid: $iid, userId: $userId }) { + errors + } +} diff --git a/app/assets/javascripts/sidebar/services/sidebar_service.js b/app/assets/javascripts/sidebar/services/sidebar_service.js index a61af631661..eef9a22cce5 100644 --- a/app/assets/javascripts/sidebar/services/sidebar_service.js +++ b/app/assets/javascripts/sidebar/services/sidebar_service.js @@ -1,6 +1,8 @@ import sidebarDetailsQuery from 'ee_else_ce/sidebar/queries/sidebarDetails.query.graphql'; import axios from '~/lib/utils/axios_utils'; import createGqClient, { fetchPolicies } from '~/lib/graphql'; +import { convertToGraphQLId } from '~/graphql_shared/utils'; +import reviewerRereviewMutation from '../queries/reviewer_rereview.mutation.graphql'; export const gqClient = createGqClient( {}, @@ -70,4 +72,15 @@ export default class SidebarService { move_to_project_id: moveToProjectId, }); } + + requestReview(userId) { + return gqClient.mutate({ + mutation: reviewerRereviewMutation, + variables: { + userId: convertToGraphQLId('User', `${userId}`), // eslint-disable-line @gitlab/require-i18n-strings + projectPath: this.fullPath, + iid: this.iid.toString(), + }, + }); + } } diff --git a/app/assets/javascripts/sidebar/sidebar_mediator.js b/app/assets/javascripts/sidebar/sidebar_mediator.js index 8ef5bc9db8d..b23788f81fe 100644 --- a/app/assets/javascripts/sidebar/sidebar_mediator.js +++ b/app/assets/javascripts/sidebar/sidebar_mediator.js @@ -1,4 +1,5 @@ import Store from 'ee_else_ce/sidebar/stores/sidebar_store'; +import toast from '~/vue_shared/plugins/global_toast'; import { __ } from '~/locale'; import { visitUrl } from '../lib/utils/url_utility'; import { deprecatedCreateFlash as Flash } from '../flash'; @@ -51,6 +52,17 @@ export default class SidebarMediator { return this.service.update(field, data); } + requestReview({ userId, callback }) { + return this.service + .requestReview(userId) + .then(() => { + this.store.updateReviewer(userId); + toast(__('Requested review')); + callback(true); + }) + .catch(() => callback(false)); + } + setMoveToProjectId(projectId) { this.store.setMoveToProjectId(projectId); } diff --git a/app/assets/javascripts/sidebar/stores/sidebar_store.js b/app/assets/javascripts/sidebar/stores/sidebar_store.js index d53393052eb..3c108b06eab 100644 --- a/app/assets/javascripts/sidebar/stores/sidebar_store.js +++ b/app/assets/javascripts/sidebar/stores/sidebar_store.js @@ -96,6 +96,14 @@ export default class SidebarStore { } } + updateReviewer(id) { + const reviewer = this.findReviewer({ id }); + + if (reviewer) { + reviewer.reviewed = false; + } + } + findAssignee(findAssignee) { return this.assignees.find(({ id }) => id === findAssignee.id); } diff --git a/app/helpers/ci/jobs_helper.rb b/app/helpers/ci/jobs_helper.rb index d47f6195c61..ec17eccf693 100644 --- a/app/helpers/ci/jobs_helper.rb +++ b/app/helpers/ci/jobs_helper.rb @@ -8,7 +8,6 @@ module Ci "project_path" => @project.full_path, "artifact_help_url" => help_page_path('user/gitlab_com/index.html', anchor: 'gitlab-cicd'), "deployment_help_url" => help_page_path('user/project/clusters/index.html', anchor: 'troubleshooting'), - "runner_help_url" => help_page_path('ci/runners/README.html', anchor: 'set-maximum-job-timeout-for-a-runner'), "runner_settings_url" => project_runners_path(@build.project, anchor: 'js-runners-settings'), "variables_settings_url" => project_variables_path(@build.project, anchor: 'js-cicd-variables-settings'), "page_path" => project_job_path(@project, @build), diff --git a/app/views/dashboard/projects/_starred_empty_state.html.haml b/app/views/dashboard/projects/_starred_empty_state.html.haml index bea27f1a456..6db018d72da 100644 --- a/app/views/dashboard/projects/_starred_empty_state.html.haml +++ b/app/views/dashboard/projects/_starred_empty_state.html.haml @@ -3,7 +3,7 @@ .svg-content.svg-250 = image_tag 'illustrations/starred_empty.svg' .text-content - %h4.text-center + %h4.gl-text-center = s_("StarredProjectsEmptyState|You don't have starred projects yet.") - %p.text-secondary + %p.gl-text-gray-500 = s_("StarredProjectsEmptyState|Visit a project page and press on a star icon. Then, you can find the project on this page.") diff --git a/app/views/shared/issuable/_sidebar_todo.html.haml b/app/views/shared/issuable/_sidebar_todo.html.haml index 7b5926fc186..1f05dcf83bc 100644 --- a/app/views/shared/issuable/_sidebar_todo.html.haml +++ b/app/views/shared/issuable/_sidebar_todo.html.haml @@ -6,7 +6,7 @@ - button_icon = has_todo ? todo_button_data[:mark_icon] : todo_button_data[:todo_icon] %button.issuable-todo-btn.js-issuable-todo{ type: 'button', - class: (is_collapsed ? 'btn-blank sidebar-collapsed-icon dont-change-state has-tooltip' : 'btn btn-default issuable-header-btn float-right'), + class: (is_collapsed ? 'btn-blank sidebar-collapsed-icon dont-change-state has-tooltip' : 'gl-button btn btn-default issuable-header-btn float-right'), title: button_title, 'aria-label' => button_title, data: todo_button_data } diff --git a/changelogs/unreleased/fix-avatar-alignment-edit-board.yml b/changelogs/unreleased/fix-avatar-alignment-edit-board.yml new file mode 100644 index 00000000000..4d42a1fd36c --- /dev/null +++ b/changelogs/unreleased/fix-avatar-alignment-edit-board.yml @@ -0,0 +1,5 @@ +--- +title: Fix assignee avatar alignment in edit board modal +merge_request: 52453 +author: Yogi (@yo) +type: fixed diff --git a/changelogs/unreleased/gl-button-issues.yml b/changelogs/unreleased/gl-button-issues.yml new file mode 100644 index 00000000000..ae00abd29e3 --- /dev/null +++ b/changelogs/unreleased/gl-button-issues.yml @@ -0,0 +1,5 @@ +--- +title: Apply new GitLab UI style for todo button in the issuable sidebar +merge_request: 52779 +author: Yogi (@yo) +type: other diff --git a/changelogs/unreleased/ph-reRequestReview.yml b/changelogs/unreleased/ph-reRequestReview.yml new file mode 100644 index 00000000000..36d8bbfc647 --- /dev/null +++ b/changelogs/unreleased/ph-reRequestReview.yml @@ -0,0 +1,5 @@ +--- +title: Allow users to re-request a review from a reviewer +merge_request: 50068 +author: +type: added diff --git a/changelogs/unreleased/update-starred-empty-state.yml b/changelogs/unreleased/update-starred-empty-state.yml new file mode 100644 index 00000000000..66577393fa3 --- /dev/null +++ b/changelogs/unreleased/update-starred-empty-state.yml @@ -0,0 +1,5 @@ +--- +title: Update starred empty state with new GitLab UI classes +merge_request: 52467 +author: Yogi (@yo) +type: other diff --git a/danger/feature_flag/Dangerfile b/danger/feature_flag/Dangerfile index 089ee2dc50b..cda0e7f3bcc 100644 --- a/danger/feature_flag/Dangerfile +++ b/danger/feature_flag/Dangerfile @@ -52,6 +52,23 @@ def message_for_feature_flag_with_group!(feature_flag:, mr_group_label:) end end -feature_flag.feature_flag_files.each do |feature_flag| +feature_flag.feature_flag_files(change_type: :added).each do |feature_flag| check_feature_flag_yaml(feature_flag) end + +if feature_flag.feature_flag_files(change_type: :added).any? || + feature_flag.feature_flag_files(change_type: :deleted).any? + new_mr_title = helper.mr_title.dup + new_mr_title << ' [RUN ALL RSPEC]' unless helper.run_all_rspec_mr? + new_mr_title << ' [RUN AS-IF-FOSS]' unless helper.run_as_if_foss_mr? + + if new_mr_title != helper.mr_title + gitlab.api.update_merge_request( + gitlab.mr_json['project_id'], + gitlab.mr_json['iid'], + title: new_mr_title + ) + gitlab.api.post("/projects/#{gitlab.mr_json['project_id']}/merge_requests/#{gitlab.mr_json['iid']}/pipelines") + message %(You're adding or removing a feature flag, and your MR title didn't include `[RUN ALL RSPEC] [RUN AS-IF-FOSS]`, so we've updated it and started a new MR pipeline to ensure everything is covered.) + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index a89ccb085e8..91300f92f80 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -23918,6 +23918,9 @@ msgstr "" msgid "Re-authentication required" msgstr "" +msgid "Re-request review" +msgstr "" + msgid "Re-verification interval" msgstr "" @@ -24737,6 +24740,9 @@ msgstr "" msgid "Requested design version does not exist." msgstr "" +msgid "Requested review" +msgstr "" + msgid "Requested states are invalid" msgstr "" diff --git a/spec/frontend/jobs/components/job_app_spec.js b/spec/frontend/jobs/components/job_app_spec.js index 657687b5e2a..b9d27932dff 100644 --- a/spec/frontend/jobs/components/job_app_spec.js +++ b/spec/frontend/jobs/components/job_app_spec.js @@ -34,7 +34,6 @@ describe('Job App', () => { const props = { artifactHelpUrl: 'help/artifact', - runnerHelpUrl: 'help/runner', deploymentHelpUrl: 'help/deployment', runnerSettingsUrl: 'settings/ci-cd/runners', variablesSettingsUrl: 'settings/ci-cd/variables', diff --git a/spec/frontend/jobs/components/job_sidebar_details_container_spec.js b/spec/frontend/jobs/components/job_sidebar_details_container_spec.js index bc0d455c309..a29d88e5c1e 100644 --- a/spec/frontend/jobs/components/job_sidebar_details_container_spec.js +++ b/spec/frontend/jobs/components/job_sidebar_details_container_spec.js @@ -116,14 +116,5 @@ describe('Job Sidebar Details Container', () => { expect(findJobTimeout().exists()).toBe(false); }); - - it('should pass the help URL', async () => { - const helpUrl = 'fakeUrl'; - const props = { runnerHelpUrl: helpUrl }; - createWrapper({ props }); - await store.dispatch('receiveJobSuccess', { metadata: { timeout_human_readable } }); - - expect(findJobTimeout().props('helpUrl')).toBe(helpUrl); - }); }); }); diff --git a/spec/frontend/sidebar/__snapshots__/todo_spec.js.snap b/spec/frontend/sidebar/__snapshots__/todo_spec.js.snap index e295c587d70..846f45345e7 100644 --- a/spec/frontend/sidebar/__snapshots__/todo_spec.js.snap +++ b/spec/frontend/sidebar/__snapshots__/todo_spec.js.snap @@ -3,7 +3,7 @@ exports[`SidebarTodo template renders component container element with proper data attributes 1`] = ` <button aria-label="Mark as done" - class="btn btn-default btn-todo issuable-header-btn float-right" + class="gl-button btn btn-default btn-todo issuable-header-btn float-right" data-issuable-id="1" data-issuable-type="epic" type="button" diff --git a/spec/frontend/sidebar/reviewers_spec.js b/spec/frontend/sidebar/reviewers_spec.js index c6d15a9270f..9208fb7288b 100644 --- a/spec/frontend/sidebar/reviewers_spec.js +++ b/spec/frontend/sidebar/reviewers_spec.js @@ -114,8 +114,7 @@ describe('Reviewer component', () => { editable: true, }); - expect(wrapper.findAll('.user-item').length).toBe(users.length); - expect(wrapper.find('.user-list-more').exists()).toBe(false); + expect(wrapper.findAll('[data-testid="reviewer"]').length).toBe(users.length); }); it('shows sorted reviewer where "can merge" users are sorted first', () => { @@ -144,10 +143,10 @@ describe('Reviewer component', () => { users, }); - const userItems = wrapper.findAll('.user-list .user-item a'); + const userItems = wrapper.findAll('[data-testid="reviewer"]'); expect(userItems.length).toBe(3); - expect(userItems.at(0).attributes('title')).toBe(users[2].name); + expect(userItems.at(0).find('a').attributes('title')).toBe(users[2].name); }); it('passes the sorted reviewers to the collapsed-reviewer-list', () => { diff --git a/spec/frontend/sidebar/todo_spec.js b/spec/frontend/sidebar/todo_spec.js index d0f92b4b802..7b7e24f2a8f 100644 --- a/spec/frontend/sidebar/todo_spec.js +++ b/spec/frontend/sidebar/todo_spec.js @@ -26,7 +26,7 @@ describe('SidebarTodo', () => { it.each` state | classes - ${false} | ${['btn', 'btn-default', 'btn-todo', 'issuable-header-btn', 'float-right']} + ${false} | ${['gl-button', 'btn', 'btn-default', 'btn-todo', 'issuable-header-btn', 'float-right']} ${true} | ${['btn-blank', 'btn-todo', 'sidebar-collapsed-icon', 'dont-change-state']} `('returns todo button classes for when `collapsed` prop is `$state`', ({ state, classes }) => { createComponent({ collapsed: state }); diff --git a/spec/support/shared_examples/features/editable_merge_request_shared_examples.rb b/spec/support/shared_examples/features/editable_merge_request_shared_examples.rb index 2fff4137934..ccd063faac4 100644 --- a/spec/support/shared_examples/features/editable_merge_request_shared_examples.rb +++ b/spec/support/shared_examples/features/editable_merge_request_shared_examples.rb @@ -48,7 +48,7 @@ RSpec.shared_examples 'an editable merge request' do end page.within '.reviewer' do - expect(page).to have_content user.name + expect(page).to have_content user.username end page.within '.milestone' do diff --git a/spec/support/shared_examples/features/multiple_reviewers_mr_shared_examples.rb b/spec/support/shared_examples/features/multiple_reviewers_mr_shared_examples.rb index 48cde90bd9b..ad6ca3e1900 100644 --- a/spec/support/shared_examples/features/multiple_reviewers_mr_shared_examples.rb +++ b/spec/support/shared_examples/features/multiple_reviewers_mr_shared_examples.rb @@ -40,7 +40,7 @@ RSpec.shared_examples 'multiple reviewers merge request' do |action, save_button # Closing dropdown to persist click_link 'Edit' - expect(page).to have_content user2.name + expect(page).to have_content user2.username end end end diff --git a/spec/tooling/danger/feature_flag_spec.rb b/spec/tooling/danger/feature_flag_spec.rb index 1ee79e3c74e..db63116cc37 100644 --- a/spec/tooling/danger/feature_flag_spec.rb +++ b/spec/tooling/danger/feature_flag_spec.rb @@ -8,7 +8,9 @@ RSpec.describe Tooling::Danger::FeatureFlag do include DangerSpecHelper let(:added_files) { nil } - let(:fake_git) { double('fake-git', added_files: added_files) } + let(:modified_files) { nil } + let(:deleted_files) { nil } + let(:fake_git) { double('fake-git', added_files: added_files, modified_files: modified_files, deleted_files: deleted_files) } let(:mr_labels) { nil } let(:mr_json) { nil } @@ -24,29 +26,72 @@ RSpec.describe Tooling::Danger::FeatureFlag do subject(:feature_flag) { fake_danger.new(git: fake_git, gitlab: fake_gitlab, helper: fake_helper) } describe '#feature_flag_files' do - context 'added files contain several feature flags' do - let(:added_files) do - [ - 'config/feature_flags/development/entry.yml', - 'ee/config/feature_flags/ops/entry.yml' - ] + let(:feature_flag_files) do + [ + 'config/feature_flags/development/entry.yml', + 'ee/config/feature_flags/ops/entry.yml' + ] + end + + let(:other_files) do + [ + 'app/models/model.rb', + 'app/assets/javascripts/file.js' + ] + end + + shared_examples 'an array of Found objects' do |change_type| + it 'returns an array of Found objects' do + expect(feature_flag.feature_flag_files(change_type: change_type)).to contain_exactly(an_instance_of(described_class::Found), an_instance_of(described_class::Found)) + expect(feature_flag.feature_flag_files(change_type: change_type).map(&:path)).to eq(feature_flag_files) end + end + shared_examples 'an empty array' do |change_type| it 'returns an array of Found objects' do - expect(feature_flag.feature_flag_files).to contain_exactly(an_instance_of(described_class::Found), an_instance_of(described_class::Found)) + expect(feature_flag.feature_flag_files(change_type: change_type)).to be_empty + end + end + + describe 'retrieves added feature flag files' do + context 'with added added feature flag files' do + let(:added_files) { feature_flag_files } + + include_examples 'an array of Found objects', :added + end + + context 'without added added feature flag files' do + let(:added_files) { other_files } + + include_examples 'an empty array', :added end end - context 'added files do not contain a feature_flag' do - let(:added_files) do - [ - 'app/models/model.rb', - 'app/assets/javascripts/file.js' - ] + describe 'retrieves modified feature flag files' do + context 'with modified modified feature flag files' do + let(:modified_files) { feature_flag_files } + + include_examples 'an array of Found objects', :modified + end + + context 'without modified modified feature flag files' do + let(:modified_files) { other_files } + + include_examples 'an empty array', :modified end + end + + describe 'retrieves deleted feature flag files' do + context 'with deleted deleted feature flag files' do + let(:deleted_files) { feature_flag_files } + + include_examples 'an array of Found objects', :deleted + end + + context 'without deleted deleted feature flag files' do + let(:deleted_files) { other_files } - it 'returns the feature flag file path' do - expect(feature_flag.feature_flag_files).to be_empty + include_examples 'an empty array', :deleted end end end diff --git a/spec/tooling/danger/helper_spec.rb b/spec/tooling/danger/helper_spec.rb index 40170dcd355..c338d138352 100644 --- a/spec/tooling/danger/helper_spec.rb +++ b/spec/tooling/danger/helper_spec.rb @@ -402,13 +402,55 @@ RSpec.describe Tooling::Danger::Helper do end end - describe '#security_mr?' do - it 'returns false when `gitlab_helper` is unavailable' do + describe '#mr_title' do + it 'returns "" when `gitlab_helper` is unavailable' do expect(helper).to receive(:gitlab_helper).and_return(nil) - expect(helper).not_to be_security_mr + expect(helper.mr_title).to eq('') end + it 'returns the MR title when `gitlab_helper` is available' do + mr_title = 'My MR title' + expect(fake_gitlab).to receive(:mr_json) + .and_return('title' => mr_title) + + expect(helper.mr_title).to eq(mr_title) + end + end + + describe '#mr_web_url' do + it 'returns "" when `gitlab_helper` is unavailable' do + expect(helper).to receive(:gitlab_helper).and_return(nil) + + expect(helper.mr_web_url).to eq('') + end + + it 'returns the MR web_url when `gitlab_helper` is available' do + mr_web_url = 'https://gitlab.com/gitlab-org/gitlab/-/merge_requests/1' + expect(fake_gitlab).to receive(:mr_json) + .and_return('web_url' => mr_web_url) + + expect(helper.mr_web_url).to eq(mr_web_url) + end + end + + describe '#mr_target_branch' do + it 'returns "" when `gitlab_helper` is unavailable' do + expect(helper).to receive(:gitlab_helper).and_return(nil) + + expect(helper.mr_target_branch).to eq('') + end + + it 'returns the MR web_url when `gitlab_helper` is available' do + mr_target_branch = 'main' + expect(fake_gitlab).to receive(:mr_json) + .and_return('target_branch' => mr_target_branch) + + expect(helper.mr_target_branch).to eq(mr_target_branch) + end + end + + describe '#security_mr?' do it 'returns false when on a normal merge request' do expect(fake_gitlab).to receive(:mr_json) .and_return('web_url' => 'https://gitlab.com/gitlab-org/gitlab/-/merge_requests/1') @@ -425,12 +467,6 @@ RSpec.describe Tooling::Danger::Helper do end describe '#draft_mr?' do - it 'returns false when `gitlab_helper` is unavailable' do - expect(helper).to receive(:gitlab_helper).and_return(nil) - - expect(helper).not_to be_draft_mr - end - it 'returns true for a draft MR' do expect(fake_gitlab).to receive(:mr_json) .and_return('title' => 'Draft: My MR title') @@ -447,12 +483,6 @@ RSpec.describe Tooling::Danger::Helper do end describe '#cherry_pick_mr?' do - it 'returns false when `gitlab_helper` is unavailable' do - expect(helper).to receive(:gitlab_helper).and_return(nil) - - expect(helper).not_to be_cherry_pick_mr - end - context 'when MR title does not mention a cherry-pick' do it 'returns false' do expect(fake_gitlab).to receive(:mr_json) @@ -478,6 +508,46 @@ RSpec.describe Tooling::Danger::Helper do end end + describe '#run_all_rspec_mr?' do + context 'when MR title does not mention RUN ALL RSPEC' do + it 'returns false' do + expect(fake_gitlab).to receive(:mr_json) + .and_return('title' => 'Add feature xyz') + + expect(helper).not_to be_run_all_rspec_mr + end + end + + context 'when MR title mentions RUN ALL RSPEC' do + it 'returns true' do + expect(fake_gitlab).to receive(:mr_json) + .and_return('title' => 'Add feature xyz RUN ALL RSPEC') + + expect(helper).to be_run_all_rspec_mr + end + end + end + + describe '#run_as_if_foss_mr?' do + context 'when MR title does not mention RUN AS-IF-FOSS' do + it 'returns false' do + expect(fake_gitlab).to receive(:mr_json) + .and_return('title' => 'Add feature xyz') + + expect(helper).not_to be_run_as_if_foss_mr + end + end + + context 'when MR title mentions RUN AS-IF-FOSS' do + it 'returns true' do + expect(fake_gitlab).to receive(:mr_json) + .and_return('title' => 'Add feature xyz RUN AS-IF-FOSS') + + expect(helper).to be_run_as_if_foss_mr + end + end + end + describe '#stable_branch?' do it 'returns false when `gitlab_helper` is unavailable' do expect(helper).to receive(:gitlab_helper).and_return(nil) diff --git a/spec/tooling/danger/title_linting_spec.rb b/spec/tooling/danger/title_linting_spec.rb index 1677223f0db..7bc1684cd87 100644 --- a/spec/tooling/danger/title_linting_spec.rb +++ b/spec/tooling/danger/title_linting_spec.rb @@ -52,4 +52,40 @@ RSpec.describe Tooling::Danger::TitleLinting do expect(described_class.has_draft_flag?('My MR title')).to be false end end + + describe '#has_cherry_pick_flag?' do + [ + 'Cherry Pick !1234', + 'cherry-pick !1234', + 'CherryPick !1234' + ].each do |mr_title| + it 'returns true for cherry-pick title' do + expect(described_class.has_cherry_pick_flag?(mr_title)).to be true + end + end + + it 'returns false for non cherry-pick title' do + expect(described_class.has_cherry_pick_flag?('My MR title')).to be false + end + end + + describe '#has_run_all_rspec_flag?' do + it 'returns true for a title that includes RUN ALL RSPEC' do + expect(described_class.has_run_all_rspec_flag?('My MR title RUN ALL RSPEC')).to be true + end + + it 'returns true for a title that does not include RUN ALL RSPEC' do + expect(described_class.has_run_all_rspec_flag?('My MR title')).to be false + end + end + + describe '#has_run_as_if_foss_flag?' do + it 'returns true for a title that includes RUN AS-IF-FOSS' do + expect(described_class.has_run_as_if_foss_flag?('My MR title RUN AS-IF-FOSS')).to be true + end + + it 'returns true for a title that does not include RUN AS-IF-FOSS' do + expect(described_class.has_run_as_if_foss_flag?('My MR title')).to be false + end + end end diff --git a/tooling/danger/feature_flag.rb b/tooling/danger/feature_flag.rb index 27352457f95..2e65831ef9f 100644 --- a/tooling/danger/feature_flag.rb +++ b/tooling/danger/feature_flag.rb @@ -5,8 +5,13 @@ require 'yaml' module Tooling module Danger module FeatureFlag - def feature_flag_files - @feature_flag_files ||= git.added_files.select { |path| path =~ %r{\A(ee/)?config/feature_flags/} }.map { |path| Found.new(path) } + # `change_type` can be: + # - :added + # - :modified + # - :deleted + def feature_flag_files(change_type:) + files = git.public_send("#{change_type}_files") # rubocop:disable GitlabSecurity/PublicSend + files.select { |path| path =~ %r{\A(ee/)?config/feature_flags/} }.map { |path| Found.new(path) } end class Found diff --git a/tooling/danger/helper.rb b/tooling/danger/helper.rb index afb6220e116..60026ee9c70 100644 --- a/tooling/danger/helper.rb +++ b/tooling/danger/helper.rb @@ -215,28 +215,46 @@ module Tooling usernames.map { |u| Tooling::Danger::Teammate.new('username' => u) } end - def draft_mr? - return false unless gitlab_helper + def mr_title + return '' unless gitlab_helper - TitleLinting.has_draft_flag?(gitlab_helper.mr_json['title']) + gitlab_helper.mr_json['title'] end - def security_mr? - return false unless gitlab_helper + def mr_web_url + return '' unless gitlab_helper + + gitlab_helper.mr_json['web_url'] + end + + def mr_target_branch + return '' unless gitlab_helper - gitlab_helper.mr_json['web_url'].include?('/gitlab-org/security/') + gitlab_helper.mr_json['target_branch'] + end + + def draft_mr? + TitleLinting.has_draft_flag?(mr_title) + end + + def security_mr? + mr_web_url.include?('/gitlab-org/security/') end def cherry_pick_mr? - return false unless gitlab_helper + TitleLinting.has_cherry_pick_flag?(mr_title) + end - /cherry[\s-]*pick/i.match?(gitlab_helper.mr_json['title']) + def run_all_rspec_mr? + TitleLinting.has_run_all_rspec_flag?(mr_title) end - def stable_branch? - return false unless gitlab_helper + def run_as_if_foss_mr? + TitleLinting.has_run_as_if_foss_flag?(mr_title) + end - /\A\d+-\d+-stable-ee/i.match?(gitlab_helper.mr_json['target_branch']) + def stable_branch? + /\A\d+-\d+-stable-ee/i.match?(mr_target_branch) end def mr_has_labels?(*labels) diff --git a/tooling/danger/title_linting.rb b/tooling/danger/title_linting.rb index 0cff16f4c6b..dcd83df7d93 100644 --- a/tooling/danger/title_linting.rb +++ b/tooling/danger/title_linting.rb @@ -4,6 +4,9 @@ module Tooling module Danger module TitleLinting DRAFT_REGEX = /\A*#{Regexp.union(/(?i)(\[WIP\]\s*|WIP:\s*|WIP$)/, /(?i)(\[draft\]|\(draft\)|draft:|draft\s\-\s|draft$)/)}+\s*/i.freeze + CHERRY_PICK_REGEX = /cherry[\s-]*pick/i.freeze + RUN_ALL_RSPEC_REGEX = /RUN ALL RSPEC/i.freeze + RUN_AS_IF_FOSS_REGEX = /RUN AS-IF-FOSS/i.freeze module_function @@ -18,6 +21,18 @@ module Tooling def has_draft_flag?(title) DRAFT_REGEX.match?(title) end + + def has_cherry_pick_flag?(title) + CHERRY_PICK_REGEX.match?(title) + end + + def has_run_all_rspec_flag?(title) + RUN_ALL_RSPEC_REGEX.match?(title) + end + + def has_run_as_if_foss_flag?(title) + RUN_AS_IF_FOSS_REGEX.match?(title) + end end end end |