Compare commits

...
3 Commits
Author SHA1 Message Date
bermudalamb 6e12567bb7 Merge branch 'main' into fix/google-button-missing-on-signup
Linting / lint (pull_request) Canceled after 0s
SonarQube Analysis / sonarqube (pull_request) Canceled after 0s
2026-09-11 10:05:58 -05:00
bermudalamb b82b989cc8 Merge pull request 'fix(e2e): match an email template tab on its whole name, not a prefix' (#353) from fix/email-template-tab-name-collision into main
Linting / lint (push) Successful in 3m2s
SonarQube Analysis / sonarqube (push) Failing after 31m8s
Reviewed-on: #353
2026-09-11 10:04:42 -05:00
synAdminandClaude Opus 5 c2ee0b0c5d fix(e2e): match an email template tab on its whole name, not a prefix
SonarQube Analysis / sonarqube (pull_request) Canceled after 0s
Linting / lint (pull_request) Successful in 3m47s
CI failed on a test nobody had touched:

    strict mode violation: getByRole('tab', { name: /Email address changed/ })
    resolved to 2 elements

The locators in AdminEmails built an unanchored regex from the label, so a template whose name merely began with another's matched both. #337 added "Email address changed by the shop" alongside "Email address changed" and broke the assertion above it.

The failure named the assertion rather than the new template, which is what made it worth more than a rename. It is the same shape as the switch locator that silently retargeted in #317: a loose locator that keeps passing until something new is added nearby, and then fails somewhere that says nothing about the cause.

So the fix is the locator rather than the label. One helper now builds a matcher anchored at both ends, allowing only the optional "Customised" suffix a tab carries once its template has been edited, and escaping the label because these are copy and copy acquires brackets and full stops eventually. Both railTab and customisedTab go through it, and callers pass plain strings instead of assembling regexes at each call site.

The new template is added to the list the test walks, which is what it should have had in #337.

Verified the matcher against every real label plus the pairs that collide, including that a customised-only match still rejects an unedited tab. Frontend tsc and lint clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-11 09:59:05 -05:00
2 changed files with 53 additions and 8 deletions
+7 -3
View File
@@ -25,14 +25,18 @@ test.describe('Editing the customer emails', () => {
'Favorited item sold',
'Favorited item withdrawn',
'Cart reminder',
'Email address changed'
'Email address changed',
// Added in #337, and the reason these are matched on the whole
// accessible name rather than as substrings: it extends the label above
// it, so an unanchored match resolved to both tabs.
'Email address changed by the shop'
]) {
await expect(adminEmails.railTab(new RegExp(label))).toBeVisible();
await expect(adminEmails.railTab(label)).toBeVisible();
}
// Only a customised template is marked, so which ones have been changed is
// visible without opening each one. An untouched template carries nothing.
await expect(adminEmails.railTab(/Password reset/)).toBeVisible();
await expect(adminEmails.railTab('Password reset')).toBeVisible();
await expect(adminEmails.customisedTab('Password reset')).toHaveCount(0);
});
+46 -5
View File
@@ -1,5 +1,29 @@
import { FrameLocator, Locator, Page, expect } from '@playwright/test';
/**
* A matcher for one template label, against a tab's *whole* accessible name.
*
* A tab is named for its template, plus the word "Customised" once it has been
* edited — the dot beside it carries that as an aria-label, so the state is not
* colour-only.
*
* Anchored at both ends, which is the entire point of this function. The
* locators here used to build an unanchored regex from the label, so a template
* whose name merely *began* with another's matched both. Adding "Email address
* changed by the shop" alongside "Email address changed" broke a passing test
* with a strict-mode violation naming the assertion rather than the new
* template — the same shape as the switch locator that silently retargeted in
* #317, and the same cost to diagnose.
*
* The escape matters for the same reason: these labels are copy, and copy
* acquires brackets and full stops eventually.
*/
function nameMatching(label: string, options: { customised?: boolean } = {}): RegExp {
const escaped = label.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
const suffix = options.customised ? '\\s+Customised' : '(?:\\s+Customised)?';
return new RegExp(`^${escaped}${suffix}$`);
}
/**
* The Emails tab: a vertical rail of template types and one editor at a time.
*
@@ -30,13 +54,30 @@ export class AdminEmails {
return this.page.getByRole('button', { name: `Insert {{${name}}}` });
}
/** One template's entry in the rail. */
railTab(label: string | RegExp): Locator {
return this.page.getByRole('tab', { name: label });
/**
* One template's entry in the rail, matched on its whole accessible name.
*
* A tab's accessible name is the template's label, plus the word "Customised"
* when it has been edited — the dot beside it carries that as an aria-label so
* the state is not colour-only.
*
* Anchored at both ends, which is the point of this helper rather than a bare
* substring match. These locators used to build an unanchored regex from the
* label, so a template whose name merely *began* with another's matched both.
* Adding "Email address changed by the shop" beside "Email address changed"
* broke a passing test with a strict-mode violation, and the failure named the
* assertion rather than the new template — the same shape as the switch
* locator that silently retargeted in #317.
*
* The escape matters for the same reason: a label is copy, and copy acquires
* brackets and full stops eventually.
*/
railTab(label: string): Locator {
return this.page.getByRole('tab', { name: nameMatching(label) });
}
customisedTab(label: string): Locator {
return this.page.getByRole('tab', { name: new RegExp(`${label}.*Customised`) });
return this.page.getByRole('tab', { name: nameMatching(label, { customised: true }) });
}
subject(label: string): Locator {
@@ -65,7 +106,7 @@ export class AdminEmails {
* resolved mid-swap finds the outgoing one.
*/
async openTemplate(label: string): Promise<void> {
await this.railTab(new RegExp(label)).click();
await this.railTab(label).click();
await expect(this.subject(label)).toBeVisible();
}
}