docs(legibility): write CONTRIBUTING.md with three concrete walkthroughs
Covers environment setup, daily workflow, three walkthroughs (add domain, add endpoint, add frontend page), and a conventions reference. All file paths verified against current main. Walkthroughs follow TDD order (Red before Green). Resolves all persona feedback from issue #398. Closes #398 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Marcel
304
CONTRIBUTING.md
Normal file
304
CONTRIBUTING.md
Normal file
@@ -0,0 +1,304 @@
|
||||
# Contributing to Familienarchiv
|
||||
|
||||
For the full collaboration rules (issue workflow, PR process, Red/Green TDD, commit conventions) see [COLLABORATING.md](./COLLABORATING.md).
|
||||
For coding style see [CODESTYLE.md](./CODESTYLE.md).
|
||||
For the system architecture see [docs/ARCHITECTURE.md](./docs/ARCHITECTURE.md).
|
||||
For domain terminology see [docs/GLOSSARY.md](./docs/GLOSSARY.md).
|
||||
|
||||
---
|
||||
|
||||
## 1. Environment setup
|
||||
|
||||
**Prerequisites:** Java 21 (SDKMAN), Node 24 (nvm), Docker
|
||||
|
||||
**Activate SDKMAN and nvm before running `java`, `mvn`, `node`, or `npm`:**
|
||||
|
||||
```bash
|
||||
source "$HOME/.sdkman/bin/sdkman-init.sh"
|
||||
export NVM_DIR="$HOME/.nvm" && [ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 2. Daily development workflow
|
||||
|
||||
**Startup order — services must start in this sequence:**
|
||||
|
||||
```bash
|
||||
# 1. Start PostgreSQL and MinIO
|
||||
docker compose up -d db minio
|
||||
|
||||
# 2. Start the backend (separate terminal)
|
||||
cd backend && ./mvnw spring-boot:run
|
||||
|
||||
# 3. Start the frontend (separate terminal)
|
||||
cd frontend && npm install && npm run dev
|
||||
```
|
||||
|
||||
> `npm install` also wires up the Husky pre-commit hook via the `prepare` script.
|
||||
> Run it before your first commit, or the hook will fail to execute.
|
||||
|
||||
> **Do not use `docker-compose.ci.yml` locally** — it disables the bind mounts that the dev workflow depends on.
|
||||
|
||||
**Regenerate TypeScript types after any backend API change:**
|
||||
|
||||
```bash
|
||||
# Backend must be running with dev profile
|
||||
cd frontend && npm run generate:api
|
||||
```
|
||||
|
||||
> ⚠️ Forgetting this step is the most common cause of "where did my TypeScript type go?" — always regenerate after changing models or endpoints.
|
||||
|
||||
**Test commands:**
|
||||
|
||||
```bash
|
||||
cd backend && ./mvnw test # backend unit + slice tests
|
||||
cd frontend && npm run test # Vitest unit tests
|
||||
cd frontend && npm run check # svelte-check (type errors)
|
||||
cd frontend && npx playwright test # Playwright e2e tests
|
||||
```
|
||||
|
||||
**Branch naming:** `<type>/<issue-number>-<short-description>`, e.g. `feat/398-contributing`
|
||||
|
||||
**Commits:** one logical change per commit; reference the Gitea issue:
|
||||
|
||||
```
|
||||
feat(person): add aliases endpoint
|
||||
|
||||
Closes #42
|
||||
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
||||
```
|
||||
|
||||
### Test-type decision matrix
|
||||
|
||||
| What you're testing | Test type | Tool |
|
||||
|---|---|---|
|
||||
| Service business logic, calculations | Unit test | JUnit + `@ExtendWith(MockitoExtension.class)` |
|
||||
| HTTP contract, request validation, error codes | Controller slice test | `@WebMvcTest` |
|
||||
| Server `load` function | Vitest unit | Import directly, mock `fetch` |
|
||||
| Shared UI component | Vitest browser-mode | `render()` + `getByRole()` |
|
||||
| Full user-facing flow, navigation, forms | E2E | Playwright |
|
||||
|
||||
---
|
||||
|
||||
## 3. Walkthrough A — Add a new domain
|
||||
|
||||
**Example:** adding a `citation` domain (formal references to documents).
|
||||
|
||||
Both the backend and frontend are organised **domain-first**. A new domain means adding a package on both sides under the same name.
|
||||
|
||||
### Backend
|
||||
|
||||
1. Create `backend/src/main/java/org/raddatz/familienarchiv/citation/`
|
||||
|
||||
2. Add entity, repository, service, controller, and DTOs flat in the package:
|
||||
- **Entity** `Citation.java` — annotate with `@Entity @Data @Builder @NoArgsConstructor @AllArgsConstructor`; use `@GeneratedValue(strategy = GenerationType.UUID)` for the `id` field; add `@Schema(requiredMode = REQUIRED)` on every field the backend always populates
|
||||
- **Repository** `CitationRepository.java` — extends `JpaRepository<Citation, UUID>`
|
||||
- **Service** `CitationService.java` — `@Service @RequiredArgsConstructor`; write methods `@Transactional`, read methods unannotated; cross-domain data goes through the other domain's service, never its repository
|
||||
- **Controller** `CitationController.java` — `@RestController @RequestMapping("/api/citations")`
|
||||
|
||||
3. Add `@RequirePermission(Permission.WRITE_ALL)` on every `POST`, `PUT`, `PATCH`, and `DELETE` endpoint — **this is not optional**. Read-only `GET` endpoints stay unannotated.
|
||||
|
||||
4. Add a Flyway migration: `backend/src/main/resources/db/migration/V{n}__{description}.sql` (use the next sequential number after the highest existing one).
|
||||
|
||||
5. **Write failing tests before any implementation** (Red step):
|
||||
- Service unit test for business logic (`@ExtendWith(MockitoExtension.class)`)
|
||||
- `@WebMvcTest` slice test for each HTTP endpoint
|
||||
|
||||
6. Rebuild with `--spring.profiles.active=dev` and run `npm run generate:api` in `frontend/`.
|
||||
|
||||
### Frontend
|
||||
|
||||
7. Create `frontend/src/lib/citation/` — domain-specific Svelte components and TypeScript utilities go here.
|
||||
|
||||
8. Add routes under `frontend/src/routes/citations/` as needed.
|
||||
|
||||
9. Add a per-domain `README.md` in both the backend package folder and `frontend/src/lib/citation/` (per DOC-6).
|
||||
|
||||
### Documentation
|
||||
|
||||
10. Update `docs/ARCHITECTURE.md` Section 2 to include the new domain.
|
||||
11. Update `docs/GLOSSARY.md` if new terms are introduced.
|
||||
12. Update the ESLint boundary allow-list in `frontend/eslint.config.js` if the domain needs to import from another domain.
|
||||
|
||||
---
|
||||
|
||||
## 4. Walkthrough B — Add a new endpoint
|
||||
|
||||
**Example:** `POST /api/persons/{id}/aliases` — attach a name alias to an existing person.
|
||||
|
||||
### Red (write failing tests first)
|
||||
|
||||
1. Write a failing `@WebMvcTest` controller slice test:
|
||||
```java
|
||||
@Test
|
||||
void addAlias_returns201_whenAliasCreated() { ... }
|
||||
```
|
||||
|
||||
2. Write a failing service unit test:
|
||||
```java
|
||||
@Test
|
||||
void addAlias_throwsNotFound_whenPersonDoesNotExist() { ... }
|
||||
```
|
||||
|
||||
### Green (implement)
|
||||
|
||||
3. Add the service method in `PersonService.java`:
|
||||
```java
|
||||
@Transactional
|
||||
public PersonNameAlias addAlias(UUID personId, PersonNameAliasDTO dto) { ... }
|
||||
```
|
||||
|
||||
4. Add the controller method in `PersonController.java`:
|
||||
```java
|
||||
@PostMapping("/{id}/aliases")
|
||||
@RequirePermission(Permission.WRITE_ALL)
|
||||
public ResponseEntity<PersonNameAlias> addAlias(@PathVariable UUID id,
|
||||
@RequestBody PersonNameAliasDTO dto) { ... }
|
||||
```
|
||||
`@RequirePermission(Permission.WRITE_ALL)` on every state-mutating endpoint — **not optional**.
|
||||
|
||||
5. Validate user-supplied inputs at the controller boundary:
|
||||
```java
|
||||
if (dto.name() == null || dto.name().isBlank())
|
||||
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "name is required");
|
||||
```
|
||||
Validate at system boundaries; trust internal service code.
|
||||
|
||||
6. Use `DomainException` for domain errors:
|
||||
```java
|
||||
DomainException.notFound(ErrorCode.PERSON_NOT_FOUND, "Person not found: " + id)
|
||||
```
|
||||
If you need a new error code, add it to `ErrorCode.java`, mirror it in
|
||||
`frontend/src/lib/shared/errors.ts`, and add translation keys in `messages/{de,en,es}.json`.
|
||||
|
||||
7. Mark every field the backend always populates with `@Schema(requiredMode = REQUIRED)` — this drives TypeScript type generation.
|
||||
|
||||
### Types and tests
|
||||
|
||||
8. Rebuild with `--spring.profiles.active=dev`, then `npm run generate:api` in `frontend/`.
|
||||
|
||||
> ⚠️ **Always regenerate types after any API change.** This is the #1 cause of "where did my TypeScript type go?"
|
||||
|
||||
9. Run the full test suite — all green before committing.
|
||||
|
||||
---
|
||||
|
||||
## 5. Walkthrough C — Add a new frontend page
|
||||
|
||||
**Example:** `/persons/[id]/timeline` — a chronological event timeline for one person.
|
||||
|
||||
### Red (write failing test first)
|
||||
|
||||
1. Write a failing Playwright E2E test for the user flow:
|
||||
```typescript
|
||||
test('timeline shows events in chronological order', async ({ page }) => {
|
||||
await page.goto('/persons/1/timeline');
|
||||
// assertions...
|
||||
});
|
||||
```
|
||||
|
||||
### Green (implement)
|
||||
|
||||
2. Create `frontend/src/routes/persons/[id]/timeline/+page.svelte`
|
||||
|
||||
3. Add `frontend/src/routes/persons/[id]/timeline/+page.server.ts` for the SSR load:
|
||||
```typescript
|
||||
import { createApiClient } from '$lib/shared/api.server';
|
||||
export const load: PageServerLoad = async ({ params, fetch }) => {
|
||||
const api = createApiClient(fetch);
|
||||
const result = await api.GET('/api/persons/{id}', { params: { path: { id: params.id } } });
|
||||
if (!result.response.ok) throw error(result.response.status, '...');
|
||||
return { person: result.data! };
|
||||
};
|
||||
```
|
||||
|
||||
4. Domain-specific components (e.g. `TimelineEntry.svelte`) → `frontend/src/lib/person/`
|
||||
|
||||
5. Shared primitives (e.g. a generic date-range display) → `frontend/src/lib/shared/primitives/`
|
||||
|
||||
6. UI patterns to follow:
|
||||
- Back navigation: `import BackButton from '$lib/shared/primitives/BackButton.svelte'`
|
||||
- Date display: always append `T12:00:00` — `new Intl.DateTimeFormat('de-DE', …).format(new Date(val + 'T12:00:00'))` — prevents UTC off-by-one errors
|
||||
- Brand colors: `brand-navy`, `brand-mint`, `brand-sand` (defined in `src/routes/layout.css`)
|
||||
- Accessibility: touch targets ≥ 44 px (`min-h-[44px]`); focus rings (`focus-visible:ring-2 focus-visible:ring-brand-navy`); `aria-label` on icon-only buttons; `aria-live="polite"` on dynamic status messages
|
||||
|
||||
7. Add Paraglide i18n keys in `messages/de.json`, `messages/en.json`, `messages/es.json`.
|
||||
|
||||
8. If adding a new error code: mirror in `frontend/src/lib/shared/errors.ts` and add translation keys.
|
||||
|
||||
9. Make all tests green before committing.
|
||||
|
||||
---
|
||||
|
||||
## 6. Conventions reference
|
||||
|
||||
### Error handling
|
||||
|
||||
| Scenario | Pattern |
|
||||
|---|---|
|
||||
| Domain entity not found | `DomainException.notFound(ErrorCode.X, "…")` |
|
||||
| Permission denied | `DomainException.forbidden("…")` |
|
||||
| Concurrent edit conflict | `DomainException.conflict(ErrorCode.X, "…")` |
|
||||
| Infrastructure failure | `DomainException.internal(ErrorCode.X, "…")` |
|
||||
| Simple controller validation | `throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "…")` |
|
||||
|
||||
New error code: `ErrorCode.java` → `frontend/src/lib/shared/errors.ts` → `messages/{de,en,es}.json`.
|
||||
|
||||
### DTOs
|
||||
|
||||
- Input DTOs live flat in the domain package (e.g. `PersonUpdateDTO.java`)
|
||||
- Responses are the entity itself — no separate response DTOs
|
||||
- `@Schema(requiredMode = REQUIRED)` on every field the backend always populates
|
||||
|
||||
### Frontend API client
|
||||
|
||||
```typescript
|
||||
const api = createApiClient(fetch); // from $lib/shared/api.server
|
||||
const result = await api.GET('/api/persons/{id}', { params: { path: { id } } });
|
||||
if (!result.response.ok) {
|
||||
const code = (result.error as unknown as { code?: string })?.code;
|
||||
throw error(result.response.status, getErrorMessage(code));
|
||||
}
|
||||
return { person: result.data! }; // non-null assertion is safe after the ok check
|
||||
```
|
||||
|
||||
For multipart/form-data (file uploads): bypass the typed client and use raw `fetch` — the client cannot handle it.
|
||||
|
||||
### Date handling
|
||||
|
||||
| Context | Pattern |
|
||||
|---|---|
|
||||
| Form display | German `dd.mm.yyyy` with auto-dot insertion via `handleDateInput()` |
|
||||
| Wire format | ISO 8601 via a hidden `<input type="hidden" name="documentDate" value={dateIso}>` |
|
||||
| Display | `new Intl.DateTimeFormat('de-DE', …).format(new Date(val + 'T12:00:00'))` |
|
||||
|
||||
### Security checklist (new endpoint)
|
||||
|
||||
- `@RequirePermission(Permission.WRITE_ALL)` on every `POST`, `PUT`, `PATCH`, `DELETE` — required, not optional
|
||||
- Validate all user-supplied inputs at the controller boundary before passing to the service
|
||||
- Parameterised queries only — never interpolate user input into JPQL/SQL strings
|
||||
- No raw user input in log messages — use `{}` placeholders: `log.warn("Not found: {}", id)`
|
||||
- Validate content-type and size on upload endpoints before reading the stream
|
||||
|
||||
### Accessibility baseline (new frontend page)
|
||||
|
||||
- Touch targets ≥ 44 px on all interactive elements (`min-h-[44px]`)
|
||||
- Focus rings on all focusable elements (`focus-visible:ring-2 focus-visible:ring-brand-navy`)
|
||||
- `aria-label` on every icon-only button
|
||||
- `aria-live="polite"` on dynamic status messages
|
||||
- Color is never the sole status indicator
|
||||
|
||||
Full WCAG 2.1 AA reference: [docs/STYLEGUIDE.md](./docs/STYLEGUIDE.md).
|
||||
|
||||
### Lint and format
|
||||
|
||||
```bash
|
||||
# Frontend
|
||||
cd frontend && npm run lint # Prettier + ESLint check
|
||||
cd frontend && npm run format # Auto-fix formatting
|
||||
cd frontend && npm run check # svelte-check (type errors)
|
||||
|
||||
# Backend — checkstyle is enforced via Maven
|
||||
cd backend && ./mvnw checkstyle:check
|
||||
```
|
||||
Reference in New Issue
Block a user