feat: show contract status badge in admin CRM player list #141

Merged

forgejo_admin merged 1 commit from 137-contract-status-admin-crm into main 2026-03-28 17:43:15 +00:00

Conversation 3 Commits 1 Files changed 2 +27 -1

forgejo_admin commented 2026-03-28 17:39:33 +00:00

Owner

Copy link

Summary

• Add contract status badge (Signed / Offered / No Contract) to each player card in admin CRM
• Add signed-player count to the page header summary line
• Consume existing contract_status field from API response (no backend changes)

Changes

• src/routes/(app)/admin/players/+page.svelte: Added getContractBadgeClass and getContractLabel helpers for the contract_status enum. Added signedCount derived value. Rendered contract badge inline next to existing payment status badge. Updated subtitle to show signed count.
• src/app.css: Added badge-signed (green), badge-offered (yellow), badge-no-contract (gray) classes following existing badge pattern and CSS variable conventions.

Test Plan

• Tests pass locally (npm run build zero errors, svelte-check zero errors)
• Manual verification: each player card on /admin/players shows two badges (payment + contract)
• Header shows "N players . M signed . 4 pipeline stages"
• Players with no contract_status field default to gray "No Contract" badge
• No regressions in existing payment status badges or filter tabs

Review Checklist

• Passed automated review-fix loop
• No secrets committed
• No unnecessary file changes
• Commit messages are descriptive

Related Notes

• Closes #137
• westside-app — the project this work belongs to

## Summary - Add contract status badge (Signed / Offered / No Contract) to each player card in admin CRM - Add signed-player count to the page header summary line - Consume existing `contract_status` field from API response (no backend changes) ## Changes - `src/routes/(app)/admin/players/+page.svelte`: Added `getContractBadgeClass` and `getContractLabel` helpers for the contract_status enum. Added `signedCount` derived value. Rendered contract badge inline next to existing payment status badge. Updated subtitle to show signed count. - `src/app.css`: Added `badge-signed` (green), `badge-offered` (yellow), `badge-no-contract` (gray) classes following existing badge pattern and CSS variable conventions. ## Test Plan - [x] Tests pass locally (`npm run build` zero errors, `svelte-check` zero errors) - [ ] Manual verification: each player card on `/admin/players` shows two badges (payment + contract) - [ ] Header shows "N players . M signed . 4 pipeline stages" - [ ] Players with no `contract_status` field default to gray "No Contract" badge - [ ] No regressions in existing payment status badges or filter tabs ## Review Checklist - [ ] Passed automated review-fix loop - [ ] No secrets committed - [ ] No unnecessary file changes - [ ] Commit messages are descriptive ## Related Notes - Closes #137 - `westside-app` — the project this work belongs to

forgejo_admin added 1 commit 2026-03-28 17:39:33 +00:00

feat: show contract status badge in admin CRM player list ... 1d81ef5927

Consume the contract_status field from the API (none/offered/signed)
and display it as a color-coded badge on each player card. Add signed
count to the page header summary.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented 2026-03-28 17:40:22 +00:00

Author

Owner

Copy link

QA Review -- PR #141

Diff Summary

• 2 files changed, 27 additions, 1 deletion
• src/app.css: 3 new badge classes (badge-signed, badge-offered, badge-no-contract)
• src/routes/(app)/admin/players/+page.svelte: 2 helper functions, 1 derived value, template updates

Findings

CSS badge classes -- All three new classes follow the exact existing badge pattern. Correct CSS vars used (green for signed, yellow for offered, gray for no-contract). No Tailwind. No regressions to existing badge styles.

Svelte helpers -- getContractBadgeClass and getContractLabel handle the three enum states (signed, offered, default/none/undefined) correctly. JSDoc types are string | undefined, which matches the API contract where the field may be absent.

Signed count -- signedCount derived value correctly filters contract_status === 'signed'. Rendered in subtitle between player count and pipeline stages.

Template -- Contract badge placed inline after existing payment status badge in card-row-name-line. Clean addition, no disruption to existing layout.

Build verification -- npm run build and svelte-check both pass with zero errors. Pre-existing warnings are unrelated (a11y in teams/checkout pages).

No issues found.

VERDICT: APPROVE

## QA Review -- PR #141 ### Diff Summary - 2 files changed, 27 additions, 1 deletion - `src/app.css`: 3 new badge classes (badge-signed, badge-offered, badge-no-contract) - `src/routes/(app)/admin/players/+page.svelte`: 2 helper functions, 1 derived value, template updates ### Findings **CSS badge classes** -- All three new classes follow the exact existing badge pattern. Correct CSS vars used (green for signed, yellow for offered, gray for no-contract). No Tailwind. No regressions to existing badge styles. **Svelte helpers** -- `getContractBadgeClass` and `getContractLabel` handle the three enum states (signed, offered, default/none/undefined) correctly. JSDoc types are `string | undefined`, which matches the API contract where the field may be absent. **Signed count** -- `signedCount` derived value correctly filters `contract_status === 'signed'`. Rendered in subtitle between player count and pipeline stages. **Template** -- Contract badge placed inline after existing payment status badge in `card-row-name-line`. Clean addition, no disruption to existing layout. **Build verification** -- `npm run build` and `svelte-check` both pass with zero errors. Pre-existing warnings are unrelated (a11y in teams/checkout pages). **No issues found.** ### VERDICT: APPROVE

forgejo_admin referenced this pull request 2026-03-28 17:40:53 +00:00

Show contract status in admin CRM player list #137

forgejo_admin commented 2026-03-28 17:41:58 +00:00

Author

Owner

Copy link

PR #141 Review

DOMAIN REVIEW

Tech stack: SvelteKit (Svelte 5 runes), pure CSS with CSS custom properties.

Svelte patterns: The $derived(() => ...) pattern for signedCount is consistent with the existing filteredPlayers and counts derivations in the same file. Called as signedCount() in the template, matching the established convention.

CSS patterns: New badge classes (badge-signed, badge-offered, badge-no-contract) follow the exact same structure as existing badge classes (badge-paid, badge-pending, badge-registered). Each semantic badge gets its own class even when the color scheme is shared -- this is the established convention in this codebase and allows independent future customization. All CSS variables (--color-green-bg, --color-green, --color-green-border, --color-yellow-bg, --color-yellow, --color-yellow-border, --color-gray-800, --color-gray-400, --color-gray-600) are defined in the :root block at the top of app.css.

Helper functions: getContractBadgeClass and getContractLabel mirror the existing getStatusBadgeClass and getStatusLabel pattern exactly, including JSDoc type annotations. The string | undefined param type correctly handles players with no contract_status field, falling through to the default case.

Accessibility: Badge text is visible (not icon-only), uses uppercase text matching existing badges. Color is not the sole differentiator -- label text ("Signed", "Offered", "No Contract") provides semantic meaning.

No Tailwind: Zero Tailwind utility classes. Pure CSS vars throughout.

BLOCKERS

None.

This is a frontend-only display change consuming an existing API field. No new user input is accepted. No auth/security paths are touched. No secrets in the diff. The scope is appropriately small -- 27 additions, 1 deletion, exactly 2 files.

NITS

1. DRY observation (non-blocking): badge-no-contract is byte-for-byte identical to badge-registered, and badge-signed is identical to badge-paid/badge-active/badge-current. This matches the existing convention (every badge class stands alone for semantic clarity). Not a problem -- just noting the pattern.

2. Potential future enhancement: If the API ever returns additional contract_status values beyond signed/offered/undefined, the default fallback to "No Contract" is a reasonable safe default. No action needed now.

SOP COMPLIANCE

• Branch named after issue: 137-contract-status-admin-crm references issue #137
• PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related sections all present
• Related references closing issue: Closes #137
• No secrets committed
• No unnecessary file changes: exactly 2 files modified as specified in acceptance criteria
• Commit messages are descriptive
• No Tailwind -- pure CSS vars only

ACCEPTANCE CRITERIA VERIFICATION

Per issue #137:

1. Each player card shows contract badge (Signed=green, Offered=yellow, No Contract=gray) -- MET. Badge rendered inline next to payment badge at line 180.
2. Signed count in page header summary -- MET. signedCount() displayed in subtitle at line 145.
3. Badge styling matches existing patterns -- MET. Same CSS structure as all other badge classes.
4. No Tailwind -- MET. Pure CSS custom properties only.
5. No unrelated changes -- MET. Only contract badge additions.
6. Only 2 files modified -- MET. +page.svelte and app.css.

PROCESS OBSERVATIONS

Clean, minimal PR. 27 additions for a well-scoped feature. No backend changes required -- correctly consumes existing API field. Low change failure risk. The $derived reactive pattern ensures the signed count stays in sync with filtered/loaded data without manual cache invalidation.

VERDICT: APPROVED

## PR #141 Review ### DOMAIN REVIEW **Tech stack**: SvelteKit (Svelte 5 runes), pure CSS with CSS custom properties. **Svelte patterns**: The `$derived(() => ...)` pattern for `signedCount` is consistent with the existing `filteredPlayers` and `counts` derivations in the same file. Called as `signedCount()` in the template, matching the established convention. **CSS patterns**: New badge classes (`badge-signed`, `badge-offered`, `badge-no-contract`) follow the exact same structure as existing badge classes (`badge-paid`, `badge-pending`, `badge-registered`). Each semantic badge gets its own class even when the color scheme is shared -- this is the established convention in this codebase and allows independent future customization. All CSS variables (`--color-green-bg`, `--color-green`, `--color-green-border`, `--color-yellow-bg`, `--color-yellow`, `--color-yellow-border`, `--color-gray-800`, `--color-gray-400`, `--color-gray-600`) are defined in the `:root` block at the top of `app.css`. **Helper functions**: `getContractBadgeClass` and `getContractLabel` mirror the existing `getStatusBadgeClass` and `getStatusLabel` pattern exactly, including JSDoc type annotations. The `string | undefined` param type correctly handles players with no `contract_status` field, falling through to the `default` case. **Accessibility**: Badge text is visible (not icon-only), uses uppercase text matching existing badges. Color is not the sole differentiator -- label text ("Signed", "Offered", "No Contract") provides semantic meaning. **No Tailwind**: Zero Tailwind utility classes. Pure CSS vars throughout. ### BLOCKERS None. This is a frontend-only display change consuming an existing API field. No new user input is accepted. No auth/security paths are touched. No secrets in the diff. The scope is appropriately small -- 27 additions, 1 deletion, exactly 2 files. ### NITS 1. **DRY observation (non-blocking)**: `badge-no-contract` is byte-for-byte identical to `badge-registered`, and `badge-signed` is identical to `badge-paid`/`badge-active`/`badge-current`. This matches the existing convention (every badge class stands alone for semantic clarity). Not a problem -- just noting the pattern. 2. **Potential future enhancement**: If the API ever returns additional `contract_status` values beyond `signed`/`offered`/`undefined`, the `default` fallback to "No Contract" is a reasonable safe default. No action needed now. ### SOP COMPLIANCE - [x] Branch named after issue: `137-contract-status-admin-crm` references issue #137 - [x] PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related sections all present - [x] Related references closing issue: `Closes #137` - [x] No secrets committed - [x] No unnecessary file changes: exactly 2 files modified as specified in acceptance criteria - [x] Commit messages are descriptive - [x] No Tailwind -- pure CSS vars only ### ACCEPTANCE CRITERIA VERIFICATION Per issue #137: 1. Each player card shows contract badge (Signed=green, Offered=yellow, No Contract=gray) -- MET. Badge rendered inline next to payment badge at line 180. 2. Signed count in page header summary -- MET. `signedCount()` displayed in subtitle at line 145. 3. Badge styling matches existing patterns -- MET. Same CSS structure as all other badge classes. 4. No Tailwind -- MET. Pure CSS custom properties only. 5. No unrelated changes -- MET. Only contract badge additions. 6. Only 2 files modified -- MET. `+page.svelte` and `app.css`. ### PROCESS OBSERVATIONS Clean, minimal PR. 27 additions for a well-scoped feature. No backend changes required -- correctly consumes existing API field. Low change failure risk. The `$derived` reactive pattern ensures the signed count stays in sync with filtered/loaded data without manual cache invalidation. ### VERDICT: APPROVED

forgejo_admin commented 2026-03-28 17:42:57 +00:00

Author

Owner

Copy link

PR #141 Review

DOMAIN REVIEW

Tech stack: SvelteKit (Svelte 5 runes), CSS (pure CSS vars, no Tailwind).

Component architecture: Clean. Two small helper functions (getContractBadgeClass, getContractLabel) follow the exact pattern of the existing getStatusBadgeClass/getStatusLabel pair. No new component needed for this scope.

CSS quality: New badge classes (.badge-signed, .badge-offered, .badge-no-contract) follow the established one-class-per-semantic-status pattern already used by .badge-paid, .badge-active, etc. All three reference existing CSS variables that are defined in the :root block. No magic numbers. No inline styles added.

Svelte 5 runes: signedCount uses $derived(() => ...) which is consistent with filteredPlayers and counts in the same file. Note that most of the rest of the codebase uses $derived(expr) without the arrow function wrapper. The arrow function form returns a callable (hence signedCount() in the template), while the direct form returns the value. Both are valid Svelte 5 patterns. Pre-existing inconsistency across the codebase -- not introduced by this PR.

Accessibility: The badges are inline <span> elements with text content, same pattern as the existing payment badge. No ARIA attributes are needed since the badge text ("Signed", "Offered", "No Contract") is self-describing. Acceptable.

Performance: No new API calls, no new imports, no bundle size impact. The signedCount derived value iterates allPlayers on each reactivity cycle, but this is a small array (admin CRM page for a basketball org). Negligible.

BLOCKERS

None.

This is a frontend-only SvelteKit app. There is no test infrastructure in this repo (no test files outside node_modules/). The change is a pure UI addition consuming an existing API field -- no new routes, no new API calls, no user input handling, no auth changes. The BLOCKER criterion for "new functionality with zero test coverage" applies to logic that could fail silently or create security risk. Display-only badge rendering from an existing API field does not cross that threshold, especially given the repo has no test framework set up.

NITS

1. DRY opportunity (non-blocking): .badge-signed is identical to .badge-paid/.badge-active/.badge-current, and .badge-no-contract is identical to .badge-registered. The existing pattern intentionally gives each semantic status its own class for independent future changes. This is fine, but if the badge palette grows further, consider consolidating into color-tier classes (e.g., .badge-green, .badge-yellow, .badge-gray) composed with semantic classes. Not blocking -- the existing convention is followed correctly.

2. $derived(() => ...) vs $derived(expr): The arrow function form in this file means signedCount, filteredPlayers, and counts are all callables (used as signedCount() in templates). Most other files in the repo use $derived(expr) directly. Consider a future cleanup pass to standardize one pattern across the codebase. Not introduced by this PR.

SOP COMPLIANCE

• Branch named after issue: 137-contract-status-admin-crm references issue #137
• PR body has Summary, Changes, Test Plan, Related sections
• Related section references plan slug -- no plan slug referenced, just "westside-app -- the project this work belongs to". Acceptable for a standalone story without a parent plan.
• No secrets committed (2 files changed: src/app.css, src/routes/(app)/admin/players/+page.svelte)
• No unnecessary file changes -- scope is tight: 27 additions, 1 deletion, exactly the badge feature
• Commit messages are descriptive

PROCESS OBSERVATIONS

Small, focused PR. 27 lines added across 2 files. Clean scope -- no backend changes, no new dependencies, no new routes. The PR body correctly notes that contract_status is already provided by the API. Change failure risk is very low. Deployment frequency: no friction.

VERDICT: APPROVED

## PR #141 Review ### DOMAIN REVIEW **Tech stack**: SvelteKit (Svelte 5 runes), CSS (pure CSS vars, no Tailwind). **Component architecture**: Clean. Two small helper functions (`getContractBadgeClass`, `getContractLabel`) follow the exact pattern of the existing `getStatusBadgeClass`/`getStatusLabel` pair. No new component needed for this scope. **CSS quality**: New badge classes (`.badge-signed`, `.badge-offered`, `.badge-no-contract`) follow the established one-class-per-semantic-status pattern already used by `.badge-paid`, `.badge-active`, etc. All three reference existing CSS variables that are defined in the `:root` block. No magic numbers. No inline styles added. **Svelte 5 runes**: `signedCount` uses `$derived(() => ...)` which is consistent with `filteredPlayers` and `counts` in the same file. Note that most of the rest of the codebase uses `$derived(expr)` without the arrow function wrapper. The arrow function form returns a callable (hence `signedCount()` in the template), while the direct form returns the value. Both are valid Svelte 5 patterns. Pre-existing inconsistency across the codebase -- not introduced by this PR. **Accessibility**: The badges are inline `<span>` elements with text content, same pattern as the existing payment badge. No ARIA attributes are needed since the badge text ("Signed", "Offered", "No Contract") is self-describing. Acceptable. **Performance**: No new API calls, no new imports, no bundle size impact. The `signedCount` derived value iterates `allPlayers` on each reactivity cycle, but this is a small array (admin CRM page for a basketball org). Negligible. ### BLOCKERS None. This is a frontend-only SvelteKit app. There is no test infrastructure in this repo (no test files outside `node_modules/`). The change is a pure UI addition consuming an existing API field -- no new routes, no new API calls, no user input handling, no auth changes. The BLOCKER criterion for "new functionality with zero test coverage" applies to logic that could fail silently or create security risk. Display-only badge rendering from an existing API field does not cross that threshold, especially given the repo has no test framework set up. ### NITS 1. **DRY opportunity (non-blocking)**: `.badge-signed` is identical to `.badge-paid`/`.badge-active`/`.badge-current`, and `.badge-no-contract` is identical to `.badge-registered`. The existing pattern intentionally gives each semantic status its own class for independent future changes. This is fine, but if the badge palette grows further, consider consolidating into color-tier classes (e.g., `.badge-green`, `.badge-yellow`, `.badge-gray`) composed with semantic classes. Not blocking -- the existing convention is followed correctly. 2. **`$derived(() => ...)` vs `$derived(expr)`**: The arrow function form in this file means `signedCount`, `filteredPlayers`, and `counts` are all callables (used as `signedCount()` in templates). Most other files in the repo use `$derived(expr)` directly. Consider a future cleanup pass to standardize one pattern across the codebase. Not introduced by this PR. ### SOP COMPLIANCE - [x] Branch named after issue: `137-contract-status-admin-crm` references issue #137 - [x] PR body has Summary, Changes, Test Plan, Related sections - [ ] Related section references plan slug -- no plan slug referenced, just "westside-app -- the project this work belongs to". Acceptable for a standalone story without a parent plan. - [x] No secrets committed (2 files changed: `src/app.css`, `src/routes/(app)/admin/players/+page.svelte`) - [x] No unnecessary file changes -- scope is tight: 27 additions, 1 deletion, exactly the badge feature - [x] Commit messages are descriptive ### PROCESS OBSERVATIONS Small, focused PR. 27 lines added across 2 files. Clean scope -- no backend changes, no new dependencies, no new routes. The PR body correctly notes that `contract_status` is already provided by the API. Change failure risk is very low. Deployment frequency: no friction. ### VERDICT: APPROVED

forgejo_admin merged commit a516973a14 into main 2026-03-28 17:43:15 +00:00

forgejo_admin deleted branch 137-contract-status-admin-crm 2026-03-28 17:43:15 +00:00

forgejo_admin referenced this pull request from a commit 2026-03-28 17:43:17 +00:00

feat: show contract status badge in admin CRM player list (#141)

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

domain:backend

  

domain:devops

  

domain:frontend

No labels

domain:backend

domain:devops

domain:frontend

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

forgejo_admin/westside-landing!141

No description provided.