Withhold the controls a shared record refuses - #711
Closed
MBombeck wants to merge 6 commits into
Closed
Conversation
Nine surfaces offered a delegate an action the server declines, and the switch had never been asked about any of them. The Today rail's dismiss and the coach check-in's keep / let-go write through routes that resolve the caller, so both refuse under a switch; they now ask canManage and are absent at either grant level. A priority action with no href and no handler is no longer rendered as an inert button at all — the type carries that rule now. The Vorsorge dashboard card offered the mark-done that /checkups already withholds, and the checkups list branch inlined its own ungated copy of the button the cards branch gates, so which view a browser last chose decided whether the action appeared. Both bind canManage. The chart overlay cog persists through a route that resolves the caller and stores a preference belonging to the person rather than the record. Gated once in the control so the three chart wrappers cannot drift. Three query parameters opened sheets past a gated button: ?add=<TYPE> on measurements, ?new=1 on medications, and ?edit=1 on a medication detail, the last of which opens a wizard refused at both levels. A deep link is the same affordance as the control that produces it. The efficacy retarget dial rewrites a setting the owner chose. The episode documents card offered a link and an upload the vault gates on the same endpoint throughout. And the dose ledger built its own success toast, missing both the Undo suppression and the "saved to" receipt its two siblings already carry — the one a delegate met on every dose. The Coach drawer is not mounted inside a shared record, so every button calling askCoach() opened nothing. The launch provider now publishes no value there and each entry point's existing null check does the rest; the documents sheet, the one that rendered without checking, now checks.
Three of the newly gated surfaces are query-backed, so the suite gains a query client and a cache seed rather than a hook mock: the component under test stays the real one. Each leg asserts the rail, the card and the reminder row are still THERE — only the mutating control is gone. Breaking each gate in turn puts every leg red.
…hing calls Three surfaces record a dose and all three had to reach the same two conclusions inside somebody else's record: name the record, and drop an Undo the server refuses. Two learned it and the dose ledger did not, because the ternary was written out three times. It is written once now, in the file the other two already share, with its own test. POST /api/allergies and POST /api/family-history leave the frozen delegable write set. The argument that admitted them still stands — an allergy is the single most useful thing a caregiver can contribute — but the only surface that posts to either lives in Settings, which a switch closes, so no delegate could reach the form at any level. That is a permission frozen ahead of the caller for it, and the list is built the other way round on purpose. Both delegable READ arms stay. A caregiver reading the allergy list is what the feature is for; only the contribute step waits, and it comes back in the same diff as the surface that offers it.
Every test in e2e/delegated-writes.spec.ts had been skipping since the day the invitation form shipped its level control. The skip guard looked for data-slot="grant-invite-level"; the control landed as "grant-invite-access-option". The file said out loud what to change and nobody changed it, so a quiet skip and a passing suite read the same in a CI summary. Two assertions inside it had never been executed and were both wrong: the header button reads "Add", not "Add measurement", and the form opens on blood pressure, which has no `value` field. Fixed against stable attributes rather than viewport text, with data-slot="measurement-add" added to the control the journey clicks. Adds the deep-link leg the SSR suite cannot hold: a query parameter is the same affordance as the button, and ?add= / ?new=1 open exactly what the resolved level admits.
… proof Taking `POST /api/allergies` and `POST /api/family-history` out of the delegable set moved one end and left the other: the integration suite still asserted that a delegate holding a WRITE grant gets a 201 from both, so ten cases went red on a decision that was deliberate. They are pinned the other way round now, with the reason in the file. A delegate is refused at either grant level, nothing lands under either account, and — the leg that matters most and that nothing else here covered — the owner's own unswitched write still succeeds. Withdrawing a delegated write by breaking the ordinary one would have been the worse bug.
Owner
Author
|
Superseded by #712, which carries this branch's commits plus the shared front door and the side-effect backup coverage. The ten red integration cases are resolved there: this branch withdrew |
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
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.
The second half of the adversarial sweep. Every control here either posted to a
route that refuses under a switch, or opened a drawer that no longer exists.
One severity claim in the review was backwards and is worth correcting. The
digest dismiss and the coach check-in were reported as writing to the delegate's
own record while presenting as the owner's. They do not:
requireAuth()throwsas soon as the session carries an acting stamp, so both refuse and no row lands
anywhere. They are gone regardless, because an affordance the server is on
record refusing does not belong on the page.
Fixed, all absent rather than disabled and all bound to the one capability hook:
the vorsorge dashboard card (matching what
/checkupsalready did), the vorsorgelist branch whose inlined button had escaped the gate its sibling carried, the
chart overlay control gated once inside itself so eight chart keys cannot drift,
the
?add=?new=1?edit=1deep links gated at the sheet rather than theparameter so the first-paint window withdraws instead of standing on a refused
form, the efficacy retarget, and the illness document link and upload.
The dose-history Undo is fixed at the root rather than as a fourth copy: the
take and skip toast decision now lives once and all three intake surfaces use it.
The coach sweep found the real shape. The drawer is unmounted under a switch
while its provider kept publishing, so the launcher set a flag nothing read. The
provider publishes nothing there now and each consumer's existing empty check
does the rest. Two corrections to the brief: there is no dashboard hero coach
call to action, and every launcher mount except one lives under a route the shell
already closes. The single reachable dead button was in the document sheet.
Allergies and family history come out of the frozen write list. The product
argument for admitting them stands; what they lack is a caller. Their only form
lives in Settings, which is not a shared destination, so no delegate reaches it
at any level. Admitting a route with no surface is the one-ended change this
repository keeps rediscovering. Re-admitting is one line per route when the
surface lands with it. Both read arms stay.
Two things the review missed. The dashboard is a shared destination but ten
of its routes refuse there, so the hero renders an error card and the tile strip
has no snapshot. That is a route-classification decision rather than a control
gate and is left for one. And
e2e/delegated-writes.spec.tshad never run: itsskip guard named a slot that shipped under a different name, so every test
skipped silently and two assertions inside had been wrong since they were
written. It passes five of five now, against stable attributes.