From 0a3b31ff7adb8e741781b7fe9688531ec3cd4467 Mon Sep 17 00:00:00 2001 From: Kyrylo Hudym-Levkovych Date: Wed, 3 Jul 2024 22:54:39 +0300 Subject: [PATCH 1/4] feat: same titles for sent email groups --- .../bulk-email-tool/BulkEmailTool.jsx | 5 +- .../bulk-email-form/BulkEmailForm.jsx | 5 +- .../BulkEmailRecipient.jsx | 7 +-- .../test/BulkEmailForm.test.jsx | 13 ++--- .../BulkEmailContentHistory.jsx | 3 +- .../BulkEmailTaskManager.jsx | 10 +++- .../ViewEmailModal.jsx | 2 +- .../BulkEmailScheduledEmailsTable.jsx | 22 +++++--- .../BulkEmailScheduledEmailsTable.test.jsx | 3 +- src/components/bulk-email-tool/utils.js | 52 +++++++++++++++++++ 10 files changed, 99 insertions(+), 23 deletions(-) create mode 100644 src/components/bulk-email-tool/utils.js diff --git a/src/components/bulk-email-tool/BulkEmailTool.jsx b/src/components/bulk-email-tool/BulkEmailTool.jsx index 1ebec152..9962d1e5 100644 --- a/src/components/bulk-email-tool/BulkEmailTool.jsx +++ b/src/components/bulk-email-tool/BulkEmailTool.jsx @@ -40,7 +40,10 @@ export default function BulkEmailTool() { />
- +
diff --git a/src/components/bulk-email-tool/bulk-email-form/BulkEmailForm.jsx b/src/components/bulk-email-tool/bulk-email-form/BulkEmailForm.jsx index 29de06d7..4aea1e64 100644 --- a/src/components/bulk-email-tool/bulk-email-form/BulkEmailForm.jsx +++ b/src/components/bulk-email-tool/bulk-email-form/BulkEmailForm.jsx @@ -28,6 +28,7 @@ import { } from './data/actions'; import { editScheduledEmailThunk, postBulkEmailThunk } from './data/thunks'; import { getScheduledBulkEmailThunk } from '../bulk-email-task-manager/bulk-email-scheduled-emails-table/data/thunks'; +import { getDisplayText } from '../utils'; import './bulkEmailForm.scss'; @@ -219,7 +220,7 @@ function BulkEmailForm(props) {

{intl.formatMessage(messages.bulkEmailTaskAlertRecipients, { subject: editor.emailSubject })}

{!isScheduled && ( @@ -246,7 +247,7 @@ function BulkEmailForm(props) {

{intl.formatMessage(messages.bulkEmailTaskAlertEditingTo)}

{intl.formatMessage(messages.bulkEmailTaskAlertEditingWarning)}

diff --git a/src/components/bulk-email-tool/bulk-email-form/bulk-email-recipient/BulkEmailRecipient.jsx b/src/components/bulk-email-tool/bulk-email-form/bulk-email-recipient/BulkEmailRecipient.jsx index d60b316b..48445990 100644 --- a/src/components/bulk-email-tool/bulk-email-form/bulk-email-recipient/BulkEmailRecipient.jsx +++ b/src/components/bulk-email-tool/bulk-email-form/bulk-email-recipient/BulkEmailRecipient.jsx @@ -2,6 +2,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import { Form } from '@openedx/paragon'; import { FormattedMessage } from '@edx/frontend-platform/i18n'; +import { RECIPIENTS_DISPLAY_NAMES } from '../../utils'; import './bulkEmailRecepient.scss'; @@ -41,7 +42,7 @@ export default function BulkEmailRecipient(props) { @@ -52,7 +53,7 @@ export default function BulkEmailRecipient(props) { > @@ -99,7 +100,7 @@ export default function BulkEmailRecipient(props) { > diff --git a/src/components/bulk-email-tool/bulk-email-form/test/BulkEmailForm.test.jsx b/src/components/bulk-email-tool/bulk-email-form/test/BulkEmailForm.test.jsx index 2d0dd1c3..92d9de91 100644 --- a/src/components/bulk-email-tool/bulk-email-form/test/BulkEmailForm.test.jsx +++ b/src/components/bulk-email-tool/bulk-email-form/test/BulkEmailForm.test.jsx @@ -13,6 +13,7 @@ import { BulkEmailContext, BulkEmailProvider } from '../../bulk-email-context'; import { formatDate } from '../../../../utils/formatDateAndTime'; import cohortFactory from '../data/__factories__/bulkEmailFormCohort.factory'; import courseModeFactory from '../data/__factories__/bulkEmailFormCourseMode.factory'; +import { RECIPIENTS_DISPLAY_NAMES } from '../../utils'; jest.mock('../../text-editor/TextEditor'); @@ -75,8 +76,8 @@ describe('bulk-email-form', () => { success: true, }); render(renderBulkEmailForm()); - fireEvent.click(screen.getByRole('checkbox', { name: 'Myself' })); - expect(screen.getByRole('checkbox', { name: 'Myself' })).toBeChecked(); + fireEvent.click(screen.getByRole('checkbox', { name: RECIPIENTS_DISPLAY_NAMES.myself })); + expect(screen.getByRole('checkbox', { name: RECIPIENTS_DISPLAY_NAMES.myself })).toBeChecked(); fireEvent.change(screen.getByRole('textbox', { name: 'Subject' }), { target: { value: 'test subject' } }); fireEvent.change(screen.getByTestId('textEditor'), { target: { value: 'test body' } }); fireEvent.click(screen.getByText('Send email')); @@ -90,7 +91,7 @@ describe('bulk-email-form', () => { axiosMock.onPost(`${getConfig().LMS_BASE_URL}/courses/test/instructor/api/send_email`).reply(500); render(renderBulkEmailForm()); const subjectLine = screen.getByRole('textbox', { name: 'Subject' }); - const recipient = screen.getByRole('checkbox', { name: 'Myself' }); + const recipient = screen.getByRole('checkbox', { name: RECIPIENTS_DISPLAY_NAMES.myself }); fireEvent.click(recipient); fireEvent.change(subjectLine, { target: { value: 'test subject' } }); fireEvent.change(screen.getByTestId('textEditor'), { target: { value: 'test body' } }); @@ -99,9 +100,9 @@ describe('bulk-email-form', () => { fireEvent.click(await screen.findByRole('button', { name: /continue/i })); expect(await screen.findByText('An error occured while attempting to send the email.')).toBeInTheDocument(); }); - test('Checking "All Learners" disables each learner group', async () => { + test('Checking "All Students" disables each learner group', async () => { render(renderBulkEmailForm()); - fireEvent.click(screen.getByRole('checkbox', { name: 'All Learners' })); + fireEvent.click(screen.getByRole('checkbox', { name: RECIPIENTS_DISPLAY_NAMES.learners })); const verifiedLearners = screen.getByRole('checkbox', { name: 'Learners in the Verified Certificate Track' }); const auditLearners = screen.getByRole('checkbox', { name: 'Learners in the Audit Track' }); const { cohorts } = cohortFactory.build(); @@ -130,7 +131,7 @@ describe('bulk-email-form', () => { test('Adds scheduling data to POST requests when schedule is selected', async () => { const postBulkEmailInstructorTask = jest.spyOn(bulkEmailFormApi, 'postBulkEmailInstructorTask'); render(renderBulkEmailForm()); - fireEvent.click(screen.getByRole('checkbox', { name: 'Myself' })); + fireEvent.click(screen.getByRole('checkbox', { name: RECIPIENTS_DISPLAY_NAMES.myself })); fireEvent.change(screen.getByRole('textbox', { name: 'Subject' }), { target: { value: 'test subject' } }); fireEvent.change(screen.getByTestId('textEditor'), { target: { value: 'test body' } }); const scheduleCheckbox = screen.getByText('Schedule this email for a future date'); diff --git a/src/components/bulk-email-tool/bulk-email-task-manager/BulkEmailContentHistory.jsx b/src/components/bulk-email-tool/bulk-email-task-manager/BulkEmailContentHistory.jsx index ef0e2446..2bd1c576 100644 --- a/src/components/bulk-email-tool/bulk-email-task-manager/BulkEmailContentHistory.jsx +++ b/src/components/bulk-email-tool/bulk-email-task-manager/BulkEmailContentHistory.jsx @@ -13,6 +13,7 @@ import messages from './messages'; import { getSentEmailHistory } from './data/api'; import BulkEmailTaskManagerTable from './BulkEmailHistoryTable'; import ViewEmailModal from './ViewEmailModal'; +import { HISTORY_RECIPIENTS_DISPLAY_NAMES } from '../utils'; function BulkEmailContentHistory({ intl }) { const { courseId } = useParams(); @@ -55,7 +56,7 @@ function BulkEmailContentHistory({ intl }) { const tableData = emailHistoryData?.map((item) => ({ ...item, subject: item.email.subject, - sent_to: item.sent_to.join(', '), + sent_to: item.sent_to.map((recipient) => HISTORY_RECIPIENTS_DISPLAY_NAMES[recipient] || recipient).join(', '), created: new Date(item.created).toLocaleString(), })); return tableData || []; diff --git a/src/components/bulk-email-tool/bulk-email-task-manager/BulkEmailTaskManager.jsx b/src/components/bulk-email-tool/bulk-email-task-manager/BulkEmailTaskManager.jsx index 43a34cdb..2bc5267f 100644 --- a/src/components/bulk-email-tool/bulk-email-task-manager/BulkEmailTaskManager.jsx +++ b/src/components/bulk-email-tool/bulk-email-task-manager/BulkEmailTaskManager.jsx @@ -9,13 +9,13 @@ import messages from './messages'; import BulkEmailScheduledEmailsTable from './bulk-email-scheduled-emails-table'; import BulkEmailPendingTasksAlert from './BulkEmailPendingTasksAlert'; -function BulkEmailTaskManager({ intl, courseId }) { +function BulkEmailTaskManager({ intl, courseId, courseModes }) { return (
{getConfig().SCHEDULE_EMAIL_SECTION && (

{intl.formatMessage(messages.scheduledEmailsTableHeader)}

- +
)}
@@ -36,6 +36,12 @@ function BulkEmailTaskManager({ intl, courseId }) { BulkEmailTaskManager.propTypes = { intl: intlShape.isRequired, courseId: PropTypes.string.isRequired, + courseModes: PropTypes.arrayOf( + PropTypes.shape({ + slug: PropTypes.string.isRequired, + name: PropTypes.string.isRequired, + }), + ).isRequired, }; export default injectIntl(BulkEmailTaskManager); diff --git a/src/components/bulk-email-tool/bulk-email-task-manager/ViewEmailModal.jsx b/src/components/bulk-email-tool/bulk-email-task-manager/ViewEmailModal.jsx index 03b9f547..03d39a0c 100644 --- a/src/components/bulk-email-tool/bulk-email-task-manager/ViewEmailModal.jsx +++ b/src/components/bulk-email-tool/bulk-email-task-manager/ViewEmailModal.jsx @@ -33,7 +33,7 @@ function ViewEmailModal({

{messageContent.created}

-

{intl.formatMessage(messages.modalMessageSentTo)}

+

{intl.formatMessage(messages.modalMessageSentTo)}

{messageContent.sent_to}

diff --git a/src/components/bulk-email-tool/bulk-email-task-manager/bulk-email-scheduled-emails-table/BulkEmailScheduledEmailsTable.jsx b/src/components/bulk-email-tool/bulk-email-task-manager/bulk-email-scheduled-emails-table/BulkEmailScheduledEmailsTable.jsx index 0e5a5487..7f0d74f0 100644 --- a/src/components/bulk-email-tool/bulk-email-task-manager/bulk-email-scheduled-emails-table/BulkEmailScheduledEmailsTable.jsx +++ b/src/components/bulk-email-tool/bulk-email-task-manager/bulk-email-scheduled-emails-table/BulkEmailScheduledEmailsTable.jsx @@ -4,6 +4,7 @@ import React, { useCallback, useContext, useState, useEffect, } from 'react'; +import PropTypes from 'prop-types'; import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; import { Alert, DataTable, Icon, IconButton, useToggle, @@ -19,8 +20,9 @@ import ViewEmailModal from '../ViewEmailModal'; import { copyToEditor } from '../../bulk-email-form/data/actions'; import TaskAlertModal from '../../task-alert-modal'; import { formatDate, formatTime } from '../../../../utils/formatDateAndTime'; +import { getDisplayText, getRecipientFromDisplayText } from '../../utils'; -function flattenScheduledEmailsArray(emails) { +function flattenScheduledEmailsArray(emails, courseModes) { return emails.map((email) => ({ schedulingId: email.id, emailId: email.courseEmail.id, @@ -28,11 +30,12 @@ function flattenScheduledEmailsArray(emails) { taskDue: new Date(email.taskDue).toLocaleString(), taskDueUTC: email.taskDue, ...email.courseEmail, - targets: email.courseEmail.targets.join(', '), + targets: email.courseEmail.targets + .map((recipient) => getDisplayText(recipient, courseModes)).join(', '), })); } -function BulkEmailScheduledEmailsTable({ intl }) { +function BulkEmailScheduledEmailsTable({ intl, courseModes }) { const { courseId } = useParams(); const [{ scheduledEmailsTable }, dispatch] = useContext(BulkEmailContext); const [tableData, setTableData] = useState([]); @@ -44,8 +47,8 @@ function BulkEmailScheduledEmailsTable({ intl }) { const [currentTask, setCurrentTask] = useState({}); useEffect(() => { - setTableData(flattenScheduledEmailsArray(scheduledEmailsTable.results)); - }, [scheduledEmailsTable.results]); + setTableData(flattenScheduledEmailsArray(scheduledEmailsTable.results, courseModes)); + }, [scheduledEmailsTable.results, courseModes]); const fetchTableData = useCallback((args) => { dispatch(getScheduledBulkEmailThunk(courseId, args.pageIndex + 1)); @@ -96,7 +99,8 @@ function BulkEmailScheduledEmailsTable({ intl }) { }, } = row; const dateTime = new Date(taskDueUTC); - const emailRecipients = targets.replaceAll('-', ':').split(', '); + const emailRecipients = targets + .split(', ').map((recipient) => getRecipientFromDisplayText(recipient, courseModes)); const scheduleDate = formatDate(dateTime); const scheduleTime = formatTime(dateTime); dispatch( @@ -198,6 +202,12 @@ function BulkEmailScheduledEmailsTable({ intl }) { BulkEmailScheduledEmailsTable.propTypes = { intl: intlShape.isRequired, + courseModes: PropTypes.arrayOf( + PropTypes.shape({ + slug: PropTypes.string.isRequired, + name: PropTypes.string.isRequired, + }), + ).isRequired, }; export default injectIntl(BulkEmailScheduledEmailsTable); diff --git a/src/components/bulk-email-tool/bulk-email-task-manager/bulk-email-scheduled-emails-table/test/BulkEmailScheduledEmailsTable.test.jsx b/src/components/bulk-email-tool/bulk-email-task-manager/bulk-email-scheduled-emails-table/test/BulkEmailScheduledEmailsTable.test.jsx index 3495bef2..f461b2d5 100644 --- a/src/components/bulk-email-tool/bulk-email-task-manager/bulk-email-scheduled-emails-table/test/BulkEmailScheduledEmailsTable.test.jsx +++ b/src/components/bulk-email-tool/bulk-email-task-manager/bulk-email-scheduled-emails-table/test/BulkEmailScheduledEmailsTable.test.jsx @@ -13,6 +13,7 @@ import { BulkEmailProvider } from '../../../bulk-email-context'; import BulkEmailScheduledEmailsTable from '..'; import scheduledEmailsFactory from './__factories__/scheduledEmails.factory'; import * as actions from '../../../bulk-email-form/data/actions'; +import { RECIPIENTS_DISPLAY_NAMES } from '../../../utils'; jest.mock('react-router-dom', () => ({ ...jest.requireActual('react-router-dom'), @@ -42,7 +43,7 @@ describe('BulkEmailScheduledEmailsTable', () => { .onGet(`${getConfig().LMS_BASE_URL}/api/instructor_task/v1/schedules/test-id/bulk_email/?page=1`) .reply(200, scheduledEmailsFactory.build(1)); render(renderBulkEmailScheduledEmailsTable()); - expect(await screen.findByText('learners')).toBeTruthy(); + expect(await screen.findByText(RECIPIENTS_DISPLAY_NAMES.learners)).toBeTruthy(); expect(await screen.findByText('subject')).toBeTruthy(); expect(await screen.findByText('edx')).toBeTruthy(); expect(await screen.findByLabelText('View')).toBeTruthy(); diff --git a/src/components/bulk-email-tool/utils.js b/src/components/bulk-email-tool/utils.js new file mode 100644 index 00000000..2e454128 --- /dev/null +++ b/src/components/bulk-email-tool/utils.js @@ -0,0 +1,52 @@ +export const RECIPIENTS_DISPLAY_NAMES = { + myself: 'Myself', + staff: 'Staff/Administrators', + learners: 'All Learners', +}; + +export const HISTORY_RECIPIENTS_DISPLAY_NAMES = { + 'Staff and instructors': 'Staff/Administrators', + 'All students': 'All Learners', +}; + +// Output: { 'Myself': 'myself', 'Staff/Administrators': 'staff', 'All Learners': 'learners' } +export const REVERSE_RECIPIENTS_DISPLAY_NAMES = Object.fromEntries( + Object.entries(RECIPIENTS_DISPLAY_NAMES).map(([key, value]) => [value, key]), +); + +export const getDisplayText = (recipient, courseModes) => { + const normalizedRecipient = recipient.replace(/-/, ':'); + + if (normalizedRecipient.startsWith('track') && courseModes) { + const courseModeSlug = normalizedRecipient.split(':')[1]; + const courseMode = courseModes.find((mode) => mode.slug === courseModeSlug); + if (courseMode) { + return `Learners in the ${courseMode.name} Track`; + } + } + + if (normalizedRecipient.startsWith('cohort')) { + const cohort = normalizedRecipient.replace(/:/, ': '); + return cohort.charAt(0).toUpperCase() + cohort.slice(1); + } + + return RECIPIENTS_DISPLAY_NAMES[recipient] || recipient; +}; + +export const getRecipientFromDisplayText = (displayText, courseModes) => { + const trackMatch = displayText.match(/^Learners in the (.+) Track$/); + if (trackMatch) { + const courseModeName = trackMatch[1]; + const courseMode = courseModes.find(mode => mode.name === courseModeName); + if (courseMode) { + return `track:${courseMode.slug}`; + } + } + + const cohortMatch = displayText.match(/^Cohort: (.+)$/); + if (cohortMatch) { + return `cohort:${cohortMatch[1].replace(' ', '-')}`; + } + + return REVERSE_RECIPIENTS_DISPLAY_NAMES[displayText] || displayText; +}; From 0d9fb9fa02ccf108f7c011c634aa7ba0d0af07f3 Mon Sep 17 00:00:00 2001 From: Kyrylo Hudym-Levkovych Date: Thu, 4 Jul 2024 17:42:49 +0300 Subject: [PATCH 2/4] feat: aligned the checkbox labels with the BE --- .../bulk-email-tool/BulkEmailTool.jsx | 5 +- .../bulk-email-form/BulkEmailForm.jsx | 6 +-- .../BulkEmailContentHistory.jsx | 3 +- .../BulkEmailTaskManager.jsx | 10 +--- .../BulkEmailScheduledEmailsTable.jsx | 17 ++----- src/components/bulk-email-tool/utils.js | 48 ++----------------- 6 files changed, 16 insertions(+), 73 deletions(-) diff --git a/src/components/bulk-email-tool/BulkEmailTool.jsx b/src/components/bulk-email-tool/BulkEmailTool.jsx index 9962d1e5..1ebec152 100644 --- a/src/components/bulk-email-tool/BulkEmailTool.jsx +++ b/src/components/bulk-email-tool/BulkEmailTool.jsx @@ -40,10 +40,7 @@ export default function BulkEmailTool() { />
- +
diff --git a/src/components/bulk-email-tool/bulk-email-form/BulkEmailForm.jsx b/src/components/bulk-email-tool/bulk-email-form/BulkEmailForm.jsx index 4aea1e64..ef334e7c 100644 --- a/src/components/bulk-email-tool/bulk-email-form/BulkEmailForm.jsx +++ b/src/components/bulk-email-tool/bulk-email-form/BulkEmailForm.jsx @@ -28,7 +28,7 @@ import { } from './data/actions'; import { editScheduledEmailThunk, postBulkEmailThunk } from './data/thunks'; import { getScheduledBulkEmailThunk } from '../bulk-email-task-manager/bulk-email-scheduled-emails-table/data/thunks'; -import { getDisplayText } from '../utils'; +import { getDisplayTextFromRecipient } from '../utils'; import './bulkEmailForm.scss'; @@ -220,7 +220,7 @@ function BulkEmailForm(props) {

{intl.formatMessage(messages.bulkEmailTaskAlertRecipients, { subject: editor.emailSubject })}

    {editor.emailRecipients.map((group) => ( -
  • {getDisplayText(group, courseModes)}
    • +
  • {getDisplayTextFromRecipient(group)}
    • ))}
{!isScheduled && ( @@ -247,7 +247,7 @@ function BulkEmailForm(props) {

{intl.formatMessage(messages.bulkEmailTaskAlertEditingTo)}

    {editor.emailRecipients.map((group) => ( -
  • {getDisplayText(group, courseModes)}
    • +
  • {getDisplayTextFromRecipient(group)}
    • ))}

{intl.formatMessage(messages.bulkEmailTaskAlertEditingWarning)}

diff --git a/src/components/bulk-email-tool/bulk-email-task-manager/BulkEmailContentHistory.jsx b/src/components/bulk-email-tool/bulk-email-task-manager/BulkEmailContentHistory.jsx index 2bd1c576..ef0e2446 100644 --- a/src/components/bulk-email-tool/bulk-email-task-manager/BulkEmailContentHistory.jsx +++ b/src/components/bulk-email-tool/bulk-email-task-manager/BulkEmailContentHistory.jsx @@ -13,7 +13,6 @@ import messages from './messages'; import { getSentEmailHistory } from './data/api'; import BulkEmailTaskManagerTable from './BulkEmailHistoryTable'; import ViewEmailModal from './ViewEmailModal'; -import { HISTORY_RECIPIENTS_DISPLAY_NAMES } from '../utils'; function BulkEmailContentHistory({ intl }) { const { courseId } = useParams(); @@ -56,7 +55,7 @@ function BulkEmailContentHistory({ intl }) { const tableData = emailHistoryData?.map((item) => ({ ...item, subject: item.email.subject, - sent_to: item.sent_to.map((recipient) => HISTORY_RECIPIENTS_DISPLAY_NAMES[recipient] || recipient).join(', '), + sent_to: item.sent_to.join(', '), created: new Date(item.created).toLocaleString(), })); return tableData || []; diff --git a/src/components/bulk-email-tool/bulk-email-task-manager/BulkEmailTaskManager.jsx b/src/components/bulk-email-tool/bulk-email-task-manager/BulkEmailTaskManager.jsx index 2bc5267f..43a34cdb 100644 --- a/src/components/bulk-email-tool/bulk-email-task-manager/BulkEmailTaskManager.jsx +++ b/src/components/bulk-email-tool/bulk-email-task-manager/BulkEmailTaskManager.jsx @@ -9,13 +9,13 @@ import messages from './messages'; import BulkEmailScheduledEmailsTable from './bulk-email-scheduled-emails-table'; import BulkEmailPendingTasksAlert from './BulkEmailPendingTasksAlert'; -function BulkEmailTaskManager({ intl, courseId, courseModes }) { +function BulkEmailTaskManager({ intl, courseId }) { return (
{getConfig().SCHEDULE_EMAIL_SECTION && (

{intl.formatMessage(messages.scheduledEmailsTableHeader)}

- +
)}
@@ -36,12 +36,6 @@ function BulkEmailTaskManager({ intl, courseId, courseModes }) { BulkEmailTaskManager.propTypes = { intl: intlShape.isRequired, courseId: PropTypes.string.isRequired, - courseModes: PropTypes.arrayOf( - PropTypes.shape({ - slug: PropTypes.string.isRequired, - name: PropTypes.string.isRequired, - }), - ).isRequired, }; export default injectIntl(BulkEmailTaskManager); diff --git a/src/components/bulk-email-tool/bulk-email-task-manager/bulk-email-scheduled-emails-table/BulkEmailScheduledEmailsTable.jsx b/src/components/bulk-email-tool/bulk-email-task-manager/bulk-email-scheduled-emails-table/BulkEmailScheduledEmailsTable.jsx index 7f0d74f0..6d7698cf 100644 --- a/src/components/bulk-email-tool/bulk-email-task-manager/bulk-email-scheduled-emails-table/BulkEmailScheduledEmailsTable.jsx +++ b/src/components/bulk-email-tool/bulk-email-task-manager/bulk-email-scheduled-emails-table/BulkEmailScheduledEmailsTable.jsx @@ -4,7 +4,6 @@ import React, { useCallback, useContext, useState, useEffect, } from 'react'; -import PropTypes from 'prop-types'; import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; import { Alert, DataTable, Icon, IconButton, useToggle, @@ -20,9 +19,9 @@ import ViewEmailModal from '../ViewEmailModal'; import { copyToEditor } from '../../bulk-email-form/data/actions'; import TaskAlertModal from '../../task-alert-modal'; import { formatDate, formatTime } from '../../../../utils/formatDateAndTime'; -import { getDisplayText, getRecipientFromDisplayText } from '../../utils'; +import { getDisplayTextFromRecipient, getRecipientFromDisplayText } from '../../utils'; -function flattenScheduledEmailsArray(emails, courseModes) { +function flattenScheduledEmailsArray(emails) { return emails.map((email) => ({ schedulingId: email.id, emailId: email.courseEmail.id, @@ -30,8 +29,7 @@ function flattenScheduledEmailsArray(emails, courseModes) { taskDue: new Date(email.taskDue).toLocaleString(), taskDueUTC: email.taskDue, ...email.courseEmail, - targets: email.courseEmail.targets - .map((recipient) => getDisplayText(recipient, courseModes)).join(', '), + targets: email.courseEmail.targets.map(getDisplayTextFromRecipient).join(', '), })); } @@ -99,8 +97,7 @@ function BulkEmailScheduledEmailsTable({ intl, courseModes }) { }, } = row; const dateTime = new Date(taskDueUTC); - const emailRecipients = targets - .split(', ').map((recipient) => getRecipientFromDisplayText(recipient, courseModes)); + const emailRecipients = targets.replaceAll('-', ':').split(', ').map(getRecipientFromDisplayText); const scheduleDate = formatDate(dateTime); const scheduleTime = formatTime(dateTime); dispatch( @@ -202,12 +199,6 @@ function BulkEmailScheduledEmailsTable({ intl, courseModes }) { BulkEmailScheduledEmailsTable.propTypes = { intl: intlShape.isRequired, - courseModes: PropTypes.arrayOf( - PropTypes.shape({ - slug: PropTypes.string.isRequired, - name: PropTypes.string.isRequired, - }), - ).isRequired, }; export default injectIntl(BulkEmailScheduledEmailsTable); diff --git a/src/components/bulk-email-tool/utils.js b/src/components/bulk-email-tool/utils.js index 2e454128..2a37c880 100644 --- a/src/components/bulk-email-tool/utils.js +++ b/src/components/bulk-email-tool/utils.js @@ -1,52 +1,14 @@ export const RECIPIENTS_DISPLAY_NAMES = { myself: 'Myself', - staff: 'Staff/Administrators', - learners: 'All Learners', + staff: 'Staff and instructors', + learners: 'All students', }; -export const HISTORY_RECIPIENTS_DISPLAY_NAMES = { - 'Staff and instructors': 'Staff/Administrators', - 'All students': 'All Learners', -}; - -// Output: { 'Myself': 'myself', 'Staff/Administrators': 'staff', 'All Learners': 'learners' } +// Output: { 'Myself': 'myself', 'Staff and instructors': 'staff', 'All students': 'learners' } export const REVERSE_RECIPIENTS_DISPLAY_NAMES = Object.fromEntries( Object.entries(RECIPIENTS_DISPLAY_NAMES).map(([key, value]) => [value, key]), ); -export const getDisplayText = (recipient, courseModes) => { - const normalizedRecipient = recipient.replace(/-/, ':'); - - if (normalizedRecipient.startsWith('track') && courseModes) { - const courseModeSlug = normalizedRecipient.split(':')[1]; - const courseMode = courseModes.find((mode) => mode.slug === courseModeSlug); - if (courseMode) { - return `Learners in the ${courseMode.name} Track`; - } - } +export const getDisplayTextFromRecipient = (recipient) => RECIPIENTS_DISPLAY_NAMES[recipient] || recipient; - if (normalizedRecipient.startsWith('cohort')) { - const cohort = normalizedRecipient.replace(/:/, ': '); - return cohort.charAt(0).toUpperCase() + cohort.slice(1); - } - - return RECIPIENTS_DISPLAY_NAMES[recipient] || recipient; -}; - -export const getRecipientFromDisplayText = (displayText, courseModes) => { - const trackMatch = displayText.match(/^Learners in the (.+) Track$/); - if (trackMatch) { - const courseModeName = trackMatch[1]; - const courseMode = courseModes.find(mode => mode.name === courseModeName); - if (courseMode) { - return `track:${courseMode.slug}`; - } - } - - const cohortMatch = displayText.match(/^Cohort: (.+)$/); - if (cohortMatch) { - return `cohort:${cohortMatch[1].replace(' ', '-')}`; - } - - return REVERSE_RECIPIENTS_DISPLAY_NAMES[displayText] || displayText; -}; +export const getRecipientFromDisplayText = (recipient) => REVERSE_RECIPIENTS_DISPLAY_NAMES[recipient] || recipient; From 56e5ba22337afad7cddf2d381cdd2b4226b6ec2f Mon Sep 17 00:00:00 2001 From: ihor-romaniuk Date: Mon, 9 Sep 2024 12:23:25 +0200 Subject: [PATCH 3/4] fix: refactor the code --- .../bulk-email-form/BulkEmailForm.jsx | 4 +-- .../BulkEmailRecipient.jsx | 36 +++++-------------- .../test/BulkEmailForm.test.jsx | 13 ++++--- .../BulkEmailScheduledEmailsTable.jsx | 17 ++++----- .../BulkEmailScheduledEmailsTable.test.jsx | 3 +- src/components/bulk-email-tool/constants.js | 8 +++++ src/components/bulk-email-tool/messages.js | 21 +++++++++++ src/components/bulk-email-tool/utils.js | 29 +++++++++------ 8 files changed, 74 insertions(+), 57 deletions(-) create mode 100644 src/components/bulk-email-tool/constants.js create mode 100644 src/components/bulk-email-tool/messages.js diff --git a/src/components/bulk-email-tool/bulk-email-form/BulkEmailForm.jsx b/src/components/bulk-email-tool/bulk-email-form/BulkEmailForm.jsx index ef334e7c..ea473f78 100644 --- a/src/components/bulk-email-tool/bulk-email-form/BulkEmailForm.jsx +++ b/src/components/bulk-email-tool/bulk-email-form/BulkEmailForm.jsx @@ -220,7 +220,7 @@ function BulkEmailForm(props) {

{intl.formatMessage(messages.bulkEmailTaskAlertRecipients, { subject: editor.emailSubject })}

    {editor.emailRecipients.map((group) => ( -
  • {getDisplayTextFromRecipient(group)}
    • +
  • {getDisplayTextFromRecipient(intl, group)}
    • ))}
{!isScheduled && ( @@ -247,7 +247,7 @@ function BulkEmailForm(props) {

{intl.formatMessage(messages.bulkEmailTaskAlertEditingTo)}

    {editor.emailRecipients.map((group) => ( -
  • {getDisplayTextFromRecipient(group)}
    • +
  • {getDisplayTextFromRecipient(intl, group)}
    • ))}

{intl.formatMessage(messages.bulkEmailTaskAlertEditingWarning)}

diff --git a/src/components/bulk-email-tool/bulk-email-form/bulk-email-recipient/BulkEmailRecipient.jsx b/src/components/bulk-email-tool/bulk-email-form/bulk-email-recipient/BulkEmailRecipient.jsx index 48445990..98ca9b67 100644 --- a/src/components/bulk-email-tool/bulk-email-form/bulk-email-recipient/BulkEmailRecipient.jsx +++ b/src/components/bulk-email-tool/bulk-email-form/bulk-email-recipient/BulkEmailRecipient.jsx @@ -1,19 +1,12 @@ import React from 'react'; import PropTypes from 'prop-types'; import { Form } from '@openedx/paragon'; -import { FormattedMessage } from '@edx/frontend-platform/i18n'; -import { RECIPIENTS_DISPLAY_NAMES } from '../../utils'; +import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; +import { getDisplayTextFromRecipient } from '../../utils'; +import { DEFAULT_RECIPIENTS_GROUPS } from '../../constants'; import './bulkEmailRecepient.scss'; -const DEFAULT_GROUPS = { - SELF: 'myself', - STAFF: 'staff', - ALL_LEARNERS: 'learners', - VERIFIED: 'track:verified', - AUDIT: 'track:audit', -}; - export default function BulkEmailRecipient(props) { const { handleCheckboxes, @@ -21,6 +14,7 @@ export default function BulkEmailRecipient(props) { additionalCohorts, courseModes, } = props; + const intl = useIntl(); const hasCourseModes = courseModes && courseModes.length > 1; return ( @@ -40,22 +34,14 @@ export default function BulkEmailRecipient(props) { value={selectedGroups} > - + {getDisplayTextFromRecipient(intl, DEFAULT_RECIPIENTS_GROUPS.SELF)} - + {getDisplayTextFromRecipient(intl, DEFAULT_RECIPIENTS_GROUPS.STAFF)} { // additional modes @@ -64,7 +50,7 @@ export default function BulkEmailRecipient(props) { group === DEFAULT_GROUPS.ALL_LEARNERS)} + disabled={selectedGroups.find((group) => group === DEFAULT_RECIPIENTS_GROUPS.ALL_LEARNERS)} className="col col-lg-4 col-sm-6 col-12" > group === DEFAULT_GROUPS.ALL_LEARNERS)} + disabled={selectedGroups.find((group) => group === DEFAULT_RECIPIENTS_GROUPS.ALL_LEARNERS)} className="col col-lg-4 col-sm-6 col-12" > - + {getDisplayTextFromRecipient(intl, DEFAULT_RECIPIENTS_GROUPS.ALL_LEARNERS)} {!props.isValid && ( diff --git a/src/components/bulk-email-tool/bulk-email-form/test/BulkEmailForm.test.jsx b/src/components/bulk-email-tool/bulk-email-form/test/BulkEmailForm.test.jsx index 92d9de91..a2b8254a 100644 --- a/src/components/bulk-email-tool/bulk-email-form/test/BulkEmailForm.test.jsx +++ b/src/components/bulk-email-tool/bulk-email-form/test/BulkEmailForm.test.jsx @@ -13,7 +13,6 @@ import { BulkEmailContext, BulkEmailProvider } from '../../bulk-email-context'; import { formatDate } from '../../../../utils/formatDateAndTime'; import cohortFactory from '../data/__factories__/bulkEmailFormCohort.factory'; import courseModeFactory from '../data/__factories__/bulkEmailFormCourseMode.factory'; -import { RECIPIENTS_DISPLAY_NAMES } from '../../utils'; jest.mock('../../text-editor/TextEditor'); @@ -76,8 +75,8 @@ describe('bulk-email-form', () => { success: true, }); render(renderBulkEmailForm()); - fireEvent.click(screen.getByRole('checkbox', { name: RECIPIENTS_DISPLAY_NAMES.myself })); - expect(screen.getByRole('checkbox', { name: RECIPIENTS_DISPLAY_NAMES.myself })).toBeChecked(); + fireEvent.click(screen.getByRole('checkbox', { name: 'Myself' })); + expect(screen.getByRole('checkbox', { name: 'Myself' })).toBeChecked(); fireEvent.change(screen.getByRole('textbox', { name: 'Subject' }), { target: { value: 'test subject' } }); fireEvent.change(screen.getByTestId('textEditor'), { target: { value: 'test body' } }); fireEvent.click(screen.getByText('Send email')); @@ -91,7 +90,7 @@ describe('bulk-email-form', () => { axiosMock.onPost(`${getConfig().LMS_BASE_URL}/courses/test/instructor/api/send_email`).reply(500); render(renderBulkEmailForm()); const subjectLine = screen.getByRole('textbox', { name: 'Subject' }); - const recipient = screen.getByRole('checkbox', { name: RECIPIENTS_DISPLAY_NAMES.myself }); + const recipient = screen.getByRole('checkbox', { name: 'Myself' }); fireEvent.click(recipient); fireEvent.change(subjectLine, { target: { value: 'test subject' } }); fireEvent.change(screen.getByTestId('textEditor'), { target: { value: 'test body' } }); @@ -100,9 +99,9 @@ describe('bulk-email-form', () => { fireEvent.click(await screen.findByRole('button', { name: /continue/i })); expect(await screen.findByText('An error occured while attempting to send the email.')).toBeInTheDocument(); }); - test('Checking "All Students" disables each learner group', async () => { + test('Checking "All learners" disables each learner group', async () => { render(renderBulkEmailForm()); - fireEvent.click(screen.getByRole('checkbox', { name: RECIPIENTS_DISPLAY_NAMES.learners })); + fireEvent.click(screen.getByRole('checkbox', { name: 'All learners' })); const verifiedLearners = screen.getByRole('checkbox', { name: 'Learners in the Verified Certificate Track' }); const auditLearners = screen.getByRole('checkbox', { name: 'Learners in the Audit Track' }); const { cohorts } = cohortFactory.build(); @@ -131,7 +130,7 @@ describe('bulk-email-form', () => { test('Adds scheduling data to POST requests when schedule is selected', async () => { const postBulkEmailInstructorTask = jest.spyOn(bulkEmailFormApi, 'postBulkEmailInstructorTask'); render(renderBulkEmailForm()); - fireEvent.click(screen.getByRole('checkbox', { name: RECIPIENTS_DISPLAY_NAMES.myself })); + fireEvent.click(screen.getByRole('checkbox', { name: 'Myself' })); fireEvent.change(screen.getByRole('textbox', { name: 'Subject' }), { target: { value: 'test subject' } }); fireEvent.change(screen.getByTestId('textEditor'), { target: { value: 'test body' } }); const scheduleCheckbox = screen.getByText('Schedule this email for a future date'); diff --git a/src/components/bulk-email-tool/bulk-email-task-manager/bulk-email-scheduled-emails-table/BulkEmailScheduledEmailsTable.jsx b/src/components/bulk-email-tool/bulk-email-task-manager/bulk-email-scheduled-emails-table/BulkEmailScheduledEmailsTable.jsx index 6d7698cf..a18fbefb 100644 --- a/src/components/bulk-email-tool/bulk-email-task-manager/bulk-email-scheduled-emails-table/BulkEmailScheduledEmailsTable.jsx +++ b/src/components/bulk-email-tool/bulk-email-task-manager/bulk-email-scheduled-emails-table/BulkEmailScheduledEmailsTable.jsx @@ -19,9 +19,9 @@ import ViewEmailModal from '../ViewEmailModal'; import { copyToEditor } from '../../bulk-email-form/data/actions'; import TaskAlertModal from '../../task-alert-modal'; import { formatDate, formatTime } from '../../../../utils/formatDateAndTime'; -import { getDisplayTextFromRecipient, getRecipientFromDisplayText } from '../../utils'; +import { getDisplayTextFromRecipient } from '../../utils'; -function flattenScheduledEmailsArray(emails) { +function flattenScheduledEmailsArray(intl, emails) { return emails.map((email) => ({ schedulingId: email.id, emailId: email.courseEmail.id, @@ -29,11 +29,12 @@ function flattenScheduledEmailsArray(emails) { taskDue: new Date(email.taskDue).toLocaleString(), taskDueUTC: email.taskDue, ...email.courseEmail, - targets: email.courseEmail.targets.map(getDisplayTextFromRecipient).join(', '), + targets: email.courseEmail.targets.join(', '), + targetsText: email.courseEmail.targets.map((mess) => getDisplayTextFromRecipient(intl, mess)).join(', '), })); } -function BulkEmailScheduledEmailsTable({ intl, courseModes }) { +function BulkEmailScheduledEmailsTable({ intl }) { const { courseId } = useParams(); const [{ scheduledEmailsTable }, dispatch] = useContext(BulkEmailContext); const [tableData, setTableData] = useState([]); @@ -45,8 +46,8 @@ function BulkEmailScheduledEmailsTable({ intl, courseModes }) { const [currentTask, setCurrentTask] = useState({}); useEffect(() => { - setTableData(flattenScheduledEmailsArray(scheduledEmailsTable.results, courseModes)); - }, [scheduledEmailsTable.results, courseModes]); + setTableData(flattenScheduledEmailsArray(intl, scheduledEmailsTable.results)); + }, [intl, scheduledEmailsTable.results]); const fetchTableData = useCallback((args) => { dispatch(getScheduledBulkEmailThunk(courseId, args.pageIndex + 1)); @@ -97,7 +98,7 @@ function BulkEmailScheduledEmailsTable({ intl, courseModes }) { }, } = row; const dateTime = new Date(taskDueUTC); - const emailRecipients = targets.replaceAll('-', ':').split(', ').map(getRecipientFromDisplayText); + const emailRecipients = targets.replaceAll('-', ':').split(', '); const scheduleDate = formatDate(dateTime); const scheduleTime = formatTime(dateTime); dispatch( @@ -155,7 +156,7 @@ function BulkEmailScheduledEmailsTable({ intl, courseModes }) { }, { Header: intl.formatMessage(messages.bulkEmailScheduledEmailsTableSendTo), - accessor: 'targets', + accessor: 'targetsText', }, { Header: intl.formatMessage(messages.bulkEmailScheduledEmailsTableSubject), diff --git a/src/components/bulk-email-tool/bulk-email-task-manager/bulk-email-scheduled-emails-table/test/BulkEmailScheduledEmailsTable.test.jsx b/src/components/bulk-email-tool/bulk-email-task-manager/bulk-email-scheduled-emails-table/test/BulkEmailScheduledEmailsTable.test.jsx index f461b2d5..e540f727 100644 --- a/src/components/bulk-email-tool/bulk-email-task-manager/bulk-email-scheduled-emails-table/test/BulkEmailScheduledEmailsTable.test.jsx +++ b/src/components/bulk-email-tool/bulk-email-task-manager/bulk-email-scheduled-emails-table/test/BulkEmailScheduledEmailsTable.test.jsx @@ -13,7 +13,6 @@ import { BulkEmailProvider } from '../../../bulk-email-context'; import BulkEmailScheduledEmailsTable from '..'; import scheduledEmailsFactory from './__factories__/scheduledEmails.factory'; import * as actions from '../../../bulk-email-form/data/actions'; -import { RECIPIENTS_DISPLAY_NAMES } from '../../../utils'; jest.mock('react-router-dom', () => ({ ...jest.requireActual('react-router-dom'), @@ -43,7 +42,7 @@ describe('BulkEmailScheduledEmailsTable', () => { .onGet(`${getConfig().LMS_BASE_URL}/api/instructor_task/v1/schedules/test-id/bulk_email/?page=1`) .reply(200, scheduledEmailsFactory.build(1)); render(renderBulkEmailScheduledEmailsTable()); - expect(await screen.findByText(RECIPIENTS_DISPLAY_NAMES.learners)).toBeTruthy(); + expect(await screen.findByText('All learners')).toBeTruthy(); expect(await screen.findByText('subject')).toBeTruthy(); expect(await screen.findByText('edx')).toBeTruthy(); expect(await screen.findByLabelText('View')).toBeTruthy(); diff --git a/src/components/bulk-email-tool/constants.js b/src/components/bulk-email-tool/constants.js new file mode 100644 index 00000000..9cd9e4f5 --- /dev/null +++ b/src/components/bulk-email-tool/constants.js @@ -0,0 +1,8 @@ +// eslint-disable-next-line import/prefer-default-export +export const DEFAULT_RECIPIENTS_GROUPS = { + SELF: 'myself', + STAFF: 'staff', + ALL_LEARNERS: 'learners', + VERIFIED: 'track:verified', + AUDIT: 'track:audit', +}; diff --git a/src/components/bulk-email-tool/messages.js b/src/components/bulk-email-tool/messages.js new file mode 100644 index 00000000..f11a684c --- /dev/null +++ b/src/components/bulk-email-tool/messages.js @@ -0,0 +1,21 @@ +import { defineMessages } from '@edx/frontend-platform/i18n'; + +const messages = defineMessages({ + bulkEmailRecipientsMyselfLabel: { + id: 'bulk.email.recipients.myself.label', + defaultMessage: 'Myself', + description: 'Label for selecting the option to send a bulk email to oneself.', + }, + bulkEmailRecipientsStaffLabel: { + id: 'bulk.email.recipients.staff.label', + defaultMessage: 'Staff and instructors', + description: 'Label for selecting the option to send a bulk email to all staff and instructors.', + }, + bulkEmailRecipientsLearnersLabel: { + id: 'bulk.email.recipients.learners.label', + defaultMessage: 'All learners', + description: 'Label for selecting the option to send a bulk email to all learners.', + }, +}); + +export default messages; diff --git a/src/components/bulk-email-tool/utils.js b/src/components/bulk-email-tool/utils.js index 2a37c880..50262db2 100644 --- a/src/components/bulk-email-tool/utils.js +++ b/src/components/bulk-email-tool/utils.js @@ -1,14 +1,21 @@ -export const RECIPIENTS_DISPLAY_NAMES = { - myself: 'Myself', - staff: 'Staff and instructors', - learners: 'All students', +import { DEFAULT_RECIPIENTS_GROUPS } from './constants'; +import messages from './messages'; + +const RECIPIENTS_DISPLAY_NAMES = { + [DEFAULT_RECIPIENTS_GROUPS.SELF]: messages.bulkEmailRecipientsMyselfLabel, + [DEFAULT_RECIPIENTS_GROUPS.STAFF]: messages.bulkEmailRecipientsStaffLabel, + [DEFAULT_RECIPIENTS_GROUPS.ALL_LEARNERS]: messages.bulkEmailRecipientsLearnersLabel, }; -// Output: { 'Myself': 'myself', 'Staff and instructors': 'staff', 'All students': 'learners' } -export const REVERSE_RECIPIENTS_DISPLAY_NAMES = Object.fromEntries( - Object.entries(RECIPIENTS_DISPLAY_NAMES).map(([key, value]) => [value, key]), +/** + * Retrieves the display text for a given recipient. + * + * @param {Object} intl - The internationalization object, provided by React Intl. + * @param {string} recipient - The recipient key used to look up the corresponding display name. + * @returns {string} - The formatted display name for the recipient, + * or the original recipient key if no display name is found. + */ +// eslint-disable-next-line import/prefer-default-export +export const getDisplayTextFromRecipient = (intl, recipient) => ( + intl.formatMessage(RECIPIENTS_DISPLAY_NAMES[recipient]) || recipient ); - -export const getDisplayTextFromRecipient = (recipient) => RECIPIENTS_DISPLAY_NAMES[recipient] || recipient; - -export const getRecipientFromDisplayText = (recipient) => REVERSE_RECIPIENTS_DISPLAY_NAMES[recipient] || recipient; From f9b7c95f9f87ffdf0e81d9fe8fc28e31149b2a6d Mon Sep 17 00:00:00 2001 From: Ihor Romaniuk Date: Tue, 3 Dec 2024 09:38:10 +0100 Subject: [PATCH 4/4] fix: add fall back to the recipient key Co-authored-by: Braden MacDonald --- src/components/bulk-email-tool/utils.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/bulk-email-tool/utils.js b/src/components/bulk-email-tool/utils.js index 50262db2..674fabaa 100644 --- a/src/components/bulk-email-tool/utils.js +++ b/src/components/bulk-email-tool/utils.js @@ -17,5 +17,6 @@ const RECIPIENTS_DISPLAY_NAMES = { */ // eslint-disable-next-line import/prefer-default-export export const getDisplayTextFromRecipient = (intl, recipient) => ( - intl.formatMessage(RECIPIENTS_DISPLAY_NAMES[recipient]) || recipient + const msg = RECIPIENTS_DISPLAY_NAMES[recipient]; + return msg ? intl.formatMessage(msg) : recipient; // Fall back to the recipient key if no display name is found. );