From ab269ec6c64b9b1ba5c819d1fadc3700016becd5 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Thu, 30 May 2024 12:12:46 +0930 Subject: [PATCH 01/17] fix: warnings about Duplicate message id [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.content-tags-drawer.content-tags-collapsible.custom-menu.placeholder-text", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "authoring.alert.error.connection", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.import.page.title", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.course-outline.card.menu.delete", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.course-outline.page-alerts.discussionNotificationLearnMore", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.course-outline.status-bar.video-sharing.title", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.course-outline.subsection.button.new-unit", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.course-team.member.button.remove", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.files-and-uploads..deleteConfirmation.message", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.files-and-videos.sort-and-filter.modal.filter.activeCheckbox.label", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.files-and-videos.sort-and-filter.modal.filter.inactiveCheckbox.label", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.course-outline.configure-modal.advanced-tab.timed-description", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.course-outline.configure-modal.advanced-tab.timed-description", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.course-outline.configure-modal.advanced-tab.timed-description", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.export.stepper.header.title", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "authoring.loading", but the `description` and/or `defaultMessage` are different. --- src/accessibility-page/messages.js | 2 +- src/content-tags-drawer/messages.js | 2 +- src/course-outline/card-header/messages.js | 2 +- src/course-outline/page-alerts/messages.js | 2 +- src/course-outline/status-bar/messages.js | 2 +- .../subsection-card/messages.js | 2 +- .../course-team-member/messages.js | 2 +- src/files-and-videos/files-page/messages.js | 4 ++-- src/files-and-videos/generic/messages.js | 6 +++--- .../table-custom-columns/ActiveColumn.jsx | 21 ++++--------------- src/generic/ConnectionErrorAlert.jsx | 3 +-- src/generic/configure-modal/messages.js | 6 +++--- src/import-page/import-stepper/messages.js | 2 +- 13 files changed, 21 insertions(+), 35 deletions(-) diff --git a/src/accessibility-page/messages.js b/src/accessibility-page/messages.js index 6b97fb96e9..b15a88832c 100644 --- a/src/accessibility-page/messages.js +++ b/src/accessibility-page/messages.js @@ -2,7 +2,7 @@ import { defineMessages } from '@edx/frontend-platform/i18n'; const messages = defineMessages({ pageTitle: { - id: 'course-authoring.import.page.title', + id: 'course-authoring.accessibility.page.title', defaultMessage: 'Studio Accessibility Policy| {siteName}', }, }); diff --git a/src/content-tags-drawer/messages.js b/src/content-tags-drawer/messages.js index ca9c7896c5..4868195336 100644 --- a/src/content-tags-drawer/messages.js +++ b/src/content-tags-drawer/messages.js @@ -64,7 +64,7 @@ const messages = defineMessages({ defaultMessage: 'Add a tag', }, collapsibleNoTagsAddedText: { - id: 'course-authoring.content-tags-drawer.content-tags-collapsible.custom-menu.placeholder-text', + id: 'course-authoring.content-tags-drawer.content-tags-collapsible.custom-menu.no-tags-added-text', defaultMessage: 'No tags added yet.', }, collapsibleAddStagedTagsButtonText: { diff --git a/src/course-outline/card-header/messages.js b/src/course-outline/card-header/messages.js index 410443d695..0aeb34b20e 100644 --- a/src/course-outline/card-header/messages.js +++ b/src/course-outline/card-header/messages.js @@ -58,7 +58,7 @@ const messages = defineMessages({ defaultMessage: 'Delete', }, menuCopy: { - id: 'course-authoring.course-outline.card.menu.delete', + id: 'course-authoring.course-outline.card.menu.copy', defaultMessage: 'Copy to clipboard', }, menuProctoringLinkText: { diff --git a/src/course-outline/page-alerts/messages.js b/src/course-outline/page-alerts/messages.js index ea67c76ad9..5373e714e9 100644 --- a/src/course-outline/page-alerts/messages.js +++ b/src/course-outline/page-alerts/messages.js @@ -22,7 +22,7 @@ const messages = defineMessages({ description: 'Learn more link in upgraded discussion notification alert', }, discussionNotificationFeedback: { - id: 'course-authoring.course-outline.page-alerts.discussionNotificationLearnMore', + id: 'course-authoring.course-outline.page-alerts.discussionNotificationFeedback', defaultMessage: 'Share feedback', description: 'Share feedback link in upgraded discussion notification alert', }, diff --git a/src/course-outline/status-bar/messages.js b/src/course-outline/status-bar/messages.js index e7c752f92b..0f3f105737 100644 --- a/src/course-outline/status-bar/messages.js +++ b/src/course-outline/status-bar/messages.js @@ -56,7 +56,7 @@ const messages = defineMessages({ defaultMessage: 'Video Sharing', }, videoSharingLink: { - id: 'course-authoring.course-outline.status-bar.video-sharing.title', + id: 'course-authoring.course-outline.status-bar.video-sharing.link', defaultMessage: 'Learn more', }, videoSharingPerVideoText: { diff --git a/src/course-outline/subsection-card/messages.js b/src/course-outline/subsection-card/messages.js index b4a0b5661a..f741b02ecc 100644 --- a/src/course-outline/subsection-card/messages.js +++ b/src/course-outline/subsection-card/messages.js @@ -6,7 +6,7 @@ const messages = defineMessages({ defaultMessage: 'New unit', }, pasteButton: { - id: 'course-authoring.course-outline.subsection.button.new-unit', + id: 'course-authoring.course-outline.subsection.button.paste-unit', defaultMessage: 'Paste unit', }, }); diff --git a/src/course-team/course-team-member/messages.js b/src/course-team/course-team-member/messages.js index 87bb371682..548b733b13 100644 --- a/src/course-team/course-team-member/messages.js +++ b/src/course-team/course-team-member/messages.js @@ -22,7 +22,7 @@ const messages = defineMessages({ defaultMessage: 'Add admin access', }, removeButton: { - id: 'course-authoring.course-team.member.button.remove', + id: 'course-authoring.course-team.member.button.remove-admin-access', defaultMessage: 'Remove admin access', }, deleteUserButton: { diff --git a/src/files-and-videos/files-page/messages.js b/src/files-and-videos/files-page/messages.js index 2f827e5c01..d2c3bf3d78 100644 --- a/src/files-and-videos/files-page/messages.js +++ b/src/files-and-videos/files-page/messages.js @@ -47,12 +47,12 @@ const messages = defineMessages({ description: 'Label for lock file checkbox in info modal', }, activeCheckboxLabel: { - id: 'course-authoring.files-and-videos.sort-and-filter.modal.filter.activeCheckbox.label', + id: 'course-authoring.files-and-videos.file-info.activeCheckbox.label', defaultMessage: 'Active', description: 'Label for active checkbox in filter section of sort and filter modal', }, inactiveCheckboxLabel: { - id: 'course-authoring.files-and-videos.sort-and-filter.modal.filter.inactiveCheckbox.label', + id: 'course-authoring.files-and-videos.file-info.inactiveCheckbox.label', defaultMessage: 'Inactive', description: 'Label for inactive checkbox in filter section of sort and filter modal', }, diff --git a/src/files-and-videos/generic/messages.js b/src/files-and-videos/generic/messages.js index 1906f1ba78..6bb73e6fd9 100644 --- a/src/files-and-videos/generic/messages.js +++ b/src/files-and-videos/generic/messages.js @@ -126,12 +126,12 @@ const messages = defineMessages({ description: 'Label for delete button in card menu dropdown', }, deleteConfirmationTitle: { - id: 'course-authoring.files-and-uploads..deleteConfirmation.title', + id: 'course-authoring.files-and-uploads.deleteConfirmation.title', defaultMessage: 'Delete {fileNumber, plural, one {{fileName}} other {{fileNumber} {fileType}s}}', description: 'Title for delete confirmation modal', }, deleteConfirmationMessage: { - id: 'course-authoring.files-and-uploads..deleteConfirmation.message', + id: 'course-authoring.files-and-uploads.deleteConfirmation.message', defaultMessage: ` Are you sure you want to delete {fileNumber, plural, one {{fileName}} other {{fileNumber} {fileType}s}}? This action cannot be undone and may break your course if the {fileNumber, plural, one {{fileType} is} other {{fileType}s are}} @@ -140,7 +140,7 @@ const messages = defineMessages({ description: 'Message presented to user listing the number of files they are attempting to delete in the delete confirmation modal', }, deleteConfirmationUsageMessage: { - id: 'course-authoring.files-and-uploads..deleteConfirmation.message', + id: 'course-authoring.files-and-uploads.deleteConfirmation.usage-message', defaultMessage: 'The following {fileNumber, plural, one {{fileType} is} other {{fileType}s are}} used in course content. Consider updating the content before deleting.', description: 'Message listing where the files the user is attempting to delete are used in the course', }, diff --git a/src/files-and-videos/generic/table-components/table-custom-columns/ActiveColumn.jsx b/src/files-and-videos/generic/table-components/table-custom-columns/ActiveColumn.jsx index f0f143a1fb..36b153c75c 100644 --- a/src/files-and-videos/generic/table-components/table-custom-columns/ActiveColumn.jsx +++ b/src/files-and-videos/generic/table-components/table-custom-columns/ActiveColumn.jsx @@ -1,29 +1,16 @@ import React from 'react'; import { PropTypes } from 'prop-types'; import { isNil } from 'lodash'; -import { injectIntl, FormattedMessage } from '@edx/frontend-platform/i18n'; -import { Icon, Spinner } from '@openedx/paragon'; +import { injectIntl } from '@edx/frontend-platform/i18n'; +import { Icon } from '@openedx/paragon'; import { Check } from '@openedx/paragon/icons'; import { RequestStatus } from '../../../../data/constants'; +import { LoadingSpinner } from '../../../../generic/Loading'; const ActiveColumn = ({ row, pageLoadStatus }) => { const { usageLocations } = row.original; if (isNil(usageLocations) || pageLoadStatus !== RequestStatus.SUCCESSFUL) { - return ( - - )} - /> - ); + return ; } const numOfUsageLocations = usageLocations.length; return numOfUsageLocations > 0 ? : null; diff --git a/src/generic/ConnectionErrorAlert.jsx b/src/generic/ConnectionErrorAlert.jsx index 3fd9989df4..44d088a66b 100644 --- a/src/generic/ConnectionErrorAlert.jsx +++ b/src/generic/ConnectionErrorAlert.jsx @@ -8,8 +8,7 @@ import messages from '../messages'; const ConnectionErrorAlert = ({ intl }) => ( diff --git a/src/generic/configure-modal/messages.js b/src/generic/configure-modal/messages.js index 5d785da574..41ef703bd8 100644 --- a/src/generic/configure-modal/messages.js +++ b/src/generic/configure-modal/messages.js @@ -196,7 +196,7 @@ const messages = defineMessages({ defaultMessage: 'Proctored', }, proctoredExamDescription: { - id: 'course-authoring.course-outline.configure-modal.advanced-tab.timed-description', + id: 'course-authoring.course-outline.configure-modal.advanced-tab.proctored-exam-description', defaultMessage: 'Proctored exams are timed and they record video of each learner taking the exam. The videos are then reviewed to ensure that learners follow all examination rules. Please note that setting this exam as proctored will change the visibility settings to "Hide content after due date."', }, onboardingExam: { @@ -204,7 +204,7 @@ const messages = defineMessages({ defaultMessage: 'Onboarding', }, onboardingExamDescription: { - id: 'course-authoring.course-outline.configure-modal.advanced-tab.timed-description', + id: 'course-authoring.course-outline.configure-modal.advanced-tab.onboarding-exam-description', defaultMessage: 'Use Onboarding to introduce learners to proctoring, verify their identity, and create an onboarding profile. Learners must complete the onboarding profile step prior to taking a proctored exam. Profile reviews take 2+ business days.', }, practiceExam: { @@ -212,7 +212,7 @@ const messages = defineMessages({ defaultMessage: 'Practice proctored', }, practiceExamDescription: { - id: 'course-authoring.course-outline.configure-modal.advanced-tab.timed-description', + id: 'course-authoring.course-outline.configure-modal.advanced-tab.practice-exam-description', defaultMessage: 'Use a practice proctored exam to introduce learners to the proctoring tools and processes. Results of a practice exam do not affect a learner\'s grade.', }, advancedTabTitle: { diff --git a/src/import-page/import-stepper/messages.js b/src/import-page/import-stepper/messages.js index 83d720600e..172b4a7338 100644 --- a/src/import-page/import-stepper/messages.js +++ b/src/import-page/import-stepper/messages.js @@ -50,7 +50,7 @@ const messages = defineMessages({ defaultMessage: 'Error importing course', }, stepperHeaderTitle: { - id: 'course-authoring.export.stepper.header.title', + id: 'course-authoring.import.stepper.header.title', defaultMessage: 'Course import status', }, }); From 2bf0d6f63a87822b6dfe985774003b8b2266f8a6 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Fri, 31 May 2024 09:41:44 +0930 Subject: [PATCH 02/17] fix: paragon's Hyperlink no longer accepts a 'content' attribute --- .../ChecklistSection/ChecklistItemComment.jsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/course-checklist/ChecklistSection/ChecklistItemComment.jsx b/src/course-checklist/ChecklistSection/ChecklistItemComment.jsx index b254a79c16..92fb83ea32 100644 --- a/src/course-checklist/ChecklistSection/ChecklistItemComment.jsx +++ b/src/course-checklist/ChecklistSection/ChecklistItemComment.jsx @@ -79,10 +79,9 @@ const ChecklistItemComment = ({
    {gradedAssignmentsOutsideDateRange.map(assignment => (
  • - + + {assignment.displayName} +
    • ))}
From 961a8a2502bb2cc266c7c1a0db6381d28e786911 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Fri, 31 May 2024 09:42:27 +0930 Subject: [PATCH 03/17] test: ensure all act() calls are async --- .../discussions/DiscussionsSettings.test.jsx | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/pages-and-resources/discussions/DiscussionsSettings.test.jsx b/src/pages-and-resources/discussions/DiscussionsSettings.test.jsx index 2f8215ceff..5499c59445 100644 --- a/src/pages-and-resources/discussions/DiscussionsSettings.test.jsx +++ b/src/pages-and-resources/discussions/DiscussionsSettings.test.jsx @@ -185,7 +185,7 @@ describe('DiscussionsSettings', () => { // content has been loaded - prior to proceeding with our expectations. await waitForElementToBeRemoved(screen.getByRole('status')); - act(async () => { + await act(async () => { userEvent.click(queryByLabelText(container, 'Select Piazza')); userEvent.click(getByRole(container, 'button', { name: 'Next' })); @@ -210,7 +210,7 @@ describe('DiscussionsSettings', () => { // content has been loaded - prior to proceeding with our expectations. await waitForElementToBeRemoved(screen.getByRole('status')); - act(async () => { + await act(async () => { userEvent.click(getByRole(container, 'checkbox', { name: 'Select Discourse' })); userEvent.click(getByRole(container, 'button', { name: 'Next' })); @@ -241,7 +241,7 @@ describe('DiscussionsSettings', () => { await waitFor(() => expect(screen.queryByRole('status')).toBeNull()); - act(async () => { + await act(async () => { expect(await findByRole(container, 'heading', { name: 'Discourse' })).toBeInTheDocument(); userEvent.type(getByRole(container, 'textbox', { name: 'Consumer Key' }), 'a'); @@ -305,7 +305,7 @@ describe('DiscussionsSettings', () => { await waitForElementToBeRemoved(screen.getByRole('status')); // Apply causes an async action to take place - act(async () => { + await act(async () => { userEvent.click(queryByText(container, appMessages.saveButton.defaultMessage)); await waitFor(() => expect(axiosMock.history.post.length).toBe(1)); @@ -350,7 +350,7 @@ describe('DiscussionsSettings', () => { // content has been loaded - prior to proceeding with our expectations. await waitForElementToBeRemoved(screen.getByRole('status')); - act(async () => { + await act(async () => { userEvent.click(getByRole(container, 'button', { name: 'Save' })); await waitFor(() => expect(axiosMock.history.post.length).toBe(1)); @@ -408,7 +408,7 @@ describe.each([ // content has been loaded - prior to proceeding with our expectations. waitForElementToBeRemoved(screen.getByRole('status')); - act(async () => { + await act(async () => { userEvent.click(await screen.findByLabelText('Select Piazza')); userEvent.click(queryByText(container, messages.nextButton.defaultMessage)); waitForElementToBeRemoved(screen.getByRole('status')); @@ -462,7 +462,7 @@ describe.each([ // content has been loaded - prior to proceeding with our expectations. waitForElementToBeRemoved(screen.getByRole('status')); - act(async () => { + await act(async () => { userEvent.click(await screen.findByLabelText('Select Piazza')); userEvent.click(await screen.findByText(messages.nextButton.defaultMessage)); From af509998b96f2129731ad09a077605edbb8058ed Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Fri, 31 May 2024 09:43:09 +0930 Subject: [PATCH 04/17] test: Removed "async" from "describe" Returning a Promise from "describe" is not supported. --- src/studio-home/StudioHome.test.jsx | 2 +- .../CollapsibleStateWithAction.test.jsx | 2 +- .../organization-section/OrganizationSection.test.jsx | 2 +- .../processing-courses/course-item/CourseItem.test.jsx | 2 +- src/taxonomy/TaxonomyLayout.test.jsx | 2 +- src/taxonomy/taxonomy-card/TaxonomyCard.test.jsx | 2 +- src/taxonomy/taxonomy-detail/TaxonomyDetailSideCard.test.jsx | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/studio-home/StudioHome.test.jsx b/src/studio-home/StudioHome.test.jsx index 7286acda0f..0d9a339b35 100644 --- a/src/studio-home/StudioHome.test.jsx +++ b/src/studio-home/StudioHome.test.jsx @@ -49,7 +49,7 @@ const RootWrapper = () => ( ); -describe('', async () => { +describe('', () => { describe('api fetch fails', () => { beforeEach(async () => { initializeMockApp({ diff --git a/src/studio-home/collapsible-state-with-action/CollapsibleStateWithAction.test.jsx b/src/studio-home/collapsible-state-with-action/CollapsibleStateWithAction.test.jsx index 4cff22993d..96cc477bc6 100644 --- a/src/studio-home/collapsible-state-with-action/CollapsibleStateWithAction.test.jsx +++ b/src/studio-home/collapsible-state-with-action/CollapsibleStateWithAction.test.jsx @@ -43,7 +43,7 @@ const props = { state: COURSE_CREATOR_STATES.unrequested, }; -describe('', async () => { +describe('', () => { beforeEach(async () => { initializeMockApp({ authenticatedUser: { diff --git a/src/studio-home/organization-section/OrganizationSection.test.jsx b/src/studio-home/organization-section/OrganizationSection.test.jsx index 62b15987b7..24a4aa5973 100644 --- a/src/studio-home/organization-section/OrganizationSection.test.jsx +++ b/src/studio-home/organization-section/OrganizationSection.test.jsx @@ -30,7 +30,7 @@ const RootWrapper = () => ( ); -describe('', async () => { +describe('', () => { beforeEach(async () => { initializeMockApp({ authenticatedUser: { diff --git a/src/studio-home/processing-courses/course-item/CourseItem.test.jsx b/src/studio-home/processing-courses/course-item/CourseItem.test.jsx index 3c55e36fc0..125f88a9bd 100644 --- a/src/studio-home/processing-courses/course-item/CourseItem.test.jsx +++ b/src/studio-home/processing-courses/course-item/CourseItem.test.jsx @@ -37,7 +37,7 @@ const course = { const props = { course }; -describe('', async () => { +describe('', () => { beforeEach(async () => { initializeMockApp({ authenticatedUser: { diff --git a/src/taxonomy/TaxonomyLayout.test.jsx b/src/taxonomy/TaxonomyLayout.test.jsx index a372b10257..dcea6c2c3c 100644 --- a/src/taxonomy/TaxonomyLayout.test.jsx +++ b/src/taxonomy/TaxonomyLayout.test.jsx @@ -54,7 +54,7 @@ const RootWrapper = () => ( ); -describe('', async () => { +describe('', () => { beforeEach(async () => { initializeMockApp({ authenticatedUser: { diff --git a/src/taxonomy/taxonomy-card/TaxonomyCard.test.jsx b/src/taxonomy/taxonomy-card/TaxonomyCard.test.jsx index a63b451d06..413623e5bc 100644 --- a/src/taxonomy/taxonomy-card/TaxonomyCard.test.jsx +++ b/src/taxonomy/taxonomy-card/TaxonomyCard.test.jsx @@ -37,7 +37,7 @@ const TaxonomyCardComponent = ({ original }) => ( TaxonomyCardComponent.propTypes = TaxonomyCard.propTypes; -describe('', async () => { +describe('', () => { beforeEach(async () => { initializeMockApp({ authenticatedUser: { diff --git a/src/taxonomy/taxonomy-detail/TaxonomyDetailSideCard.test.jsx b/src/taxonomy/taxonomy-detail/TaxonomyDetailSideCard.test.jsx index 7b2dd1fd3a..bee3c39a4a 100644 --- a/src/taxonomy/taxonomy-detail/TaxonomyDetailSideCard.test.jsx +++ b/src/taxonomy/taxonomy-detail/TaxonomyDetailSideCard.test.jsx @@ -26,7 +26,7 @@ const TaxonomyCardComponent = ({ taxonomy }) => ( TaxonomyCardComponent.propTypes = TaxonomyDetailSideCard.propTypes; -describe('', async () => { +describe('', () => { beforeEach(async () => { initializeMockApp({ authenticatedUser: { From fb7391ef6d1c7d113a4d1ce016538b8353506890 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Fri, 31 May 2024 14:19:45 +0930 Subject: [PATCH 05/17] fix: DiscussionsSettings tests Previous commit revealed several issues with these tests: * Don't nest userAction.click in act() -- nested act() statements have indeterminent behaviour. * Use getBy* instead of findBy* with userAction to avoid nested act() statements * Always await userEvent.click * Use fireEvent.click when the onClick handlers need to be called * Use queryBy* instead of getBy* when using .toBeInTheDocument or waitForElementToBeRemoved queryBy* return null when the element is not found. --- .../discussions/DiscussionsSettings.test.jsx | 169 +++++++++--------- 1 file changed, 81 insertions(+), 88 deletions(-) diff --git a/src/pages-and-resources/discussions/DiscussionsSettings.test.jsx b/src/pages-and-resources/discussions/DiscussionsSettings.test.jsx index 5499c59445..7c69e7aa1d 100644 --- a/src/pages-and-resources/discussions/DiscussionsSettings.test.jsx +++ b/src/pages-and-resources/discussions/DiscussionsSettings.test.jsx @@ -5,8 +5,8 @@ import { import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; import { AppProvider, PageWrap } from '@edx/frontend-platform/react'; import { - act, findByRole, getByRole, queryByLabelText, queryByRole, queryByTestId, queryByText, render, - screen, waitFor, waitForElementToBeRemoved, + act, findByRole, fireEvent, getByRole, queryByLabelText, queryByRole, queryByTestId, queryByText, + render, screen, waitFor, waitForElementToBeRemoved, } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import MockAdapter from 'axios-mock-adapter'; @@ -94,7 +94,7 @@ describe('DiscussionsSettings', () => { renderComponent(`/course/${courseId}/pages-and-resources/discussion`); // This is an important line that ensures the spinner has been removed - and thus our main // content has been loaded - prior to proceeding with our expectations. - await waitForElementToBeRemoved(screen.getByRole('status')); + await waitForElementToBeRemoved(screen.queryByRole('status')); expect(queryByTestId(container, 'appList')).toBeInTheDocument(); expect(queryByTestId(container, 'appConfigForm')).not.toBeInTheDocument(); @@ -104,7 +104,7 @@ describe('DiscussionsSettings', () => { renderComponent(`/course/${courseId}/pages-and-resources/discussion/configure/piazza`); // This is an important line that ensures the spinner has been removed - and thus our main // content has been loaded - prior to proceeding with our expectations. - await waitForElementToBeRemoved(screen.getByRole('status')); + await waitForElementToBeRemoved(screen.queryByRole('status')); expect(queryByTestId(container, 'appList')).not.toBeInTheDocument(); expect(queryByTestId(container, 'appConfigForm')).toBeInTheDocument(); @@ -114,7 +114,7 @@ describe('DiscussionsSettings', () => { renderComponent(`/course/${courseId}/pages-and-resources/discussion`); // This is an important line that ensures the spinner has been removed - and thus our main // content has been loaded - prior to proceeding with our expectations. - await waitForElementToBeRemoved(screen.getByRole('status')); + await waitForElementToBeRemoved(screen.queryByRole('status')); await waitFor(() => { userEvent.click(queryByLabelText(container, 'Select Piazza')); @@ -134,7 +134,7 @@ describe('DiscussionsSettings', () => { renderComponent(`/course/${courseId}/pages-and-resources/discussion`); // This is an important line that ensures the spinner has been removed - and thus our main // content has been loaded - prior to proceeding with our expectations. - await waitForElementToBeRemoved(screen.getByRole('status')); + await waitForElementToBeRemoved(screen.queryByRole('status')); await waitFor(() => { userEvent.click(queryByLabelText(container, 'Select edX')); @@ -151,11 +151,11 @@ describe('DiscussionsSettings', () => { renderComponent(`/course/${courseId}/pages-and-resources/discussion/configure/piazza`); // This is an important line that ensures the spinner has been removed - and thus our main // content has been loaded - prior to proceeding with our expectations. - await waitForElementToBeRemoved(screen.getByRole('status')); + await waitForElementToBeRemoved(screen.queryByRole('status')); expect(queryByTestId(container, 'appConfigForm')).toBeInTheDocument(); - await act(() => userEvent.click(queryByText(container, appMessages.backButton.defaultMessage))); + await waitFor(() => userEvent.click(queryByText(container, appMessages.backButton.defaultMessage))); await waitFor(() => { expect(queryByTestId(container, 'appList')).toBeInTheDocument(); @@ -167,11 +167,11 @@ describe('DiscussionsSettings', () => { renderComponent(`/course/${courseId}/pages-and-resources/discussion`); // This is an important line that ensures the spinner has been removed - and thus our main // content has been loaded - prior to proceeding with our expectations. - await waitForElementToBeRemoved(screen.getByRole('status')); + await waitForElementToBeRemoved(screen.queryByRole('status')); expect(queryByTestId(container, 'appList')).toBeInTheDocument(); - userEvent.click(queryByLabelText(container, 'Close')); + await userEvent.click(queryByLabelText(container, 'Close')); expect(queryByTestId(container, 'appList')).not.toBeInTheDocument(); expect(queryByTestId(container, 'appConfigForm')).not.toBeInTheDocument(); @@ -183,22 +183,25 @@ describe('DiscussionsSettings', () => { // This is an important line that ensures the spinner has been removed - and thus our main // content has been loaded - prior to proceeding with our expectations. - await waitForElementToBeRemoved(screen.getByRole('status')); + await waitForElementToBeRemoved(screen.queryByRole('status')); - await act(async () => { - userEvent.click(queryByLabelText(container, 'Select Piazza')); + await userEvent.click(screen.getByLabelText('Select Piazza')); - userEvent.click(getByRole(container, 'button', { name: 'Next' })); - - userEvent.click(await findByRole(container, 'button', { name: 'Save' })); + // Have to use fireEvent.click with these Stepper buttons so that the + // onClick handler is triggered. (userEvent.click doesn't trigger onClick). + await act(async () => { + fireEvent.click(getByRole(container, 'button', { name: 'Next' })); + }); + await act(async () => { + fireEvent.click(getByRole(container, 'button', { name: 'Save' })); + }); - // This is an important line that ensures the Close button has been removed, which implies that - // the full screen modal has been closed following our click of Apply. Once this has happened, - // then it's safe to proceed with our expectations. - await waitFor(() => expect(screen.queryByRole(container, 'button', { name: 'Close' })).toBeNull()); + // This is an important line that ensures the Close button has been removed, which implies that + // the full screen modal has been closed following our click of Apply. Once this has happened, + // then it's safe to proceed with our expectations. + await waitFor(() => expect(screen.queryByRole(container, 'button', { name: 'Close' })).toBeNull()); - await waitFor(() => expect(window.location.pathname).toEqual(`/course/${courseId}/pages-and-resources`)); - }); + await waitFor(() => expect(window.location.pathname).toEqual(`/course/${courseId}/pages-and-resources`)); }); test('requires confirmation if changing provider', async () => { @@ -208,20 +211,18 @@ describe('DiscussionsSettings', () => { renderComponent(`/course/${courseId}/pages-and-resources/discussion`); // This is an important line that ensures the spinner has been removed - and thus our main // content has been loaded - prior to proceeding with our expectations. - await waitForElementToBeRemoved(screen.getByRole('status')); + await waitForElementToBeRemoved(screen.queryByRole('status')); - await act(async () => { - userEvent.click(getByRole(container, 'checkbox', { name: 'Select Discourse' })); - userEvent.click(getByRole(container, 'button', { name: 'Next' })); + await userEvent.click(getByRole(container, 'checkbox', { name: 'Select Discourse' })); + await userEvent.click(getByRole(container, 'button', { name: 'Next' })); - await findByRole(container, 'button', { name: 'Save' }); - userEvent.type(getByRole(container, 'textbox', { name: 'Consumer Key' }), 'key'); - userEvent.type(getByRole(container, 'textbox', { name: 'Consumer Secret' }), 'secret'); - userEvent.type(getByRole(container, 'textbox', { name: 'Launch URL' }), 'http://example.test'); - userEvent.click(getByRole(container, 'button', { name: 'Save' })); + await findByRole(container, 'button', { name: 'Save' }); + await userEvent.type(getByRole(container, 'textbox', { name: 'Consumer Key' }), 'key'); + await userEvent.type(getByRole(container, 'textbox', { name: 'Consumer Secret' }), 'secret'); + await userEvent.type(getByRole(container, 'textbox', { name: 'Launch URL' }), 'http://example.test'); + await userEvent.click(getByRole(container, 'button', { name: 'Save' })); - await waitFor(() => expect(queryByRole(container, 'dialog', { name: 'OK' })).toBeInTheDocument()); - }); + await waitFor(() => expect(queryByRole(container, 'dialog', { name: 'OK' })).toBeInTheDocument()); }); test('can cancel confirmation', async () => { @@ -231,30 +232,28 @@ describe('DiscussionsSettings', () => { renderComponent(`/course/${courseId}/pages-and-resources/discussion`); // This is an important line that ensures the spinner has been removed - and thus our main // content has been loaded - prior to proceeding with our expectations. - await waitForElementToBeRemoved(screen.getByRole('status')); + await waitForElementToBeRemoved(screen.queryByRole('status')); const discourseBox = getByRole(container, 'checkbox', { name: 'Select Discourse' }); expect(discourseBox).not.toBeDisabled(); - userEvent.click(discourseBox); + await userEvent.click(discourseBox); - userEvent.click(getByRole(container, 'button', { name: 'Next' })); + await userEvent.click(getByRole(container, 'button', { name: 'Next' })); await waitFor(() => expect(screen.queryByRole('status')).toBeNull()); - await act(async () => { - expect(await findByRole(container, 'heading', { name: 'Discourse' })).toBeInTheDocument(); + expect(await findByRole(container, 'heading', { name: 'Discourse' })).toBeInTheDocument(); - userEvent.type(getByRole(container, 'textbox', { name: 'Consumer Key' }), 'a'); - userEvent.type(getByRole(container, 'textbox', { name: 'Consumer Secret' }), 'secret'); - userEvent.type(getByRole(container, 'textbox', { name: 'Launch URL' }), 'http://example.test'); - userEvent.click(getByRole(container, 'button', { name: 'Save' })); + await userEvent.type(getByRole(container, 'textbox', { name: 'Consumer Key' }), 'a'); + await userEvent.type(getByRole(container, 'textbox', { name: 'Consumer Secret' }), 'secret'); + await userEvent.type(getByRole(container, 'textbox', { name: 'Launch URL' }), 'http://example.test'); + await userEvent.click(getByRole(container, 'button', { name: 'Save' })); - waitFor(() => expect(getByRole(container, 'dialog', { name: 'OK' })).toBeInTheDocument()); - userEvent.click(getByRole(container, 'button', { name: 'Cancel' })); + await waitFor(() => expect(getByRole(container, 'dialog', { name: 'OK' })).toBeInTheDocument()); + await userEvent.click(getByRole(container, 'button', { name: 'Cancel' })); - expect(queryByRole(container, 'dialog', { name: 'Confirm' })).not.toBeInTheDocument(); - expect(queryByRole(container, 'dialog', { name: 'Configure discussion' })); - }); + expect(queryByRole(container, 'dialog', { name: 'Confirm' })).not.toBeInTheDocument(); + expect(queryByRole(container, 'dialog', { name: 'Configure discussion' })); }); }); @@ -274,7 +273,7 @@ describe('DiscussionsSettings', () => { renderComponent(`/course/${courseId}/pages-and-resources/discussion`); // This is an important line that ensures the spinner has been removed - and thus our main // content has been loaded - prior to proceeding with our expectations. - await waitForElementToBeRemoved(screen.getByRole('status')); + await waitForElementToBeRemoved(screen.queryByRole('status')); const alert = queryByRole(container, 'alert'); expect(alert).toBeInTheDocument(); @@ -302,19 +301,17 @@ describe('DiscussionsSettings', () => { renderComponent(`/course/${courseId}/pages-and-resources/discussion/configure/piazza`); // This is an important line that ensures the spinner has been removed - and thus our main // content has been loaded - prior to proceeding with our expectations. - await waitForElementToBeRemoved(screen.getByRole('status')); + await waitForElementToBeRemoved(screen.queryByRole('status')); // Apply causes an async action to take place - await act(async () => { - userEvent.click(queryByText(container, appMessages.saveButton.defaultMessage)); - await waitFor(() => expect(axiosMock.history.post.length).toBe(1)); - - expect(queryByTestId(container, 'appConfigForm')).toBeInTheDocument(); - const alert = await findByRole(container, 'alert'); - expect(alert).toBeInTheDocument(); - expect(alert.textContent).toEqual(expect.stringContaining('We encountered a technical error when applying changes.')); - expect(alert.innerHTML).toEqual(expect.stringContaining(getConfig().SUPPORT_URL)); - }); + await userEvent.click(queryByText(container, appMessages.saveButton.defaultMessage)); + await waitFor(() => expect(axiosMock.history.post.length).toBe(1)); + + expect(queryByTestId(container, 'appConfigForm')).toBeInTheDocument(); + const alert = await findByRole(container, 'alert'); + expect(alert).toBeInTheDocument(); + expect(alert.textContent).toEqual(expect.stringContaining('We encountered a technical error when applying changes.')); + expect(alert.innerHTML).toEqual(expect.stringContaining(getConfig().SUPPORT_URL)); }); }); @@ -328,7 +325,7 @@ describe('DiscussionsSettings', () => { renderComponent(`/course/${courseId}/pages-and-resources/discussion`); // This is an important line that ensures the spinner has been removed - and thus our main // content has been loaded - prior to proceeding with our expectations. - await waitForElementToBeRemoved(screen.getByRole('status')); + await waitForElementToBeRemoved(screen.queryByRole('status')); const alert = queryByRole(container, 'alert'); expect(alert).toBeInTheDocument(); @@ -348,23 +345,21 @@ describe('DiscussionsSettings', () => { renderComponent(`/course/${courseId}/pages-and-resources/discussion/configure/piazza`); // This is an important line that ensures the spinner has been removed - and thus our main // content has been loaded - prior to proceeding with our expectations. - await waitForElementToBeRemoved(screen.getByRole('status')); + await waitForElementToBeRemoved(screen.queryByRole('status')); - await act(async () => { - userEvent.click(getByRole(container, 'button', { name: 'Save' })); + await userEvent.click(getByRole(container, 'button', { name: 'Save' })); - await waitFor(() => expect(axiosMock.history.post.length).toBe(1)); + await waitFor(() => expect(axiosMock.history.post.length).toBe(1)); - expect(queryByTestId(container, 'appList')).not.toBeInTheDocument(); - expect(queryByTestId(container, 'appConfigForm')).not.toBeInTheDocument(); + expect(queryByTestId(container, 'appList')).not.toBeInTheDocument(); + expect(queryByTestId(container, 'appConfigForm')).not.toBeInTheDocument(); - // We don't technically leave the route in this case, though the modal is hidden. - expect(window.location.pathname).toEqual(`/course/${courseId}/pages-and-resources/discussion/configure/piazza`); + // We don't technically leave the route in this case, though the modal is hidden. + expect(window.location.pathname).toEqual(`/course/${courseId}/pages-and-resources/discussion/configure/piazza`); - const alert = await findByRole(container, 'alert'); - expect(alert).toBeInTheDocument(); - expect(alert.textContent).toEqual(expect.stringContaining('You are not authorized to view this page.')); - }); + const alert = await findByRole(container, 'alert'); + expect(alert).toBeInTheDocument(); + expect(alert.textContent).toEqual(expect.stringContaining('You are not authorized to view this page.')); }); }); }); @@ -406,13 +401,13 @@ describe.each([ renderComponent(`/course/${courseId}/pages-and-resources/discussion`); // This is an important line that ensures the spinner has been removed - and thus our main // content has been loaded - prior to proceeding with our expectations. - waitForElementToBeRemoved(screen.getByRole('status')); + await waitForElementToBeRemoved(screen.queryByRole('status')); - await act(async () => { - userEvent.click(await screen.findByLabelText('Select Piazza')); - userEvent.click(queryByText(container, messages.nextButton.defaultMessage)); - waitForElementToBeRemoved(screen.getByRole('status')); + await userEvent.click(screen.getByLabelText('Select Piazza')); + await userEvent.click(queryByText(container, messages.nextButton.defaultMessage)); + expect(screen.queryByRole('status')).not.toBeInTheDocument(); + await waitFor(() => { if (showLTIConfig) { expect(queryByText(container, ltiMessages.formInstructions.defaultMessage)).toBeInTheDocument(); expect(queryByTestId(container, 'ltiConfigFields')).toBeInTheDocument(); @@ -460,18 +455,16 @@ describe.each([ renderComponent(`/course/${courseId}/pages-and-resources/discussion`); // This is an important line that ensures the spinner has been removed - and thus our main // content has been loaded - prior to proceeding with our expectations. - waitForElementToBeRemoved(screen.getByRole('status')); + await waitForElementToBeRemoved(screen.queryByRole('status')); - await act(async () => { - userEvent.click(await screen.findByLabelText('Select Piazza')); - userEvent.click(await screen.findByText(messages.nextButton.defaultMessage)); + await userEvent.click(screen.getByLabelText('Select Piazza')); + await userEvent.click(screen.getByText(messages.nextButton.defaultMessage)); - waitForElementToBeRemoved(screen.getByRole('status')); - if (enablePIISharing) { - expect(queryByTestId(container, 'piiSharingFields')).toBeInTheDocument(); - } else { - expect(queryByTestId(container, 'piiSharingFields')).not.toBeInTheDocument(); - } - }); + expect(screen.queryByRole('status')).not.toBeInTheDocument(); + if (enablePIISharing) { + expect(queryByTestId(container, 'piiSharingFields')).toBeInTheDocument(); + } else { + expect(queryByTestId(container, 'piiSharingFields')).not.toBeInTheDocument(); + } }); }); From 3c48d814383b948a0f40d21866492c5c3a7ac3e9 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Fri, 31 May 2024 15:45:58 +0930 Subject: [PATCH 06/17] fix: typo in data-testid Warning: React does not recognize the `data-testId` prop on a DOM element. If you intentionally want it to appear in the DOM as a custom attribute, spell it as lowercase `data-testid` instead. --- .../app-config-form/apps/shared/DiscussionRestriction.jsx | 2 +- .../apps/shared/discussion-restrictions/RestrictDatesInput.jsx | 2 +- .../shared/discussion-restrictions/RestrictionSchedules.jsx | 2 +- src/pages-and-resources/discussions/app-list/AppList.jsx | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/pages-and-resources/discussions/app-config-form/apps/shared/DiscussionRestriction.jsx b/src/pages-and-resources/discussions/app-config-form/apps/shared/DiscussionRestriction.jsx index fce616a4aa..3fbee41c10 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/shared/DiscussionRestriction.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/shared/DiscussionRestriction.jsx @@ -40,7 +40,7 @@ const DiscussionRestriction = () => { const discussionRestrictionButtons = useMemo(() => discussionRestrictionOptions.map((restriction) => (