🐛 Fixed data-locale being ignored in Portal (#25190)

no issue

- when we updated `data-attributes.js` to use the importable `t` function that uses a single instance of the `i18n` library we inadvertently introduced an override of the locale so it always matches the site locale despite using a `data-locale=` attribute in Portal's `<script>` tag
- ultimately this uncovered a long-standing bug from before the switch to importable `t` where custom forms would always use the site locale rather than a data-attribute override
- removed the now-unnecessary language change calls along with any tests for them
- added test to ensure all calls to `changeLanguage` that occur during App startup use the prop rather than site locale

Author: kevinansfield

apps/portal/src/data-attributes.js

@@ -1,7 +1,7 @@
 /* eslint-disable no-console */
 import {getCheckoutSessionDataFromPlanAttribute, getUrlHistory} from './utils/helpers';
 import {HumanReadableError, chooseBestErrorMessage} from './utils/errors';
-import i18n, {t} from './utils/i18n';
+import {t} from './utils/i18n';
 
 function displayErrorIfElementExists(errorEl, message) {
     if (errorEl) {
@@ -125,8 +125,6 @@ export async function formSubmitHandler(
 }
 
 export function planClickHandler({event, el, errorEl, siteUrl, site, member, clickHandler}) {
-    i18n.changeLanguage(site.locale || 'en');
-
     el.removeEventListener('click', clickHandler);
     event.preventDefault();
     let plan = el.dataset.membersPlan;
@@ -211,8 +209,6 @@ export function handleDataAttributes({siteUrl, site = {}, member, labs = {}, doA
         return;
     }
 
-    i18n.changeLanguage(site.locale || 'en');
-
     siteUrl = siteUrl.replace(/\/$/, '');
     Array.prototype.forEach.call(document.querySelectorAll('form[data-members-form]'), function (form) {
         let errorEl = form.querySelector('[data-members-error]');

apps/portal/src/tests/App.test.js

@@ -2,6 +2,17 @@ import App from '../App';
 import setupGhostApi from '../utils/api';
 import {appRender} from '../utils/test-utils';
 import {site as FixtureSite, member as FixtureMember} from '../utils/test-fixtures';
+import i18n from '../utils/i18n';
+import {vi} from 'vitest';
+
+vi.mock('../utils/i18n', () => ({
+    default: {
+        changeLanguage: vi.fn(),
+        dir: vi.fn(),
+        t: vi.fn(str => str)
+    },
+    t: vi.fn(str => str)
+}));
 
 describe('App', function () {
     beforeEach(function () {
@@ -11,24 +22,61 @@ describe('App', function () {
         window.location = location;
     });
 
-    test('transforms portal links on render', async () => {
-        const link = document.createElement('a');
-        link.setAttribute('href', 'http://example.com/#/portal/signup');
-        document.body.appendChild(link);
+    function setupApi({site = {}, member = {}} = {}) {
+        const defaultSite = FixtureSite.singleTier.basic;
+        const defaultMember = FixtureMember.free;
+
+        const siteFixtures = {
+            ...defaultSite,
+            ...site
+        };
+
+        const memberFixtures = {
+            ...defaultMember,
+            ...member
+        };
 
         const ghostApi = setupGhostApi({siteUrl: 'http://example.com'});
         ghostApi.init = vi.fn(() => {
             return Promise.resolve({
-                site: FixtureSite.singleTier.basic,
-                member: FixtureMember.free
+                site: siteFixtures,
+                member: memberFixtures
             });
         });
+
+        return ghostApi;
+    }
+
+    test('transforms portal links on render', async () => {
+        const link = document.createElement('a');
+        link.setAttribute('href', 'http://example.com/#/portal/signup');
+        document.body.appendChild(link);
+
+        const ghostApi = setupApi();
         const utils = appRender(
-            <App api={ghostApi} />
+            <App siteUrl="http://example.com" api={ghostApi} />
         );
 
         await utils.findByTitle(/portal-popup/i);
 
         expect(link.getAttribute('href')).toBe('#/portal/signup');
     });
+
+    test('prefers locale prop over site locale for i18n language', async () => {
+        const ghostApi = setupApi({
+            site: {
+                locale: 'de'
+            }
+        });
+
+        const utils = appRender(
+            <App siteUrl="http://example.com" api={ghostApi} locale="en" />
+        );
+
+        await utils.findByTitle(/portal-popup/i);
+
+        i18n.changeLanguage.mock.calls.forEach((call) => {
+            expect(call[0]).toBe('en');
+        });
+    });
 });

apps/portal/src/tests/data-attributes.test.js

@@ -3,19 +3,9 @@ import {site as FixturesSite, member as FixtureMember} from '../utils/test-fixtu
 import {fireEvent, appRender, within} from '../utils/test-utils';
 import setupGhostApi from '../utils/api';
 import * as helpers from '../utils/helpers';
-import {formSubmitHandler, planClickHandler, handleDataAttributes} from '../data-attributes';
-import i18n from '../utils/i18n';
+import {formSubmitHandler, planClickHandler} from '../data-attributes';
 import {vi} from 'vitest';
 
-vi.mock('../utils/i18n', () => ({
-    default: {
-        changeLanguage: vi.fn(),
-        dir: vi.fn(),
-        t: vi.fn(str => str)
-    },
-    t: vi.fn(str => str)
-}));
-
 // Mock data
 function getMockData({newsletterQuerySelectorResult = null} = {}) {
     const site = FixturesSite.singleTier.basic;
@@ -322,17 +312,6 @@ describe('Member Data attributes:', () => {
             );
         });
 
-        test('sets correct i18n language based on site locale', async () => {
-            const {event, errorEl, siteUrl, clickHandler, site, member, element} = getMockData();
-            // Override the site locale to 'de'
-            site.locale = 'de';
-
-            await planClickHandler({event, errorEl, siteUrl, clickHandler, site, member, el: element});
-
-            // Verify i18nLib was called with correct locale
-            expect(i18n.changeLanguage).toHaveBeenCalledWith('de');
-        });
-
         test('allows free member upgrade via direct checkout', async () => {
             let {event, errorEl, siteUrl, clickHandler, site, member, element} = getMockData();
             member = FixtureMember.free;
@@ -718,16 +697,4 @@ describe('Portal Data attributes:', () => {
             expect(errorEl.innerText).toBe('No member exists with this e-mail address. Please sign up first.');
         });
     });
-
-    describe('handleDataAttributes', () => {
-        test('sets correct i18n language based on site locale', () => {
-            const {siteUrl, site, member} = getMockData();
-            // Override the site locale to 'de'
-            site.locale = 'de';
-
-            handleDataAttributes({siteUrl, site, member});
-
-            expect(i18n.changeLanguage).toHaveBeenCalledWith('de');
-        });
-    });
 });