fix: prorated fee, local schedules, remove local rules of conduct #18

Merged

forgejo_admin merged 1 commit from 17-fix-contract-prorated-fee-schedules into main

2026-03-24 21:02:42 +00:00

forgejo_admin commented

2026-03-24 20:56:49 +00:00

Owner

Summary

Fixes three contract display issues and removes inapplicable content from the local contract: adds prorated first-month fee calculation, restores team-specific local practice schedules, and deletes the Rules of Conduct section from local contracts.

Changes

src/routes/contract/[token]/+page.svelte
- Added proratedFee calculation (Math.round(monthlyFee * 25 / 30 / 5) * 5) after monthlyFee const
- Updated "First monthly fee" label in both travel and local payment schedules to show prorated amount instead of full monthly fee
- Replaced generic "Practice days and locations vary by team" paragraph with team-specific schedules using player.team_name conditional (17U Local, 16U Local, fallback)
- Removed Rules of Conduct <h3> + <ul> block from local contract section (not applicable to local players)

Test Plan

npm run build passes (no new warnings)
npm test passes (12/12 tests)
Verify travel contract still shows Rules of Conduct
Verify local contract no longer shows Rules of Conduct
Verify prorated fee displays correctly for both contract types (e.g., $200 -> $165)
Verify 17U Local, 16U Local, and fallback practice schedules render correctly

Review Checklist

Build passes (npm run build)
Tests pass (npm test -- 12/12)
Only one file changed, scoped to contract page template
Travel contract Rules of Conduct untouched
Prorated fee formula matches issue spec

Closes #17

## Summary Fixes three contract display issues and removes inapplicable content from the local contract: adds prorated first-month fee calculation, restores team-specific local practice schedules, and deletes the Rules of Conduct section from local contracts. ## Changes - `src/routes/contract/[token]/+page.svelte` - Added `proratedFee` calculation (`Math.round(monthlyFee * 25 / 30 / 5) * 5`) after `monthlyFee` const - Updated "First monthly fee" label in both travel and local payment schedules to show prorated amount instead of full monthly fee - Replaced generic "Practice days and locations vary by team" paragraph with team-specific schedules using `player.team_name` conditional (17U Local, 16U Local, fallback) - Removed Rules of Conduct `<h3>` + `<ul>` block from local contract section (not applicable to local players) ## Test Plan - `npm run build` passes (no new warnings) - `npm test` passes (12/12 tests) - Verify travel contract still shows Rules of Conduct - Verify local contract no longer shows Rules of Conduct - Verify prorated fee displays correctly for both contract types (e.g., $200 -> $165) - Verify 17U Local, 16U Local, and fallback practice schedules render correctly ## Review Checklist - [x] Build passes (`npm run build`) - [x] Tests pass (`npm test` -- 12/12) - [x] Only one file changed, scoped to contract page template - [x] Travel contract Rules of Conduct untouched - [x] Prorated fee formula matches issue spec ## Related - Closes #17

forgejo_admin added 1 commit

2026-03-24 20:56:49 +00:00

fix: prorated fee, local schedules, remove local rules of conduct d24dde96db

- Add proratedFee calculation (monthlyFee * 25/30 rounded to nearest $5)
- Update "First monthly fee" label to show prorated amount on both
  travel and local contracts
- Replace generic local practice schedule with team-specific schedules
  (17U Local, 16U Local, fallback)
- Remove Rules of Conduct section from local contract (not applicable)

Closes #17

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

forgejo_admin commented

2026-03-24 20:57:28 +00:00

Author

Owner

QA Review — PR #18

Diff Summary

Single file changed: src/routes/contract/[token]/+page.svelte (+14/-10)

Findings

1. proratedFee calculation (line 9) -- Correct. Math.round(monthlyFee * 25 / 30 / 5) * 5 matches the issue spec. For default $200: Math.round(200 * 25 / 30 / 5) * 5 = Math.round(33.33) * 5 = 33 * 5 = $165.

2. Travel payment schedule label (line 304) -- Correct. First monthly fee — prorated (${proratedFee}) replaces the old (${monthlyFee}) reference.

3. Local payment schedule label (line 380) -- Correct. Same change applied to local contract section.

4. Local practice schedule (lines 354-364) -- Correct. Generic paragraph replaced with conditional {#if}/{:else if}/{:else} block keyed on player.team_name?.includes(). Optional chaining handles null/undefined team_name safely. Three branches: 17U Local, 16U Local, fallback -- all match issue spec.

5. Rules of Conduct removed from local contract (lines 391-397) -- Correct. The <h3>Rules of Conduct</h3> and <ul> block deleted from local section only. Travel contract Rules of Conduct is untouched (verified in diff -- no changes in that region).

Scope Check

No unrelated changes
Build passes, 12/12 tests pass
All four issue requirements addressed

VERDICT: PASS

No issues found. All changes match the issue spec exactly. Ready to merge.

## QA Review — PR #18 ### Diff Summary Single file changed: `src/routes/contract/[token]/+page.svelte` (+14/-10) ### Findings **1. proratedFee calculation (line 9)** -- Correct. `Math.round(monthlyFee * 25 / 30 / 5) * 5` matches the issue spec. For default $200: `Math.round(200 * 25 / 30 / 5) * 5 = Math.round(33.33) * 5 = 33 * 5 = $165`. **2. Travel payment schedule label (line 304)** -- Correct. `First monthly fee — prorated (${proratedFee})` replaces the old `(${monthlyFee})` reference. **3. Local payment schedule label (line 380)** -- Correct. Same change applied to local contract section. **4. Local practice schedule (lines 354-364)** -- Correct. Generic paragraph replaced with conditional `{#if}`/`{:else if}`/`{:else}` block keyed on `player.team_name?.includes()`. Optional chaining handles null/undefined team_name safely. Three branches: 17U Local, 16U Local, fallback -- all match issue spec. **5. Rules of Conduct removed from local contract (lines 391-397)** -- Correct. The `<h3>Rules of Conduct</h3>` and `<ul>` block deleted from local section only. Travel contract Rules of Conduct is untouched (verified in diff -- no changes in that region). ### Scope Check - No unrelated changes - Build passes, 12/12 tests pass - All four issue requirements addressed ### VERDICT: PASS No issues found. All changes match the issue spec exactly. Ready to merge.

forgejo_admin commented

2026-03-24 21:01:45 +00:00

Author

Owner

PR #18 Review

DOMAIN REVIEW

Tech stack: SvelteKit 5 (runes mode), TypeScript, server-side Postgres via pg pool, MinIO for signature storage.

Changes scoped to: src/routes/contract/[token]/+page.svelte (template only -- 14 additions, 10 deletions).

1. Proration formula (line 9)

const proratedFee = Math.round(monthlyFee * 25 / 30 / 5) * 5;

Arithmetic is correct. For the default monthlyFee = 200: Math.round(200 * 25 / 30 / 5) * 5 = Math.round(33.33) * 5 = 165. The round-to-nearest-$5 pattern is clean. The formula is used consistently in both the travel payment schedule (line 304) and the local payment schedule (line 380) via the same proratedFee const -- no DRY violation.

One observation: the 25/30 ratio represents "25 days remaining in a 30-day month" which is a hardcoded business assumption. This is acceptable for a seasonal contract with a fixed April 6 start date, but if the start date ever changes, this formula silently produces wrong results. Not a blocker for a fixed-season contract.

2. Local practice schedule conditionals (lines 358-367)

The player.team_name?.includes('17U Local') and player.team_name?.includes('16U Local') conditionals are sound. The optional chaining is defensive but correct -- within the {:else} (isLocal) branch, team_name is guaranteed non-null by the server load function (isLocal is only true when team_name.includes('Local')), so the ?. is belt-and-suspenders. No harm.

The fallback {:else} branch duplicates the 17U Local schedule (Monday BWill + Tuesday West High). This is a reasonable safe default for any future local teams. All three branches share "Monday BWill, 7-9 PM" -- this is conditional template branching, not a meaningful DRY violation.

3. Rules of Conduct removal from local contract (lines 391-397 removed)

Clean removal. The travel contract retains its own (different, more detailed) Rules of Conduct section at lines 326-333. Verified these are distinct sections -- the travel rules include travel-specific items (group travel, property damage, playing time consequences) while the removed local rules were a subset. No shared template logic was broken.

Accessibility: Schedule rows use <span> elements with descriptive class names (schedule-day, schedule-detail). These are display-only, consistent with existing travel schedule markup pattern (lines 277-288). No new interactive elements requiring ARIA labels.

No security concerns: No new user input handling, no new endpoints, no new data flows. All changes are read-only template rendering.

BLOCKERS

None.

On the test coverage question: this PR modifies only Svelte template markup (display labels, conditional rendering, and removal of a static section). The proration formula is a single-line const using only the existing monthlyFee value. No new endpoints, validation paths, or data flows were introduced. The existing 12 validation tests pass. While the proration formula could be extracted to a utility and unit-tested, requiring component rendering tests for a contract display fix would be disproportionate. No blocker here.

NITS

Hardcoded proration ratio (25/30): The formula bakes in "25 days remaining out of 30" as magic numbers. A comment explaining the business rationale would help future maintainers. Something like // 25 of 30 days remaining in April (start April 6), rounded to nearest $5. Low priority -- the PR body documents it, but code outlives PR descriptions.
Hardcoded schedule data in template: Practice locations ("BWill", "West High Field House") and times are inline in the Svelte template. If schedules change next season, someone must edit the .svelte file. For a seasonal contract app this is pragmatic, but worth noting if the team count grows.
Fallback schedule is implicit 17U clone: The {:else} branch silently uses the 17U Local schedule. A comment like  would make the intent explicit and prevent someone from assuming it is a bug.

SOP COMPLIANCE

Branch named after issue: 17-fix-contract-prorated-fee-schedules
PR body has Summary, Changes, Test Plan, Related sections
Related references parent issue: Closes #17
No secrets committed
No unnecessary file changes -- single file, scoped to contract display
Commit message is descriptive (matches PR title)
Related does not reference a plan slug (no plan slug present -- acceptable if this is a standalone board item, not plan-driven work)

PROCESS OBSERVATIONS

Change failure risk: Low. Template-only change to a contract display page. No backend logic, no database changes, no new API surface. The proration formula is deterministic arithmetic.
Deployment frequency: Clean single-file PR ships fast. Good scope discipline.
Test plan quality: The PR body lists manual verification steps (travel vs local rendering, proration math, schedule conditionals). For a display fix, this is adequate. The build and existing test suite provide regression safety.

VERDICT: APPROVED

## PR #18 Review ### DOMAIN REVIEW **Tech stack**: SvelteKit 5 (runes mode), TypeScript, server-side Postgres via `pg` pool, MinIO for signature storage. **Changes scoped to**: `src/routes/contract/[token]/+page.svelte` (template only -- 14 additions, 10 deletions). **1. Proration formula** (line 9) ``` const proratedFee = Math.round(monthlyFee * 25 / 30 / 5) * 5; ``` Arithmetic is correct. For the default `monthlyFee = 200`: `Math.round(200 * 25 / 30 / 5) * 5 = Math.round(33.33) * 5 = 165`. The round-to-nearest-$5 pattern is clean. The formula is used consistently in both the travel payment schedule (line 304) and the local payment schedule (line 380) via the same `proratedFee` const -- no DRY violation. One observation: the 25/30 ratio represents "25 days remaining in a 30-day month" which is a hardcoded business assumption. This is acceptable for a seasonal contract with a fixed April 6 start date, but if the start date ever changes, this formula silently produces wrong results. Not a blocker for a fixed-season contract. **2. Local practice schedule conditionals** (lines 358-367) The `player.team_name?.includes('17U Local')` and `player.team_name?.includes('16U Local')` conditionals are sound. The optional chaining is defensive but correct -- within the `{:else}` (isLocal) branch, `team_name` is guaranteed non-null by the server load function (`isLocal` is only true when `team_name.includes('Local')`), so the `?.` is belt-and-suspenders. No harm. The fallback `{:else}` branch duplicates the 17U Local schedule (Monday BWill + Tuesday West High). This is a reasonable safe default for any future local teams. All three branches share "Monday BWill, 7-9 PM" -- this is conditional template branching, not a meaningful DRY violation. **3. Rules of Conduct removal from local contract** (lines 391-397 removed) Clean removal. The travel contract retains its own (different, more detailed) Rules of Conduct section at lines 326-333. Verified these are distinct sections -- the travel rules include travel-specific items (group travel, property damage, playing time consequences) while the removed local rules were a subset. No shared template logic was broken. **Accessibility**: Schedule rows use `<span>` elements with descriptive class names (`schedule-day`, `schedule-detail`). These are display-only, consistent with existing travel schedule markup pattern (lines 277-288). No new interactive elements requiring ARIA labels. **No security concerns**: No new user input handling, no new endpoints, no new data flows. All changes are read-only template rendering. ### BLOCKERS None. On the test coverage question: this PR modifies only Svelte template markup (display labels, conditional rendering, and removal of a static section). The proration formula is a single-line const using only the existing `monthlyFee` value. No new endpoints, validation paths, or data flows were introduced. The existing 12 validation tests pass. While the proration formula *could* be extracted to a utility and unit-tested, requiring component rendering tests for a contract display fix would be disproportionate. No blocker here. ### NITS 1. **Hardcoded proration ratio (25/30)**: The formula bakes in "25 days remaining out of 30" as magic numbers. A comment explaining the business rationale would help future maintainers. Something like `// 25 of 30 days remaining in April (start April 6), rounded to nearest $5`. Low priority -- the PR body documents it, but code outlives PR descriptions. 2. **Hardcoded schedule data in template**: Practice locations ("BWill", "West High Field House") and times are inline in the Svelte template. If schedules change next season, someone must edit the `.svelte` file. For a seasonal contract app this is pragmatic, but worth noting if the team count grows. 3. **Fallback schedule is implicit 17U clone**: The `{:else}` branch silently uses the 17U Local schedule. A comment like `` would make the intent explicit and prevent someone from assuming it is a bug. ### SOP COMPLIANCE - [x] Branch named after issue: `17-fix-contract-prorated-fee-schedules` - [x] PR body has Summary, Changes, Test Plan, Related sections - [x] Related references parent issue: `Closes #17` - [x] No secrets committed - [x] No unnecessary file changes -- single file, scoped to contract display - [x] Commit message is descriptive (matches PR title) - [ ] Related does not reference a plan slug (no plan slug present -- acceptable if this is a standalone board item, not plan-driven work) ### PROCESS OBSERVATIONS - **Change failure risk**: Low. Template-only change to a contract display page. No backend logic, no database changes, no new API surface. The proration formula is deterministic arithmetic. - **Deployment frequency**: Clean single-file PR ships fast. Good scope discipline. - **Test plan quality**: The PR body lists manual verification steps (travel vs local rendering, proration math, schedule conditionals). For a display fix, this is adequate. The build and existing test suite provide regression safety. ### VERDICT: APPROVED