Skip to content

refactor(frontend): Split collective-detail into data-driven sections (#187) - #189

Merged
enko merged 1 commit into
mainfrom
refactor/187-split-collective-detail
Jun 29, 2026
Merged

refactor(frontend): Split collective-detail into data-driven sections (#187)#189
enko merged 1 commit into
mainfrom
refactor/187-split-collective-detail

Conversation

@enko

@enko enko commented Jun 24, 2026

Copy link
Copy Markdown
Member

Closes #187.

What

collective-detail.svelte (893 LOC) was the lone grab-bag component flagged in the PR #186 review — it mixed the header/notes template, collective-delete state, per-subresource CRUD for five types, member lifecycle, and keyboard wiring via a central editingType discriminator with a parallel if/else save chain and a switch delete chain.

This decomposes it behavior-preservingly, using a data-driven descriptor (per the issue's first bullet) plus an extracted member section:

File Role
subresource-descriptors.ts One descriptor per type (phone/email/address/url/circle): icon, i18n keys, API calls, Row/Form components, and per-type quirks (address geocoding refetch, circle add-only)
subresource-section.svelte A single generic, branch-free section driven by a descriptor — owns its CRUD/modal/delete state and add-shortcut listener, mirroring the friend section lifecycle
member-section.svelte Member list, add-member modal, deactivate modal, the member add-shortcut, and the visibleMemberContactIds effect
collective-detail.svelte Thin container (893 → 193 LOC) — header, notes, collective-delete confirmation, and section/member orchestration

The central if/else save chain and switch delete chain collapse into the descriptor table. Reuses the existing friends/subresources shared components, the $lib/api/collectives CRUD fns, and the same markup/strings/shortcuts, so behavior is unchanged. Sections are always-mounted so the add-shortcut works even when a section is empty. Drops the dead availableCircles fetch (CircleEditForm uses the current circles).

Tests

Adds the collective component's first coverage via the $lib/test infra from PR #186:

  • subresource-descriptors.test.ts — delete-name extraction, row-prop mapping, editable/error mapping
  • subresource-section.test.ts — render, add, create, delete, shortcut-opens-when-empty, non-editable affordance (injects a fake descriptor with vi.fn() CRUD — no API mocking needed)
  • member-section.test.ts — member render, add modal, deactivate/reactivate/remove through the store

Verification

Acceptance criteria

  • No behavioral change (add/edit/delete each subresource, member lifecycle, shortcuts) — preserved via shared components/API/markup; covered by tests
  • Component test coverage for the extracted pieces (uses $lib/test)
  • aube type-check, aube run check, and the full test suite pass
  • Manually verified in-browser (aube dev) — pending

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Jun 24, 2026

Copy link
Copy Markdown

Coverage Report

👍 Test coverage is looking good.

Impacted Files % Stmts % Branch % Funcs % Lines Uncovered Lines
apps/frontend/src/lib/
components/collectives/
collective-detail.svelte
100 91.67 100 100
apps/frontend/src/lib/
components/collectives/
member-section.svelte
90.82 77.27 86.84 90.82 54, 59, 60, 112, 113, 157, 167, 196, 209
apps/frontend/src/lib/
components/collectives/
subresource-descriptors.ts
100 95.24 97.44 100
and 2 more...
Impacted Files % Stmts % Branch % Funcs % Lines Uncovered Lines
apps/frontend/src/lib/
components/collectives/
subresource-section.svelte
99.15 75.56 95.45 99.15 134
apps/frontend/src/lib/
components/friends/
subresources/
detail-edit-modal.svelte
89.13 75 88.89 89.13 59, 60, 66, 121, 122

Generated by 🚫 dangerJS against 6e7f82d

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors the large collective-detail.svelte component into smaller, data-driven Svelte components to preserve existing collective subresource/member behavior while making the implementation easier to maintain and test.

Changes:

  • Introduces SubresourceDescriptor definitions to drive per-type CRUD behavior (phones/emails/addresses/urls/circles) without central if/switch chains.
  • Adds a generic SubresourceSection component and a dedicated MemberSection component, with new component tests for each.
  • Slims collective-detail.svelte down to a container that orchestrates the new sections.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
apps/frontend/src/lib/components/collectives/collective-detail.svelte Replaces in-component CRUD/member logic with orchestrated section components.
apps/frontend/src/lib/components/collectives/subresource-descriptors.ts Defines the data-driven descriptor table for collective subresource CRUD.
apps/frontend/src/lib/components/collectives/subresource-section.svelte Implements the generic, descriptor-driven subresource section (load/create/update/delete + modals + shortcut).
apps/frontend/src/lib/components/collectives/member-section.svelte Extracts member lifecycle UI/shortcuts and visible-member tracking into its own component.
apps/frontend/src/lib/components/collectives/subresource-descriptors.test.ts Adds unit coverage for descriptor mapping and per-type behaviors.
apps/frontend/src/lib/components/collectives/subresource-section.test.ts Adds component tests for generic subresource section rendering, create/delete, and shortcut behavior.
apps/frontend/src/lib/components/collectives/member-section.test.ts Adds component tests for member add/deactivate/reactivate/remove flows via the store.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread apps/frontend/src/lib/components/collectives/member-section.svelte
@enko

enko commented Jun 25, 2026

Copy link
Copy Markdown
Member Author

Pushed 0e3c9c6 addressing both review comments and the coverage report:

  • member deactivate payload — explicit empty-string/length checks instead of truthiness coercion.
  • update-path test — added for the generic section's create-vs-update split.
  • coverage — added subresource-descriptors.api.test.ts (load/create/update/remove delegation per type, formProps defaults, address geocoding afterSave backoff) plus the reloadAfterMutate create/delete branches, lifting the new sections past the 80% branch threshold.

24 tests pass; biome + frontend type-check clean.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Comment thread apps/frontend/src/lib/components/collectives/member-section.svelte
@enko

enko commented Jun 25, 2026

Copy link
Copy Markdown
Member Author

Pushed 7f2d457 addressing this round:

  • shortcut suppression (both modals) — the deactivate-member dialog and the collective delete-confirm overlay now set isModalOpen on open and clear it on close/confirm/error, matching the shared modal components. Real bug; thanks Copilot.
  • coverage — added a collective-detail test suite (was 0%) plus member-section tests for the modal flag and the deactivate/reactivate/remove error paths.

43 collective tests pass; biome + frontend type-check clean (0 errors).

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Comment thread apps/frontend/src/lib/components/collectives/collective-detail.test.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Comment thread apps/frontend/src/lib/components/collectives/member-section.svelte

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Comment thread apps/frontend/src/lib/components/collectives/subresource-section.svelte Outdated
Comment thread apps/frontend/src/lib/components/collectives/member-section.svelte

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

@enko
enko force-pushed the refactor/187-split-collective-detail branch from 0321b6e to 5c9e0d9 Compare June 29, 2026 20:00
…#187)

collective-detail.svelte (893 LOC) was the lone grab-bag component flagged
in the PR #186 review: it mixed the header/notes template, collective-delete
state, per-subresource CRUD for five types, member lifecycle, and keyboard
wiring — via a central editingType discriminator with a parallel if/else save
chain and a switch delete chain.

Decompose it behavior-preservingly:

- subresource-descriptors.ts: one descriptor per type (phone/email/address/
  url/circle) capturing icon, i18n keys, API calls, Row/Form components, and
  per-type quirks (address geocoding refetch, circle add-only).
- subresource-section.svelte: a single generic, branch-free section driven by
  a descriptor — owns its CRUD/modal/delete state and add-shortcut listener,
  mirroring the friend section lifecycle. Drops stale in-flight loads and
  resets its items/modals when the collective changes (the instance is reused
  across collectives without unmounting).
- member-section.svelte: member list, add-member modal, deactivate modal, the
  member add-shortcut, and the visibleMemberContactIds effect. Marks the
  deactivate dialog as a modal (isModalOpen) to suppress global shortcuts, and
  resets its modal state when the collective changes.
- collective-detail.svelte: thin container (893 -> 193 LOC) — header, notes,
  collective-delete confirmation (suppressing shortcuts while open and resetting
  the deleting state in a finally), and the section/member orchestration.

DetailEditModal gains an asForm prop so a section whose children supply their
own <form> (add-member) avoids invalid nested forms; its footer Save still
calls onSave whether or not it owns the form.

Drops the dead availableCircles fetch (CircleEditForm uses the current
circles). Reuses the existing friends/subresources shared components, the
$lib/api/collectives CRUD fns, and the same markup/strings/shortcuts, so
behavior is unchanged.

Adds first component coverage for the collective pieces via $lib/test:
descriptor mapping and delegation, the generic section (render/add/create/
update/delete/shortcut/non-editable/stale-load/reset-on-change), member
management (lifecycle, modal flag, reset-on-change), the collective-detail
delete lifecycle, and DetailEditModal's form/non-form Save.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@enko
enko force-pushed the refactor/187-split-collective-detail branch from 5c9e0d9 to 6e7f82d Compare June 29, 2026 20:06

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

@enko
enko merged commit f589a18 into main Jun 29, 2026
12 checks passed
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 2.89.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor: split collective-detail.svelte (893 LOC) into focused pieces

2 participants