summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2021-02-04 06:09:22 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2021-02-04 06:09:22 +0000
commita0806c73204d0fe3c3986b5c8140c582283f83c8 (patch)
treee54aeca8607f2d150decfc6f7624d6a5f22e6cf8
parent112200c9a9a19061aa5c74394bbc2409041c4c20 (diff)
downloadgitlab-ce-a0806c73204d0fe3c3986b5c8140c582283f83c8.tar.gz
Add latest changes from gitlab-org/gitlab@master
-rw-r--r--.gitlab/ci/rails.gitlab-ci.yml7
-rw-r--r--.gitlab/ci/rules.gitlab-ci.yml1
-rw-r--r--app/assets/javascripts/jobs/components/job_app.vue7
-rw-r--r--app/assets/javascripts/jobs/components/sidebar.vue7
-rw-r--r--app/assets/javascripts/jobs/components/sidebar_job_details_container.vue13
-rw-r--r--app/assets/javascripts/jobs/index.js2
-rw-r--r--app/assets/javascripts/sidebar/components/reviewers/reviewer_avatar_link.vue4
-rw-r--r--app/assets/javascripts/sidebar/components/reviewers/reviewers.vue4
-rw-r--r--app/assets/javascripts/sidebar/components/reviewers/sidebar_reviewers.vue4
-rw-r--r--app/assets/javascripts/sidebar/components/reviewers/uncollapsed_reviewer_list.vue82
-rw-r--r--app/assets/javascripts/sidebar/components/todo_toggle/todo.vue2
-rw-r--r--app/assets/javascripts/sidebar/queries/reviewer_rereview.mutation.graphql5
-rw-r--r--app/assets/javascripts/sidebar/services/sidebar_service.js13
-rw-r--r--app/assets/javascripts/sidebar/sidebar_mediator.js12
-rw-r--r--app/assets/javascripts/sidebar/stores/sidebar_store.js8
-rw-r--r--app/helpers/ci/jobs_helper.rb1
-rw-r--r--app/views/dashboard/projects/_starred_empty_state.html.haml4
-rw-r--r--app/views/shared/issuable/_sidebar_todo.html.haml2
-rw-r--r--changelogs/unreleased/fix-avatar-alignment-edit-board.yml5
-rw-r--r--changelogs/unreleased/gl-button-issues.yml5
-rw-r--r--changelogs/unreleased/ph-reRequestReview.yml5
-rw-r--r--changelogs/unreleased/update-starred-empty-state.yml5
-rw-r--r--danger/feature_flag/Dangerfile19
-rw-r--r--locale/gitlab.pot6
-rw-r--r--spec/frontend/jobs/components/job_app_spec.js1
-rw-r--r--spec/frontend/jobs/components/job_sidebar_details_container_spec.js9
-rw-r--r--spec/frontend/sidebar/__snapshots__/todo_spec.js.snap2
-rw-r--r--spec/frontend/sidebar/reviewers_spec.js7
-rw-r--r--spec/frontend/sidebar/todo_spec.js2
-rw-r--r--spec/support/shared_examples/features/editable_merge_request_shared_examples.rb2
-rw-r--r--spec/support/shared_examples/features/multiple_reviewers_mr_shared_examples.rb2
-rw-r--r--spec/tooling/danger/feature_flag_spec.rb77
-rw-r--r--spec/tooling/danger/helper_spec.rb100
-rw-r--r--spec/tooling/danger/title_linting_spec.rb36
-rw-r--r--tooling/danger/feature_flag.rb9
-rw-r--r--tooling/danger/helper.rb40
-rw-r--r--tooling/danger/title_linting.rb15
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