Skip to content

Partner dashboard branch (#1262): review findings to resolve before merge #1324

Description

@TaprootFreak

EN: Review of the partner-dashboard branch (#1262) surfaced one item that needs handling outside the branch — a test fixture carries real partner production figures in this public repository — plus a few branch-local defects worth fixing before merge.
DE: Das Review des Partner-Dashboard-Branches (#1262) hat einen Punkt aufgedeckt, der ausserhalb des Branches gehört — eine Test-Fixture trägt echte Partner-Produktionszahlen in diesem öffentlichen Repository — sowie einige branch-lokale Defekte, die vor dem Merge behoben werden sollten.

1. Real partner figures in a public fixture — needs a purge, not just an edit

src/partner-dashboard/fixtures/partner-statistic.fixture.ts documents its own values as real, production-checked partner figures (all-time volume and user counts). Those numbers do not belong in a public repository. Two actions are needed and neither can be done from a normal follow-up commit:

  • Replace the figures with clearly invented placeholder values in the fixture (and in any handbook baseline derived from them).
  • The real values are already in the public git history across several commits; a content edit does not remove them. A history purge (git history rewrite / GitHub Support) is required, and this should be treated internally as a possible disclosure to be assessed.

The PR body already flags the fixture values as an open owner decision, so this is a known item — this issue is to track it to closure rather than let it merge as-is.

2. Reality declaration missing (CONTRIBUTING requirement)

docs/test-architecture.md requires a docs/test-reality.md reality declaration for any PR that introduces a declarable fake. This branch introduces a handbook e2e that runs entirely on a synthetic, unsigned JWT with all API traffic mocked — the first spec in the repo to do so — and ships no reality declaration. Add one with the fields the architecture doc specifies.

3. Time-zone-dependent date rendering (functional defect)

toLocaleDateString(...) is called without timeZone: 'UTC' in the dashboard's date rendering (axis labels, tooltips and table rows across src/partner-dashboard/util/timeline-axis.ts and the two time-chart components). A UTC bucket boundary then renders as the previous calendar day in negative-UTC-offset time zones. The e2e baselines pin timezoneId: 'UTC', which hides this in CI but not for real users. Pass timeZone: 'UTC' at each call.

4. Dashboard fetch fires before the role guard redirects (defense in depth)

The screen renders the view unconditionally while the role guard's redirect lives in a passive effect, and the data load has no abort on unmount. An unauthorized visitor's browser can dispatch the statistic requests before being redirected away. The API side is the real gate, but the frontend should not fire the fetch before the guard has passed.

5. Smaller items

  • src/partner-dashboard/styles/tokens.css references source files that are not in the tree (primitives.json, themes.json) and names an internal preview host in a header comment — make the source reproducible and keep the comment generic for a public repo.
  • .gitignore carries a /partner-dist entry left over from an earlier build-target iteration that no longer exists; remove it.
  • New test files use any casts and a few relative imports where the repo standard is absolute src/... imports.

Out of scope (pre-existing, repo-wide)

The role guard's fail-open shape on an undefined session and dashboard state that is not re-keyed on account identity both mirror the existing support/compliance dashboards; they are not specific to this branch and belong to a separate, repo-wide discussion rather than this PR.


Context: this came out of an independent two-dimension review (conformance + logic) of the branch; each finding was checked against the cited source. Items 1 and 2 are the merge blockers; 3–5 are branch-local fixes.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions