Migrate Catalog (Categories/Services/Tags) to Result errors; remove Services; physical reorg - #75
Merged
Conversation
…ervices vertical; physical reorg Split out of #69 (part 3 of 4 — see that PR for the full picture). Recreated from origin/main after #70 and #71 merged, since this repo's convention (and the split-large-coderabbit-pr skill) is a sequential series, not stacking on an unmerged branch — CodeRabbit also doesn't review PRs whose base isn't the default branch, so stacking silently skipped review for this and the next PR in the series. Same content as the original #72, just re-based; no functional change. This PR is larger than the <100-file target used for the other PRs in this series, deliberately — see "Why this couldn't be split further" below. - Categories, Services, and Tags all move to the Result-based error convention docs/adr/014 establishes: domain entities' create() methods, mappers, and API repositories return Result<T, AppError> instead of throwing; useAsync.ts (already Result-based, landed in #71) is the one hook every feature's data layer builds on now. - app/composition/container.ts's CatalogFacade drops the use-case-class indirection (ListCategories/CreateTag/etc. as separate classes) for direct repository delegation (`{ execute: repo.method }`) - there's no orchestration between the facade and the repository, so the extra class per operation wasn't earning its keep. The 24 now-orphaned use-case-class files (application/use-cases/{categories,services,tags}/) are deleted. - Services' frontend implementation (ServicesPage, ServiceForm, six ServicesPage.*.test.tsx files, all its components/hooks/models) is fully removed, reverting `/services` to a placeholder page (app/pages/ServicesPage/ServicesPage.tsx) - this vertical is going back to `stub` status, see docs/STATUS.md. - Categories moves to a routed create/edit dialog (features/catalog/presentation/categories/pages/CategoriesListPage/, .../CategoryEditorDialog/) per docs/adr/012, replacing the old flat CategoriesPage.tsx/useCategories.ts/CategoryEditorDialog.tsx shape. - Tags gets the equivalent internal move (hooks/useTagEditor.ts, pages/TagEditorDialog.tsx) and its own Result migration (Tag.ts/tagMapper.ts/ApiTagRepository.ts) - Tags itself is not being removed here, just migrated; its removal is a later PR in this stack (docs/adr/016). - shared/: AuthenticatedHttpClient's get/post/put/delete now return Result<T, AppError> instead of throwing; DeleteConfirmationDialog takes entityName/entityType instead of a raw title/description pair; useCreateInline is removed (no longer used once Services - its only consumer - is gone). - Also removes .husky/pre-commit and fixes architecture_guard.py's precommit check accordingly (already merged independently via #70; included here too since this branch's own ancestry needed it before #70 existed). ## Why this couldn't be split further I initially tried a narrower "Category-only foundation" PR (~99 files) deferring Services/Tags. That failed a real build: app/composition/ container.ts wires TagRepository with its *new* method signature directly (tagRepository.listAll(options) instead of the old (tenantContext, options) two-arg form) - not just a return-type change useAsync-style, but the interface itself. Making that build without also migrating TagRepository/ApiTagRepository/tagMapper/Tag.ts for real isn't a smaller wrapper shim - it's the same size of work as just finishing the migration, since there's no reduced version of an interface signature. Categories, Services, and Tags share container.ts's catalog wiring, router.tsx, and AuthenticatedHttpClient tightly enough that they're one atomic, verified-buildable unit at this layer - mirroring why Auth couldn't be split from Catalog either, just one layer down. ## Test plan - [x] `npm install` + `npm run build --workspace=apps/admin-frontend` — green - [x] `npm run lint --workspace=apps/admin-frontend` — clean, 0 warnings - [x] `npm run format:check --workspace=apps/admin-frontend` — clean - [x] `npm run test --workspace=apps/admin-frontend` — 368/368 passing - [x] `scripts/sync_agent_skills.py --check`, `scripts/check_agent_governance.py`, `scripts/architecture_guard.py` — all pass Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Important Review skippedToo many files! This PR contains 149 files, which is 49 over the limit of 100. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (149)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Closed
5 tasks
useCategoryEditor fetches its own category via GET /api/v1/categories/{id}
(docs/adr/013), but this spec's route mock matched any /api/v1/categories*
path and always returned the full list array regardless of whether the
request was for the collection or a single id - so the by-id fetch
received an array instead of a CategoryDto, and the edit dialog's Nome
field never populated. Mock now inspects the last path segment and
returns the matching single category (404 if not found) for a by-id GET,
the full list otherwise.
Verified against the real Playwright suite (production build + preview,
matching CI): all 10 e2e specs pass, including this one.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
This was referenced Aug 2, 2026
evertonschuster
added a commit
that referenced
this pull request
Aug 2, 2026
Split out of #69 (final part of this series — see that PR for the full picture). Recreated from origin/main after #70/#71/#75 merged, since this repo's convention (and the split-large-coderabbit-pr skill) is a sequential series, not stacking on an unmerged branch. Same content as the original #73, just re-based; no functional change. Removes the entire Tags vertical from apps/admin-frontend (domain, application, infrastructure, presentation, MSW handlers, E2E specs, nav entry, route, and catalog facade wiring) while intentionally retaining the backend Tag domain entity and /api/v1/tags endpoints, including Service's many-to-many relationship to Tag - a project-owner decision, see docs/adr/016-remove-tags-frontend.md. Categories replaces Tags as the reference CRUD implementation throughout the docs and the agenza-frontend-feature skill. ## Test plan - [x] `npm install` + `npm run build --workspace=apps/admin-frontend` — green - [x] `npm run lint --workspace=apps/admin-frontend` — clean, 0 warnings - [x] `npm run format:check --workspace=apps/admin-frontend` — clean - [x] `npm run test --workspace=apps/admin-frontend` — 305/305 passing - [x] `npx playwright test` (full e2e suite, production build + preview) — 8/8 passing - [x] `scripts/sync_agent_skills.py --check`, `scripts/check_agent_governance.py`, `scripts/architecture_guard.py` — all pass Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
4 tasks
evertonschuster
added a commit
that referenced
this pull request
Aug 2, 2026
…#77) These 3 files (agent-skills/agenza-frontend-feature/SKILL.md and its two synced copies under .claude/skills/ and .agents/skills/) live at the repo root, outside apps/admin-frontend/ — every diff I computed while splitting #69 into #70/#71/#75/#76 was scoped to apps/admin-frontend (and backend/ for #70), so these files' accumulated updates from this session (Catalog Result migration, Auth Result migration, and finally the Tags-removal doc pass replacing TagsPage/TagForm with Categories as the reference implementation) never made it into any of the split PRs, even though the actual code changes they describe are all correctly merged. Content taken directly from the original branch's final commit (4911abb), already reviewed and governance-checked at the time. Verified again here against the current merged main: sync_agent_skills.py --check, check_agent_governance.py, and architecture_guard.py all pass, and the file paths the skill references (CategoriesListPage.tsx, CategoryForm.tsx, categoryMapper.ts, AdminLayout.tsx) all exist in the current tree. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Split out of #69 (part 3 — see that PR for the full picture). Replaces #72, which was built stacked on #71's now-merged branch; this repo's convention (and the
split-large-coderabbit-prskill) calls for a sequential series instead — CodeRabbit also doesn't review PRs whose base isn't the default branch, so stacking silently skipped review for #72. This PR is the same content, re-based onmainnow that #70 and #71 are merged. No functional change from #72.This PR is larger than the <100-file target the other PRs in this series hit — deliberately, see "Why this couldn't be split further" below.
docs/adr/014establishes: domain entities'create()methods, mappers, and API repositories returnResult<T, AppError>instead of throwing;useAsync.ts(already Result-based, landed in Migrate Auth to Result-based error handling; add shared Result/error infra #71) is the one hook every feature's data layer builds on now.app/composition/container.ts'sCatalogFacadedrops the use-case-class indirection (ListCategories/CreateTag/etc. as separate classes) for direct repository delegation ({ execute: repo.method }) — there's no orchestration between the facade and the repository, so the extra class per operation wasn't earning its keep. The 24 now-orphaned use-case-class files (application/use-cases/{categories,services,tags}/) are deleted.ServicesPage,ServiceForm, sixServicesPage.*.test.tsxfiles, all its components/hooks/models) is fully removed, reverting/servicesto a placeholder page (app/pages/ServicesPage/ServicesPage.tsx) — this vertical goes back tostubstatus, seedocs/STATUS.md.features/catalog/presentation/categories/pages/CategoriesListPage/,.../CategoryEditorDialog/) perdocs/adr/012, replacing the old flatCategoriesPage.tsx/useCategories.ts/CategoryEditorDialog.tsxshape.hooks/useTagEditor.ts,pages/TagEditorDialog.tsx) and its own Result migration (Tag.ts/tagMapper.ts/ApiTagRepository.ts) — Tags itself is not being removed here, just migrated; its removal is the next PR in this series (docs/adr/016).shared/:AuthenticatedHttpClient'sget/post/put/deletenow returnResult<T, AppError>instead of throwing;DeleteConfirmationDialogtakesentityName/entityTypeinstead of a raw title/description pair;useCreateInlineis removed (no longer used once Services — its only consumer — is gone).Why this couldn't be split further
I initially tried a narrower "Category-only foundation" PR (~99 files) deferring Services/Tags. That failed a real build:
app/composition/container.tswiresTagRepositorywith its new method signature directly (tagRepository.listAll(options)instead of the old(tenantContext, options)two-arg form) — not just a return-type changeuseAsync-style, but the interface itself. Making that build without also migratingTagRepository/ApiTagRepository/tagMapper/Tag.tsfor real isn't a smaller wrapper shim — it's the same size of work as just finishing the migration, since there's no reduced version of an interface signature. Categories, Services, and Tags sharecontainer.ts's catalog wiring,router.tsx, andAuthenticatedHttpClienttightly enough that they're one atomic, verified-buildable unit at this layer — mirroring why Auth couldn't be split from Catalog either, just one layer down.Test plan
npm install+npm run build --workspace=apps/admin-frontend— greennpm run lint --workspace=apps/admin-frontend— clean, 0 warningsnpm run format:check --workspace=apps/admin-frontend— cleannpm run test --workspace=apps/admin-frontend— 368/368 passingscripts/sync_agent_skills.py --check,scripts/check_agent_governance.py,scripts/architecture_guard.py— all pass🤖 Generated with Claude Code