diff --git a/ai/backend-context.md b/ai/backend-context.md index e7825fa..f3b5ddd 100644 --- a/ai/backend-context.md +++ b/ai/backend-context.md @@ -15,11 +15,14 @@ intentionally omitted here - update this section as entities land. - Look at similar entities for reference before writing anything new - Entities: constructor with properties, getters -- DTOs: simple data containers for creation (e.g. `CreateSetDto`) +- DTOs: simple data containers for creation (e.g. `CreateElementDto`) - Repositories: interfaces that define data access - Do not write unit tests for concrete repository implementations (e.g. `Postgres*Repository`). They are exercised by e2e tests. Use cases are tested with fake repositories. + - Repository methods that find records by a foreign key should accept + the related entity, not a raw id (e.g. `findBySet(Set $set)`, not + `findBySetId(int $setId)`). - Use cases: business logic with Request objects - When throwing exceptions, add `@throws` docblock - Fakes: in-memory implementations for testing @@ -50,16 +53,33 @@ to be added to the repo; the Nix flake already provides Constructs LLMs default to that this project forbids. Name the trap explicitly so you catch yourself before writing it. -| Anti-pattern | Forbidden | Required | -|---|---|---| -| Arrow function | `fn ($x) => $x->getId()` | `function ($x) { return $x->getId(); }` | -| Inline FQCN type | `function f(): \Psr\Http\Message\ResponseInterface` | `use Psr\Http\Message\ResponseInterface;` then `function f(): ResponseInterface` | -| Inline `::class` | `Container::get(\App\Foo\Bar::class)` | `use App\Foo\Bar;` then `Container::get(Bar::class)` | -| Default param | `function f(int $count = 10)` | `function f(int $count)` | -| Default in fake | `public function create(Dto $dto, bool $strict = true)` | no default; every caller passes a value | -| Lookup returns stored ref | `return $this->items[$id] ?? null;` | rebuild a new instance with the stored fields | -| Short variable name | `$t`, `$n`, `$res`, `$req`, `$e` | `$text`, `$node`, `$response`, `$request`, `$exception` | -| Em dash | `// fetches user — by email` | `// fetches user - by email` | +- Arrow function: + - Forbidden: `fn ($node) => $node->getId()` + - Required: `function ($node) { return $node->getId(); }` +- Inline FQCN type: + - Forbidden: `function f(): \Psr\Http\Message\ResponseInterface` + - Required: import `ResponseInterface`, then return `ResponseInterface` +- Inline `::class`: + - Forbidden: `Container::get(\App\Foo\Bar::class)` + - Required: import `Bar`, then call `Container::get(Bar::class)` +- Default param: + - Forbidden: `function f(int $count = 10)` + - Required: `function f(int $count)` +- Default in fake: + - Forbidden: `public function create(Dto $dto, bool $strict = true)` + - Required: no default; every caller passes a value +- Lookup returns stored ref: + - Forbidden: `return $this->items[$id] ?? null;` + - Required: rebuild a new instance with the stored fields +- FK lookup by id: + - Forbidden: `findBySetId(int $setId)` + - Required: `findBySet(Set $set)` +- Short variable name: + - Forbidden: one-letter or abbreviated names (`$t`, `$n`, `$res`) + - Required: explicit names (`$text`, `$node`, `$response`) +- Em dash: + - Forbidden: comment or docblock containing an em dash character + - Required: use `-` When generating code, scan the diff for these patterns before writing it to disk. If you catch one mid-write, rewrite that line. diff --git a/ai/shared.md b/ai/shared.md index c07271e..3d0522e 100644 --- a/ai/shared.md +++ b/ai/shared.md @@ -16,7 +16,9 @@ guides (`backend-context.md`, `frontend-context.md`) extend these. 4. Implement the code to make the test pass 5. Run the test to confirm it passes 6. Commit the implementation -7. Repeat for each new behavior +7. Repeat this red/green commit cycle for each new behavior. Do not + batch multiple behaviors into one failing-test commit and one + implementation commit when they can be reviewed separately. ## Code style @@ -32,8 +34,8 @@ guides (`backend-context.md`, `frontend-context.md`) extend these. and fakes - if a helper accepts a value, every caller must supply it. - First, explore the codebase to understand existing patterns - look at similar files for reference before writing anything -- Never use em dashes (—) in code, comments, or docblocks - use hyphens (-) - instead +- Never use em dash characters in code, comments, or docblocks - use + hyphens (-) instead ## Git commit style @@ -51,9 +53,21 @@ guides (`backend-context.md`, `frontend-context.md`) extend these. ## Git commits - Tests should be committed first, before implementation +- Prefer small, reviewable commits. Commit each meaningful step as soon as + it is green and the pre-commit checklist passes. - One logical change per commit - a commit may span multiple files when they form a single logical unit (e.g. a use case with its request and exception, or a Vue SFC with its Pinia store and route entry) +- Split commits by behavior, public contract, migration, wiring, docs, or + mechanical formatting when those parts can be reviewed independently. +- A multi-file commit is okay only when the files form one inseparable + change. Do not use "one logical change" to justify batching adjacent work. +- Do not batch several behaviors into one "tests" commit and one + "implementation" commit. Repeat the red/green cycle per behavior. +- Do not bundle backend and frontend changes unless both are required for + the same user-facing behavior. +- Do not bundle cleanup, refactors, renames, or formatting with feature + behavior. - Keep commits focused: not one file per commit, not unrelated work batched - Make commits frequent - commit each meaningful logical step as you go - Commits are for reviewing and documenting the development of code @@ -83,6 +97,8 @@ mechanical, not aspirational - a "yes" to all is required. - [ ] On a feature branch (not master/main). - [ ] This commit is one logical change. If it spans unrelated changes, stop and split it. +- [ ] This commit has been split as far as it can be while staying + reviewable. If any part can land independently, split it. - [ ] Tests for new behavior were committed BEFORE this implementation (or this commit IS the failing-test commit). @@ -95,6 +111,8 @@ mechanical, not aspirational - a "yes" to all is required. (PHP or TS). - [ ] Find/lookup repository methods return new instances, not stored references. +- [ ] Backend repository methods that find by a foreign key accept the + related entity, not a raw id. - [ ] No em dashes (use hyphens). - [ ] Variable names are explicit (no `$t`, `n`, `res`, `req`, `e`, etc.). - [ ] No `any` in TypeScript - use concrete types or `unknown` with a diff --git a/backend/app/Element/CreateElementDto.php b/backend/app/Element/CreateElementDto.php new file mode 100644 index 0000000..db77dd7 --- /dev/null +++ b/backend/app/Element/CreateElementDto.php @@ -0,0 +1,14 @@ +id; + } + + public function getTitle(): string + { + return $this->title; + } + + public function getSet(): Set + { + return $this->set; + } + + public function getParentElement(): ?Element + { + return $this->parentElement; + } +} diff --git a/backend/app/Element/ElementModel.php b/backend/app/Element/ElementModel.php new file mode 100644 index 0000000..119d287 --- /dev/null +++ b/backend/app/Element/ElementModel.php @@ -0,0 +1,40 @@ +|ElementModel newModelQuery() + * @method static Builder|ElementModel newQuery() + * @method static Builder|ElementModel query() + * @method static Builder|ElementModel whereId($value) + * @method static Builder|ElementModel whereParentElementId($value) + * @method static Builder|ElementModel whereSetId($value) + * @method static Builder|ElementModel whereTitle($value) + * + * @mixin \Eloquent + */ +class ElementModel extends Model +{ + protected $table = 'elements'; + + public $timestamps = false; + + protected $fillable = [ + 'set_id', + 'title', + 'parent_element_id', + ]; + + protected $casts = [ + 'set_id' => 'integer', + 'parent_element_id' => 'integer', + ]; +} diff --git a/backend/app/Element/ElementRepository.php b/backend/app/Element/ElementRepository.php new file mode 100644 index 0000000..4f39fb6 --- /dev/null +++ b/backend/app/Element/ElementRepository.php @@ -0,0 +1,17 @@ + $dto->set->getId(), + 'title' => $dto->title, + 'parent_element_id' => $dto->parentElement?->getId(), + ]); + + return new Element( + id: $model->id, + title: $dto->title, + set: $dto->set, + parentElement: $dto->parentElement, + ); + } + + public function find(int $id): ?Element + { + $model = ElementModel::find($id); + + return $model === null ? null : $this->toDomain($model); + } + + /** + * @return Element[] + */ + public function findBySet(DomainSet $set): array + { + $models = ElementModel::where('set_id', $set->getId()) + ->orderBy('id') + ->get(); + $elements = []; + foreach ($models as $model) { + $elements[] = $this->toDomain($model); + } + + return $elements; + } + + private function toDomain(ElementModel $model): Element + { + $set = $this->setRepo->find($model->set_id); + if ($set === null) { + throw new DomainException( + "Set with id: {$model->set_id} doesnt exist" + ); + } + + $parentElement = null; + if ($model->parent_element_id !== null) { + $parentElement = $this->find($model->parent_element_id); + if ($parentElement === null) { + throw new DomainException( + "Element with id: {$model->parent_element_id} doesnt exist" + ); + } + } + + return new Element( + id: $model->id, + title: $model->title, + set: $set, + parentElement: $parentElement, + ); + } +} diff --git a/backend/app/Element/UseCases/CreateElement/CreateElement.php b/backend/app/Element/UseCases/CreateElement/CreateElement.php new file mode 100644 index 0000000..6656338 --- /dev/null +++ b/backend/app/Element/UseCases/CreateElement/CreateElement.php @@ -0,0 +1,85 @@ +setId === null) { + throw new BadRequestException('setId is required'); + } + if ($request->title === null || $request->title === '') { + throw new BadRequestException('title is required'); + } + + $set = $this->setRepo->find($request->setId); + if ($set === null) { + throw new DomainException( + "Set with id: {$request->setId} doesnt exist" + ); + } + + if ($request->parentElementId === null) { + $this->validateNoRootElementExists($set); + + return $this->elementRepo->create(new CreateElementDto( + set: $set, + title: $request->title, + parentElement: null, + )); + } + + $parentElement = $this->elementRepo->find( + $request->parentElementId + ); + if ($parentElement === null) { + throw new DomainException( + "Element with id: {$request->parentElementId} doesnt exist" + ); + } + if ($parentElement->getSet()->getId() !== $set->getId()) { + throw new DomainException( + 'Parent element must belong to the same set' + ); + } + + return $this->elementRepo->create(new CreateElementDto( + set: $set, + title: $request->title, + parentElement: $parentElement, + )); + } + + /** + * @throws DomainException + */ + private function validateNoRootElementExists(DomainSet $set): void + { + $elements = $this->elementRepo->findBySet($set); + foreach ($elements as $element) { + if ($element->getParentElement() === null) { + throw new DomainException( + 'A root element already exists for this set' + ); + } + } + } +} diff --git a/backend/app/Element/UseCases/CreateElement/CreateElementRequest.php b/backend/app/Element/UseCases/CreateElement/CreateElementRequest.php new file mode 100644 index 0000000..b9f0dbd --- /dev/null +++ b/backend/app/Element/UseCases/CreateElement/CreateElementRequest.php @@ -0,0 +1,12 @@ +app->bind( + SetRepository::class, + EloquentSetRepository::class + ); + $this->app->bind( + ElementRepository::class, + EloquentElementRepository::class + ); } } diff --git a/backend/app/Set/CreateSetDto.php b/backend/app/Set/CreateSetDto.php new file mode 100644 index 0000000..2999a88 --- /dev/null +++ b/backend/app/Set/CreateSetDto.php @@ -0,0 +1,10 @@ + $dto->name, + ]); + + return $this->toDomain($model); + } + + public function find(int $id): ?Set + { + $model = SetModel::find($id); + + return $model === null ? null : $this->toDomain($model); + } + + public function getAll(): array + { + $models = SetModel::orderBy('id')->get(); + $sets = []; + foreach ($models as $model) { + $sets[] = $this->toDomain($model); + } + + return $sets; + } + + private function toDomain(SetModel $model): Set + { + return new Set( + id: $model->id, + name: $model->name, + ); + } +} diff --git a/backend/app/Set/Set.php b/backend/app/Set/Set.php new file mode 100644 index 0000000..d045e70 --- /dev/null +++ b/backend/app/Set/Set.php @@ -0,0 +1,21 @@ +id; + } + + public function getName(): string + { + return $this->name; + } +} diff --git a/backend/app/Set/SetModel.php b/backend/app/Set/SetModel.php new file mode 100644 index 0000000..c30e2c9 --- /dev/null +++ b/backend/app/Set/SetModel.php @@ -0,0 +1,27 @@ +|SetModel newModelQuery() + * @method static Builder|SetModel newQuery() + * @method static Builder|SetModel query() + * @method static Builder|SetModel whereId($value) + * @method static Builder|SetModel whereName($value) + * + * @mixin \Eloquent + */ +class SetModel extends Model +{ + protected $table = 'sets'; + + public $timestamps = false; + + protected $fillable = ['name']; +} diff --git a/backend/app/Set/SetRepository.php b/backend/app/Set/SetRepository.php new file mode 100644 index 0000000..7f1efcd --- /dev/null +++ b/backend/app/Set/SetRepository.php @@ -0,0 +1,15 @@ +id(); + $table->string('name'); + }); + } + + public function down(): void + { + Schema::dropIfExists('sets'); + } +}; diff --git a/backend/database/migrations/2026_05_24_000001_elements_table.php b/backend/database/migrations/2026_05_24_000001_elements_table.php new file mode 100644 index 0000000..377ba38 --- /dev/null +++ b/backend/database/migrations/2026_05_24_000001_elements_table.php @@ -0,0 +1,25 @@ +id(); + $table->foreignId('set_id')->constrained('sets'); + $table->string('title'); + $table->foreignId('parent_element_id') + ->nullable() + ->constrained('elements'); + }); + } + + public function down(): void + { + Schema::dropIfExists('elements'); + } +}; diff --git a/backend/tests/Fakes/FakeElementRepository.php b/backend/tests/Fakes/FakeElementRepository.php new file mode 100644 index 0000000..65dbcad --- /dev/null +++ b/backend/tests/Fakes/FakeElementRepository.php @@ -0,0 +1,69 @@ +elementsById) + 1; + $element = new Element( + id: $id, + title: $dto->title, + set: $dto->set, + parentElement: $dto->parentElement, + ); + $this->elementsById[$id] = $element; + + return $element; + } + + public function find(int $id): ?Element + { + if (! isset($this->elementsById[$id])) { + return null; + } + + return $this->cloneElement($this->elementsById[$id]); + } + + /** + * @return Element[] + */ + public function findBySet(DomainSet $set): array + { + $elements = []; + foreach ($this->elementsById as $element) { + if ($element->getSet()->getId() === $set->getId()) { + $elements[] = $this->cloneElement($element); + } + } + + return $elements; + } + + private function cloneElement(Element $element): Element + { + $parentElement = $element->getParentElement(); + if ($parentElement !== null) { + $parentElement = $this->cloneElement($parentElement); + } + + return new Element( + id: $element->getId(), + title: $element->getTitle(), + set: $element->getSet(), + parentElement: $parentElement, + ); + } +} diff --git a/backend/tests/Fakes/FakeSetRepository.php b/backend/tests/Fakes/FakeSetRepository.php new file mode 100644 index 0000000..295e619 --- /dev/null +++ b/backend/tests/Fakes/FakeSetRepository.php @@ -0,0 +1,57 @@ +setsById) + 1; + $set = new DomainSet( + id: $id, + name: $dto->name, + ); + $this->setsById[$id] = $set; + + return $set; + } + + public function find(int $id): ?DomainSet + { + if (! isset($this->setsById[$id])) { + return null; + } + + return $this->cloneSet($this->setsById[$id]); + } + + /** + * @return DomainSet[] + */ + public function getAll(): array + { + $sets = []; + foreach ($this->setsById as $set) { + $sets[] = $this->cloneSet($set); + } + + return $sets; + } + + private function cloneSet(DomainSet $set): DomainSet + { + return new DomainSet( + id: $set->getId(), + name: $set->getName(), + ); + } +} diff --git a/backend/tests/Unit/Element/ElementTest.php b/backend/tests/Unit/Element/ElementTest.php new file mode 100644 index 0000000..3e914d9 --- /dev/null +++ b/backend/tests/Unit/Element/ElementTest.php @@ -0,0 +1,33 @@ +assertSame(2, $childElement->getId()); + $this->assertSame('Child', $childElement->getTitle()); + $this->assertSame($set, $childElement->getSet()); + $this->assertSame($rootElement, $childElement->getParentElement()); + $this->assertNull($rootElement->getParentElement()); + } +} diff --git a/backend/tests/Unit/Element/UseCases/CreateElementTest.php b/backend/tests/Unit/Element/UseCases/CreateElementTest.php new file mode 100644 index 0000000..2d01fc6 --- /dev/null +++ b/backend/tests/Unit/Element/UseCases/CreateElementTest.php @@ -0,0 +1,183 @@ +setRepo = new FakeSetRepository(); + $this->elementRepo = new FakeElementRepository(); + $this->createElement = new CreateElement( + $this->elementRepo, + $this->setRepo, + ); + } + + public function testCreatesRootElement(): void + { + $set = $this->setRepo->create( + new CreateSetDto('Daily learning') + ); + + $element = $this->createElement->execute(new CreateElementRequest( + setId: $set->getId(), + title: 'Root', + parentElementId: null, + )); + + $this->assertInstanceOf(Element::class, $element); + $this->assertSame('Root', $element->getTitle()); + $this->assertSame($set->getId(), $element->getSet()->getId()); + $this->assertNull($element->getParentElement()); + } + + public function testCreatesChildElement(): void + { + $set = $this->setRepo->create( + new CreateSetDto('Daily learning') + ); + $rootElement = $this->createElement->execute( + new CreateElementRequest( + setId: $set->getId(), + title: 'Root', + parentElementId: null, + ) + ); + + $childElement = $this->createElement->execute( + new CreateElementRequest( + setId: $set->getId(), + title: 'Child', + parentElementId: $rootElement->getId(), + ) + ); + + $this->assertSame('Child', $childElement->getTitle()); + $this->assertSame( + $rootElement->getId(), + $childElement->getParentElement()->getId(), + ); + } + + public function testThrowsWhenSetIdMissing(): void + { + $this->expectException(BadRequestException::class); + $this->expectExceptionMessage('setId is required'); + + $this->createElement->execute(new CreateElementRequest( + setId: null, + title: 'Root', + parentElementId: null, + )); + } + + public function testThrowsWhenTitleMissing(): void + { + $this->expectException(BadRequestException::class); + $this->expectExceptionMessage('title is required'); + + $this->createElement->execute(new CreateElementRequest( + setId: 1, + title: null, + parentElementId: null, + )); + } + + public function testThrowsWhenSetDoesNotExist(): void + { + $this->expectException(DomainException::class); + $this->expectExceptionMessage('Set with id: 99 doesnt exist'); + + $this->createElement->execute(new CreateElementRequest( + setId: 99, + title: 'Root', + parentElementId: null, + )); + } + + public function testThrowsWhenParentElementDoesNotExist(): void + { + $set = $this->setRepo->create( + new CreateSetDto('Daily learning') + ); + + $this->expectException(DomainException::class); + $this->expectExceptionMessage( + 'Element with id: 99 doesnt exist' + ); + + $this->createElement->execute(new CreateElementRequest( + setId: $set->getId(), + title: 'Child', + parentElementId: 99, + )); + } + + public function testThrowsWhenRootElementAlreadyExists(): void + { + $set = $this->setRepo->create( + new CreateSetDto('Daily learning') + ); + $this->createElement->execute(new CreateElementRequest( + setId: $set->getId(), + title: 'Root', + parentElementId: null, + )); + + $this->expectException(DomainException::class); + $this->expectExceptionMessage( + 'A root element already exists for this set' + ); + + $this->createElement->execute(new CreateElementRequest( + setId: $set->getId(), + title: 'Another root', + parentElementId: null, + )); + } + + public function testThrowsWhenParentBelongsToAnotherSet(): void + { + $parentSet = $this->setRepo->create( + new CreateSetDto('Parent set') + ); + $childSet = $this->setRepo->create( + new CreateSetDto('Child set') + ); + $parentElement = $this->createElement->execute( + new CreateElementRequest( + setId: $parentSet->getId(), + title: 'Parent root', + parentElementId: null, + ) + ); + + $this->expectException(DomainException::class); + $this->expectExceptionMessage( + 'Parent element must belong to the same set' + ); + + $this->createElement->execute(new CreateElementRequest( + setId: $childSet->getId(), + title: 'Invalid child', + parentElementId: $parentElement->getId(), + )); + } +} diff --git a/backend/tests/Unit/Set/SetTest.php b/backend/tests/Unit/Set/SetTest.php new file mode 100644 index 0000000..a98ac5f --- /dev/null +++ b/backend/tests/Unit/Set/SetTest.php @@ -0,0 +1,17 @@ +assertSame(1, $set->getId()); + $this->assertSame('Daily learning', $set->getName()); + } +}