Skip to content

Commit c1b4620

Browse files
authored
Merge pull request #3319 from AtCoder-NoviSteps/feature/atcoder-verified-voting
Feature/atcoder verified voting
2 parents 8757704 + caedfa9 commit c1b4620

20 files changed

Lines changed: 526 additions & 301 deletions

File tree

.claude/rules/svelte-components.md

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,3 +154,46 @@ const map: Partial<Record<WorkBookType, Component<Props>>> = {
154154
};
155155
// Safe to guard with: {#if map[type]} or if (map[type])
156156
```
157+
158+
## Props — Pass Domain Model Objects; Derive Computed Values Internally
159+
160+
When a component's data comes from a domain model, pass the model object as a single prop
161+
rather than individual fields. This keeps the call site in sync with the model automatically.
162+
163+
Computed values (status, labels, flags derived from multiple fields) must NOT be props —
164+
they belong inside the component as `$derived`.
165+
166+
```typescript
167+
// Bad: individual fields + derived value as prop
168+
interface Props {
169+
handle: string;
170+
validationCode: string;
171+
isValidated: boolean;
172+
status: string; // derived — should not be a prop
173+
}
174+
175+
// Good: model object as prop; status derived inside
176+
// (username is from User model; atCoderAccount is from a separate domain model — two props is correct here)
177+
interface Props {
178+
username: string;
179+
atCoderAccount: { handle: string; validationCode: string; isValidated: boolean };
180+
}
181+
let { username, atCoderAccount }: Props = $props();
182+
const status = $derived(atCoderAccount.isValidated ? 'validated' : ...);
183+
```
184+
185+
Call site passes the object directly from `$derived(data.atCoderAccount)`.
186+
187+
## $derived for data.\* Fields in +page.svelte
188+
189+
When reading fields from `data` in a `+page.svelte`, use `$derived` rather than plain assignment:
190+
191+
```typescript
192+
// Bad: stale after load() re-runs following a form action
193+
const atCoderAccount = data.atCoderAccount;
194+
195+
// Good: stays in sync when SvelteKit re-runs load() after an action
196+
const atCoderAccount = $derived(data.atCoderAccount);
197+
```
198+
199+
`data` is a reactive prop that SvelteKit updates after each form action. A plain assignment captures the initial value only.

.claude/rules/sveltekit.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,3 +76,25 @@ The same pattern applies to `url.searchParams.get()` in `+server.ts` handlers.
7676
## Page Component Props
7777

7878
SvelteKit page components (`+page.svelte`) accept only `data` and `form` as props (`svelte/valid-prop-names-in-kit-pages`). Commented-out features that reference other props are not "dead code" — remove only the violating prop declaration, preserve the feature code.
79+
80+
## load() — Group Related Model Fields as Objects
81+
82+
When a `load()` function returns fields from the same domain model (e.g., `AtCoderAccount`),
83+
group them as an object rather than flattening to top-level keys.
84+
Apply default values at this boundary so the page component typically does not need to handle `undefined`.
85+
86+
```typescript
87+
// Bad: flat, scattered across top-level keys
88+
atcoder_username: user?.atCoderAccount?.handle ?? '',
89+
atcoder_validationcode: user?.atCoderAccount?.validationCode ?? '',
90+
is_validated: user?.atCoderAccount?.isValidated ?? false,
91+
92+
// Good: grouped by model, defaults absorbed here
93+
atCoderAccount: {
94+
handle: user?.atCoderAccount?.handle ?? '',
95+
validationCode: user?.atCoderAccount?.validationCode ?? '',
96+
isValidated: user?.atCoderAccount?.isValidated ?? false,
97+
},
98+
```
99+
100+
When consuming in `+page.svelte`, use `$derived` to maintain reactivity across load() re-runs after form actions (see svelte-components.md).

.claude/rules/testing-e2e.md

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
---
2+
description: E2E testing rules and patterns (Playwright)
3+
paths:
4+
- '**/*.spec.ts'
5+
- 'e2e/**'
6+
---
7+
8+
# E2E Tests (Playwright)
9+
10+
## No Path Aliases
11+
12+
The `e2e/` directory is outside SvelteKit's build pipeline — `$lib`, `$features`, and other path aliases are not resolved. Define string constant values as local constants with a reference comment:
13+
14+
```typescript
15+
// Mirrors WorkBookTab.SOLUTION from $features/workbooks/types/workbook
16+
const TAB_SOLUTION = 'solution';
17+
```
18+
19+
Avoid importing values from `src/` in E2E test files. Type-only imports (`import type`) are acceptable since they are erased at compile time:
20+
21+
```typescript
22+
// Bad: runtime import — path alias not resolved in e2e/
23+
import { TAB_SOLUTION } from '$features/workbooks/types/workbook';
24+
25+
// Good: type-only import — compile-time only
26+
import type { WorkBookTab } from '$features/workbooks/types/workbook';
27+
const TAB_SOLUTION: WorkBookTab = 'solution';
28+
```
29+
30+
## Describe Hierarchy
31+
32+
When a `describe` block for a user role grows large, split it by behavioral dimension rather than adding more flat `test()` calls:
33+
34+
```typescript
35+
test.describe('logged-in user', () => {
36+
test.describe('tab visibility', () => { ... });
37+
test.describe('URL parameter handling', () => { ... });
38+
test.describe('navigation interactions', () => { ... });
39+
test.describe('session state', () => { ... });
40+
});
41+
```
42+
43+
## Parameterized Tests
44+
45+
Playwright has no native `test.each`. Use `for...of` loops — the official recommended pattern.
46+
47+
**Single test with a loop** — use when testing a sequence or workflow within one test:
48+
49+
```typescript
50+
// Mirrors TaskGrade from $lib/types/task — do not import from src/ in E2E files
51+
const GRADES = ['Q10', 'Q9', 'Q8'] as const;
52+
53+
for (const grade of GRADES) {
54+
await gradeButton(page, grade).click();
55+
await expect(page).toHaveURL(`?grades=${grade}`);
56+
}
57+
```
58+
59+
**Multiple tests from parameters** — use when each parameter represents an independent case:
60+
61+
```typescript
62+
const GRADES = ['Q10', 'Q9', 'Q8'] as const;
63+
64+
for (const grade of GRADES) {
65+
test(`filters by grade ${grade}`, async ({ page }) => {
66+
await page.goto('/tasks');
67+
await gradeButton(page, grade).click();
68+
await expect(page).toHaveURL(`?grades=${grade}`);
69+
});
70+
}
71+
```
72+
73+
## Assertions
74+
75+
After an interaction that changes element state (active tab, toggle, selection), assert the _new_ state — not just that the element is visible, which may have been true before the interaction. Assert an active CSS class, `aria-selected`, or similar attribute instead of `toBeVisible()`.
76+
77+
## Flowbite Toggle
78+
79+
Flowbite's `Toggle` renders an `sr-only` `<input type="checkbox">` inside a `<label>`. Clicking the input directly fails because the visual `<span>` sibling intercepts pointer events. Click the label wrapper instead:
80+
81+
```typescript
82+
const toggleInput = page.locator('input[aria-label="<aria-label value>"]');
83+
const toggleLabel = page.locator('label:has(input[aria-label="<aria-label value>"])');
84+
85+
await toggleLabel.click();
86+
await expect(toggleInput).toBeChecked({ checked: true });
87+
```
88+
89+
The same pattern applies to any Flowbite component that visually overlays its native input (e.g. `Checkbox`, `Radio`).
90+
91+
## Strict Mode: Scope Locators to the Content Area
92+
93+
When the navbar and page body both contain a link or button with the same text (e.g., a breadcrumb and a nav link share the same label), `getByRole` in strict mode will find multiple matches and throw. Scope the locator to the page's content container:
94+
95+
```typescript
96+
// Bad: matches navbar link AND breadcrumb link
97+
await page.getByRole('link', { name: 'グレード投票' }).click();
98+
99+
// Good: scoped to page content only
100+
await page.locator('.container').locator('nav').getByRole('link', { name: 'グレード投票' }).click();
101+
await page.locator('.container').getByRole('link', { name: 'ログイン' }).click();
102+
```
103+
104+
Use `.container` (page content wrapper) to exclude the global navbar. Prefer the narrowest scope that remains stable — breadcrumb `nav` inside `.container` is more precise than `.container` alone when the link only appears there.
105+
106+
## Conditional Skip Based on Runtime State
107+
108+
When a test depends on DB or session state that may vary across environments (e.g., a user's AtCoder verification status), use `test.skip(condition, reason)` inside the test body instead of a static `test.skip`. This way the test runs automatically when the precondition is met:
109+
110+
```typescript
111+
test('sees vote grade buttons', async ({ page }) => {
112+
await page.goto(url);
113+
114+
const isUnverified = await page.getByText('AtCoderアカウントの認証が必要です').isVisible();
115+
test.skip(isUnverified, 'test user is not AtCoder-verified');
116+
117+
// assertions below run only when precondition holds
118+
await expect(page.locator('form[action="?/voteAbsoluteGrade"]')).toBeVisible();
119+
});
120+
```
121+
122+
Prefer this over a hard-coded `test.skip` whenever the condition is observable on the page.

.claude/rules/testing.md

Lines changed: 40 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -57,20 +57,6 @@ E2E test files must use the `.spec.ts` extension. `playwright.config.ts` matches
5757
- Use `toBe(true)` / `toBe(false)` over `toBeTruthy()` / `toBeFalsy()`
5858
- For DB query tests, assert `orderBy`, `include`, and other significant parameters with `expect.objectContaining` — not just `where`. When a returned field (e.g. `authorName`) depends on an `include` relation, that `include` clause must be part of the assertion, or a regression in the query shape will go undetected
5959
- Enum membership: `in` traverses the prototype chain; use `Object.hasOwn(Enum, value)` instead
60-
- **E2E state transitions**: after an interaction that changes element state (active tab, toggle, selection), assert the _new_ state — not just that the element is visible, which may have been true before the interaction. Assert an active CSS class, `aria-selected`, or similar attribute instead of `toBeVisible()`
61-
62-
## Cleanup in Tests
63-
64-
Wrap DB-mutating cleanup in `try/finally` — a failing assertion skips cleanup and contaminates later tests:
65-
66-
```typescript
67-
try {
68-
await doSomething();
69-
expect(result).toBe(expected);
70-
} finally {
71-
await restoreState();
72-
}
73-
```
7460

7561
## Test Data
7662

@@ -79,9 +65,22 @@ try {
7965
- After `.filter()` on fixtures, verify actual contents — same ID may refer to a different entity after fixture updates
8066
- **Description ↔ code path alignment**: when a test name describes a specific scenario (e.g. "tie-break"), verify the fixture actually exercises that code path. A test that passes without reaching the branch it claims to cover gives false confidence
8167

82-
## Mock Helpers
68+
## Coverage
69+
70+
- Run `pnpm coverage` for coverage report
71+
- Target: 80% lines, 80% branches
72+
73+
## Test Order Mirrors Source Order
74+
75+
Order `describe` blocks in service and utils test files to match the declaration order of functions in the source file. Misalignment makes it harder to cross-reference tests and implementation.
76+
77+
## Service Layer Unit Tests
8378

84-
Extract repeated mock patterns into helpers in the test file. For Prisma service tests, define the return type alias once and use it across all helpers:
79+
Service tests mock Prisma via `vi.mock('$lib/server/database', ...)` — no real DB mutations occur.
80+
81+
### Mock Helpers
82+
83+
Extract repeated mock patterns into helpers in the test file. Define the return type alias once and use it across all helpers:
8584

8685
```typescript
8786
type PrismaWorkBook = Awaited<ReturnType<typeof prisma.workBook.findUnique>>;
@@ -104,27 +103,24 @@ function mockCount(value: number) {
104103

105104
Extract `mockFindUnique`, `mockFindMany`, and `mockCount` as the standard trio for service tests that touch a single Prisma model. Add `mockCreate`, `mockTransaction`, and `mockDelete` when those operations are also tested.
106105

107-
## Component Vitest Unit Tests
106+
### Cleanup for Integration Tests and Tests with Real Side Effects
108107

109-
Omit Vitest unit tests for a Svelte component when **both** conditions hold:
108+
This does not apply to standard service layer unit tests that use Prisma mocks.
110109

111-
1. The component is template-only (no logic beyond prop bindings and basic conditionals)
112-
2. The component is covered by E2E tests
113-
114-
When a component contains extracted logic (e.g. derived values, event handlers, utility calls), add unit tests for that logic in the nearest `utils/` file instead of testing the component directly.
115-
116-
## Testing Extracted Utilities
117-
118-
- Add tests at extraction time, not later
119-
- For URL manipulation: assert the original URL is not mutated
120-
- For multi-column operations (e.g., DnD): assert both source and destination columns
110+
If a test performs real DB mutations, file system changes, external API calls, or other stateful side effects that persist beyond the test (e.g., integration tests, seed scripts), wrap assertions in `try/finally` — a failing assertion skips cleanup and contaminates later tests:
121111

122-
## Coverage
112+
```typescript
113+
try {
114+
await doSomething();
115+
expect(result).toBe(expected);
116+
} finally {
117+
await restoreState();
118+
}
119+
```
123120

124-
- Run `pnpm coverage` for coverage report
125-
- Target: 80% lines, 70% branches
121+
This is not needed for standard service unit tests that use Prisma mocks.
126122

127-
## Service Layer Split for Testability
123+
### File Split for Testability
128124

129125
When a service file mixes DB operations and pure functions, split it into two files:
130126

@@ -133,64 +129,23 @@ When a service file mixes DB operations and pure functions, split it into two fi
133129

134130
Stop the split if internal helpers (e.g. `fetchUnplacedWorkbooks`) would be fragmented across files — cohesion matters more than the split itself.
135131

136-
## HTTP Mocking
137-
138-
Use Nock for external HTTP calls. See `src/test/lib/clients/` for examples.
139-
140-
## Test Order Mirrors Source Order
141-
142-
Order `describe` blocks in service and utils test files to match the declaration order of functions in the source file. Misalignment makes it harder to cross-reference tests and implementation.
143-
144-
## E2E Tests
145-
146-
### No Path Aliases
147-
148-
The `e2e/` directory is outside SvelteKit's build pipeline — `$lib`, `$features`, and other path aliases are not resolved. Define URL string values as local constants with a reference comment:
149-
150-
```typescript
151-
// Mirrors WorkBookTab.SOLUTION from $features/workbooks/types/workbook
152-
const TAB_SOLUTION = 'solution';
153-
```
154-
155-
Avoid importing types from `src/` in E2E test files.
156-
157-
### Describe Hierarchy
158-
159-
When a `describe` block for a user role grows large, split it by behavioral dimension rather than adding more flat `test()` calls:
160-
161-
```typescript
162-
test.describe('logged-in user', () => {
163-
test.describe('tab visibility', () => { ... });
164-
test.describe('URL parameter handling', () => { ... });
165-
test.describe('navigation interactions', () => { ... });
166-
test.describe('session state', () => { ... });
167-
});
168-
```
169-
170-
### Parameterized Tests
132+
## Component Vitest Unit Tests
171133

172-
Playwright has no native `test.each`. Use `for...of` loops — the official recommended pattern:
134+
E2E tests are complementary to, not a substitute for, unit tests. Add Vitest unit tests for any component logic (derived values, event handlers, utility calls) by extracting it to the nearest `utils/` file and testing there.
173135

174-
```typescript
175-
// Mirrors TaskGrade from $lib/types/task — do not import from src/ in E2E files
176-
const GRADES = ['Q10', 'Q9', 'Q8'] as const;
136+
You may omit a component-level Vitest test when **both** conditions hold:
177137

178-
for (const grade of GRADES) {
179-
await gradeButton(grade).click();
180-
await expect(page).toHaveURL(`?grades=${grade}`);
181-
}
182-
```
138+
1. The component is template-only (no logic beyond prop bindings and simple `{#if}`/`{#each}` blocks that only render — no inline function calls, ternaries with side effects, derived computations, or nested logic)
139+
2. The component's rendering paths are covered by E2E tests
183140

184-
### Flowbite Toggle
141+
When a component contains extracted logic (e.g. derived values, event handlers, utility calls), add unit tests for that logic in the nearest `utils/` file instead of testing the component directly.
185142

186-
Flowbite's `Toggle` renders an `sr-only` `<input type="checkbox">` inside a `<label>`. Clicking the input directly fails because the visual `<span>` sibling intercepts pointer events. Click the label wrapper instead:
143+
## Testing Extracted Utilities
187144

188-
```typescript
189-
const toggleInput = page.locator('input[aria-label="<aria-label value>"]');
190-
const toggleLabel = page.locator('label:has(input[aria-label="<aria-label value>"])');
145+
- Add tests at extraction time, not later
146+
- For URL manipulation: assert the original URL is not mutated
147+
- For multi-column operations (e.g., DnD): assert both source and destination columns
191148

192-
await toggleLabel.click();
193-
await expect(toggleInput).toBeChecked({ checked: true });
194-
```
149+
## HTTP Mocking
195150

196-
The same pattern applies to any Flowbite component that visually overlays its native input (e.g. `Checkbox`, `Radio`).
151+
Use Nock for external HTTP calls. See `src/test/lib/clients/` for examples.

.env.example

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
# AtCoder affiliation confirmation API endpoint
2-
# Set this to the actual endpoint URL in your .env file (do not commit real URLs here)
1+
# AtCoder affiliation confirmation API endpoint (NoviSteps organization crawler)
2+
# See team documentation for the actual URL.
33
CONFIRM_API_URL=https://your-confirm-api-endpoint.example.com/confirm

e2e/custom_colors.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ test.describe('Custom colors for TailwindCSS v4 configuration', () => {
7272
.join('\n');
7373

7474
// Verify xs breakpoint media query is generated (26.25rem = 420px)
75-
// @media(min-width:26.25rem) confirms the xs breakpoint is defined correctly
76-
expect(allCss).toMatch(/@media\(min-width:26\.25rem\)/);
75+
// TailwindCSS v4 outputs range syntax: @media (width>=26.25rem) or @media (width >= 26.25rem)
76+
expect(allCss).toMatch(/@media\s*\(width\s*>=\s*26\.25rem\)/);
7777
});
7878
});

0 commit comments

Comments
 (0)