From 8e1afe6078376b91a462635c69ef912a9f683b17 Mon Sep 17 00:00:00 2001 From: Chawn Neal Date: Fri, 3 Sep 2021 22:48:15 -0500 Subject: [PATCH] fix query for go to reply MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fixes VICE-2028 flag=isolated_view Basically 2 issues: 1) we need to pass the parent entry of a reply. Non legacy will always be rootEntry. But for legacy it could be parent or root. Its safer to check if root present (thus its a child), then pass parent or root. 2) root || entry give preference to root. we need to give preference to entry. Test Plan: 1) Create a deeply nested entry. 1a.) To do this turn off isolated view and discussion_design feature flags. 1b.) Then create your entries 1b.) See the ticket for more details if needed. 2) search for the entry with search input. 3) Click go to reply should not break and should work. 4) Work means: entry will be in thread and parent will be the IsolateParent. Change-Id: If9530418c00804a88540f4652e1fa690f3711f46 Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/272955 Tested-by: Service Cloud Jenkins Reviewed-by: Omar Soto-Fortuño QA-Review: Omar Soto-Fortuño Product-Review: Omar Soto-Fortuño --- .../react/DiscussionTopicManager.js | 2 +- .../ThreadingToolbar/ThreadingToolbar.js | 2 +- .../__tests__/ThreadingToolbar.test.js | 61 ++++++++++++++----- 3 files changed, 47 insertions(+), 18 deletions(-) diff --git a/ui/features/discussion_topics_post/react/DiscussionTopicManager.js b/ui/features/discussion_topics_post/react/DiscussionTopicManager.js index 822e815e78f..5029bc268b0 100644 --- a/ui/features/discussion_topics_post/react/DiscussionTopicManager.js +++ b/ui/features/discussion_topics_post/react/DiscussionTopicManager.js @@ -84,7 +84,7 @@ const DiscussionTopicManager = props => { }, [highlightEntryId]) const openIsolatedView = (discussionEntryId, rootEntryId, withRCE, relativeId = null) => { - setIsolatedEntryId(rootEntryId || discussionEntryId) + setIsolatedEntryId(discussionEntryId || rootEntryId) setReplyId(discussionEntryId) setIsolatedViewOpen(true) setEditorExpanded(withRCE) diff --git a/ui/features/discussion_topics_post/react/components/ThreadingToolbar/ThreadingToolbar.js b/ui/features/discussion_topics_post/react/components/ThreadingToolbar/ThreadingToolbar.js index f8923c57e91..3ea8adf4ac3 100644 --- a/ui/features/discussion_topics_post/react/components/ThreadingToolbar/ThreadingToolbar.js +++ b/ui/features/discussion_topics_post/react/components/ThreadingToolbar/ThreadingToolbar.js @@ -54,7 +54,7 @@ export function ThreadingToolbar({...props}) { data-testid="go-to-reply" onClick={() => { const isolatedId = props.discussionEntry.rootEntryId - ? props.discussionEntry.rootEntryId + ? props.discussionEntry.parentId || props.discussionEntry.rootEntryId : props.discussionEntry._id const relativeId = props.discussionEntry.rootEntryId ? props.discussionEntry._id diff --git a/ui/features/discussion_topics_post/react/components/ThreadingToolbar/__tests__/ThreadingToolbar.test.js b/ui/features/discussion_topics_post/react/components/ThreadingToolbar/__tests__/ThreadingToolbar.test.js index 19ff6865722..d3c179e1452 100644 --- a/ui/features/discussion_topics_post/react/components/ThreadingToolbar/__tests__/ThreadingToolbar.test.js +++ b/ui/features/discussion_topics_post/react/components/ThreadingToolbar/__tests__/ThreadingToolbar.test.js @@ -91,23 +91,52 @@ describe('PostToolbar', () => { expect(queryByText('Go to Reply')).toBeNull() }) - it('calls the onOpenIsolatedView callback with the root entry id', async () => { - window.ENV.isolated_view = true - const onOpenIsolatedView = jest.fn() - const container = render( - - ) + describe('when rootEntryId is present', () => { + it('calls the onOpenIsolatedView callback with the parent entry id', async () => { + window.ENV.isolated_view = true + const onOpenIsolatedView = jest.fn() + const container = render( + + ) - fireEvent.click(container.getByText('Go to Reply')) - await waitFor(() => expect(onOpenIsolatedView).toHaveBeenCalledWith('2', '2', false, '1', '1')) + fireEvent.click(container.getByText('Go to Reply')) + await waitFor(() => + expect(onOpenIsolatedView).toHaveBeenCalledWith('3', '2', false, '1', '1') + ) + }) + }) + + describe('when rootEntryId is not present', () => { + it('calls the onOpenIsolatedView callback with the entry id', async () => { + window.ENV.isolated_view = true + const onOpenIsolatedView = jest.fn() + const container = render( + + ) + + fireEvent.click(container.getByText('Go to Reply')) + await waitFor(() => + expect(onOpenIsolatedView).toHaveBeenCalledWith('1', null, false, null, '1') + ) + }) }) it('calls the onOpenIsolatedView callback with its own id if it is a root entry', async () => {