Research reports on datetime handling, RFC 9457, font selection. Implementation plans for US-1 create event and post-review fixes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
200 lines
8.4 KiB
Markdown
200 lines
8.4 KiB
Markdown
# US-1 Post-Review Fixes — Implementation Plan
|
|
|
|
Date: 2026-03-05
|
|
Origin: Deep review of all unstaged US-1 changes before commit
|
|
|
|
## Context
|
|
|
|
US-1 "Create Event" is fully implemented (backend + frontend, 7 phases) with 4 review fixes already applied (reactive error clearing, network error handling, page title, favicon). A comprehensive review of ALL unstaged files revealed additional issues that must be fixed before committing.
|
|
|
|
## Task 1: Backend — Clock injection in EventService [x]
|
|
|
|
**Problem:** `EventService` uses `LocalDate.now()` and `OffsetDateTime.now()` directly, making deterministic time-based testing impossible.
|
|
|
|
**Files:**
|
|
- `backend/src/main/java/de/fete/application/service/EventService.java`
|
|
- `backend/src/test/java/de/fete/application/service/EventServiceTest.java`
|
|
|
|
**Fix:**
|
|
1. Inject a `java.time.Clock` bean into `EventService` via constructor
|
|
2. Replace `LocalDate.now()` with `LocalDate.now(clock)` and `OffsetDateTime.now()` with `OffsetDateTime.now(clock)`
|
|
3. Add a `Clock` bean to the Spring config (or rely on a `@Bean Clock clock() { return Clock.systemDefaultZone(); }` in a config class)
|
|
4. Update `EventServiceTest` to use `Clock.fixed(...)` for deterministic tests
|
|
|
|
**Verification:** `cd backend && ./mvnw test`
|
|
|
|
## Task 2: Frontend A11y — Error spans should only render when error present [x]
|
|
|
|
**Problem:** Every form field has `<span class="field-error" role="alert">{{ errors.title }}</span>` that is always in the DOM, even when empty. Screen readers may announce empty `role="alert"` elements.
|
|
|
|
**File:** `frontend/src/views/EventCreateView.vue`
|
|
|
|
**Fix:** Use `v-if` to conditionally render error spans:
|
|
```html
|
|
<span v-if="errors.title" class="field-error" role="alert">{{ errors.title }}</span>
|
|
```
|
|
|
|
Apply to all 5 field error spans (title, description, dateTime, location, expiryDate).
|
|
|
|
**Note:** This removes the `min-height: 1.2em` layout reservation. Accept the layout shift as a trade-off for accessibility, OR add a wrapper div with `min-height` that doesn't carry `role="alert"`.
|
|
|
|
**Verification:** `cd frontend && npm run test:unit` — existing tests use `.querySelector('[role="alert"]')` so they may need adjustment since empty alerts will no longer be in the DOM.
|
|
|
|
## Task 3: Frontend A11y — aria-invalid and aria-describedby on fields [x]
|
|
|
|
**Problem:** When a field fails validation, there is no `aria-invalid="true"` or `aria-describedby` linking the input to its error message. Assistive technologies cannot associate errors with fields.
|
|
|
|
**File:** `frontend/src/views/EventCreateView.vue`
|
|
|
|
**Fix:**
|
|
1. Add unique `id` to each error span (e.g., `id="title-error"`)
|
|
2. Add `:aria-describedby="errors.title ? 'title-error' : undefined"` to each input
|
|
3. Add `:aria-invalid="!!errors.title"` to each input
|
|
|
|
Example for title:
|
|
```html
|
|
<input
|
|
id="title"
|
|
v-model="form.title"
|
|
type="text"
|
|
class="form-field"
|
|
required
|
|
maxlength="200"
|
|
placeholder="What's the event?"
|
|
:aria-invalid="!!errors.title"
|
|
:aria-describedby="errors.title ? 'title-error' : undefined"
|
|
/>
|
|
<span v-if="errors.title" id="title-error" class="field-error" role="alert">{{ errors.title }}</span>
|
|
```
|
|
|
|
Apply the same pattern to all 5 fields (title, description, dateTime, location, expiryDate).
|
|
|
|
**Verification:** `cd frontend && npm run test:unit`
|
|
|
|
## Task 4: Frontend A11y — Error text contrast [x]
|
|
|
|
**Problem:** White (`#fff`) error text on the pink gradient start (`#F06292`) has a contrast ratio of only 3.06:1, which fails WCAG AA for small text (0.8rem). The project statute requires WCAG AA compliance.
|
|
|
|
**File:** `frontend/src/assets/main.css`
|
|
|
|
**Fix options (pick one):**
|
|
- **Option A:** Use a light yellow/cream color like `#FFF9C4` or `#FFECB3` that has higher contrast on the gradient
|
|
- **Option B:** Add a subtle dark text-shadow to the error text: `text-shadow: 0 1px 2px rgba(0,0,0,0.3)`
|
|
- **Option C:** Make error text slightly larger/bolder to qualify for WCAG AA-large (18px+ or 14px+ bold)
|
|
|
|
**Recommended:** Option C — bump `.field-error` to `font-size: 0.85rem; font-weight: 600;` which at 600 weight qualifies for AA-large text at 14px+ (0.85rem ≈ 13.6px — close but may not quite qualify). Alternatively combine with option B for safety.
|
|
|
|
**Note:** Verify the final choice against the design system spec in `spec/design-system.md`. The spec notes that gradient start only passes AA-large. The error text must work across the full gradient.
|
|
|
|
**Verification:** Manual contrast check with a tool like WebAIM contrast checker.
|
|
|
|
## Task 5: Test — Happy-path submission in EventCreateView [x]
|
|
|
|
**Problem:** No test verifies successful form submission (the most important behavior).
|
|
|
|
**File:** `frontend/src/views/__tests__/EventCreateView.spec.ts`
|
|
|
|
**Fix:** Add a test that:
|
|
1. Mocks `api.POST` to return `{ data: { eventToken: 'abc', organizerToken: 'xyz', title: 'Test', dateTime: '...', expiryDate: '...' } }`
|
|
2. Fills all required fields
|
|
3. Submits the form
|
|
4. Asserts `api.POST` was called with the correct body
|
|
5. Asserts navigation to `/events/abc` occurred
|
|
6. Asserts `saveCreatedEvent` was called (need to mock `useEventStorage`)
|
|
|
|
**Note:** `useEventStorage` must be mocked. Use `vi.mock('@/composables/useEventStorage')`.
|
|
|
|
**Verification:** `cd frontend && npm run test:unit`
|
|
|
|
## Task 6: Test — EventStubView component tests [x]
|
|
|
|
**Problem:** No test file exists for `EventStubView.vue`.
|
|
|
|
**New file:** `frontend/src/views/__tests__/EventStubView.spec.ts`
|
|
|
|
**Fix:** Create tests covering:
|
|
1. Renders the event URL based on route param `:token`
|
|
2. Shows the correct share URL (`window.location.origin + /events/:token`)
|
|
3. Copy button exists
|
|
4. Back link navigates to home
|
|
|
|
**Note:** Read `frontend/src/views/EventStubView.vue` first to understand the component structure.
|
|
|
|
**Verification:** `cd frontend && npm run test:unit`
|
|
|
|
## Task 7: Test — Server-side field errors in EventCreateView [x]
|
|
|
|
**Problem:** The `fieldErrors` handling branch (lines 184-196 of EventCreateView.vue) is untested.
|
|
|
|
**File:** `frontend/src/views/__tests__/EventCreateView.spec.ts`
|
|
|
|
**Fix:** Add a test that:
|
|
1. Mocks `api.POST` to return `{ error: { fieldErrors: [{ field: 'title', message: 'Title already taken' }] } }`
|
|
2. Fills all required fields and submits
|
|
3. Asserts the title field error shows "Title already taken"
|
|
4. Asserts other field errors are empty
|
|
|
|
**Verification:** `cd frontend && npm run test:unit`
|
|
|
|
## Task 8: Fix border-radius on EventStubView copy button [x]
|
|
|
|
**Problem:** `border-radius: 10px` is hardcoded instead of using the design token `var(--radius-button)` (14px).
|
|
|
|
**File:** `frontend/src/views/EventStubView.vue`
|
|
|
|
**Fix:** Replace `border-radius: 10px` with `border-radius: var(--radius-button)` in the `.stub__copy` CSS class.
|
|
|
|
**Verification:** Visual check.
|
|
|
|
## Task 9: Add 404 catch-all route user story [x]
|
|
|
|
**Problem:** Navigating to an unknown path shows a blank page.
|
|
|
|
**File:** `spec/userstories.md`
|
|
|
|
**Fix:** Add a new user story for a 404/catch-all route. Something like:
|
|
|
|
```
|
|
### US-X: 404 Page
|
|
|
|
As a user who navigates to a non-existent URL, I want to see a helpful error page so I can find my way back.
|
|
|
|
Acceptance Criteria:
|
|
- [ ] Unknown routes show a "Page not found" message
|
|
- [ ] The page includes a link back to the home page
|
|
- [ ] The page follows the design system
|
|
```
|
|
|
|
Read the existing user stories first to match the format.
|
|
|
|
**Verification:** N/A (spec only).
|
|
|
|
## Task 10: EventStubView silent clipboard failure [x]
|
|
|
|
**Problem:** In `EventStubView.vue`, the `catch` block on `navigator.clipboard.writeText()` is empty. If clipboard is unavailable (HTTP, older browser), the user gets no feedback.
|
|
|
|
**File:** `frontend/src/views/EventStubView.vue`
|
|
|
|
**Fix:** In the catch block, show a fallback message (e.g., set `copied` text to "Copy failed" or select the URL text for manual copying).
|
|
|
|
**Verification:** `cd frontend && npm run test:unit`
|
|
|
|
## Execution Order
|
|
|
|
1. Task 1 (Clock injection — backend, independent)
|
|
2. Tasks 2 + 3 (A11y fixes — can be done together since they touch the same file)
|
|
3. Task 4 (Contrast fix — CSS only)
|
|
4. Tasks 5 + 7 (EventCreateView tests — same test file)
|
|
5. Task 6 (EventStubView tests — new file)
|
|
6. Tasks 8 + 10 (EventStubView fixes — same file)
|
|
7. Task 9 (User story — spec only)
|
|
8. Run all tests: `cd backend && ./mvnw test` and `cd frontend && npm run test:unit`
|
|
|
|
## Constraints
|
|
|
|
- TDD: write/update tests first, then fix (where applicable)
|
|
- Follow existing code style and patterns
|
|
- Do not refactor unrelated code
|
|
- Do not add dependencies
|
|
- Update design system spec if contrast solution changes the spec
|