From 1f05a7890f52573b83578944456fc3579917c09a Mon Sep 17 00:00:00 2001 From: Steve Larson <9larsons@gmail.com> Date: Fri, 26 Jul 2024 21:20:13 -0500 Subject: [PATCH] Added test coverage over newsletter flows (#20672) no ref - while reviewing the newsletter flows, it was apparent that we were missing test coverage Some of the tests in Portal are a bit redundant with tests added for child components, but it didn't seem worth removing them after getting them to work. There was a bug in our Portal fixture data that requires a few changes, as well as some small adjustments for making tests easier (testing-lib-react has `getByTestId` and simply a `querySelector` to use alternate test attributes). --- .../components/common/NewsletterManagement.js | 4 +- .../components/pages/AccountEmailPage.test.js | 117 ++++++++++++++++++ .../AccountHomePage/AccountHomePage.test.js | 4 +- .../pages/NewsletterSelectionPage.js | 4 +- .../src/tests/EmailSubscriptionsFlow.test.js | 31 ++++- apps/portal/src/utils/fixtures-generator.js | 2 +- .../server/services/members/middleware.js | 2 +- .../shared/config/env/config.development.json | 12 ++ .../e2e-browser/portal/member-actions.spec.js | 2 +- .../services/members/middleware.test.js | 93 +++++++++++++- 10 files changed, 261 insertions(+), 10 deletions(-) create mode 100644 apps/portal/src/components/pages/AccountEmailPage.test.js diff --git a/apps/portal/src/components/common/NewsletterManagement.js b/apps/portal/src/components/common/NewsletterManagement.js index b36406ce90..3bc9782de2 100644 --- a/apps/portal/src/components/common/NewsletterManagement.js +++ b/apps/portal/src/components/common/NewsletterManagement.js @@ -46,7 +46,7 @@ function NewsletterPrefSection({newsletter, subscribedNewsletters, setSubscribed const [showUpdated, setShowUpdated] = useState(false); const [timeoutId, setTimeoutId] = useState(null); return ( -
+

{newsletter.name}

{newsletter?.description}

@@ -95,7 +95,7 @@ function CommentsSection({updateCommentNotifications, isCommentsEnabled, enableC } return ( -
+

{t('Comments')}

{t('Get notified when someone replies to your comment')}

diff --git a/apps/portal/src/components/pages/AccountEmailPage.test.js b/apps/portal/src/components/pages/AccountEmailPage.test.js new file mode 100644 index 0000000000..87c5f8ea57 --- /dev/null +++ b/apps/portal/src/components/pages/AccountEmailPage.test.js @@ -0,0 +1,117 @@ +import {getSiteData, getNewslettersData, getMemberData} from '../../utils/fixtures-generator'; +import {render, fireEvent} from '../../utils/test-utils'; +import AccountEmailPage from './AccountEmailPage'; + +const setup = (overrides) => { + const {mockOnActionFn, context, ...utils} = render( + , + { + overrideContext: { + ...overrides + } + } + ); + const unsubscribeAllBtn = utils.getByText('Unsubscribe from all emails'); + const closeBtn = utils.getByTestId('close-popup'); + + return { + unsubscribeAllBtn, + closeBtn, + mockOnActionFn, + context, + ...utils + }; +}; + +describe('Account Email Page', () => { + test('renders', () => { + const newsletterData = getNewslettersData({numOfNewsletters: 2}); + const siteData = getSiteData({ + newsletters: newsletterData, + member: getMemberData({newsletters: newsletterData}) + }); + const {unsubscribeAllBtn, getAllByTestId, getByText} = setup({site: siteData}); + const unsubscribeBtns = getAllByTestId(`toggle-wrapper`); + expect(getByText('Email preferences')).toBeInTheDocument(); + // one for each newsletter and one for comments + expect(unsubscribeBtns).toHaveLength(3); + expect(unsubscribeAllBtn).toBeInTheDocument(); + }); + + test('can unsubscribe from all emails', async () => { + const newsletterData = getNewslettersData({numOfNewsletters: 2}); + const siteData = getSiteData({ + newsletters: newsletterData + }); + const {mockOnActionFn, unsubscribeAllBtn, getAllByTestId} = setup({site: siteData, member: getMemberData({newsletters: newsletterData})}); + let checkmarkContainers = getAllByTestId('checkmark-container'); + // each newsletter should have the checked class (this is how we know they're enabled/subscribed to) + expect(checkmarkContainers[0]).toHaveClass('gh-portal-toggle-checked'); + expect(checkmarkContainers[1]).toHaveClass('gh-portal-toggle-checked'); + + fireEvent.click(unsubscribeAllBtn); + expect(mockOnActionFn).toHaveBeenCalledTimes(2); + expect(mockOnActionFn).toHaveBeenCalledWith('showPopupNotification', {action: 'updated:success', message: 'Unsubscribed from all emails.'}); + expect(mockOnActionFn).toHaveBeenLastCalledWith('updateNewsletterPreference', {newsletters: [], enableCommentNotifications: false}); + + checkmarkContainers = getAllByTestId('checkmark-container'); + expect(checkmarkContainers).toHaveLength(3); + checkmarkContainers.forEach((newsletter) => { + // each newsletter htmlElement should not have the checked class + expect(newsletter).not.toHaveClass('gh-portal-toggle-checked'); + }); + }); + + test('unsubscribe all is disabled when no newsletters are subscribed to', async () => { + const siteData = getSiteData({ + newsletters: getNewslettersData({numOfNewsletters: 2}) + }); + const {unsubscribeAllBtn} = setup({site: siteData, member: getMemberData()}); + expect(unsubscribeAllBtn).toBeDisabled(); + }); + + test('can update newsletter preferences', async () => { + const newsletterData = getNewslettersData({numOfNewsletters: 2}); + const siteData = getSiteData({ + newsletters: newsletterData + }); + const {mockOnActionFn, getAllByTestId} = setup({site: siteData, member: getMemberData({newsletters: newsletterData})}); + let checkmarkContainers = getAllByTestId('checkmark-container'); + // each newsletter should have the checked class (this is how we know they're enabled/subscribed to) + expect(checkmarkContainers[0]).toHaveClass('gh-portal-toggle-checked'); + let subscriptionToggles = getAllByTestId('switch-input'); + fireEvent.click(subscriptionToggles[0]); + expect(mockOnActionFn).toHaveBeenCalledWith('updateNewsletterPreference', {newsletters: [{id: newsletterData[1].id}]}); + fireEvent.click(subscriptionToggles[0]); + expect(mockOnActionFn).toHaveBeenCalledWith('updateNewsletterPreference', {newsletters: [{id: newsletterData[1].id}, {id: newsletterData[0].id}]}); + }); + + test('can update comment notifications', async () => { + const siteData = getSiteData(); + const {mockOnActionFn, getAllByTestId} = setup({site: siteData, member: getMemberData()}); + let subscriptionToggles = getAllByTestId('switch-input'); + fireEvent.click(subscriptionToggles[0]); + expect(mockOnActionFn).toHaveBeenCalledWith('updateNewsletterPreference', {enableCommentNotifications: true}); + fireEvent.click(subscriptionToggles[0]); + expect(mockOnActionFn).toHaveBeenCalledWith('updateNewsletterPreference', {enableCommentNotifications: false}); + }); + + test('displays help for members with email suppressions', async () => { + const newsletterData = getNewslettersData({numOfNewsletters: 2}); + const siteData = getSiteData({ + newsletters: newsletterData + }); + const {getByText} = setup({site: siteData, member: getMemberData({newsletters: newsletterData, email_suppressions: {suppressed: false}})}); + expect(getByText('Not receiving emails?')).toBeInTheDocument(); + expect(getByText('Get help →')).toBeInTheDocument(); + }); + + test('redirects to signin page if no member', async () => { + const newsletterData = getNewslettersData({numOfNewsletters: 2}); + const siteData = getSiteData({ + newsletters: newsletterData + }); + const {mockOnActionFn} = setup({site: siteData, member: null}); + expect(mockOnActionFn).toHaveBeenCalledWith('switchPage', {page: 'signin'}); + }); +}); diff --git a/apps/portal/src/components/pages/AccountHomePage/AccountHomePage.test.js b/apps/portal/src/components/pages/AccountHomePage/AccountHomePage.test.js index 98ab7bebcd..ac6679bf47 100644 --- a/apps/portal/src/components/pages/AccountHomePage/AccountHomePage.test.js +++ b/apps/portal/src/components/pages/AccountHomePage/AccountHomePage.test.js @@ -1,6 +1,7 @@ import {render, fireEvent} from '../../../utils/test-utils'; import AccountHomePage from './AccountHomePage'; import {site} from '../../../utils/fixtures'; +import {getSiteData} from '../../../utils/fixtures-generator'; const setup = (overrides) => { const {mockOnActionFn, ...utils} = render( @@ -21,7 +22,8 @@ const setup = (overrides) => { describe('Account Home Page', () => { test('renders', () => { - const {logoutBtn, utils} = setup(); + const siteData = getSiteData({commentsEnabled: 'off'}); + const {logoutBtn, utils} = setup({site: siteData}); expect(logoutBtn).toBeInTheDocument(); expect(utils.queryByText('You\'re currently not receiving emails')).not.toBeInTheDocument(); expect(utils.queryByText('Email newsletter')).toBeInTheDocument(); diff --git a/apps/portal/src/components/pages/NewsletterSelectionPage.js b/apps/portal/src/components/pages/NewsletterSelectionPage.js index 4707fa8bae..067c9aa2e5 100644 --- a/apps/portal/src/components/pages/NewsletterSelectionPage.js +++ b/apps/portal/src/components/pages/NewsletterSelectionPage.js @@ -11,7 +11,7 @@ function NewsletterPrefSection({newsletter, subscribedNewsletters, setSubscribed }); if (newsletter.paid) { return ( -
+

{newsletter.name}

{newsletter.description}

@@ -23,7 +23,7 @@ function NewsletterPrefSection({newsletter, subscribedNewsletters, setSubscribed ); } return ( -
+

{newsletter.name}

{newsletter.description}

diff --git a/apps/portal/src/tests/EmailSubscriptionsFlow.test.js b/apps/portal/src/tests/EmailSubscriptionsFlow.test.js index 0ecace4b9c..960a240858 100644 --- a/apps/portal/src/tests/EmailSubscriptionsFlow.test.js +++ b/apps/portal/src/tests/EmailSubscriptionsFlow.test.js @@ -160,7 +160,7 @@ describe('Newsletter Subscriptions', () => { fireEvent.click(unsubscribeAllButton); - expect(ghostApi.member.update).toHaveBeenCalledWith({newsletters: []}); + expect(ghostApi.member.update).toHaveBeenCalledWith({newsletters: [], enableCommentNotifications: false}); // Verify the local state shows the newsletter as unsubscribed let newsletterToggles = within(popupIframeDocument).queryAllByTestId('checkmark-container'); let newsletter1Toggle = newsletterToggles[0]; @@ -254,4 +254,33 @@ describe('Newsletter Subscriptions', () => { expect(newsletter2Toggle).toHaveClass('gh-portal-toggle-checked'); }); }); + + // describe('navigating straight to /portal/account/newsletters', () => { + // it('shows the newsletter management page when signed in', async () => { + // const {popupFrame, triggerButton, queryAllByText, popupIframeDocument} = await setup({ + // site: FixtureSite.singleTier.onlyFreePlanWithoutStripe, + // member: FixtureMember.subbedToNewsletter, + // newsletters: Newsletters + // }); + + // const manageSubscriptionsButton = within(popupIframeDocument).queryByRole('button', {name: 'Manage'}); + // await userEvent.click(manageSubscriptionsButton); + + // const newsletter1 = within(popupIframeDocument).queryAllByText('Newsletter 1'); + // expect(newsletter1).toBeInTheDocument(); + // }); + + // it('redirects to the sign in page when not signed in', async () => { + // const {popupFrame, queryByTitle, popupIframeDocument} = await setup({ + // site: FixtureSite.singleTier.onlyFreePlanWithoutStripe, + // member: FixtureMember.subbedToNewsletter, + // newsletters: Newsletters + // }, true); + + // // console.log(`popupFrame`, popupFrame); + // // console.log(`queryByTitle`, queryByTitle); + // // console.log(`popupIframeDocument`, popupIframeDocument); + + // }); + // }); }); diff --git a/apps/portal/src/utils/fixtures-generator.js b/apps/portal/src/utils/fixtures-generator.js index 5f850686f4..4fda8d6de3 100644 --- a/apps/portal/src/utils/fixtures-generator.js +++ b/apps/portal/src/utils/fixtures-generator.js @@ -66,7 +66,7 @@ export function getSiteData({ portal_button_signup_text, portal_button_style, members_support_address, - comments_enabled: !!commentsEnabled, + comments_enabled: commentsEnabled !== 'off', newsletters, recommendations, recommendations_enabled: !!recommendationsEnabled diff --git a/ghost/core/core/server/services/members/middleware.js b/ghost/core/core/server/services/members/middleware.js index e31d407a90..e73bfd65f4 100644 --- a/ghost/core/core/server/services/members/middleware.js +++ b/ghost/core/core/server/services/members/middleware.js @@ -110,7 +110,7 @@ const authMemberByUuid = async function authMemberByUuid(req, res, next) { } throw new errors.UnauthorizedError({ - messsage: tpl(messages.missingUuid) + message: tpl(messages.missingUuid) }); } diff --git a/ghost/core/core/shared/config/env/config.development.json b/ghost/core/core/shared/config/env/config.development.json index 5cd8e57c8b..d9551493f5 100644 --- a/ghost/core/core/shared/config/env/config.development.json +++ b/ghost/core/core/shared/config/env/config.development.json @@ -1,5 +1,17 @@ { "url": "http://localhost:2368", + "mail": { + "from": "test@example.com", + "transport": "SMTP", + "options": { + "host": "127.0.0.1", + "port": 1025, + "auth": { + "user": "user", + "pass": "unsecure" + } + } + }, "database": { "client": "sqlite3", "connection": { diff --git a/ghost/core/test/e2e-browser/portal/member-actions.spec.js b/ghost/core/test/e2e-browser/portal/member-actions.spec.js index 1f158e5cad..e0ac61631a 100644 --- a/ghost/core/test/e2e-browser/portal/member-actions.spec.js +++ b/ghost/core/test/e2e-browser/portal/member-actions.spec.js @@ -107,7 +107,7 @@ test.describe('Portal', () => { await portalFrame.locator('[data-test-button="manage-newsletters"]').click(); // check amount of newsletterss - const newsletters = await portalFrame.locator('[data-test-toggle-wrapper="true"]'); + const newsletters = await portalFrame.locator('[data-testid="toggle-wrapper"]'); const count = await newsletters.count(); await expect(count).toEqual(2); diff --git a/ghost/core/test/unit/server/services/members/middleware.test.js b/ghost/core/test/unit/server/services/members/middleware.test.js index cca19f1411..94d0242984 100644 --- a/ghost/core/test/unit/server/services/members/middleware.test.js +++ b/ghost/core/test/unit/server/services/members/middleware.test.js @@ -192,4 +192,95 @@ describe('Members Service Middleware', function () { res.redirect.firstCall.args[0].should.eql('/blah/?action=signin&success=true'); }); }); -}); + + describe('updateMemberNewsletters', function () { + // let oldMembersService; + let req; + let res; + + before(function () { + models.init(); + }); + + beforeEach(function () { + req = {body: {newsletters: [], enable_comment_notifications: null}}; + res = {writeHead: sinon.stub(), end: sinon.stub()}; + }); + + afterEach(function () { + sinon.restore(); + }); + + it('returns 400 if no member uuid is part of the request', async function () { + req.query = {}; + + // Call the middleware + await membersMiddleware.updateMemberNewsletters(req, res); + + // Check behavior + res.writeHead.calledOnce.should.be.true(); + res.writeHead.firstCall.args[0].should.eql(400); + res.end.calledOnce.should.be.true(); + res.end.firstCall.args[0].should.eql('Invalid member uuid'); + }); + + it('returns 404 if member uuid is not found', async function () { + req.query = {uuid: 'test'}; + sinon.stub(membersService, 'api').get(() => { + return { + members: { + get: sinon.stub().resolves() + } + }; + }); + + // Call the middleware + await membersMiddleware.updateMemberNewsletters(req, res); + + // Check behavior + res.writeHead.calledOnce.should.be.true(); + res.writeHead.firstCall.args[0].should.eql(404); + res.end.calledOnce.should.be.true(); + res.end.firstCall.args[0].should.eql('Email address not found.'); + }); + + it('attempts to update newsletters', async function () { + res.json = sinon.stub(); + req.query = {uuid: 'test'}; + const memberData = { + id: 'test', + email: 'test@email.com', + name: 'Test Name', + newsletters: [], + enable_comment_notifications: false, + status: 'free' + }; + sinon.stub(membersService, 'api').get(() => { + return { + members: { + get: sinon.stub().resolves({id: 'test', email: 'test@email.com', get: () => 'test'}), + update: sinon.stub().resolves({ + ...memberData, + toJSON: () => JSON.stringify(memberData) + }) + } + }; + }); + await membersMiddleware.updateMemberNewsletters(req, res); + // the stubbing of the api is difficult to test with the current design, so we just check that the response is sent + res.json.calledOnce.should.be.true(); + }); + + it('returns 400 on error', async function () { + // use a malformed request to trigger an error + req = {}; + await membersMiddleware.updateMemberNewsletters(req, res); + + // Check behavior + res.writeHead.calledOnce.should.be.true(); + res.writeHead.firstCall.args[0].should.eql(400); + res.end.calledOnce.should.be.true(); + res.end.firstCall.args[0].should.eql('Failed to update newsletters'); + }); + }); +}); \ No newline at end of file