From dd3bc0676104f3cd6c002e1a15f115dfc2cd526d Mon Sep 17 00:00:00 2001
From: Kevin Ansfield <kevin@ghost.org>
Date: Tue, 12 Mar 2024 17:39:36 +0000
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20comments=20block=20disap?=
 =?UTF-8?q?pearing=20when=20performing=20certain=20actions=20(#19846)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

closes https://linear.app/tryghost/issue/ENG-739

- `props.innerRef` in `<IFrame>` was inadvertently assumed to always exist, sometimes throwing an error on render when certain popups like the profile settings modal were opened resulting in the app crashing and the whole comments block disappearing
- added a guard to ensure the ref exists before trying to set it
- updated tests so the profile modal route is tested
---
 apps/comments-ui/src/components/IFrame.tsx    |  4 +-
 .../src/components/content/forms/Form.tsx     |  1 +
 .../src/components/popups/AddDetailsPopup.tsx |  5 ++-
 apps/comments-ui/test/e2e/actions.test.ts     | 41 +++++++++++++++++++
 apps/comments-ui/test/utils/MockedApi.ts      | 13 ++++--
 5 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/apps/comments-ui/src/components/IFrame.tsx b/apps/comments-ui/src/components/IFrame.tsx
index 611a7d03e5..09660ec3f6 100644
--- a/apps/comments-ui/src/components/IFrame.tsx
+++ b/apps/comments-ui/src/components/IFrame.tsx
@@ -59,7 +59,9 @@ class IFrame extends Component<any> {
 
     setNode(node: any) {
         this.node = node;
-        this.props.innerRef.current = node;
+        if (this.props.innerRef) {
+            this.props.innerRef.current = node;
+        }
     }
 
     render() {
diff --git a/apps/comments-ui/src/components/content/forms/Form.tsx b/apps/comments-ui/src/components/content/forms/Form.tsx
index 0e4e74bea9..6e5f22fafe 100644
--- a/apps/comments-ui/src/components/content/forms/Form.tsx
+++ b/apps/comments-ui/src/components/content/forms/Form.tsx
@@ -160,6 +160,7 @@ const FormHeader: React.FC<FormHeaderProps> = ({show, name, expertise, editName,
             <div className="flex items-baseline justify-start">
                 <button
                     className={`group flex max-w-[80%] items-center justify-start whitespace-nowrap text-left font-sans text-[14px] tracking-tight text-[rgba(0,0,0,0.5)] transition duration-150 hover:text-[rgba(0,0,0,0.75)] sm:max-w-[90%] dark:text-[rgba(255,255,255,0.5)] dark:hover:text-[rgba(255,255,255,0.4)] ${!expertise && 'text-[rgba(0,0,0,0.3)] hover:text-[rgba(0,0,0,0.5)] dark:text-[rgba(255,255,255,0.3)]'}`}
+                    data-testid="expertise-button"
                     type="button"
                     onClick={editExpertise}
                 >
diff --git a/apps/comments-ui/src/components/popups/AddDetailsPopup.tsx b/apps/comments-ui/src/components/popups/AddDetailsPopup.tsx
index 31de9077ff..9cc5eea656 100644
--- a/apps/comments-ui/src/components/popups/AddDetailsPopup.tsx
+++ b/apps/comments-ui/src/components/popups/AddDetailsPopup.tsx
@@ -116,7 +116,7 @@ const AddDetailsPopup = (props: Props) => {
     });
 
     return (
-        <div className="shadow-modal relative h-screen w-screen overflow-hidden rounded-none bg-white p-[28px] text-center sm:h-auto sm:w-[720px] sm:rounded-xl sm:p-0" onMouseDown={stopPropagation}>
+        <div className="shadow-modal relative h-screen w-screen overflow-hidden rounded-none bg-white p-[28px] text-center sm:h-auto sm:w-[720px] sm:rounded-xl sm:p-0" data-testid="profile-modal" onMouseDown={stopPropagation}>
             <div className="flex">
                 {!isMobile() &&
                     <div className={`flex w-[40%] flex-col items-center justify-center bg-[#1C1C1C]`}>
@@ -146,6 +146,7 @@ const AddDetailsPopup = (props: Props) => {
                         <input
                             ref={inputNameRef}
                             className={`flex h-[42px] w-full items-center rounded border border-neutral-200 px-3 font-sans text-[16px] outline-0 transition-[border-color] duration-200 focus:border-neutral-300 ${error.name && 'border-red-500 focus:border-red-500'}`}
+                            data-testid="name-input"
                             id="comments-name"
                             maxLength={64}
                             name="name"
@@ -170,6 +171,7 @@ const AddDetailsPopup = (props: Props) => {
                         <input
                             ref={inputExpertiseRef}
                             className={`flex h-[42px] w-full items-center rounded border border-neutral-200 px-3 font-sans text-[16px] outline-0 transition-[border-color] duration-200 focus:border-neutral-300 ${(expertiseCharsLeft === 0) && 'border-red-500 focus:border-red-500'}`}
+                            data-testid="expertise-input"
                             id="comments-expertise"
                             maxLength={maxExpertiseChars}
                             name="expertise"
@@ -191,6 +193,7 @@ const AddDetailsPopup = (props: Props) => {
                         />
                         <button
                             className={`mt-10 flex h-[42px] w-full items-center justify-center rounded-md px-8 font-sans text-[15px] font-semibold text-white opacity-100 transition-opacity duration-200 ease-linear hover:opacity-90`}
+                            data-testid="save-button"
                             style={{backgroundColor: accentColor ?? '#000000'}}
                             type="button"
                             onClick={() => {
diff --git a/apps/comments-ui/test/e2e/actions.test.ts b/apps/comments-ui/test/e2e/actions.test.ts
index 2f2bd79e29..f2e7b8a635 100644
--- a/apps/comments-ui/test/e2e/actions.test.ts
+++ b/apps/comments-ui/test/e2e/actions.test.ts
@@ -105,5 +105,46 @@ test.describe('Actions', async () => {
         await expect(frame.getByTestId('comment-component')).toHaveCount(4);
         await expect(frame.getByText('This is a reply 123')).toHaveCount(1);
     });
+
+    test('Can add expertise', async ({page}) => {
+        const mockedApi = new MockedApi({});
+        mockedApi.setMember({name: 'John Doe', expertise: null});
+
+        mockedApi.addComment({
+            html: '<p>This is comment 1</p>'
+        });
+
+        const {frame} = await initialize({
+            mockedApi,
+            page,
+            publication: 'Publisher Weekly'
+        });
+
+        const editor = frame.getByTestId('form-editor');
+        await editor.click({force: true});
+        await waitEditorFocused(editor);
+
+        const expertiseButton = frame.getByTestId('expertise-button');
+        await expect(expertiseButton).toBeVisible();
+        await expect(expertiseButton).toHaveText('Add your expertise');
+        await expertiseButton.click();
+
+        const detailsFrame = page.frameLocator('iframe[title="addDetailsPopup"]');
+        const profileModal = detailsFrame.getByTestId('profile-modal');
+        await expect(profileModal).toBeVisible();
+
+        await expect(detailsFrame.getByTestId('name-input')).toHaveValue('John Doe');
+        await expect(detailsFrame.getByTestId('expertise-input')).toHaveValue('');
+
+        await detailsFrame.getByTestId('name-input').fill('Testy McTest');
+        await detailsFrame.getByTestId('expertise-input').fill('Software development');
+
+        await detailsFrame.getByTestId('save-button').click();
+
+        await expect(profileModal).not.toBeVisible();
+
+        await expect(frame.getByTestId('member-name')).toHaveText('Testy McTest');
+        await expect(frame.getByTestId('expertise-button')).toHaveText('Software development');
+    });
 });
 
diff --git a/apps/comments-ui/test/utils/MockedApi.ts b/apps/comments-ui/test/utils/MockedApi.ts
index 8ae893425b..42b07a0555 100644
--- a/apps/comments-ui/test/utils/MockedApi.ts
+++ b/apps/comments-ui/test/utils/MockedApi.ts
@@ -159,6 +159,14 @@ export class MockedApi {
                 });
             }
 
+            if (route.request().method() === 'PUT') {
+                const payload = JSON.parse(route.request().postData());
+                this.member = {
+                    ...this.member,
+                    ...payload
+                };
+            }
+
             await route.fulfill({
                 status: 200,
                 body: JSON.stringify(this.member)
@@ -200,8 +208,8 @@ export class MockedApi {
             });
         });
 
-         // LIKE a single comment
-         await page.route(`${path}/members/api/comments/*/like/`, async (route) => {
+        // LIKE a single comment
+        await page.route(`${path}/members/api/comments/*/like/`, async (route) => {
             const url = new URL(route.request().url());
             const commentId = url.pathname.split('/').reverse()[2];
 
@@ -233,7 +241,6 @@ export class MockedApi {
             });
         });
 
-
         // GET a single comment
         await page.route(`${path}/members/api/comments/*/`, async (route) => {
             const url = new URL(route.request().url());