diff --git a/app/assets/javascripts/notes/store/legacy_notes/actions.js b/app/assets/javascripts/notes/store/legacy_notes/actions.js index 1824cf0792b6ca756f5c284e4e980fb522920089..8d6814cc38e7e943b6f7c7168c444da8d46fdc11 100644 --- a/app/assets/javascripts/notes/store/legacy_notes/actions.js +++ b/app/assets/javascripts/notes/store/legacy_notes/actions.js @@ -411,7 +411,6 @@ export function resolveDiscussion({ discussionId }) { } export function toggleResolveNote({ endpoint, isResolved, discussion }) { - const discussionIdMatch = (endpoint || '').match(/discussions\/([^/]+)\/resolve$/); const method = isResolved ? constants.UNRESOLVE_NOTE_METHOD_NAME : constants.RESOLVE_NOTE_METHOD_NAME; @@ -423,24 +422,6 @@ export function toggleResolveNote({ endpoint, isResolved, discussion }) { this.updateResolvableDiscussionsCounts(); this.updateMergeRequestWidget(); - - if (!isResolved && discussion && discussionIdMatch) { - const discussions = this.discussions || []; - const discussionId = discussionIdMatch[1]; - const actualDiscussion = discussions.find((d) => d.id === discussionId); - const currentLineCode = actualDiscussion.line_code; - const lineDiscussions = discussions.filter((d) => d.line_code === currentLineCode); - const allLineDiscussionsResolved = lineDiscussions.every((d) => d.resolved); - - if (allLineDiscussionsResolved && currentLineCode && lineDiscussions.length > 0) { - // tryStore only used for migration, refactor the store to avoid using this helper - this.tryStore('legacyDiffs').toggleLineDiscussions({ - lineCode: actualDiscussion.line_code, - fileHash: actualDiscussion.diff_file?.file_hash, - expanded: false, - }); - } - } }); } diff --git a/qa/qa/page/merge_request/show.rb b/qa/qa/page/merge_request/show.rb index e5ea16be682e2c40c5db2372f6c0123a6fa4322e..d558bf58514de80c61f1c8afdfa9244225c0b408 100644 --- a/qa/qa/page/merge_request/show.rb +++ b/qa/qa/page/merge_request/show.rb @@ -551,10 +551,6 @@ def has_exposed_artifact_with_name?(name) has_link?(name) end - def expand_collapsed_discussions - all_elements('left-discussions', minimum: 1).each(&:click) - end - private def submit_commit diff --git a/qa/qa/specs/features/browser_ui/3_create/merge_request/suggestions/batch_suggestion_spec.rb b/qa/qa/specs/features/browser_ui/3_create/merge_request/suggestions/batch_suggestion_spec.rb index 5cd78fa717b4333016fd6cd19b32677801dd04cd..abc5b51c53b801ea68870db4603d0de0c32fc2b8 100644 --- a/qa/qa/specs/features/browser_ui/3_create/merge_request/suggestions/batch_suggestion_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/merge_request/suggestions/batch_suggestion_spec.rb @@ -42,8 +42,6 @@ module QA 4.times { merge_request.add_suggestion_to_batch } merge_request.apply_suggestion_with_message("Custom commit message") - merge_request.expand_collapsed_discussions - expect(merge_request).to have_suggestions_applied(4) end end diff --git a/qa/qa/specs/features/browser_ui/3_create/merge_request/suggestions/custom_commit_suggestion_spec.rb b/qa/qa/specs/features/browser_ui/3_create/merge_request/suggestions/custom_commit_suggestion_spec.rb index ac9c8640460e258d021fa0578b2e3963ccb22e47..6bd1c7d226edb9b074491f63d8e39cd462f189c1 100644 --- a/qa/qa/specs/features/browser_ui/3_create/merge_request/suggestions/custom_commit_suggestion_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/merge_request/suggestions/custom_commit_suggestion_spec.rb @@ -38,8 +38,6 @@ module QA merge_request.click_diffs_tab merge_request.apply_suggestion_with_message(commit_message) - merge_request.expand_collapsed_discussions - expect(merge_request).to have_suggestions_applied merge_request.click_commits_tab diff --git a/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb b/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb index 9a2ec37d5b3b37492b7c8182aa5073e248ba84fd..dd9dc31c2d4f6a0f3ba4685de46361a05d7c06f5 100644 --- a/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb +++ b/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb @@ -105,11 +105,7 @@ def expect_suggestion_has_content(element, expected_changing_content, expected_s click_button('Apply suggestion') click_button('Apply') wait_for_requests - end - - expand_collapsed_discussions - page.within('.diff-discussions') do expect(page).to have_content('Applied') end end @@ -214,8 +210,6 @@ def hash(path) wait_for_requests end - expand_all_collapsed_discussions - expect(page).to have_content('Applied').twice end end @@ -278,11 +272,7 @@ def hash(path) all('button', text: 'Apply suggestion').first.click click_button('Apply') wait_for_requests - end - - expand_collapsed_discussions - page.within(container) do expect(page).to have_content('Applied').once expect(page).to have_button('Apply suggestion').once @@ -402,11 +392,7 @@ def hash(path) click_button('Apply suggestion') click_button('Apply') wait_for_requests - end - - expand_collapsed_discussions - page.within("[id='#{hash}']") do expect(page).to have_content('Applied') end end @@ -418,11 +404,7 @@ def hash(path) click_button('Apply suggestion') click_button('Apply') wait_for_requests - end - - expand_collapsed_discussions - page.within("[id='#{hash}']") do expect(page).to have_content('Reopen thread') end end diff --git a/spec/frontend/notes/store/legacy_notes/actions_spec.js b/spec/frontend/notes/store/legacy_notes/actions_spec.js index 1fdbc0ac59b7d4caf28500b256e53ee10504a9f5..238f7a6dd15b50722af48bd34799dae3a4ad7635 100644 --- a/spec/frontend/notes/store/legacy_notes/actions_spec.js +++ b/spec/frontend/notes/store/legacy_notes/actions_spec.js @@ -43,6 +43,7 @@ jest.mock('~/alert', () => ({ dismiss: mockAlertDismiss, })), })); + jest.mock('~/vue_shared/plugins/global_toast'); describe('Actions Notes Store', () => { @@ -638,169 +639,6 @@ describe('Actions Notes Store', () => { ], ); }); - - describe('auto-collapse', () => { - let legacyDiffsStore; - let mockToggleLineDiscussions; - - beforeEach(() => { - legacyDiffsStore = useLegacyDiffs(); - mockToggleLineDiscussions = jest.fn(); - legacyDiffsStore.toggleLineDiscussions = mockToggleLineDiscussions; - }); - - it('collapses line discussions when all discussions on the line are resolved', () => { - const localRes = { - resolved: true, - expanded: true, - discussion_id: '1', - id: '1', - }; - - axiosMock.reset(); - axiosMock.onAny().reply(HTTP_STATUS_OK, localRes); - axiosMock.onPut(`${TEST_HOST}/discussions/1/resolve`).replyOnce(200, localRes); - - return testAction( - store.toggleResolveNote, - { endpoint: `${TEST_HOST}/discussions/1/resolve`, isResolved: false, discussion: true }, - { - discussions: [ - { - resolved: true, - discussion_id: '1', - id: '1', - individual_note: true, - notes: [], - line_code: 'abc123', - diff_file: { file_hash: 'file123' }, - }, - { - resolved: true, - discussion_id: '2', - id: '2', - individual_note: true, - notes: [], - line_code: 'abc123', - }, - ], - }, - [ - { - type: store[types.UPDATE_DISCUSSION], - payload: localRes, - }, - ], - [ - { - type: store.updateResolvableDiscussionsCounts, - }, - { - type: store.updateMergeRequestWidget, - }, - ], - ).then(() => { - expect(mockToggleLineDiscussions).toHaveBeenCalledWith({ - lineCode: 'abc123', - fileHash: 'file123', - expanded: false, - }); - }); - }); - - it('does not collapse when there are unresolved discussions on the same line', () => { - return testAction( - store.toggleResolveNote, - { endpoint: `${TEST_HOST}`, isResolved: false, discussion: true }, - { - discussions: [ - { - resolved: true, - discussion_id: 1, - id: 1, - individual_note: true, - notes: [], - line_code: 'abc123', - }, - { - resolved: false, - discussion_id: 2, - id: 2, - individual_note: true, - notes: [], - line_code: 'abc123', - }, - ], - }, - [ - { - type: store[types.UPDATE_DISCUSSION], - payload: res, - }, - ], - [ - { - type: store.updateResolvableDiscussionsCounts, - }, - { - type: store.updateMergeRequestWidget, - }, - ], - ); - }); - - it('does not collapse when unresolving a discussion', () => { - return testAction( - store.toggleResolveNote, - { endpoint: `${TEST_HOST}`, isResolved: true, discussion: true }, - { - discussions: [ - { resolved: false, discussion_id: 1, id: 1, individual_note: true, notes: [] }, - ], - }, - [ - { - type: store[types.UPDATE_DISCUSSION], - payload: res, - }, - ], - [ - { - type: store.updateResolvableDiscussionsCounts, - }, - { - type: store.updateMergeRequestWidget, - }, - ], - ); - }); - - it('does not collapse when discussion parameter is false', () => { - return testAction( - store.toggleResolveNote, - { endpoint: `${TEST_HOST}`, isResolved: false, discussion: false }, - { - discussions: [ - { resolved: true, discussion_id: 1, id: 1, individual_note: true, notes: [] }, - ], - }, - [ - { - type: store[types.UPDATE_NOTE], - payload: res, - }, - ], - [ - { - type: store.updateResolvableDiscussionsCounts, - }, - { - type: store.updateMergeRequestWidget, - }, - ], - ); - }); - }); }); }); diff --git a/spec/support/helpers/merge_request_diff_helpers.rb b/spec/support/helpers/merge_request_diff_helpers.rb index dd296bba31bda88fb756330f534537716b6aa15f..bbd9382fcc2defe92c3fb4343aaea430c7a0371d 100644 --- a/spec/support/helpers/merge_request_diff_helpers.rb +++ b/spec/support/helpers/merge_request_diff_helpers.rb @@ -93,12 +93,4 @@ def find_by_scrolling(selector, **options) end element end - - def expand_collapsed_discussions - first('.js-diff-comment-avatar').click - end - - def expand_all_collapsed_discussions - all('.js-diff-comment-avatar').each(&:click) - end end