PR #317 was merged while its run was still failing, so four failures landed on main: one integration and three end-to-end. All of them are consequences of #56, and none could have been caught on the dev machine, which has no database and no browser to run those suites with. The schema mirror was never regenerated. The migration added three columns to customers and src/db-kysely/schema.ts still described the table without them, which is the drift guard from #305 doing exactly what it exists for. Hand-edited to match what kysely-codegen emits — alphabetical, and Generated on the column that has a default — because regenerating properly needs a live database. The other three are the same mistake three times: a control addressed by position, and the position moved. An unscoped getByRole('checkbox') became ambiguous once the register form had two consents. A toHaveCount(2) on the account modal's switches became three. And favoriteAlertsSwitch was getByRole('switch').last(), which did not error when a switch was appended below it — it silently retargeted, toggled analytics consent instead of favourite alerts, and then failed on a text assertion in favorites.spec.ts, naming neither the file nor the control actually at fault. The reason position was ever used is that antd's Switch renders a bare role="switch" with no accessible name; the adjacent Text is a sibling, not a label. So each one now carries an explicit aria-label and is addressed by it. That is what makes them addressable from a test, and it is what a screen reader needed regardless — the fix and the accessibility improvement are the same change. The count assertion stays, but alongside naming each switch, because a count on its own would pass if two of them were swapped for each other. Two coverage gaps closed while here, both properties the compliance work in #56 depends on and neither previously asserted anywhere a customer could see: the analytics checkbox is unchecked on the register form, and the account toggle is off for a new customer. Quebec's Law 25 s.8.1 requires profiling to start off, the integration suite asserts the server half of that, and nothing asserted the half rendered on screen. The fourth Playwright entry, the logged-out header surviving a reload, is reported flaky rather than failed and passed on retry. Left alone; it is unrelated to #56 and #257 covers flakes in this suite. Verified: backend tsc clean, frontend tsc against the test config clean, production build green, both lint suites 0 errors, 478 unit tests passing. The integration and e2e suites still cannot run here, so whether this actually clears run 875's failures is for CI to say — which is the same gap that produced them. Closes #320 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
0ec6064391
commit
5b40ac25db
@@ -72,6 +72,9 @@ export interface Checkouts {
|
||||
}
|
||||
|
||||
export interface Customers {
|
||||
analytics_consent: Generated<boolean>;
|
||||
analytics_consent_at: Timestamp | null;
|
||||
analytics_consent_text: string | null;
|
||||
created_at: Generated<Timestamp>;
|
||||
disabled_at: Timestamp | null;
|
||||
email: string;
|
||||
|
||||
@@ -141,7 +141,7 @@ export default function Account({ onClose }: Props) {
|
||||
|
||||
<Divider />
|
||||
<Space align="center">
|
||||
<Switch checked={customer.marketing_consent} onChange={handleConsentToggle} />
|
||||
<Switch aria-label="Receive emails about new items" checked={customer.marketing_consent} onChange={handleConsentToggle} />
|
||||
<Text>Receive emails about new items</Text>
|
||||
</Space>
|
||||
|
||||
@@ -149,7 +149,7 @@ export default function Account({ onClose }: Props) {
|
||||
customer can hold one without the other. */}
|
||||
<div style={{ marginTop: 12 }}>
|
||||
<Space align="center">
|
||||
<Switch checked={customer.favorite_alerts} onChange={handleFavoriteAlertsToggle} />
|
||||
<Switch aria-label="Email me when an item I favorited is sold" checked={customer.favorite_alerts} onChange={handleFavoriteAlertsToggle} />
|
||||
<Text>Email me when an item I favorited is sold</Text>
|
||||
</Space>
|
||||
</div>
|
||||
@@ -161,7 +161,7 @@ export default function Account({ onClose }: Props) {
|
||||
control that has to make withdrawal as easy as consenting. */}
|
||||
<div style={{ marginTop: 12 }}>
|
||||
<Space align="center">
|
||||
<Switch checked={customer.analytics_consent} onChange={handleAnalyticsConsentToggle} />
|
||||
<Switch aria-label="Share what I browse and buy with Brevo" checked={customer.analytics_consent} onChange={handleAnalyticsConsentToggle} />
|
||||
<Text>Share what I browse and buy with Brevo, to make emails relevant</Text>
|
||||
</Space>
|
||||
<div style={{ marginTop: 4 }}>
|
||||
|
||||
@@ -78,7 +78,28 @@ test.describe('My Account opens as a modal', () => {
|
||||
await expect(accountModal.orderHistoryButton).toBeVisible();
|
||||
// Scoped to the modal: the storefront behind it has a theme switch of its
|
||||
// own, so an unscoped switch locator would be ambiguous.
|
||||
await expect(accountModal.themeSwitches).toHaveCount(2);
|
||||
//
|
||||
// Three since #56 added analytics consent: marketing email, favourite
|
||||
// alerts, analytics. The count is a guard against one appearing or
|
||||
// vanishing unnoticed, so it is asserted alongside naming each of them —
|
||||
// a count alone would pass if two were swapped for each other.
|
||||
await expect(accountModal.themeSwitches).toHaveCount(3);
|
||||
await expect(accountModal.marketingConsentSwitch).toBeVisible();
|
||||
await expect(accountModal.favoriteAlertsSwitch).toBeVisible();
|
||||
await expect(accountModal.analyticsConsentSwitch).toBeVisible();
|
||||
});
|
||||
|
||||
// Quebec's Law 25 s.8.1 requires profiling to be off until the person turns
|
||||
// it on. The server defaults the column to false and the integration suite
|
||||
// asserts that; this is the half of the promise the customer can actually
|
||||
// see, and the two have to agree.
|
||||
test('analytics consent is off until the customer turns it on', async ({
|
||||
customer,
|
||||
accountModal
|
||||
}) => {
|
||||
await accountModal.open();
|
||||
|
||||
await expect(accountModal.analyticsConsentSwitch).not.toBeChecked();
|
||||
});
|
||||
|
||||
test('deleting the account does not leave the page behind it looking signed in', async ({
|
||||
|
||||
@@ -20,6 +20,15 @@ test.describe('Customer accounts', () => {
|
||||
await expect(authModal.marketingConsent).not.toBeChecked();
|
||||
});
|
||||
|
||||
// A second, separate consent (#56). Unchecked for the same reason as the one
|
||||
// above, and asserted separately because the two must be independently
|
||||
// refusable — a single control covering both is the bundling GDPR treats as
|
||||
// invalid, and Law 25 requires this one to start off.
|
||||
test('analytics consent checkbox is unchecked by default', async ({ authModal }) => {
|
||||
await authModal.gotoRegister();
|
||||
await expect(authModal.analyticsConsent).not.toBeChecked();
|
||||
});
|
||||
|
||||
test('the consent label is the exact wording the server records', async ({ authModal }) => {
|
||||
await authModal.gotoRegister();
|
||||
|
||||
|
||||
@@ -16,9 +16,15 @@ export class AccountModal {
|
||||
readonly orderHistoryButton: Locator;
|
||||
readonly closeButton: Locator;
|
||||
readonly resendVerificationButton: Locator;
|
||||
/**
|
||||
* Every switch in the modal. Kept for the count assertion that guards against
|
||||
* a control appearing or vanishing unnoticed — the individual switches below
|
||||
* are addressed by name, not by position.
|
||||
*/
|
||||
readonly themeSwitches: Locator;
|
||||
/** The second switch in the modal; the first is the theme. */
|
||||
readonly marketingConsentSwitch: Locator;
|
||||
readonly favoriteAlertsSwitch: Locator;
|
||||
readonly analyticsConsentSwitch: Locator;
|
||||
readonly notVerifiedNotice: Locator;
|
||||
|
||||
readonly firstName: Locator;
|
||||
@@ -46,7 +52,25 @@ export class AccountModal {
|
||||
this.closeButton = this.dialog.getByRole('button', { name: 'Close' });
|
||||
this.resendVerificationButton = this.dialog.getByRole('button', { name: 'Send it again' });
|
||||
this.themeSwitches = this.dialog.getByRole('switch');
|
||||
this.favoriteAlertsSwitch = this.dialog.getByRole('switch').last();
|
||||
// Addressed by accessible name rather than by position. `favoriteAlertsSwitch`
|
||||
// used to be `.last()`, which silently retargeted the moment the analytics
|
||||
// consent switch was added below it (#56): the test that meant to turn
|
||||
// favourite alerts off toggled analytics consent on instead, and failed on
|
||||
// the message rather than on the switch, which said nothing about why.
|
||||
//
|
||||
// antd's Switch renders a bare `role="switch"` with no accessible name — the
|
||||
// adjacent Text is a sibling, not a label — so each one carries an explicit
|
||||
// aria-label in Account.tsx. That is what makes these addressable, and it is
|
||||
// also what a screen reader needed.
|
||||
this.marketingConsentSwitch = this.dialog.getByRole('switch', {
|
||||
name: 'Receive emails about new items'
|
||||
});
|
||||
this.favoriteAlertsSwitch = this.dialog.getByRole('switch', {
|
||||
name: 'Email me when an item I favorited is sold'
|
||||
});
|
||||
this.analyticsConsentSwitch = this.dialog.getByRole('switch', {
|
||||
name: 'Share what I browse and buy with Brevo'
|
||||
});
|
||||
this.notVerifiedNotice = this.dialog.getByText('Email not verified');
|
||||
|
||||
this.firstName = this.dialog.getByLabel('First name', { exact: true });
|
||||
|
||||
@@ -22,6 +22,7 @@ export class AuthModal {
|
||||
readonly lastName: Locator;
|
||||
readonly password: Locator;
|
||||
readonly marketingConsent: Locator;
|
||||
readonly analyticsConsent: Locator;
|
||||
readonly createAccountButton: Locator;
|
||||
readonly createAccountTab: Locator;
|
||||
readonly logInTab: Locator;
|
||||
@@ -34,7 +35,17 @@ export class AuthModal {
|
||||
this.firstName = page.getByRole('textbox', { name: 'First name' });
|
||||
this.lastName = page.getByRole('textbox', { name: 'Last name' });
|
||||
this.password = page.getByLabel('Password');
|
||||
this.marketingConsent = page.getByRole('checkbox');
|
||||
// Named rather than "the checkbox on the form". There are two consents now
|
||||
// and they are deliberately separate (#56), so an unscoped checkbox locator
|
||||
// is ambiguous in strict mode — which is how this broke. Matched on the
|
||||
// opening words of each sentence so the locator survives the wording being
|
||||
// revised, which it has been once already.
|
||||
this.marketingConsent = page.getByRole('checkbox', {
|
||||
name: /^I want to receive occasional emails/
|
||||
});
|
||||
this.analyticsConsent = page.getByRole('checkbox', {
|
||||
name: /^I agree that what I browse and buy/
|
||||
});
|
||||
this.createAccountButton = page.getByRole('button', { name: 'Create account' });
|
||||
this.createAccountTab = page.getByRole('tab', { name: 'Create Account' });
|
||||
this.logInTab = page.getByRole('tab', { name: 'Log In' });
|
||||
|
||||
Reference in New Issue
Block a user