diff options
14 files changed, 243 insertions, 85 deletions
diff --git a/app/assets/javascripts/boards/components/sidebar/board_sidebar_labels_select.vue b/app/assets/javascripts/boards/components/sidebar/board_sidebar_labels_select.vue index aedd695d07b..919ef0d3783 100644 --- a/app/assets/javascripts/boards/components/sidebar/board_sidebar_labels_select.vue +++ b/app/assets/javascripts/boards/components/sidebar/board_sidebar_labels_select.vue @@ -1,10 +1,12 @@ <script> import { GlLabel } from '@gitlab/ui'; import { mapGetters, mapActions } from 'vuex'; +import Api from '~/api'; import BoardEditableItem from '~/boards/components/sidebar/board_editable_item.vue'; import createFlash from '~/flash'; import { getIdFromGraphQLId } from '~/graphql_shared/utils'; import { isScopedLabel } from '~/lib/utils/common_utils'; +import { mergeUrlParams } from '~/lib/utils/url_utility'; import { __ } from '~/locale'; import LabelsSelect from '~/vue_shared/components/sidebar/labels_select_vue/labels_select_root.vue'; @@ -14,7 +16,13 @@ export default { LabelsSelect, GlLabel, }, - inject: ['labelsFetchPath', 'labelsManagePath', 'labelsFilterBasePath'], + inject: { + labelsFetchPath: { + default: null, + }, + labelsManagePath: {}, + labelsFilterBasePath: {}, + }, data() { return { loading: false, @@ -38,6 +46,32 @@ export default { scoped: isScopedLabel(label), })); }, + fetchPath() { + /* + Labels fetched in epic boards are always group-level labels + and the correct path are passed from the backend (injected through labelsFetchPath) + + For issue boards, we should always include project-level labels and use a different endpoint. + (it requires knowing the project path of a selected issue.) + + Note 1. that we will be using GraphQL to fetch labels when we create a labels select widget. + And this component will be removed _wholesale_ https://gitlab.com/gitlab-org/gitlab/-/issues/300653. + + Note 2. Moreover, 'fetchPath' needs to be used as a key for 'labels-select' component to force updates. + 'labels-select' has its own vuex store and initializes the passed props as states + and these states aren't reactively bound to the passed props. + */ + + const projectLabelsFetchPath = mergeUrlParams( + { include_ancestor_groups: true }, + Api.buildUrl(Api.projectLabelsPath).replace( + ':namespace_path/:project_path', + this.projectPathForActiveIssue, + ), + ); + + return this.labelsFetchPath || projectLabelsFetchPath; + }, }, methods: { ...mapActions(['setActiveBoardItemLabels']), @@ -100,12 +134,13 @@ export default { <template #default="{ edit }"> <labels-select ref="labelsSelect" + :key="fetchPath" :allow-label-edit="false" :allow-label-create="false" :allow-multiselect="true" :allow-scoped-labels="true" :selected-labels="selectedLabels" - :labels-fetch-path="labelsFetchPath" + :labels-fetch-path="fetchPath" :labels-manage-path="labelsManagePath" :labels-filter-base-path="labelsFilterBasePath" :labels-list-title="__('Select label')" diff --git a/app/assets/javascripts/boards/index.js b/app/assets/javascripts/boards/index.js index 53825d2c045..1888645ef78 100644 --- a/app/assets/javascripts/boards/index.js +++ b/app/assets/javascripts/boards/index.js @@ -97,7 +97,6 @@ export default () => { currentUserId: gon.current_user_id || null, canUpdate: parseBoolean($boardApp.dataset.canUpdate), canAdminList: parseBoolean($boardApp.dataset.canAdminList), - labelsFetchPath: $boardApp.dataset.labelsFetchPath, labelsManagePath: $boardApp.dataset.labelsManagePath, labelsFilterBasePath: $boardApp.dataset.labelsFilterBasePath, timeTrackingLimitToHours: parseBoolean($boardApp.dataset.timeTrackingLimitToHours), diff --git a/app/assets/javascripts/pipelines/components/graph/graph_component_wrapper.vue b/app/assets/javascripts/pipelines/components/graph/graph_component_wrapper.vue index 4bef6f66be2..9329a35ba99 100644 --- a/app/assets/javascripts/pipelines/components/graph/graph_component_wrapper.vue +++ b/app/assets/javascripts/pipelines/components/graph/graph_component_wrapper.vue @@ -7,7 +7,7 @@ import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { DEFAULT, DRAW_FAILURE, LOAD_FAILURE } from '../../constants'; import DismissPipelineGraphCallout from '../../graphql/mutations/dismiss_pipeline_notification.graphql'; import getUserCallouts from '../../graphql/queries/get_user_callouts.query.graphql'; -import { reportToSentry } from '../../utils'; +import { reportToSentry, reportMessageToSentry } from '../../utils'; import { listByLayers } from '../parsing_utils'; import { IID_FAILURE, LAYER_VIEW, STAGE_VIEW, VIEW_TYPE_KEY } from './constants'; import PipelineGraph from './graph_component.vue'; @@ -109,14 +109,16 @@ export default { }, error(err) { this.reportFailure({ type: LOAD_FAILURE, skipSentry: true }); - reportToSentry( + + reportMessageToSentry( this.$options.name, - `| type: ${LOAD_FAILURE} | - | rawError: ${JSON.stringify(err)} | - | info: ${serializeLoadErrors(err)} | - | graphqlResourceEtag: ${this.graphqlResourceEtag} | - | projectPath: ${this.projectPath} | - | iid: ${this.pipelineIid} |`, + `| type: ${LOAD_FAILURE} , info: ${serializeLoadErrors(err)}`, + { + projectPath: this.projectPath, + pipelineIid: this.pipelineIid, + pipelineStages: this.pipeline?.stages?.length || 0, + nbOfDownstreams: this.pipeline?.downstream?.length || 0, + }, ); }, result({ error }) { diff --git a/app/assets/javascripts/pipelines/components/graph/utils.js b/app/assets/javascripts/pipelines/components/graph/utils.js index 373aa6bf9a1..163b3898c28 100644 --- a/app/assets/javascripts/pipelines/components/graph/utils.js +++ b/app/assets/javascripts/pipelines/components/graph/utils.js @@ -1,3 +1,4 @@ +import { isEmpty } from 'lodash'; import Visibility from 'visibilityjs'; import { getIdFromGraphQLId } from '~/graphql_shared/utils'; import { unwrapStagesWithNeedsAndLookup } from '../unwrapping_utils'; @@ -39,15 +40,15 @@ const serializeGqlErr = (gqlError) => { const serializeLoadErrors = (errors) => { const { gqlError, graphQLErrors, networkError, message } = errors; - if (graphQLErrors) { + if (!isEmpty(graphQLErrors)) { return graphQLErrors.map((err) => serializeGqlErr(err)).join('; '); } - if (gqlError) { + if (!isEmpty(gqlError)) { return serializeGqlErr(gqlError); } - if (networkError) { + if (!isEmpty(networkError)) { return `Network error: ${networkError.message}`; } diff --git a/app/assets/javascripts/pipelines/utils.js b/app/assets/javascripts/pipelines/utils.js index 0a6c326fa3d..800a363cada 100644 --- a/app/assets/javascripts/pipelines/utils.js +++ b/app/assets/javascripts/pipelines/utils.js @@ -73,3 +73,12 @@ export const reportToSentry = (component, failureType) => { Sentry.captureException(failureType); }); }; + +export const reportMessageToSentry = (component, message, context) => { + Sentry.withScope((scope) => { + // eslint-disable-next-line @gitlab/require-i18n-strings + scope.setContext('Vue data', context); + scope.setTag('component', component); + Sentry.captureMessage(message); + }); +}; diff --git a/doc/user/project/code_owners.md b/doc/user/project/code_owners.md index 4fb65608b58..ea7bcce99c1 100644 --- a/doc/user/project/code_owners.md +++ b/doc/user/project/code_owners.md @@ -22,19 +22,19 @@ The GitLab Code Owners feature defines who owns specific files or paths in a repository, allowing other users to understand who is responsible for each file or path. +As an alternative to using Code Owners for approvals, you can instead +[configure rules](merge_requests/approvals/rules.md). + ## Why is this useful? Code Owners allows for a version controlled, single source of truth file outlining the exact GitLab users or groups that -own certain files or paths in a repository. Code Owners can be -used in the merge request approval process which can streamline -the process of finding the right reviewers and approvers for a given -merge request. - -In larger organizations or popular open source projects, Code Owners -can help you understand who to contact if you have -a question that may not be related to code review or a merge request -approval. +own certain files or paths in a repository. In larger organizations +or popular open source projects, Code Owners can help you understand +who to contact if you have a question about a specific portion of +the codebase. Code Owners can also streamline the merge request approval +process, identifying the most relevant reviewers and approvers for a +given change. ## How to set up Code Owners diff --git a/doc/user/project/merge_requests/approvals/index.md b/doc/user/project/merge_requests/approvals/index.md index aa668e85156..1f64eee033c 100644 --- a/doc/user/project/merge_requests/approvals/index.md +++ b/doc/user/project/merge_requests/approvals/index.md @@ -10,14 +10,15 @@ disqus_identifier: 'https://docs.gitlab.com/ee/user/project/merge_requests/merge > Redesign [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/1979) in [GitLab Premium](https://about.gitlab.com/pricing/) 11.8 and [feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/10685) in 12.0. -Successful projects depend on code reviews. Merge request approvals clearly communicate -someone's ability to merge proposed changes. Approvals [are optional](#optional-approvals) -in GitLab Free, but you can require them for your project in higher tiers. - -Merge request approvals are configured at the project level. Administrator users -of self-managed GitLab installations can also configure -[instance-level approval rules](../../../admin_area/merge_requests_approvals.md) -that cannot be overridden on a project-level basis. +You can configure your merge requests so that they must be approved before +they can be merged. You can do this by creating [rules](rules.md) or by specifying +a list of users who act as [code owners](../../code_owners.md) for specific files. + +You can configure merge request approvals for each project. In higher GitLab tiers, +Administrators of self-managed GitLab instances can configure approvals +[for the entire instance](../../../admin_area/merge_requests_approvals.md). + +## How approvals work With [merge request approval rules](rules.md), you can set the minimum number of required approvals before work can merge into your project. You can also extend these diff --git a/spec/features/boards/sidebar_labels_in_namespaces_spec.rb b/spec/features/boards/sidebar_labels_in_namespaces_spec.rb new file mode 100644 index 00000000000..8395a0b33c0 --- /dev/null +++ b/spec/features/boards/sidebar_labels_in_namespaces_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Issue boards sidebar labels select', :js do + include BoardHelpers + + include_context 'labels from nested groups and projects' + + let(:card) { find('.board:nth-child(1)').first('[data-testid="board_card"]') } + + context 'group boards' do + context 'in the top-level group board' do + let_it_be(:group_board) { create(:board, group: group) } + let_it_be(:board_list) { create(:backlog_list, board: group_board) } + + before do + load_board group_board_path(group, group_board) + end + + context 'selecting an issue from a direct descendant project' do + let_it_be(:project_issue) { create(:issue, project: project) } + + include_examples 'an issue from a direct descendant project is selected' + end + + context "selecting an issue from a subgroup's project" do + let_it_be(:subproject_issue) { create(:issue, project: subproject) } + + include_examples "an issue from a subgroup's project is selected" + end + end + end +end diff --git a/spec/features/boards/sub_group_project_spec.rb b/spec/features/boards/sub_group_project_spec.rb deleted file mode 100644 index bde5f061a67..00000000000 --- a/spec/features/boards/sub_group_project_spec.rb +++ /dev/null @@ -1,46 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe 'Sub-group project issue boards', :js do - let(:group) { create(:group) } - let(:nested_group_1) { create(:group, parent: group) } - let(:project) { create(:project, group: nested_group_1) } - let(:board) { create(:board, project: project) } - let(:label) { create(:label, project: project) } - let(:user) { create(:user) } - let!(:list1) { create(:list, board: board, label: label, position: 0) } - let!(:issue) { create(:labeled_issue, project: project, labels: [label]) } - - before do - project.add_maintainer(user) - - sign_in(user) - - visit project_board_path(project, board) - wait_for_requests - end - - # TODO https://gitlab.com/gitlab-org/gitlab/-/issues/324290 - xit 'creates new label from sidebar' do - find('.board-card').click - - page.within '.labels' do - click_link 'Edit' - click_link 'Create project label' - end - - page.within '.dropdown-new-label' do - fill_in 'new_label_name', with: 'test label' - first('.suggest-colors-dropdown a').click - - click_button 'Create' - - wait_for_requests - end - - page.within '.labels' do - expect(page).to have_link 'test label' - end - end -end diff --git a/spec/features/profiles/user_visits_notifications_tab_spec.rb b/spec/features/profiles/user_visits_notifications_tab_spec.rb index 939e791c75d..e960cc76219 100644 --- a/spec/features/profiles/user_visits_notifications_tab_spec.rb +++ b/spec/features/profiles/user_visits_notifications_tab_spec.rb @@ -12,6 +12,14 @@ RSpec.describe 'User visits the notifications tab', :js do visit(profile_notifications_path) end + it 'turns on the receive product marketing emails setting' do + expect(page).to have_content('Notifications') + + expect do + check 'Receive product marketing emails' + end.to change { user.reload.email_opted_in }.to(true) + end + it 'changes the project notifications setting' do expect(page).to have_content('Notifications') diff --git a/spec/frontend/boards/components/sidebar/board_sidebar_labels_select_spec.js b/spec/frontend/boards/components/sidebar/board_sidebar_labels_select_spec.js index 153d0640b23..ad682774ee6 100644 --- a/spec/frontend/boards/components/sidebar/board_sidebar_labels_select_spec.js +++ b/spec/frontend/boards/components/sidebar/board_sidebar_labels_select_spec.js @@ -1,7 +1,11 @@ import { GlLabel } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; import { TEST_HOST } from 'helpers/test_constants'; -import { labels as TEST_LABELS, mockIssue as TEST_ISSUE } from 'jest/boards/mock_data'; +import { + labels as TEST_LABELS, + mockIssue as TEST_ISSUE, + mockIssueFullPath as TEST_ISSUE_FULLPATH, +} from 'jest/boards/mock_data'; import BoardEditableItem from '~/boards/components/sidebar/board_editable_item.vue'; import BoardSidebarLabelsSelect from '~/boards/components/sidebar/board_sidebar_labels_select.vue'; import { createStore } from '~/boards/stores'; @@ -23,7 +27,7 @@ describe('~/boards/components/sidebar/board_sidebar_labels_select.vue', () => { wrapper = null; }); - const createWrapper = ({ labels = [] } = {}) => { + const createWrapper = ({ labels = [], providedValues = {} } = {}) => { store = createStore(); store.state.boardItems = { [TEST_ISSUE.id]: { ...TEST_ISSUE, labels } }; store.state.activeId = TEST_ISSUE.id; @@ -32,9 +36,9 @@ describe('~/boards/components/sidebar/board_sidebar_labels_select.vue', () => { store, provide: { canUpdate: true, - labelsFetchPath: TEST_HOST, labelsManagePath: TEST_HOST, labelsFilterBasePath: TEST_HOST, + ...providedValues, }, stubs: { BoardEditableItem, @@ -48,6 +52,22 @@ describe('~/boards/components/sidebar/board_sidebar_labels_select.vue', () => { wrapper.findAll(GlLabel).wrappers.map((item) => item.props('title')); const findCollapsed = () => wrapper.find('[data-testid="collapsed-content"]'); + describe('when labelsFetchPath is provided', () => { + it('uses injected labels fetch path', () => { + createWrapper({ providedValues: { labelsFetchPath: 'foobar' } }); + + expect(findLabelsSelect().props('labelsFetchPath')).toEqual('foobar'); + }); + }); + + it('uses the default project label endpoint', () => { + createWrapper(); + + expect(findLabelsSelect().props('labelsFetchPath')).toEqual( + `/${TEST_ISSUE_FULLPATH}/-/labels?include_ancestor_groups=true`, + ); + }); + it('renders "None" when no labels are selected', () => { createWrapper(); @@ -78,7 +98,7 @@ describe('~/boards/components/sidebar/board_sidebar_labels_select.vue', () => { it('commits change to the server', () => { expect(wrapper.vm.setActiveBoardItemLabels).toHaveBeenCalledWith({ addLabelIds: TEST_LABELS.map((label) => label.id), - projectPath: 'gitlab-org/test-subgroup/gitlab-test', + projectPath: TEST_ISSUE_FULLPATH, removeLabelIds: [], }); }); @@ -103,7 +123,7 @@ describe('~/boards/components/sidebar/board_sidebar_labels_select.vue', () => { expect(wrapper.vm.setActiveBoardItemLabels).toHaveBeenCalledWith({ addLabelIds: [5, 7], removeLabelIds: [6], - projectPath: 'gitlab-org/test-subgroup/gitlab-test', + projectPath: TEST_ISSUE_FULLPATH, }); }); }); @@ -122,7 +142,7 @@ describe('~/boards/components/sidebar/board_sidebar_labels_select.vue', () => { expect(wrapper.vm.setActiveBoardItemLabels).toHaveBeenCalledWith({ removeLabelIds: [getIdFromGraphQLId(testLabel.id)], - projectPath: 'gitlab-org/test-subgroup/gitlab-test', + projectPath: TEST_ISSUE_FULLPATH, }); }); }); diff --git a/spec/frontend/boards/mock_data.js b/spec/frontend/boards/mock_data.js index 1c5b7cf8248..bcaca9522e4 100644 --- a/spec/frontend/boards/mock_data.js +++ b/spec/frontend/boards/mock_data.js @@ -151,6 +151,8 @@ export const rawIssue = { }, }; +export const mockIssueFullPath = 'gitlab-org/test-subgroup/gitlab-test'; + export const mockIssue = { id: 'gid://gitlab/Issue/436', iid: '27', @@ -159,8 +161,8 @@ export const mockIssue = { timeEstimate: 0, weight: null, confidential: false, - referencePath: 'gitlab-org/test-subgroup/gitlab-test#27', - path: '/gitlab-org/test-subgroup/gitlab-test/-/issues/27', + referencePath: `${mockIssueFullPath}#27`, + path: `/${mockIssueFullPath}/-/issues/27`, assignees, labels: [ { diff --git a/spec/support/helpers/board_helpers.rb b/spec/support/helpers/board_helpers.rb index dff3f9d976c..c4e69d06f52 100644 --- a/spec/support/helpers/board_helpers.rb +++ b/spec/support/helpers/board_helpers.rb @@ -7,4 +7,20 @@ module BoardHelpers wait_for_requests end end + + def load_board(board_path) + visit board_path + + wait_for_requests + end + + def click_card_and_edit_label + click_card(card) + + page.within(labels_select) do + click_button 'Edit' + + wait_for_requests + end + end end diff --git a/spec/support/shared_examples/features/board_sidebar_labels_examples.rb b/spec/support/shared_examples/features/board_sidebar_labels_examples.rb new file mode 100644 index 00000000000..520980c2615 --- /dev/null +++ b/spec/support/shared_examples/features/board_sidebar_labels_examples.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +RSpec.shared_context 'labels from nested groups and projects' do + let_it_be(:group) { create(:group) } + let_it_be(:group_label) { create(:group_label, group: group, name: 'Group label') } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:project_label) { create(:label, project: project, name: 'Project label') } + + let_it_be(:subgroup) { create(:group, parent: group) } + let_it_be(:subgroup_label) { create(:group_label, group: subgroup, name: 'Subgroup label') } + let_it_be(:subproject) { create(:project, group: subgroup) } + let_it_be(:subproject_label) { create(:label, project: subproject, name: 'Subproject label') } + + let_it_be(:subgroup2) { create(:group, parent: group) } + let_it_be(:subgroup2_label) { create(:group_label, group: subgroup2, name: 'Subgroup2 label') } + + let_it_be(:maintainer) { create(:user) } + + let(:labels_select) { find("[data-testid='sidebar-labels']") } + let(:labels_dropdown) { labels_select.find('[data-testid="dropdown-content"]')} + + before do + group.add_maintainer(maintainer) + + sign_in(maintainer) + end +end + +RSpec.shared_examples "an issue from a subgroup's project is selected" do + context 'when editing labels' do + before do + click_card_and_edit_label + end + + it 'displays the label from the top-level group' do + expect(labels_dropdown).to have_content(group_label.name) + end + + it 'displays the label from the subgroup' do + expect(labels_dropdown).to have_content(subgroup_label.name) + end + + it 'displays the label from the project' do + expect(labels_dropdown).to have_content(subproject_label.name) + end + + it "does not display labels from the subgroup's siblings (project or group)" do + aggregate_failures do + expect(labels_dropdown).not_to have_content(project_label.name) + expect(labels_dropdown).not_to have_content(subgroup2_label.name) + end + end + end +end + +RSpec.shared_examples 'an issue from a direct descendant project is selected' do + context 'when editing labels' do + before do + click_card_and_edit_label + end + + it 'displays the label from the top-level group' do + expect(labels_dropdown).to have_content(group_label.name) + end + + it 'displays the label from the project' do + expect(labels_dropdown).to have_content(project_label.name) + end + + it "does not display labels from the project's siblings or their descendents" do + aggregate_failures do + expect(labels_dropdown).not_to have_content(subgroup_label.name) + expect(labels_dropdown).not_to have_content(subproject_label.name) + end + end + end +end |