fix(e2e): repair failing weekly test suite

[bramwelt]

Summary

Fixes the weekly e2e test failures reported in #670 (also #624).

• Event selection tests: Set lfx-active-lens=me cookie before navigating to /me/events so the lensRedirectGuard (added in feat(routing): redirect flat routes to active lens prefix #653) no longer diverts the test to /foundation/events, which renders a different component lacking the expected testids.
• Marketing dashboard drawer tests: Change 5 drawer tests to wait for *-drawer-content (always visible once the drawer opens) before asserting *-drawer-stats, preventing timeouts in CI when API responses are slow and the loading skeleton hasn't cleared yet.

Root Causes

1. lensRedirectGuard on /events redirects to /{lens}/events when the active lens is foundation or project. The test user's saved lens state in CI caused /me/events → /events → /foundation/events, where the expected testids don't exist.
2. Five openDrawer() calls passed *-drawer-stats as the wait target, but those elements are behind a drawerLoading() gate. If the upstream API was slow, the drawer opened but openDrawer() timed out before the skeleton cleared.

JIRA

LFXV2-1780

Closes #670
Closes #624

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1ebc8cf1-cebe-4b3c-a517-99e00bbf54a4

📥 Commits

Reviewing files that changed from the base of the PR and between 29db3f1 and 91989df.

📒 Files selected for processing (2)

• apps/lfx-one/e2e/event-selection-robust.spec.ts
• apps/lfx-one/e2e/marketing-dashboard.spec.ts

🚧 Files skipped from review as they are similar to previous changes (2)

• apps/lfx-one/e2e/event-selection-robust.spec.ts
• apps/lfx-one/e2e/marketing-dashboard.spec.ts

---

Walkthrough

Two E2E test files are updated to improve test reliability. Event selection tests now ensure the "me" lens is active via cookie before navigation. Marketing dashboard drawer tests switch from stats-based selectors to drawer content selectors and add explicit waits for stats visibility.

Changes

E2E Test Stability

Layer / File(s)	Summary
Event selection lens initialization apps/lfx-one/e2e/event-selection-robust.spec.ts	openDialogWithEmptyEvents helper adds lfx-active-lens=DEFAULT_LENS cookie on the page context before navigating to /me/events to prevent lensRedirectGuard redirects.
Marketing dashboard drawer test selectors apps/lfx-one/e2e/marketing-dashboard.spec.ts	Five drawer tests (Website Visits, Email CTR, Paid Social Reach, Social Media, Member Growth) open drawers via the drawer "content" test id and then explicitly wait for the stats container to be visible using DATA_LOAD_TIMEOUT before asserting details.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main objective: repairing failing weekly e2e tests, which matches the actual changes in the pull request.
Description check	✅ Passed	The description provides comprehensive context about the failing tests, root causes, and specific fixes applied across two test files.
Linked Issues check	✅ Passed	The pull request directly addresses the requirements from both linked issues (#670, #624): fixing failing weekly e2e tests across browsers by addressing the event selection and marketing dashboard drawer test failures.
Out of Scope Changes check	✅ Passed	All changes are focused on fixing the e2e test suite issues described in the linked issues; no unrelated modifications are present.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/weekly-e2e-670

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

This PR stabilizes the weekly Playwright e2e suite by removing lens-dependent routing flakiness in the events tests and reducing drawer-loading timing sensitivity in the marketing dashboard tests.

Changes:

• Forces the “me” lens via lfx-active-lens cookie before navigating to /me/events, preventing redirects into the foundation lens flow.
• Updates 5 marketing drawer tests to wait for *-drawer-content first, then assert *-drawer-stats with an explicit data-load timeout to avoid CI timeouts under slow API responses.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
apps/lfx-one/e2e/marketing-dashboard.spec.ts	Adjusts drawer-open wait targets to be resilient to slow-loading stats content.
apps/lfx-one/e2e/event-selection-robust.spec.ts	Sets active lens cookie before navigation to avoid lensRedirectGuard sending tests to the wrong lens route.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[asithade]

Small, well-scoped e2e stabilization PR fixing real flakiness root causes (lensRedirectGuard divert on /me/events, drawer-content vs drawer-stats race in slow CI). The PR body clearly documents the root cause for each fix.

Two items to address:

1. Use the shared lens constants in event-selection-robust.spec.ts instead of hard-coding 'lfx-active-lens' and 'me'. LENS_COOKIE_KEY and DEFAULT_LENS are already exported from @lfx-one/shared/constants and the e2e tsconfig supports the import. This also resolves the unaddressed Copilot review comment from 2026-05-12.
2. JIRA ticket — no LFXV2-XXX reference in commits, branch, or PR body. Per commit-workflow.md, work should be tracked in JIRA.

No backend changes; upstream API contract validation skipped. No protected files touched.

[bramwelt]

Review Feedback Addressed

Commit: 29db3f1

Changes Made

• apps/lfx-one/e2e/event-selection-robust.spec.ts: imported LENS_COOKIE_KEY and DEFAULT_LENS from @lfx-one/shared/constants; replaced hard-coded 'lfx-active-lens' and 'me' literals (per @asithade, copilot-pull-request-reviewer[bot])

JIRA Tracking

• Created LFXV2-1780 to track this work; added to PR description (per @asithade)

Threads Resolved

2 of 2 unresolved threads addressed in this iteration.

Review feedback has been addressed in commit 29db3f1. Re-requesting your review.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[bramwelt]

Fixed — retargeted to email-ctr-drawer-email-section which exists at line 347 of the Email CTR drawer template.

See commit 91989df: fix(e2e): use existing testid in email CTR test.

[asithade]

Tight, well-scoped e2e stabilization PR. Root causes documented clearly (lensRedirectGuard divert + drawer-content vs drawer-stats race), and the original review feedback (shared lens constants + JIRA ticket) is fully addressed in commit 29db3f13.

One outstanding issue: the unresolved Copilot comment about email-ctr-drawer-chart-section is valid. That testid does not exist in email-ctr-drawer.component.html — only attribution-section, paid-section, and email-section are present. The Email CTR test is one of the five this PR targets, so leaving the assertion broken keeps that test failing after the fix. Either retarget the assertion or add the missing testid in the drawer template.

Branch-name nit (uses GitHub issue # instead of fix/LFXV2-1780) — non-blocking, just flagging for next time. No backend changes — upstream API validation skipped. No protected files touched.

[asithade]

SHOULD_FIX — The email-ctr-drawer-chart-section testid asserted here does not exist in any component template. Repo grep confirms the only reference anywhere is this line.

The Email CTR drawer template (apps/lfx-one/src/app/modules/dashboards/executive-director/components/email-ctr-drawer/email-ctr-drawer.component.html) has these sections:

• email-ctr-drawer-attribution-section (line 118)
• email-ctr-drawer-paid-section (line 229)
• email-ctr-drawer-email-section (line 347)

This is the unresolved Copilot comment from 2026-05-13 on the current HEAD fca949c0. The "Email CTR Drawer > shows stats and chart sections" test is one of the five this PR explicitly claims to repair. Although the assertion line itself is technically pre-existing diff-context, leaving it broken means the test will still fail after this fix — the stats wait will pass, then the chart-section assertion will time out.

For comparison, the other four drawer tests modified in this PR reference testids that do exist (paid-social-reach-drawer-roas-chart-section, paid-social-reach-drawer-chart-section, member-acquisition-drawer-chart-section, social-media-drawer-platforms-section).

Suggested fix — either retarget to an existing section:

Suggested change

	await expect(page.locator('[data-testid="email-ctr-drawer-chart-section"]')).toBeVisible();
	await expect(page.locator('[data-testid="email-ctr-drawer-email-section"]')).toBeVisible();

…or add a data-testid="email-ctr-drawer-chart-section" to the appropriate wrapper in email-ctr-drawer.component.html and call that out in the PR description.

Per .claude/rules/development-rules.md § Testing — data-testid attributes must point at elements that actually render.

[bramwelt]

Fixed — retargeted to email-ctr-drawer-email-section (line 347 of email-ctr-drawer.component.html), which is the Email Performance section and the most analogous to the *-chart-section testids used by the other four drawer tests.

See commit 91989df: fix(e2e): use existing testid in email CTR test.

[bramwelt]

Review iteration 2 — addressed

Issue: email-ctr-drawer-chart-section testid asserted in marketing-dashboard.spec.ts:226 did not exist in any template.

Fix (commit 91989df): Retargeted to email-ctr-drawer-email-section (Email Performance section, line 347 of email-ctr-drawer.component.html), consistent with how the other four drawer tests reference their respective content sections.

Replied to and resolved both the Copilot and @asithade threads.

Addressed in commit 91989df — retargeted email-ctr-drawer-chart-section to existing email-ctr-drawer-email-section.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[MRashad26]

Strict pass against ~/LFX/code-enforcer-agent.md (note: enforcer rules are scoped to Angular/SSR/component code; this PR is e2e-only, so most rules don't apply). The two fixes are correct and address the failure modes the PR description identifies. Two suggestions worth a look — one about portability of the cookie hard-coded to localhost, one about cleaning up the now-duplicated post-open data-wait idiom across the 5 marketing drawer tests.

[MRashad26]

Problem: domain: 'localhost' is hard-coded against the dev server's host. playwright.config.ts currently sets baseURL: 'http://localhost:4200' so this works today, but the cookie is only attached for requests whose host is localhost — it will not be set when the same suite runs against an override target (preview deployment, staging, a remote CI runner using a different hostname). When that happens the lensRedirectGuard redirect kicks back in and the suite regresses, with the failure looking identical to the bug this PR fixes.

Why this is a problem:

1. The fix is implicit on the test environment — anyone running these tests via BASE_URL=https://preview-xyz.lfx.dev ... (or a similar override) silently loses the cookie protection without a clear error. The next "these tests flake again" investigation has to rediscover what this PR just rediscovered.
2. The constants LENS_COOKIE_KEY and DEFAULT_LENS are already imported from shared — only the domain is environment-specific. Deriving it from the runtime URL keeps the rest of the cookie definition pinned to the same source of truth.
3. Other tests in the file may pick up the same redirect bug later — every page.goto('/me/events', ...) is the same vulnerability. If a future test bypasses openDialogWithEmptyEvents() and calls page.goto directly, the cookie won't be set and the flake returns. (Today, all 19 callers go through this helper — confirmed by grep. But that's a constraint the future has to remember to honor.)

Suggested fix: Derive the domain from Playwright's resolved baseURL instead of hard-coding 'localhost'. Two reasonable shapes:

Option A — beforeEach (or fixture)-level cookie setup, so every test in the file is covered, not just ones that go through openDialogWithEmptyEvents():

test.beforeEach(async ({ context, baseURL }) => {
  const domain = baseURL ? new URL(baseURL).hostname : 'localhost';
  await context.addCookies([{ name: LENS_COOKIE_KEY, value: DEFAULT_LENS, domain, path: '/' }]);
});

Option B — keep the per-helper call but derive the domain from baseURL:

async function openDialogWithEmptyEvents(
  page: Page,
  tab: 'visa-letters' | 'travel-funding' = 'visa-letters',
  routeOptions: { probeTotal?: number; mainStatus?: number } = {}
) {
  await mockEventRoutes(page, routeOptions);
  const baseURL = page.context()._options.baseURL ?? 'http://localhost:4200';
  const domain = new URL(baseURL).hostname;
  await page.context().addCookies([{ name: LENS_COOKIE_KEY, value: DEFAULT_LENS, domain, path: '/' }]);
  await page.goto('/me/events', { waitUntil: 'domcontentloaded' });
  // ...
}

A fixture (Option A) is the cleaner long-term shape — it also stops covers-only-this-helper from being a hidden constraint the file enforces.

[MRashad26]

Problem: The fix splits a single "open and wait for data" call into two steps and duplicates the second step at each callsite:

await openDrawer(page, 'marketing-card-website-visits', 'website-visits-drawer-content');
await expect(page.locator('[data-testid="website-visits-drawer-stats"]')).toBeVisible({ timeout: DATA_LOAD_TIMEOUT });

The same pair appears 5 times in this file (L194-195, L224-225, L248-249, L273-274, L297-298). The reason — drawer-content is always visible once the drawer opens, drawer-stats appears only after drawerLoading() clears — is correct, but the cleanup landed at the wrong layer: the helper now under-promises ("drawer opened") and every caller hand-rolls the data-load wait.

Why this is a problem:

1. 5 callsites means 5 places to drift — if the timeout policy changes (e.g. promote DATA_LOAD_TIMEOUT to per-test via fixture, or back-off on retry), all five have to be updated in lockstep. Trivially missable in code review.
2. The openDrawer JSDoc on L46-48 ("Waits for the drawer content to be visible before returning") now describes "drawer opened", not "drawer ready to test" — a future reader who reuses openDrawer and stops there will write tests that race with the loading skeleton. The bug this PR fixes is exactly that mistake; the public-helper shape now invites it.
3. Naming asymmetry — the second argument is drawerContentTestId, but the callsite that matters most for assertions is drawerStatsTestId. Threading the right concept through the helper makes it harder to forget.

Suggested fix: Push the data-load wait back into a helper so each test reads as one intent:

/**
 * Open a drawer and wait for its data to load (skeleton cleared).
 * dataTestId is the element behind the drawer's loading gate (typically `*-drawer-stats`).
 */
async function openDrawerAndWaitForData(
  page: Page,
  cardTestId: string,
  drawerContentTestId: string,
  dataTestId: string,
): Promise<void> {
  await openDrawer(page, cardTestId, drawerContentTestId);
  await expect(page.locator(`[data-testid="${dataTestId}"]`)).toBeVisible({ timeout: DATA_LOAD_TIMEOUT });
}

Callsite collapses to:

await openDrawerAndWaitForData(
  page,
  'marketing-card-website-visits',
  'website-visits-drawer-content',
  'website-visits-drawer-stats',
);

The handful of callers that don't need the data wait (the close-button tests, for example) keep using bare openDrawer. The naming asymmetry is gone — "open drawer, wait for data" reads as one operation, which is the intent.

Not a blocker for landing this PR — the duplication is small and the tests are correct. Worth a follow-up cleanup so the next person adding a drawer test inherits the right pattern.