diff options
21 files changed, 466 insertions, 206 deletions
diff --git a/app/assets/javascripts/alert_management/components/alert_management_table.vue b/app/assets/javascripts/alert_management/components/alert_management_table.vue index 0fd00fe90eb..fc87252f772 100644 --- a/app/assets/javascripts/alert_management/components/alert_management_table.vue +++ b/app/assets/javascripts/alert_management/components/alert_management_table.vue @@ -267,8 +267,8 @@ export default { this.searchTerm = trimmedInput; } }, 500), - navigateToAlertDetails({ iid }) { - return visitUrl(joinPaths(window.location.pathname, iid, 'details')); + navigateToAlertDetails({ iid }, index, { metaKey }) { + return visitUrl(joinPaths(window.location.pathname, iid, 'details'), metaKey); }, trackPageViews() { const { category, action } = trackAlertListViewsOptions; diff --git a/app/assets/javascripts/serverless/components/functions.vue b/app/assets/javascripts/serverless/components/functions.vue index e15549f5864..d662cc7b802 100644 --- a/app/assets/javascripts/serverless/components/functions.vue +++ b/app/assets/javascripts/serverless/components/functions.vue @@ -1,7 +1,6 @@ <script> -/* eslint-disable vue/no-v-html */ import { mapState, mapActions, mapGetters } from 'vuex'; -import { GlLink, GlLoadingIcon } from '@gitlab/ui'; +import { GlLink, GlLoadingIcon, GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui'; import { sprintf, s__ } from '~/locale'; import EnvironmentRow from './environment_row.vue'; import EmptyState from './empty_state.vue'; @@ -14,6 +13,9 @@ export default { GlLink, GlLoadingIcon, }, + directives: { + SafeHtml, + }, computed: { ...mapState(['installed', 'isLoading', 'hasFunctionData', 'helpPath', 'statusPath']), ...mapGetters(['getFunctions']), @@ -92,9 +94,9 @@ export default { }} </p> <ul> - <li v-html="noServerlessConfigFile"></li> - <li v-html="noGitlabYamlConfigured"></li> - <li v-html="mismatchedServerlessFunctions"></li> + <li v-safe-html="noServerlessConfigFile"></li> + <li v-safe-html="noGitlabYamlConfigured"></li> + <li v-safe-html="mismatchedServerlessFunctions"></li> <li>{{ s__('Serverless|The deploy job has not finished.') }}</li> </ul> diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index b2732d83aac..3d41218a29e 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -71,4 +71,5 @@ module Types end ::Types::MutationType.prepend(::Types::DeprecatedMutations) +::Types::MutationType.prepend_if_ee('EE::Types::DeprecatedMutations') ::Types::MutationType.prepend_if_ee('::EE::Types::MutationType') diff --git a/app/views/layouts/_page.html.haml b/app/views/layouts/_page.html.haml index 5184bc93a81..9b925369660 100644 --- a/app/views/layouts/_page.html.haml +++ b/app/views/layouts/_page.html.haml @@ -19,7 +19,6 @@ = yield :customize_homepage_banner - unless @hide_breadcrumbs = render "layouts/nav/breadcrumbs" - .d-flex %div{ class: "#{(container_class unless @no_container)} #{@content_class}" } .content{ id: "content-body" } = render "layouts/flash", extra_flash_class: 'limit-container-width' diff --git a/app/views/profiles/two_factor_auths/show.html.haml b/app/views/profiles/two_factor_auths/show.html.haml index 82265938180..d6a4ac09915 100644 --- a/app/views/profiles/two_factor_auths/show.html.haml +++ b/app/views/profiles/two_factor_auths/show.html.haml @@ -102,7 +102,12 @@ %tbody - @registrations.each do |registration| %tr - %td= registration[:name].presence || html_escape_once(_("<no name set>")).html_safe + %td + - if registration[:name].present? + = registration[:name] + - else + %span.gl-text-gray-500 + = _("no name set") %td= registration[:created_at].to_date.to_s(:medium) %td= link_to _('Delete'), registration[:delete_path], method: :delete, class: "btn btn-danger float-right", data: { confirm: _('Are you sure you want to delete this device? This action cannot be undone.') } diff --git a/changelogs/unreleased/212373-249588-enable-ci_new_artifact_file_reader.yml b/changelogs/unreleased/212373-249588-enable-ci_new_artifact_file_reader.yml new file mode 100644 index 00000000000..c5b485e9951 --- /dev/null +++ b/changelogs/unreleased/212373-249588-enable-ci_new_artifact_file_reader.yml @@ -0,0 +1,5 @@ +--- +title: Fix triggering multiple children pipeline with the same artifact +merge_request: 42595 +author: +type: fixed diff --git a/changelogs/unreleased/225655-alert-new-page.yml b/changelogs/unreleased/225655-alert-new-page.yml new file mode 100644 index 00000000000..baae40eccfb --- /dev/null +++ b/changelogs/unreleased/225655-alert-new-page.yml @@ -0,0 +1,5 @@ +--- +title: Allow alerts to open on new tab +merge_request: 42691 +author: +type: changed diff --git a/changelogs/unreleased/247476-standardise-usage-of-angle-brackets.yml b/changelogs/unreleased/247476-standardise-usage-of-angle-brackets.yml new file mode 100644 index 00000000000..eec8ad098a1 --- /dev/null +++ b/changelogs/unreleased/247476-standardise-usage-of-angle-brackets.yml @@ -0,0 +1,5 @@ +--- +title: Remove angle brackets from empty name in U2F device settings +merge_request: 42440 +author: +type: changed diff --git a/changelogs/unreleased/drop-an-element.yml b/changelogs/unreleased/drop-an-element.yml new file mode 100644 index 00000000000..958d7a47678 --- /dev/null +++ b/changelogs/unreleased/drop-an-element.yml @@ -0,0 +1,5 @@ +--- +title: Remove an unnecessary element from every page +merge_request: 42769 +author: Takuya Noguchi +type: other diff --git a/config/feature_flags/development/ci_new_artifact_file_reader.yml b/config/feature_flags/development/ci_new_artifact_file_reader.yml index a6e9c67bd7e..9c0ad7a2d9c 100644 --- a/config/feature_flags/development/ci_new_artifact_file_reader.yml +++ b/config/feature_flags/development/ci_new_artifact_file_reader.yml @@ -4,4 +4,4 @@ introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40268 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/249588 group: group::pipeline authoring type: development -default_enabled: false +default_enabled: true diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql index dde1412bcfd..22224022325 100644 --- a/doc/api/graphql/reference/gitlab_schema.graphql +++ b/doc/api/graphql/reference/gitlab_schema.graphql @@ -10812,7 +10812,7 @@ type Mutation { Toggles the resolved state of a discussion """ discussionToggleResolve(input: DiscussionToggleResolveInput!): DiscussionToggleResolvePayload - dismissVulnerability(input: DismissVulnerabilityInput!): DismissVulnerabilityPayload + dismissVulnerability(input: DismissVulnerabilityInput!): DismissVulnerabilityPayload @deprecated(reason: "Use vulnerabilityDismiss. Deprecated in 13.5") epicAddIssue(input: EpicAddIssueInput!): EpicAddIssuePayload epicSetSubscription(input: EpicSetSubscriptionInput!): EpicSetSubscriptionPayload epicTreeReorder(input: EpicTreeReorderInput!): EpicTreeReorderPayload @@ -10876,6 +10876,7 @@ type Mutation { updateRequirement(input: UpdateRequirementInput!): UpdateRequirementPayload updateSnippet(input: UpdateSnippetInput!): UpdateSnippetPayload vulnerabilityConfirm(input: VulnerabilityConfirmInput!): VulnerabilityConfirmPayload + vulnerabilityDismiss(input: VulnerabilityDismissInput!): VulnerabilityDismissPayload vulnerabilityResolve(input: VulnerabilityResolveInput!): VulnerabilityResolvePayload } @@ -18837,6 +18838,46 @@ type VulnerabilityConnection { } """ +Autogenerated input type of VulnerabilityDismiss +""" +input VulnerabilityDismissInput { + """ + A unique identifier for the client performing the mutation. + """ + clientMutationId: String + + """ + Reason why vulnerability should be dismissed + """ + comment: String + + """ + ID of the vulnerability to be dismissed + """ + id: ID! +} + +""" +Autogenerated return type of VulnerabilityDismiss +""" +type VulnerabilityDismissPayload { + """ + A unique identifier for the client performing the mutation. + """ + clientMutationId: String + + """ + Errors encountered during execution of the mutation. + """ + errors: [String!]! + + """ + The vulnerability after dismissal + """ + vulnerability: Vulnerability +} + +""" An edge in a connection. """ type VulnerabilityEdge { diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json index 9c2357cdc35..71642c465f7 100644 --- a/doc/api/graphql/reference/gitlab_schema.json +++ b/doc/api/graphql/reference/gitlab_schema.json @@ -31135,8 +31135,8 @@ "name": "DismissVulnerabilityPayload", "ofType": null }, - "isDeprecated": false, - "deprecationReason": null + "isDeprecated": true, + "deprecationReason": "Use vulnerabilityDismiss. Deprecated in 13.5" }, { "name": "epicAddIssue", @@ -32435,6 +32435,33 @@ "deprecationReason": null }, { + "name": "vulnerabilityDismiss", + "description": null, + "args": [ + { + "name": "input", + "description": null, + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "INPUT_OBJECT", + "name": "VulnerabilityDismissInput", + "ofType": null + } + }, + "defaultValue": null + } + ], + "type": { + "kind": "OBJECT", + "name": "VulnerabilityDismissPayload", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + }, + { "name": "vulnerabilityResolve", "description": null, "args": [ @@ -55101,6 +55128,118 @@ "possibleTypes": null }, { + "kind": "INPUT_OBJECT", + "name": "VulnerabilityDismissInput", + "description": "Autogenerated input type of VulnerabilityDismiss", + "fields": null, + "inputFields": [ + { + "name": "id", + "description": "ID of the vulnerability to be dismissed", + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "ID", + "ofType": null + } + }, + "defaultValue": null + }, + { + "name": "comment", + "description": "Reason why vulnerability should be dismissed", + "type": { + "kind": "SCALAR", + "name": "String", + "ofType": null + }, + "defaultValue": null + }, + { + "name": "clientMutationId", + "description": "A unique identifier for the client performing the mutation.", + "type": { + "kind": "SCALAR", + "name": "String", + "ofType": null + }, + "defaultValue": null + } + ], + "interfaces": null, + "enumValues": null, + "possibleTypes": null + }, + { + "kind": "OBJECT", + "name": "VulnerabilityDismissPayload", + "description": "Autogenerated return type of VulnerabilityDismiss", + "fields": [ + { + "name": "clientMutationId", + "description": "A unique identifier for the client performing the mutation.", + "args": [ + + ], + "type": { + "kind": "SCALAR", + "name": "String", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "errors", + "description": "Errors encountered during execution of the mutation.", + "args": [ + + ], + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "LIST", + "name": null, + "ofType": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "String", + "ofType": null + } + } + } + }, + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "vulnerability", + "description": "The vulnerability after dismissal", + "args": [ + + ], + "type": { + "kind": "OBJECT", + "name": "Vulnerability", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + } + ], + "inputFields": null, + "interfaces": [ + + ], + "enumValues": null, + "possibleTypes": null + }, + { "kind": "OBJECT", "name": "VulnerabilityEdge", "description": "An edge in a connection.", diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 46910357229..d43f9f021e2 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -2710,6 +2710,16 @@ Autogenerated return type of VulnerabilityConfirm. | `errors` | String! => Array | Errors encountered during execution of the mutation. | | `vulnerability` | Vulnerability | The vulnerability after state change | +### VulnerabilityDismissPayload + +Autogenerated return type of VulnerabilityDismiss. + +| Field | Type | Description | +| ----- | ---- | ----------- | +| `clientMutationId` | String | A unique identifier for the client performing the mutation. | +| `errors` | String! => Array | Errors encountered during execution of the mutation. | +| `vulnerability` | Vulnerability | The vulnerability after dismissal | + ### VulnerabilityIdentifier Represents a vulnerability identifier. diff --git a/haml_lint/linter/documentation_links.rb b/haml_lint/linter/documentation_links.rb index f8e0eec5cdc..a36b095bc11 100644 --- a/haml_lint/linter/documentation_links.rb +++ b/haml_lint/linter/documentation_links.rb @@ -13,7 +13,7 @@ module HamlLint DOCS_DIRECTORY = File.join(File.expand_path('../..', __dir__), 'doc') HELP_PATH_LINK_PATTERN = <<~PATTERN - `(send nil? :help_page_path $...) + `(send nil? {:help_page_url :help_page_path} $...) PATTERN MARKDOWN_HEADER = %r{\A\#{1,6}\s+(?<header>.+)\Z}.freeze diff --git a/lib/gitlab/ci/features.rb b/lib/gitlab/ci/features.rb index e770187b124..6bde1c12ce9 100644 --- a/lib/gitlab/ci/features.rb +++ b/lib/gitlab/ci/features.rb @@ -72,7 +72,7 @@ module Gitlab end def self.new_artifact_file_reader_enabled?(project) - ::Feature.enabled?(:ci_new_artifact_file_reader, project, default_enabled: false) + ::Feature.enabled?(:ci_new_artifact_file_reader, project, default_enabled: true) end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 1a3525408c6..fe45045996f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -857,9 +857,6 @@ msgstr "" msgid "< 1 hour" msgstr "" -msgid "<no name set>" -msgstr "" - msgid "<no scopes selected>" msgstr "" @@ -30684,6 +30681,9 @@ msgstr "" msgid "no expiration" msgstr "" +msgid "no name set" +msgstr "" + msgid "no one can merge" msgstr "" diff --git a/qa/qa/page/project/packages/index.rb b/qa/qa/page/project/packages/index.rb index 6d55d1d04b6..396d3373b8a 100644 --- a/qa/qa/page/project/packages/index.rb +++ b/qa/qa/page/project/packages/index.rb @@ -26,3 +26,5 @@ module QA end end end + +QA::Page::Project::Packages::Index.prepend_if_ee('QA::EE::Page::Project::Packages::Index') diff --git a/qa/qa/support/wait_for_requests.rb b/qa/qa/support/wait_for_requests.rb index 943d7d510df..2a72f833edc 100644 --- a/qa/qa/support/wait_for_requests.rb +++ b/qa/qa/support/wait_for_requests.rb @@ -11,6 +11,8 @@ module QA Waiter.wait_until(log: false) do finished_all_ajax_requests? && finished_all_axios_requests? && (!skip_finished_loading_check ? finished_loading?(wait: 1) : true) end + rescue Repeater::WaitExceededError + raise $!, 'Page did not fully load. This could be due to an unending async request or loading icon.' end def finished_all_axios_requests? diff --git a/spec/frontend/alert_management/components/alert_management_table_spec.js b/spec/frontend/alert_management/components/alert_management_table_spec.js index bcad415eb19..63050ece462 100644 --- a/spec/frontend/alert_management/components/alert_management_table_spec.js +++ b/spec/frontend/alert_management/components/alert_management_table_spec.js @@ -295,10 +295,30 @@ describe('AlertManagementTable', () => { loading: false, }); + expect(visitUrl).not.toHaveBeenCalled(); + findAlerts() .at(0) .trigger('click'); - expect(visitUrl).toHaveBeenCalledWith('/1527542/details'); + expect(visitUrl).toHaveBeenCalledWith('/1527542/details', false); + }); + + it('navigates to the detail page in new tab when alert row is clicked with the metaKey', () => { + mountComponent({ + props: { alertManagementEnabled: true, userCanEnableAlertManagement: true }, + data: { alerts: { list: mockAlerts }, alertsCount, hasError: false }, + loading: false, + }); + + expect(visitUrl).not.toHaveBeenCalled(); + + findAlerts() + .at(0) + .trigger('click', { + metaKey: true, + }); + + expect(visitUrl).toHaveBeenCalledWith('/1527542/details', true); }); describe('alert issue links', () => { diff --git a/spec/haml_lint/linter/documentation_links_spec.rb b/spec/haml_lint/linter/documentation_links_spec.rb index 68de8317b82..5de455b6e8c 100644 --- a/spec/haml_lint/linter/documentation_links_spec.rb +++ b/spec/haml_lint/linter/documentation_links_spec.rb @@ -8,75 +8,85 @@ require Rails.root.join('haml_lint/linter/documentation_links') RSpec.describe HamlLint::Linter::DocumentationLinks do include_context 'linter' - context 'when link_to points to the existing file path' do - let(:haml) { "= link_to 'Description', help_page_path('README.md')" } + shared_examples 'link validation rules' do |link_pattern| + context 'when link_to points to the existing file path' do + let(:haml) { "= link_to 'Description', #{link_pattern}('README.md')" } - it { is_expected.not_to report_lint } - end + it { is_expected.not_to report_lint } + end - context 'when link_to points to the existing file with valid anchor' do - let(:haml) { "= link_to 'Description', help_page_path('README.md', anchor: 'overview'), target: '_blank'" } + context 'when link_to points to the existing file with valid anchor' do + let(:haml) { "= link_to 'Description', #{link_pattern}('README.md', anchor: 'overview'), target: '_blank'" } - it { is_expected.not_to report_lint } - end + it { is_expected.not_to report_lint } + end - context 'when link_to points to the existing file path without .md extension' do - let(:haml) { "= link_to 'Description', help_page_path('README')" } + context 'when link_to points to the existing file path without .md extension' do + let(:haml) { "= link_to 'Description', #{link_pattern}('README')" } - it { is_expected.not_to report_lint } - end + it { is_expected.not_to report_lint } + end - context 'when anchor is not correct' do - let(:haml) { "= link_to 'Description', help_page_path('README.md', anchor: 'wrong')" } + context 'when anchor is not correct' do + let(:haml) { "= link_to 'Description', #{link_pattern}('README.md', anchor: 'wrong')" } - it { is_expected.to report_lint } + it { is_expected.to report_lint } - context 'when help_page_path has multiple options' do - let(:haml) { "= link_to 'Description', help_page_path('README.md', key: :value, anchor: 'wrong')" } + context "when #{link_pattern} has multiple options" do + let(:haml) { "= link_to 'Description', #{link_pattern}('README.md', key: :value, anchor: 'wrong')" } + + it { is_expected.to report_lint } + end + end + + context 'when file path is wrong' do + let(:haml) { "= link_to 'Description', #{link_pattern}('wrong.md'), target: '_blank'" } it { is_expected.to report_lint } end - end - context 'when file path is wrong' do - let(:haml) { "= link_to 'Description', help_page_path('wrong.md'), target: '_blank'" } + context 'when link with wrong file path is assigned to a variable' do + let(:haml) { "- my_link = link_to 'Description', #{link_pattern}('wrong.md')" } - it { is_expected.to report_lint } - end + it { is_expected.to report_lint } + end - context 'when link with wrong file path is assigned to a variable' do - let(:haml) { "- my_link = link_to 'Description', help_page_path('wrong.md')" } + context 'when it is a broken code' do + let(:haml) { "= I am broken! ]]]]" } - it { is_expected.to report_lint } - end + it { is_expected.not_to report_lint } + end - context 'when it is a broken code' do - let(:haml) { "= I am broken! ]]]]" } + context 'when anchor belongs to a different element' do + let(:haml) { "= link_to 'Description', #{link_pattern}('README.md'), target: (anchor: 'blank')" } - it { is_expected.not_to report_lint } - end + it { is_expected.not_to report_lint } + end - context 'when anchor belongs to a different element' do - let(:haml) { "= link_to 'Description', help_page_path('README.md'), target: (anchor: 'blank')" } + context "when a simple #{link_pattern}" do + let(:haml) { "- url = #{link_pattern}('wrong.md')" } - it { is_expected.not_to report_lint } - end + it { is_expected.to report_lint } + end - context 'when a simple help_page_path' do - let(:haml) { "- url = help_page_path('wrong.md')" } + context 'when link is not a string' do + let(:haml) { "- url = #{link_pattern}(help_url)" } - it { is_expected.to report_lint } - end + it { is_expected.not_to report_lint } + end - context 'when link is not a string' do - let(:haml) { "- url = help_page_path(help_url)" } + context 'when link is a part of the tag' do + let(:haml) { ".data-form{ data: { url: #{link_pattern}('wrong.md') } }" } - it { is_expected.not_to report_lint } + it { is_expected.to report_lint } + end end - context 'when link is a part of the tag' do - let(:haml) { ".data-form{ data: { url: help_page_path('wrong.md') } }" } + context 'help_page_path' do + it_behaves_like 'link validation rules', 'help_page_path' + end - it { is_expected.to report_lint } + context 'help_page_url' do + it_behaves_like 'link validation rules', 'help_page_url' end end diff --git a/spec/services/todos/destroy/entity_leave_service_spec.rb b/spec/services/todos/destroy/entity_leave_service_spec.rb index ccafe3bb7a8..30083357d23 100644 --- a/spec/services/todos/destroy/entity_leave_service_spec.rb +++ b/spec/services/todos/destroy/entity_leave_service_spec.rb @@ -3,77 +3,111 @@ require 'spec_helper' RSpec.describe Todos::Destroy::EntityLeaveService do - let(:group) { create(:group, :private) } - let(:project) { create(:project, group: group) } - let(:user) { create(:user) } - let(:user2) { create(:user) } - let(:issue) { create(:issue, project: project, confidential: true) } - let(:mr) { create(:merge_request, source_project: project) } - - let!(:todo_mr_user) { create(:todo, user: user, target: mr, project: project) } - let!(:todo_issue_user) { create(:todo, user: user, target: issue, project: project) } - let!(:todo_group_user) { create(:todo, user: user, group: group) } - let!(:todo_issue_user2) { create(:todo, user: user2, target: issue, project: project) } - let!(:todo_group_user2) { create(:todo, user: user2, group: group) } + let_it_be(:group, reload: true) { create(:group, :private) } + let_it_be(:project, reload: true) { create(:project, group: group) } + let_it_be(:user, reload: true) { create(:user) } + let_it_be(:user2, reload: true) { create(:user) } + let_it_be(:issue) { create(:issue, project: project) } + let_it_be(:issue_c) { create(:issue, project: project, confidential: true) } + let_it_be(:todo_group_user) { create(:todo, user: user, group: group) } + let_it_be(:todo_group_user2) { create(:todo, user: user2, group: group) } + + let(:mr) { create(:merge_request, source_project: project) } + let!(:todo_mr_user) { create(:todo, user: user, target: mr, project: project) } + let!(:todo_issue_user) { create(:todo, user: user, target: issue, project: project) } + let!(:todo_issue_c_user) { create(:todo, user: user, target: issue_c, project: project) } + let!(:todo_issue_c_user2) { create(:todo, user: user2, target: issue_c, project: project) } + + shared_examples 'using different access permissions' do |access_table| + using RSpec::Parameterized::TableSyntax + + where(:group_access, :project_access, :c_todos, :mr_todos, :method, &access_table) + + with_them do + before do + set_access(project, user, project_access) if project_access + set_access(group, user, group_access) if group_access + end - describe '#execute' do - context 'when a user leaves a project' do - subject { described_class.new(user.id, project.id, 'Project').execute } + it "#{params[:method].to_s.humanize(capitalize: false)}" do + send(method) + end + end + end - context 'when project is private' do - it 'removes project todos for the provided user' do - expect { subject }.to change { Todo.count }.from(5).to(3) + shared_examples 'does not remove any todos' do + it { does_not_remove_any_todos } + end - expect(user.todos).to match_array([todo_group_user]) - expect(user2.todos).to match_array([todo_issue_user2, todo_group_user2]) - end + shared_examples 'removes only confidential issues todos' do + it { removes_only_confidential_issues_todos } + end - context 'when the user is member of the project' do - before do - project.add_developer(user) - end + shared_examples 'removes confidential issues and merge request todos' do + it { removes_confidential_issues_and_merge_request_todos } + end - it 'does not remove any todos' do - expect { subject }.not_to change { Todo.count } - end - end + def does_not_remove_any_todos + expect { subject }.not_to change { Todo.count } + end - context 'when the user is a project guest' do - before do - project.add_guest(user) - end + def removes_only_confidential_issues_todos + expect { subject }.to change { Todo.count }.from(6).to(5) + end - it 'removes only confidential issues todos' do - expect { subject }.to change { Todo.count }.from(5).to(4) - end - end + def removes_confidential_issues_and_merge_request_todos + expect { subject }.to change { Todo.count }.from(6).to(4) + expect(user.todos).to match_array([todo_issue_user, todo_group_user]) + end - context 'when the user is member of a parent group' do - before do - group.add_developer(user) - end + def set_access(object, user, access_name) + case access_name + when :developer + object.add_developer(user) + when :reporter + object.add_reporter(user) + when :guest + object.add_guest(user) + end + end + + describe '#execute' do + describe 'updating a Project' do + subject { described_class.new(user.id, project.id, 'Project').execute } + + context 'when project is private' do + context 'when a user leaves a project' do + it 'removes project todos for the provided user' do + expect { subject }.to change { Todo.count }.from(6).to(3) - it 'does not remove any todos' do - expect { subject }.not_to change { Todo.count } + expect(user.todos).to match_array([todo_group_user]) + expect(user2.todos).to match_array([todo_issue_c_user2, todo_group_user2]) end end - context 'when the user is guest of a parent group' do - before do - project.add_guest(user) - end + context 'access permissions' do + # rubocop:disable RSpec/LeakyConstantDeclaration + PROJECT_PRIVATE_ACCESS_TABLE = + lambda do |_| + [ + # :group_access, :project_access, :c_todos, :mr_todos, :method + [nil, :reporter, :keep, :keep, :does_not_remove_any_todos], + [nil, :guest, :delete, :keep, :removes_only_confidential_issues_todos], + [:reporter, nil, :keep, :keep, :does_not_remove_any_todos], + [:guest, nil, :delete, :keep, :removes_only_confidential_issues_todos] + ] + end + # rubocop:enable RSpec/LeakyConstantDeclaration - it 'removes only confidential issues todos' do - expect { subject }.to change { Todo.count }.from(5).to(4) - end + it_behaves_like 'using different access permissions', PROJECT_PRIVATE_ACCESS_TABLE end end context 'when project is not private' do - before do - group.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) - project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) - end + let_it_be(:group, reload: true) { create(:group, :internal) } + let_it_be(:project, reload: true) { create(:project, :internal, group: group) } + let_it_be(:issue, reload: true) { create(:issue, project: project) } + let_it_be(:issue_c, reload: true) { create(:issue, project: project, confidential: true) } it 'enqueues the PrivateFeaturesWorker' do expect(TodosDestroyer::PrivateFeaturesWorker) @@ -84,50 +118,41 @@ RSpec.describe Todos::Destroy::EntityLeaveService do context 'confidential issues' do context 'when a user is not an author of confidential issue' do - it 'removes only confidential issues todos' do - expect { subject }.to change { Todo.count }.from(5).to(4) - end + it_behaves_like 'removes only confidential issues todos' end context 'when a user is an author of confidential issue' do before do - issue.update!(author: user) + issue_c.update!(author: user) end - it 'does not remove any todos' do - expect { subject }.not_to change { Todo.count } - end + it_behaves_like 'does not remove any todos' end context 'when a user is an assignee of confidential issue' do before do - issue.assignees << user - end - - it 'does not remove any todos' do - expect { subject }.not_to change { Todo.count } - end - end - - context 'when a user is a project guest' do - before do - project.add_guest(user) + issue_c.assignees << user end - it 'removes only confidential issues todos' do - expect { subject }.to change { Todo.count }.from(5).to(4) - end + it_behaves_like 'does not remove any todos' end - context 'when a user is a project guest but group developer' do - before do - project.add_guest(user) - group.add_developer(user) - end - - it 'does not remove any todos' do - expect { subject }.not_to change { Todo.count } - end + context 'access permissions' do + # rubocop:disable RSpec/LeakyConstantDeclaration + PROJECT_NOT_PRIVATE_ACCESS_TABLE = + lambda do |_| + [ + # :group_access, :project_access, :c_todos, :mr_todos, :method + [nil, :reporter, :keep, :keep, :does_not_remove_any_todos], + [nil, :guest, :delete, :keep, :removes_only_confidential_issues_todos], + [:reporter, nil, :keep, :keep, :does_not_remove_any_todos], + [:guest, nil, :delete, :keep, :removes_only_confidential_issues_todos], + [:reporter, :guest, :keep, :keep, :does_not_remove_any_todos] + ] + end + # rubocop:enable RSpec/LeakyConstantDeclaration + + it_behaves_like 'using different access permissions', PROJECT_NOT_PRIVATE_ACCESS_TABLE end end @@ -138,42 +163,39 @@ RSpec.describe Todos::Destroy::EntityLeaveService do end it 'removes only users issue todos' do - expect { subject }.to change { Todo.count }.from(5).to(4) + expect { subject }.to change { Todo.count }.from(6).to(5) end end end end end - context 'when a user leaves a group' do + describe 'updating a Group' do subject { described_class.new(user.id, group.id, 'Group').execute } context 'when group is private' do - it 'removes group and subproject todos for the user' do - expect { subject }.to change { Todo.count }.from(5).to(2) - - expect(user.todos).to be_empty - expect(user2.todos).to match_array([todo_issue_user2, todo_group_user2]) - end - - context 'when the user is member of the group' do - before do - group.add_developer(user) - end + context 'when a user leaves a group' do + it 'removes group and subproject todos for the user' do + expect { subject }.to change { Todo.count }.from(6).to(2) - it 'does not remove any todos' do - expect { subject }.not_to change { Todo.count } + expect(user.todos).to be_empty + expect(user2.todos).to match_array([todo_issue_c_user2, todo_group_user2]) end end - context 'when the user is member of the group project but not the group' do - before do - project.add_developer(user) - end + context 'access permissions' do + # rubocop:disable RSpec/LeakyConstantDeclaration + GROUP_PRIVATE_ACCESS_TABLE = + lambda do |_| + [ + # :group_access, :project_access, :c_todos, :mr_todos, :method + [nil, :reporter, :keep, :keep, :does_not_remove_any_todos], + [:reporter, nil, :keep, :keep, :does_not_remove_any_todos] + ] + end + # rubocop:enable RSpec/LeakyConstantDeclaration - it 'does not remove any todos' do - expect { subject }.not_to change { Todo.count } - end + it_behaves_like 'using different access permissions', GROUP_PRIVATE_ACCESS_TABLE end context 'with nested groups' do @@ -191,12 +213,12 @@ RSpec.describe Todos::Destroy::EntityLeaveService do context 'when the user is not a member of any groups/projects' do it 'removes todos for the user including subprojects todos' do - expect { subject }.to change { Todo.count }.from(11).to(4) + expect { subject }.to change { Todo.count }.from(12).to(4) expect(user.todos).to be_empty expect(user2.todos) .to match_array( - [todo_issue_user2, todo_group_user2, todo_subproject_user2, todo_subpgroup_user2] + [todo_issue_c_user2, todo_group_user2, todo_subproject_user2, todo_subpgroup_user2] ) end end @@ -208,9 +230,7 @@ RSpec.describe Todos::Destroy::EntityLeaveService do parent_group.add_developer(user) end - it 'does not remove any todos' do - expect { subject }.not_to change { Todo.count } - end + it_behaves_like 'does not remove any todos' end context 'when the user is member of a subgroup' do @@ -219,12 +239,12 @@ RSpec.describe Todos::Destroy::EntityLeaveService do end it 'does not remove group and subproject todos' do - expect { subject }.to change { Todo.count }.from(11).to(7) + expect { subject }.to change { Todo.count }.from(12).to(7) expect(user.todos).to match_array([todo_group_user, todo_subgroup_user, todo_subproject_user]) expect(user2.todos) .to match_array( - [todo_issue_user2, todo_group_user2, todo_subproject_user2, todo_subpgroup_user2] + [todo_issue_c_user2, todo_group_user2, todo_subproject_user2, todo_subpgroup_user2] ) end end @@ -235,12 +255,12 @@ RSpec.describe Todos::Destroy::EntityLeaveService do end it 'does not remove subproject and group todos' do - expect { subject }.to change { Todo.count }.from(11).to(7) + expect { subject }.to change { Todo.count }.from(12).to(7) expect(user.todos).to match_array([todo_subgroup_user, todo_group_user, todo_subproject_user]) expect(user2.todos) .to match_array( - [todo_issue_user2, todo_group_user2, todo_subproject_user2, todo_subpgroup_user2] + [todo_issue_c_user2, todo_group_user2, todo_subproject_user2, todo_subpgroup_user2] ) end end @@ -248,10 +268,10 @@ RSpec.describe Todos::Destroy::EntityLeaveService do end context 'when group is not private' do - before do - group.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) - project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) - end + let_it_be(:group, reload: true) { create(:group, :internal) } + let_it_be(:project, reload: true) { create(:project, :internal, group: group) } + let_it_be(:issue, reload: true) { create(:issue, project: project) } + let_it_be(:issue_c, reload: true) { create(:issue, project: project, confidential: true) } it 'enqueues the PrivateFeaturesWorker' do expect(TodosDestroyer::PrivateFeaturesWorker) @@ -260,31 +280,20 @@ RSpec.describe Todos::Destroy::EntityLeaveService do subject end - context 'when user is not member' do - it 'removes only confidential issues todos' do - expect { subject }.to change { Todo.count }.from(5).to(4) - end - end - - context 'when user is a project guest' do - before do - project.add_guest(user) - end - - it 'removes only confidential issues todos' do - expect { subject }.to change { Todo.count }.from(5).to(4) - end - end - - context 'when user is a project guest & group developer' do - before do - project.add_guest(user) - group.add_developer(user) - end + context 'access permissions' do + # rubocop:disable RSpec/LeakyConstantDeclaration + GROUP_NOT_PRIVATE_ACCESS_TABLE = + lambda do |_| + [ + # :group_access, :project_access, :c_todos, :mr_todos, :method + [nil, nil, :delete, :keep, :removes_only_confidential_issues_todos], + [nil, :guest, :delete, :keep, :removes_only_confidential_issues_todos], + [:reporter, :guest, :keep, :keep, :does_not_remove_any_todos] + ] + end + # rubocop:enable RSpec/LeakyConstantDeclaration - it 'does not remove any todos' do - expect { subject }.not_to change { Todo.count } - end + it_behaves_like 'using different access permissions', GROUP_NOT_PRIVATE_ACCESS_TABLE end end end |