Skip to content

feat(sharing): let a listed key be edited through PATCH /api/sharings/:id - #333

Merged
argszero merged 1 commit into
mainfrom
feat/sharing-edit-endpoint
Sep 30, 2026
Merged

argszero merged 1 commit into
mainfrom
feat/sharing-edit-endpoint

Conversation

@argszero

Copy link
Copy Markdown
Owner

Summary

A listed key could only be paused, resumed or soft-deleted. PATCH /api/sharings/:id took exactly one field (status), so a wrong quota, plan, model, note or time window could only be fixed by deleting the row and listing it again — losing the row's id, its listing time and its ledger ownership.

This first PR is the backend half: PATCH now accepts the same field set as POST /api/sharings as a partial update. The in-page edit form follows in a second PR.

Related Issue

None — this is the backend half of the host's rant 2026-09-30T13:12:07 (translated from the Chinese original: "A listed shared key should be editable in-page, and every setting from listing time must be changeable"), which asks for the endpoint capability separately from the UI work.

Changes

  • src/routes/sharing.rs — PatchSharingReq widened to status + provider / plan / model / key / quota / available / note, all optional: an omitted field is left untouched, a given field is replaced wholesale. status keeps its three values and semantics, soft delete off included. The update keeps WHERE id = ? AND owner_id = ?, and the response still comes from ROW_SELECT, so a row's id, created_at and ledger key_id do not move.
  • src/routes/sharing.rs — the two listing guards (the (provider, model) pair must be priceable, the plan must be routable) were inlined in create; they are now one validate_listing helper called by create with the request's values and by patch with the merged values. This endpoint has already grown a second, hand-written copy of one rule once (C2052), so the tests assert more than "both return 400": they assert the two paths answer with the byte-identical error body, and that a rejected request writes nothing.
  • src/routes/sharing.rs — the upstream key is three-state. The client only ever holds the server-made mask (mask_upstream_key), so key omitted / empty / whitespace keeps the stored ciphertext and only a real new value re-encrypts. Writing the mask back would invalidate the original key permanently (every later call would 401) with nothing in the UI to say so.
  • src/routes/sharing.rs — available distinguishes "omitted" from null via a small serde shim (double_option): omitted = do not touch the window; explicit null = any day, all day, clearing available_days / _start / _end together. A plain Option<Avail> collapses the two and would silently keep a stale window — or, with a per-subfield unwrap_or(cur), leave a half-replaced one. days is serialised exactly as create does it.
  • docs/architecture.md — the API list said PATCH was "pause / offline"; it now names the edit capability, so this line does not drift the moment this merges.
  • No config / data-structure change (no new column, no migration).

Tests

  • cargo test — 451 passed, 0 failed (baseline f63dd66: 446) — 5 new cases.
  • cargo fmt --check — clean.
  • cargo clippy --all-targets -- -D warnings — clean. RUSTFLAGS="-D warnings" cargo build (the non-test build, the only witness for a field read solely from tests) — clean.
  • New tests: full in-place edit of every listing setting (same row, same created_at, days serialised as create does); the stored key survives omitted / empty / whitespace and is replaced on a new value (decrypted encrypted_key must equal the original plaintext and never the mask); partial semantics including available: null clearing all three columns; create/patch validation parity by byte-identical error body plus "no write on 400"; another user's row is 404 while your own row still edits (positive control).
  • A/B (each mutation run, then the tree restored byte-identically): always re-encrypt the key → patch_keeps_the_stored_key_when_no_new_key_is_given fails ("" vs "sk-original9999"); drop the serde shim → patch_only_touches_the_fields_it_is_given fails ("[3]" vs "", i.e. the stale window survived); skip the shared validation in patch → patch_reuses_the_create_validation fails (200 where 400 was required). Each mutation turned exactly its own test red.

Checklist

  • Branch name follows convention (feat/…)
  • Conventional Commits format, no (#N) in the title
  • Single responsibility, minimal change

…/:id

A listing could only be paused, resumed or soft-deleted: PATCH took one
field (`status`), so a wrong quota, plan, model, note or time window could
only be fixed by deleting the row and listing it again.

PATCH now accepts the same field set as POST (provider / plan / model /
key / quota / available / note) as a partial update — an omitted field is
left untouched — while keeping the row's id, its listing time and its
ledger ownership. `status` keeps its values and semantics (including the
soft delete `off`).

Notes on the two spots that are easy to get wrong:

- The upstream key. The client only ever holds the server-made mask
  (`sharing.rs::mask_upstream_key`), so `key` is three-state: omitted or
  blank keeps the stored ciphertext, a real new value replaces it. Writing
  the mask back would invalidate the original key permanently and nothing
  in the UI would say so. The test decrypts `encrypted_key` and asserts it
  still equals the original plaintext and never the mask.
- `available`. Omitted means "do not touch the window"; an explicit `null`
  means "any day, all day" and clears all three columns together. The two
  are told apart by a small serde shim, because a plain `Option<Avail>`
  collapses them and would silently keep a stale window.

The listing guards (the (provider, model) pair must be priceable, the plan
must be routable) were inlined in `create`; they are now one
`validate_listing` helper called by both entry points, so the rule cannot
fork the way it did once before (C2052). The tests assert the two paths
answer with the same error body, not merely the same status.

Tests: 5 new cases (full in-place edit; key preserved on omitted / empty /
whitespace and replaced on a new value; partial semantics incl.
`available: null`; create/patch validation parity; another user's row is
404). Full suite 446 -> 451, `cargo fmt --check` and
`clippy --all-targets -- -D warnings` clean. The three mutations that
should make these fail (always re-encrypt, drop the serde shim, skip the
shared validation) were each run and each turned exactly its own test red.

Backend half of the host rant 2026-09-30T13:12:07 ("a listed key should be
editable in-page, and every setting from listing time must be changeable");
the in-page edit form follows in a second PR.
@argszero

Copy link
Copy Markdown
Owner Author

Self-review (this repo allows self-merge; GitHub will not accept an approval from the author, so the review is recorded here).

Read through the diff on the pushed tree and re-ran the suite locally at ca28626:

  • cargo test — 451 passed, 0 failed.
  • cargo fmt --check, cargo clippy --all-targets -- -D warnings, RUSTFLAGS="-D warnings" cargo build — all clean.
  • CI on this PR — test / fmt / clippy and msrv both green.

Three things I checked by hand rather than trusting the tests:

  1. status is still optional-validated the same way. {"status":"paused"} / {"status":"on"} / {"status":"off"} behave exactly as before; {"status":"deleted"} is still 400 and another user's id is still 404. Making the field optional does not change any existing client's path.
  2. The mask cannot be written back. The only way key reaches encrypt() is req.key being present and non-blank after trimming; the test decrypts the stored ciphertext and asserts it equals the original plaintext and never the mask.
  3. available cannot end up half-replaced. All three columns are written from one match arm each, so there is no path that updates available_days while leaving the old start/end (or the reverse).

Scope note: this is the backend half of the host rant; the in-page edit form is a separate PR, so the capability has no UI consumer yet by design.

@argszero
argszero merged commit eb2c8ca into main Sep 30, 2026
2 checks passed
@argszero
argszero deleted the feat/sharing-edit-endpoint branch September 30, 2026 05:59
argszero added a commit that referenced this pull request Sep 30, 2026
…#334)

A listing could only be paused, resumed or deleted, so a wrong quota,
plan, model, note or time window could only be fixed by deleting the row
and listing it again — which also threw away the row's id, its listing
time and its earnings attribution.

This is the frontend half of the host's work order
`2026-09-30T13:12:07` ("a listed shared key should be editable in page,
and every listing-time setting must be changeable"); the endpoint half
landed as #333, where PATCH /api/sharings/:id started accepting the full
create field set as a partial update.

Each row gains an edit action that opens the **same** inline form
(`#share-form-card`), pre-filled with that row's current values: edit mode
is a module-level `editingShareId`, and create and edit share one submit
handler, one payload builder, one availability encoding and one
validation path — mirroring the server, where create and patch share
`validate_listing`. No second form, no whole-row inline editor.

The pre-fill walks provider -> Plan -> model in order, dispatching a
`change` at each level, because the model list is built from the plan's
provider; the "every day" shortcut's property-based `disabled` is restored
from the row's day set on the same path.

The key field is the part that is easy to get wrong, so it is the part
under a gate. The client only ever holds the server-made mask, and the
mask belongs in `placeholder`, never in `value`: a mask in `value` would
be submitted as the new key, the server would encrypt the mask into
`encrypted_key`, the original key would stop working permanently and
nothing in the UI would say so. The field's only way into the payload is
the guarded `if (key) payload.key = key;` (omitted = keep the stored
ciphertext), and every programmatic write to that field writes the empty
string.

Also: `parseShareDays` becomes the single parse point for
`available_days` (the list renderer and the pre-fill had two copies of the
same `JSON.parse`), and i18n gains `share.form.editTitle` /
`share.edit.ok` / `share.edit.fail` in both packs.

Tests: `state_gate::the_share_edit_form_never_submits_the_stored_key_mask`
with three derived rules plus roster / teeth / scanner self-checks.
`cargo test` 455 passed (baseline 451).
@argszero argszero mentioned this pull request Sep 30, 2026
12 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant