From 7aa009028662fc24851f2ceb25f6fd40da6e437e Mon Sep 17 00:00:00 2001 From: Austin Regnery Date: Wed, 1 Nov 2023 13:14:20 -0400 Subject: [PATCH 1/2] Update locked issuable text Changelog: other Update sidebar edit message Fix a batch of tests Fix more tests and snapshots Fix update service specs Update terminology in docs --- .../issuable/components/locked_badge.vue | 9 ++- .../components/discussion_locked_widget.vue | 4 +- .../sidebar/components/lock/edit_form.vue | 4 +- .../components/lock/issuable_lock_form.vue | 14 ++--- .../components/notes/noteable_warning.vue | 4 +- .../system_notes/issuables_service.rb | 2 +- app/views/shared/_md_preview.html.haml | 2 +- doc/user/discussions/index.md | 4 +- locale/gitlab.pot | 56 ++++++++++--------- spec/features/issues/discussion_lock_spec.rb | 6 +- .../merge_request_discussion_lock_spec.rb | 4 +- .../user_locks_discussion_spec.rb | 2 +- .../issuable/components/locked_badge_spec.js | 2 +- .../lock/__snapshots__/edit_form_spec.js.snap | 8 +-- .../lock/issuable_lock_form_spec.js | 4 +- .../noteable_warning_spec.js.snap | 6 +- .../components/notes/noteable_warning_spec.js | 18 ++++-- .../common_system_notes_service_spec.rb | 2 +- spec/services/issues/update_service_spec.rb | 4 +- .../merge_requests/update_service_spec.rb | 4 +- .../system_notes/issuables_service_spec.rb | 4 +- 21 files changed, 90 insertions(+), 73 deletions(-) diff --git a/app/assets/javascripts/issuable/components/locked_badge.vue b/app/assets/javascripts/issuable/components/locked_badge.vue index f97ac888417cf8..652d02e8f9db10 100644 --- a/app/assets/javascripts/issuable/components/locked_badge.vue +++ b/app/assets/javascripts/issuable/components/locked_badge.vue @@ -20,9 +20,12 @@ export default { }, computed: { title() { - return sprintf(__('This %{issuable} is locked. Only project members can comment.'), { - issuable: issuableTypeText[this.issuableType], - }); + return sprintf( + __('The discussion in this %{issuable} is locked. Only project members can comment.'), + { + issuable: issuableTypeText[this.issuableType], + }, + ); }, }, }; diff --git a/app/assets/javascripts/notes/components/discussion_locked_widget.vue b/app/assets/javascripts/notes/components/discussion_locked_widget.vue index bcf9b4cf89301d..a999b633f64c9e 100644 --- a/app/assets/javascripts/notes/components/discussion_locked_widget.vue +++ b/app/assets/javascripts/notes/components/discussion_locked_widget.vue @@ -24,7 +24,9 @@ export default { }, lockedIssueWarning() { return sprintf( - __('This %{issuableDisplayName} is locked. Only project members can comment.'), + __( + 'The discussion in this %{issuableDisplayName} is locked. Only project members can comment.', + ), { issuableDisplayName: this.issuableDisplayName }, ); }, diff --git a/app/assets/javascripts/sidebar/components/lock/edit_form.vue b/app/assets/javascripts/sidebar/components/lock/edit_form.vue index c9e651370f9c7c..f783d2d091aba4 100644 --- a/app/assets/javascripts/sidebar/components/lock/edit_form.vue +++ b/app/assets/javascripts/sidebar/components/lock/edit_form.vue @@ -27,7 +27,7 @@ export default { @@ -42,7 +42,7 @@ export default { diff --git a/app/assets/javascripts/sidebar/components/lock/issuable_lock_form.vue b/app/assets/javascripts/sidebar/components/lock/issuable_lock_form.vue index 16235275a54837..977d1d6f668fcd 100644 --- a/app/assets/javascripts/sidebar/components/lock/issuable_lock_form.vue +++ b/app/assets/javascripts/sidebar/components/lock/issuable_lock_form.vue @@ -50,12 +50,12 @@ export default { issueCapitalized: __('Issue'), mergeRequest: __('merge request'), mergeRequestCapitalized: __('Merge request'), - lockingMergeRequest: __('Locking %{issuableDisplayName}'), - unlockingMergeRequest: __('Unlocking %{issuableDisplayName}'), - lockMergeRequest: __('Lock %{issuableDisplayName}'), - unlockMergeRequest: __('Unlock %{issuableDisplayName}'), - lockedMessage: __('%{issuableDisplayName} locked.'), - unlockedMessage: __('%{issuableDisplayName} unlocked.'), + lockingMergeRequest: __('Locking discussion'), + unlockingMergeRequest: __('Unlocking discussion'), + lockMergeRequest: __('Lock discussion'), + unlockMergeRequest: __('Unlock discussion'), + lockedMessage: __('Discussion locked.'), + unlockedMessage: __('Discussion unlocked.'), }, data() { return { @@ -152,7 +152,7 @@ export default { }) .catch(() => { const alertMessage = __( - 'Something went wrong trying to change the locked state of this %{issuableDisplayName}', + 'Something went wrong trying to change the locked state of the discussion', ); createAlert({ message: sprintf(alertMessage, { issuableDisplayName: this.issuableDisplayName }), diff --git a/app/assets/javascripts/vue_shared/components/notes/noteable_warning.vue b/app/assets/javascripts/vue_shared/components/notes/noteable_warning.vue index 0ec8b6e2a0a961..3bee539688bd34 100644 --- a/app/assets/javascripts/vue_shared/components/notes/noteable_warning.vue +++ b/app/assets/javascripts/vue_shared/components/notes/noteable_warning.vue @@ -64,7 +64,7 @@ export default { }); }, lockedContextText() { - return sprintf(__('This %{noteableTypeText} is locked.'), { + return sprintf(__('The discussion in this %{noteableTypeText} is locked.'), { noteableTypeText: this.noteableTypeText, }); }, @@ -80,7 +80,7 @@ export default { diff --git a/app/services/system_notes/issuables_service.rb b/app/services/system_notes/issuables_service.rb index 8442ff81d4136b..c584d5ccca3a64 100644 --- a/app/services/system_notes/issuables_service.rb +++ b/app/services/system_notes/issuables_service.rb @@ -437,7 +437,7 @@ def add_email_participants(body) def discussion_lock action = noteable.discussion_locked? ? 'locked' : 'unlocked' - body = "#{action} this #{noteable.class.to_s.titleize.downcase}" + body = "#{action} the discussion in this #{noteable.class.to_s.titleize.downcase}" if action == 'locked' track_issue_event(:track_issue_locked_action) diff --git a/app/views/shared/_md_preview.html.haml b/app/views/shared/_md_preview.html.haml index 1fd430527a1e6a..7ac6a822420882 100644 --- a/app/views/shared/_md_preview.html.haml +++ b/app/views/shared/_md_preview.html.haml @@ -5,7 +5,7 @@ .issuable-note-warning = sprite_icon('lock', css_class: 'icon') %span - = _('This merge request is locked.') + = _('The discussion in this merge request is locked.') = _('Only project members can comment.') .md-area.position-relative diff --git a/doc/user/discussions/index.md b/doc/user/discussions/index.md index 50f2eca8d0563e..a3ed888ed53d34 100644 --- a/doc/user/discussions/index.md +++ b/doc/user/discussions/index.md @@ -156,12 +156,12 @@ Prerequisite: To lock an issue or merge request: -1. On the right sidebar, next to **Lock issue** or **Lock merge request**, select **Edit**. +1. On the right sidebar, next to **Lock discussion**, select **Edit**. 1. On the confirmation dialog, select **Lock**. Notes are added to the page details. -If an issue or merge request is locked and closed, you cannot reopen it. +If an issue or merge request is closed with a locked discussion, then you cannot reopen it until the discussion is unlocked. If you don't see this action on the right sidebar, your project or instance might have [moved sidebar actions](../project/merge_requests/index.md#move-sidebar-actions) enabled. diff --git a/locale/gitlab.pot b/locale/gitlab.pot index ffabf9084f7639..e0071804d131be 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -792,12 +792,6 @@ msgstr "" msgid "%{integrations_link_start}Integrations%{link_end} enable you to make third-party applications part of your GitLab workflow. If the available integrations don't meet your needs, consider using a %{webhooks_link_start}webhook%{link_end}." msgstr "" -msgid "%{issuableDisplayName} locked." -msgstr "" - -msgid "%{issuableDisplayName} unlocked." -msgstr "" - msgid "%{issuableType} will be removed! Are you sure?" msgstr "" @@ -17553,9 +17547,15 @@ msgstr "" msgid "Discuss a specific suggestion or question." msgstr "" +msgid "Discussion locked." +msgstr "" + msgid "Discussion to reply to cannot be found" msgstr "" +msgid "Discussion unlocked." +msgstr "" + msgid "Disk Usage" msgstr "" @@ -28613,15 +28613,15 @@ msgstr "" msgid "Lock" msgstr "" -msgid "Lock %{issuableDisplayName}" -msgstr "" - msgid "Lock %{issuableType}" msgstr "" msgid "Lock File?" msgstr "" +msgid "Lock discussion" +msgstr "" + msgid "Lock label after a merge request is merged" msgstr "" @@ -28643,7 +28643,7 @@ msgstr "" msgid "Lock the discussion" msgstr "" -msgid "Lock this %{issuableDisplayName}? Only %{strongStart}project members%{strongEnd} will be able to comment." +msgid "Lock this discussion? Only %{strongStart}project members%{strongEnd} will be able to comment." msgstr "" msgid "Lock to current projects" @@ -28661,7 +28661,7 @@ msgstr "" msgid "Locked the discussion." msgstr "" -msgid "Locking %{issuableDisplayName}" +msgid "Locking discussion" msgstr "" msgid "Locks the discussion." @@ -45490,6 +45490,9 @@ msgstr "" msgid "Something went wrong on our end. Please try again." msgstr "" +msgid "Something went wrong trying to change the locked state of the discussion" +msgstr "" + msgid "Something went wrong trying to change the locked state of this %{issuableDisplayName}" msgstr "" @@ -47976,6 +47979,18 @@ msgstr "" msgid "The directory has been successfully created." msgstr "" +msgid "The discussion in this %{issuableDisplayName} is locked. Only project members can comment." +msgstr "" + +msgid "The discussion in this %{issuable} is locked. Only project members can comment." +msgstr "" + +msgid "The discussion in this %{noteableTypeText} is locked." +msgstr "" + +msgid "The discussion in this merge request is locked." +msgstr "" + msgid "The domain you entered is misformatted." msgstr "" @@ -48781,16 +48796,10 @@ msgstr "" msgid "This %{issuable} is locked. Only %{strong_open}project members%{strong_close} can comment." msgstr "" -msgid "This %{issuable} is locked. Only project members can comment." -msgstr "" - msgid "This %{issuable} would exceed the maximum number of linked %{issuables} (%{limit})." msgstr "" -msgid "This %{noteableTypeText} is %{confidentialLinkStart}confidential%{confidentialLinkEnd} and %{lockedLinkStart}locked%{lockedLinkEnd}." -msgstr "" - -msgid "This %{noteableTypeText} is locked." +msgid "This %{noteableTypeText} is %{confidentialLinkStart}confidential%{confidentialLinkEnd} and its %{lockedLinkStart}discussion is locked%{lockedLinkEnd}." msgstr "" msgid "This %{viewer} could not be displayed because %{reason}. You can %{options} instead." @@ -49246,9 +49255,6 @@ msgstr "" msgid "This merge request is from an internal project to a public project." msgstr "" -msgid "This merge request is locked." -msgstr "" - msgid "This merge request was merged. To apply this suggestion, edit this file directly." msgstr "" @@ -51152,10 +51158,10 @@ msgstr "" msgid "Unlock" msgstr "" -msgid "Unlock %{issuableDisplayName}" +msgid "Unlock account" msgstr "" -msgid "Unlock account" +msgid "Unlock discussion" msgstr "" msgid "Unlock more features with GitLab Ultimate" @@ -51164,7 +51170,7 @@ msgstr "" msgid "Unlock the discussion" msgstr "" -msgid "Unlock this %{issuableDisplayName}? %{strongStart}Everyone%{strongEnd} will be able to comment." +msgid "Unlock this discussion? %{strongStart}Everyone%{strongEnd} will be able to comment." msgstr "" msgid "Unlocked" @@ -51173,7 +51179,7 @@ msgstr "" msgid "Unlocked the discussion." msgstr "" -msgid "Unlocking %{issuableDisplayName}" +msgid "Unlocking discussion" msgstr "" msgid "Unlocks the discussion." diff --git a/spec/features/issues/discussion_lock_spec.rb b/spec/features/issues/discussion_lock_spec.rb index fb9addff1a288d..04d59854ddce53 100644 --- a/spec/features/issues/discussion_lock_spec.rb +++ b/spec/features/issues/discussion_lock_spec.rb @@ -28,7 +28,7 @@ click_button('Lock') end - expect(find('#notes')).to have_content('locked this issue') + expect(find('#notes')).to have_content('locked the discussion in this issue') end end @@ -46,7 +46,7 @@ click_button('Unlock') end - expect(find('#notes')).to have_content('unlocked this issue') + expect(find('#notes')).to have_content('unlocked the discussion in this issue') expect(find('.issuable-sidebar')).to have_content('Unlocked') end @@ -101,7 +101,7 @@ page.within('#notes') do expect(page).not_to have_selector('js-main-target-form') expect(page.find('.disabled-comments')) - .to have_content('This issue is locked. Only project members can comment.') + .to have_content('The discussion in this issue is locked. Only project members can comment.') end end end diff --git a/spec/features/merge_request/merge_request_discussion_lock_spec.rb b/spec/features/merge_request/merge_request_discussion_lock_spec.rb index 782c4af58acebb..7e01063816f435 100644 --- a/spec/features/merge_request/merge_request_discussion_lock_spec.rb +++ b/spec/features/merge_request/merge_request_discussion_lock_spec.rb @@ -92,7 +92,7 @@ it 'the user can lock the merge_request' do find('#new-actions-header-dropdown button').click - expect(page).to have_content('Lock merge request') + expect(page).to have_content('Lock discussion') end end @@ -105,7 +105,7 @@ it 'the user can unlock the merge_request' do find('#new-actions-header-dropdown button').click - expect(page).to have_content('Unlock merge request') + expect(page).to have_content('Unlock discussion') end end end diff --git a/spec/features/merge_request/user_locks_discussion_spec.rb b/spec/features/merge_request/user_locks_discussion_spec.rb index a603a5c1e0bbc6..d4cc6c9410c439 100644 --- a/spec/features/merge_request/user_locks_discussion_spec.rb +++ b/spec/features/merge_request/user_locks_discussion_spec.rb @@ -43,7 +43,7 @@ page.within('.js-vue-notes-event') do expect(page).not_to have_selector('js-main-target-form') expect(page.find('.issuable-note-warning')) - .to have_content('This merge request is locked. Only project members can comment.') + .to have_content('The discussion in this merge request is locked. Only project members can comment.') end end end diff --git a/spec/frontend/issuable/components/locked_badge_spec.js b/spec/frontend/issuable/components/locked_badge_spec.js index 73ab6e36ba16a5..46143d16712625 100644 --- a/spec/frontend/issuable/components/locked_badge_spec.js +++ b/spec/frontend/issuable/components/locked_badge_spec.js @@ -39,7 +39,7 @@ describe('LockedBadge component', () => { it('has title', () => { expect(findBadge().attributes('title')).toBe( - 'This issue is locked. Only project members can comment.', + 'The discussion in this issue is locked. Only project members can comment.', ); }); }); diff --git a/spec/frontend/sidebar/components/lock/__snapshots__/edit_form_spec.js.snap b/spec/frontend/sidebar/components/lock/__snapshots__/edit_form_spec.js.snap index d5bbd3bb3c9a1a..48ba23ac0a1152 100644 --- a/spec/frontend/sidebar/components/lock/__snapshots__/edit_form_spec.js.snap +++ b/spec/frontend/sidebar/components/lock/__snapshots__/edit_form_spec.js.snap @@ -9,7 +9,7 @@ exports[`Edit Form Dropdown In issue page when locked the appropriate warning te class="text" >

{ it.each` locked | message - ${true} | ${'Merge request locked.'} - ${false} | ${'Merge request unlocked.'} + ${true} | ${'Discussion locked.'} + ${false} | ${'Discussion unlocked.'} `('displays $message when merge request is $locked', async ({ locked, message }) => { initStore(locked); diff --git a/spec/frontend/vue_shared/components/notes/__snapshots__/noteable_warning_spec.js.snap b/spec/frontend/vue_shared/components/notes/__snapshots__/noteable_warning_spec.js.snap index 891b0c95f0eaf8..ad0e260ad7071a 100644 --- a/spec/frontend/vue_shared/components/notes/__snapshots__/noteable_warning_spec.js.snap +++ b/spec/frontend/vue_shared/components/notes/__snapshots__/noteable_warning_spec.js.snap @@ -2,7 +2,7 @@ exports[`Issue Warning Component when issue is locked but not confidential renders information about locked issue 1`] = ` - This issue is locked. Only project members can comment. + The discussion in this issue is locked. Only project members can comment. confidential - and + and its - locked + discussion is locked . diff --git a/spec/frontend/vue_shared/components/notes/noteable_warning_spec.js b/spec/frontend/vue_shared/components/notes/noteable_warning_spec.js index d7fcb9a25d4f2e..d73356e00da148 100644 --- a/spec/frontend/vue_shared/components/notes/noteable_warning_spec.js +++ b/spec/frontend/vue_shared/components/notes/noteable_warning_spec.js @@ -126,12 +126,14 @@ describe('Issue Warning Component', () => { }); it('renders confidential & locked messages with noteable "issue"', () => { - expect(findLockedBlock(wrapperLocked).text()).toContain('This issue is locked.'); + expect(findLockedBlock(wrapperLocked).text()).toContain( + 'The discussion in this issue is locked.', + ); expect(findConfidentialBlock(wrapperConfidential).text()).toContain( 'This is a confidential issue.', ); expect(findLockedAndConfidentialBlock(wrapperLockedAndConfidential).text()).toContain( - 'This issue is confidential and locked.', + 'This issue is confidential and its discussion is locked.', ); }); @@ -147,7 +149,9 @@ describe('Issue Warning Component', () => { }); await nextTick(); - expect(findLockedBlock(wrapperLocked).text()).toContain('This epic is locked.'); + expect(findLockedBlock(wrapperLocked).text()).toContain( + 'The discussion in this epic is locked.', + ); await nextTick(); expect(findConfidentialBlock(wrapperConfidential).text()).toContain( @@ -156,7 +160,7 @@ describe('Issue Warning Component', () => { await nextTick(); expect(findLockedAndConfidentialBlock(wrapperLockedAndConfidential).text()).toContain( - 'This epic is confidential and locked.', + 'This epic is confidential and its discussion is locked.', ); }); @@ -172,7 +176,9 @@ describe('Issue Warning Component', () => { }); await nextTick(); - expect(findLockedBlock(wrapperLocked).text()).toContain('This merge request is locked.'); + expect(findLockedBlock(wrapperLocked).text()).toContain( + 'The discussion in this merge request is locked.', + ); await nextTick(); expect(findConfidentialBlock(wrapperConfidential).text()).toContain( @@ -181,7 +187,7 @@ describe('Issue Warning Component', () => { await nextTick(); expect(findLockedAndConfidentialBlock(wrapperLockedAndConfidential).text()).toContain( - 'This merge request is confidential and locked.', + 'This merge request is confidential and its discussion is locked.', ); }); }); diff --git a/spec/services/issuable/common_system_notes_service_spec.rb b/spec/services/issuable/common_system_notes_service_spec.rb index 9306aeaac447f4..3d83c9ec9c2879 100644 --- a/spec/services/issuable/common_system_notes_service_spec.rb +++ b/spec/services/issuable/common_system_notes_service_spec.rb @@ -42,7 +42,7 @@ context 'on issuable update' do it_behaves_like 'system note creation', { title: 'New title' }, 'changed title' it_behaves_like 'system note creation', { description: 'New description' }, 'changed the description' - it_behaves_like 'system note creation', { discussion_locked: true }, 'locked this issue' + it_behaves_like 'system note creation', { discussion_locked: true }, 'locked the discussion in this issue' it_behaves_like 'system note creation', { time_estimate: 5 }, 'changed time estimate' context 'when new label is added' do diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index c4ad4039b45674..0cb13bfb9173d7 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -491,9 +491,9 @@ def update_issue(opts) end it 'creates system note about discussion lock' do - note = find_note('locked this issue') + note = find_note('locked the discussion in this issue') - expect(note.note).to eq 'locked this issue' + expect(note.note).to eq 'locked the discussion in this issue' end end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index f5494f429c37f8..53dd4263770310 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -351,10 +351,10 @@ def update_merge_request(opts) end it 'creates system note about discussion lock' do - note = find_note('locked this merge request') + note = find_note('locked the discussion in this merge request') expect(note).not_to be_nil - expect(note.note).to eq 'locked this merge request' + expect(note.note).to eq 'locked the discussion in this merge request' end context 'when current user cannot admin issues in the project' do diff --git a/spec/services/system_notes/issuables_service_spec.rb b/spec/services/system_notes/issuables_service_spec.rb index bcca1ed0b23d51..ca6feb6fde27e5 100644 --- a/spec/services/system_notes/issuables_service_spec.rb +++ b/spec/services/system_notes/issuables_service_spec.rb @@ -784,7 +784,7 @@ def build_note(added_count, removed_count) service = described_class.new(noteable: issuable, author: author) expect(service.discussion_lock.note) - .to eq("unlocked this #{type.to_s.titleize.downcase}") + .to eq("unlocked the discussion in this #{type.to_s.titleize.downcase}") end end end @@ -804,7 +804,7 @@ def build_note(added_count, removed_count) service = described_class.new(noteable: issuable, author: author) expect(service.discussion_lock.note) - .to eq("locked this #{type.to_s.titleize.downcase}") + .to eq("locked the discussion in this #{type.to_s.titleize.downcase}") end end end -- GitLab From 98e07c7f6a107c054994da8bdf8012a371b92837 Mon Sep 17 00:00:00 2001 From: Austin Regnery Date: Mon, 6 Nov 2023 14:32:34 -0500 Subject: [PATCH 2/2] Remove unused template --- app/assets/javascripts/sidebar/components/lock/edit_form.vue | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/assets/javascripts/sidebar/components/lock/edit_form.vue b/app/assets/javascripts/sidebar/components/lock/edit_form.vue index f783d2d091aba4..1497b229a592ce 100644 --- a/app/assets/javascripts/sidebar/components/lock/edit_form.vue +++ b/app/assets/javascripts/sidebar/components/lock/edit_form.vue @@ -31,7 +31,6 @@ export default { ) " > - @@ -46,7 +45,6 @@ export default { ) " > - -- GitLab