Skip to content

refactor(management): the manager pages ask a domain - #1210

Open
ExtraToast wants to merge 1 commit into
mainfrom
refactor/the-managers-ask-a-domain
Open

refactor(management): the manager pages ask a domain#1210
ExtraToast wants to merge 1 commit into
mainfrom
refactor/the-managers-ask-a-domain

Conversation

@ExtraToast

@ExtraToast ExtraToast commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Third slice of #1167.

Why

AddressManager, CommitteeManager, RecoveryManager and UserManager read the generated client directly. ADR-001 forbids it, and #1183 listed all four as debt.

How

domains/user gains what these pages read: listUsers, readUser, listDeletedUsers, listMemberships, listAddresses, deleteUser.

Two new domains, because the alternative was putting them somewhere they do not belong: domains/committees for the committee list and its removal, and domains/recovery for which activation an account is waiting for. A committee is not the user domain's business, and neither is a recovery token's purpose.

A behaviour change worth naming

The adapters throw on a refusal rather than answering with an empty list.

CommitteeManager already worked this way — it passes throwOnError: true, catches, and reports through $handleNetworkError, and it distinguishes "could not be read" from "there are none". AddressManager and RecoveryManager did not: they checked status === 200 and otherwise called console.log. A board member with a failed read saw an association with nobody in it and no indication anything had gone wrong.

All four now report. That is a small, deliberate improvement rather than a pure move, and it is why two tests changed from asserting console.log to asserting the handler.

The tests moved with the pages

They mocked the client the pages no longer call. Each now stands in for the door its page depends on, which is also the boundary that matters when the domain's internals change.

A side effect worth having: the assertions stopped naming request shapes. expect(mockDeleteUserById).toHaveBeenCalledWith(71) says what the page meant; {path: {userId: 71}, throwOnError: true} said how the client spells it, and would have had to change again the next time the wire did.

Verification

yarn vitest run — 189 files, 1627 tests. yarn typecheck and eslint clean.

grep -rl "@/services/api" src/pages/management returns nothing, including the import("@/services/api").MembershipResponse inline type annotation that a file-level grep would have missed.

Debt list: 55 → 51.


Diff breakdown added removed, scaled to the largest row.

frontend                                          +253   -200   15
  production         ██████████████████░░░░░░░    +157    -63   10
  unit tests         ███████████░░░░░░░░░░░░░░░    +96   -133    4
  build & config     ░                              +0     -4    1

──────────────────────────────────────────────────────────────────
production                                        +157    -63
tests                                              +96   -133  0.61 test lines per prod line
total (hand-written)                              +253   -200  15 files

The four manager pages read the generated client directly, which frontend
ADR-001 forbids and #1183 listed as debt.

The user domain gains what they read: the listing, one account, the deleted
accounts, the memberships, the addresses, and the delete. Committees and
recovery get a domain each, because a committee list is not the user domain's
business and neither is which activation an account is waiting for.

The adapters throw on a refusal rather than answering with an empty list. Three
of the four pages already treated a failed read that way; AddressManager and
RecoveryManager logged to the console instead, which tells a board member the
association has nobody in it. All four now report through $handleNetworkError.

The tests move with the pages. They mocked the client the pages no longer call,
and now stand in for the door each page actually depends on — which also means
they assert an intention, deleteUser(71), rather than a request shape,
{path: {userId: 71}, throwOnError: true}.

Fifty-one files left on the list, from fifty-five.

Part of #1167
@ExtraToast ExtraToast added the enhancement New feature or request label Sep 7, 2026
@ExtraToast ExtraToast self-assigned this Sep 7, 2026
@ExtraToast

Copy link
Copy Markdown
Contributor Author

Carried forward from the old file, so worth catching while this page is open: lines 138 and 205 bind @submitting, but CommitteeForm only ever emits submitted (CommitteeForm.vue:45-47, emitted at :114, :130, :135).

The handlers therefore never run, so loading and submittingId are never set and the edit and create buttons never show a saving state. A board member pressing Save on a slow connection sees nothing happen, and nothing stops them pressing again.

Tracked as #1211. I am not opening a competing PR since this file is yours right now — either fix it here and close #1211, or say and I will take it once this lands.

Found while diagnosing #1042, which turned out to be unrelated (a click landing on the wrong element mid-transition, fixed in #1212).

ExtraToast added a commit that referenced this pull request Sep 7, 2026
`RecoveryManagerHelper.openSection` clicked the toggle and returned. Under
`v-expand-transition` the pane shows at about two pixels and grows for some
280ms, and a row inside it holds one bounding box long enough for Playwright to
call it stable — so a restore click is delivered to whatever has arrived at
those coordinates once the pane moves on. The action is never entered, nothing
is refused, and no request is made.

That is what `board deletes user from member manager and restores in recovery
manager` failed with on #1210: the row was there, the click landed, and the
request log ends with the pane's own reads.

#1212 proved the shape on the committee panel and left ExpandPanelHelper behind
for it. The pane gains a test id so the same wait can be used here.

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant