Claude: changes for issue #111 #127

Merged
Grandiras merged 1 commit from claude/issue-111 into main 2026-08-19 19:32:53 +00:00
Collaborator

Closes #111.

Closes #111.
Let a card move to another board
All checks were successful
CI / build (pull_request) Successful in 36s
8b26379a8b
A card could only ever be re-filed within the board it was on: a drag
never leaves it, and CardService.MoveCardAsync refused a column belonging
to another board outright. The reason it refused was the interop the
issue names — a board owns its columns and its tags, so a card arriving
from somewhere else arrives carrying names that mean nothing.

The card dialog now has a fourth details panel, "Move to another board":
a menu of the other boards this member can see, a sentence saying what
the move would do, and a Move button. The rule both halves of that answer
share is one sentence — the card keeps whatever the other board also has
a word for. It lands in the column named the same as the one it is
leaving, or in that board's first column; it arrives wearing that board's
own tags of the same names, and what has no counterpart is left behind
rather than created. Matching is by name, trimmed and case-insensitive
with exact spelling preferred, because the names are ciphertext and the
database can neither join nor compare them.

Nothing is dropped quietly: the dialog names the tags that will not come
along before the move, and the toast names them after it.

- Services/CardMove.cs is that rule, pure and static like BoardFilter, so
  CardMoveTests can pin the cases that would cost a board apiece in
  Playwright.
- CardService.MoveCardToBoardAsync performs it and PlanMoveAsync answers
  the same question without writing. Both take the actor: this is a card
  edit (it changes what the card is tagged with and takes it off the
  board its readers were on), so it follows the card's own rule rather
  than the drag's, and both check the target board against the viewer —
  it is the one board id in this service that a member puts on the wire.
- A target board with no columns is refused by name
  (BoardHasNoColumnsException) and said so on screen.
- Both boards are notified; the one being left is the one that would
  otherwise go stale.
- The calendar's copy of the dialog passes no boards, for the reason it
  passes IsNew="false".

tests/e2e/boards.spec.js covers it end to end with the only fixture that
can: two boards, a tag both have and a tag only one has, and a card in a
column the other board also has a word for.
Author
Collaborator

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/309); the reply will link the full session transcript. <!-- claude-run:pr_review --> 📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-127/run-309.md) — updating every ~5 minutes while the task runs.
claude-reviewer left a comment

Verdict: approve — no blocking issues found.

Solid, well-scoped implementation of cross-board card moves. CardMove (pure, static) cleanly separates the name-matching rule from CardService, which mirrors the pattern used for BoardFilter; the unit tests in CardMoveTests cover the interesting edge cases (case/whitespace matching, exact-over-case-insensitive preference, duplicate merge-on-arrival, no-columns, no-matching-tags). MoveCardToBoardAsync and PlanMoveAsync are symmetric (both re-check target board access via RequireTargetBoardAsync, both use the same CardMove rule), so the plan the dialog shows can't drift from what the move actually does. The write path (db.SaveChangesAsync once, after all reads) is atomic, and BoardHasNoColumnsException is thrown before any mutation happens, so a refused move leaves no partial state. Permission handling looks right: MoveCardToBoardAsync requires CardPermissions.RequireCardEdit (author-or-admin, matching the doc's stated rule for this operation), while PlanMoveAsync being read-only doesn't need it — the source card is always one the actor can already see (opened from a board they're on), so the only access boundary that matters for a preview is the target board, and that is checked. Docs (boards-and-permissions.md, calendar.md, orientation.md, ui.md) are updated in step with the code, and the changelog/locale entries are added in both en/de.

One minor, non-blocking observation: in CardModal.razor, OnMoveTargetPicked/OnSelectedTagsChanged call RequestMovePlan() without any request-ordering guard, and BoardPage.OnMoveTargetChanged doesn't check that the TargetBoardId it's about to answer for is still the one currently selected before assigning _movePlan. If a member switches the target-board dropdown twice in quick succession, the two PlanMoveAsync calls could resolve out of order and the UI could briefly show the sentence ("Lands in ‚Done'", dropped tags) for the board that is not the one currently selected. It's cosmetic only — MoveCardToBoardAsync recomputes the real plan fresh from the database at write time, so the actual move is unaffected — but worth a stale-response guard (e.g. compare args.TargetBoardId against _moveTargetId, or a generation counter) if it's ever noticed in practice.

Also worth double-checking outside this diff: MoveCardToBoardAsync's landing-column max-position computation (db.Cards.Where(...).MaxAsync() ?? 0.0, then +1.0) is a read-then-write without a transaction/lock, so two concurrent moves into the same column could compute the same max and collide on position. If that's an accepted, pre-existing pattern elsewhere in CardService (e.g. MoveCardAsync), this is just consistent with it and not a new risk introduced here.


⏱ 2m 45s · 3 turns · tokens: 58.6k in / 14.7k out (+168.5k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. Solid, well-scoped implementation of cross-board card moves. `CardMove` (pure, static) cleanly separates the name-matching rule from `CardService`, which mirrors the pattern used for `BoardFilter`; the unit tests in `CardMoveTests` cover the interesting edge cases (case/whitespace matching, exact-over-case-insensitive preference, duplicate merge-on-arrival, no-columns, no-matching-tags). `MoveCardToBoardAsync` and `PlanMoveAsync` are symmetric (both re-check target board access via `RequireTargetBoardAsync`, both use the same `CardMove` rule), so the plan the dialog shows can't drift from what the move actually does. The write path (`db.SaveChangesAsync` once, after all reads) is atomic, and `BoardHasNoColumnsException` is thrown before any mutation happens, so a refused move leaves no partial state. Permission handling looks right: `MoveCardToBoardAsync` requires `CardPermissions.RequireCardEdit` (author-or-admin, matching the doc's stated rule for this operation), while `PlanMoveAsync` being read-only doesn't need it — the source card is always one the actor can already see (opened from a board they're on), so the only access boundary that matters for a preview is the target board, and that is checked. Docs (`boards-and-permissions.md`, `calendar.md`, `orientation.md`, `ui.md`) are updated in step with the code, and the changelog/locale entries are added in both `en`/`de`. One minor, non-blocking observation: in `CardModal.razor`, `OnMoveTargetPicked`/`OnSelectedTagsChanged` call `RequestMovePlan()` without any request-ordering guard, and `BoardPage.OnMoveTargetChanged` doesn't check that the `TargetBoardId` it's about to answer for is still the one currently selected before assigning `_movePlan`. If a member switches the target-board dropdown twice in quick succession, the two `PlanMoveAsync` calls could resolve out of order and the UI could briefly show the sentence ("Lands in ‚Done'", dropped tags) for the board that is *not* the one currently selected. It's cosmetic only — `MoveCardToBoardAsync` recomputes the real plan fresh from the database at write time, so the actual move is unaffected — but worth a stale-response guard (e.g. compare `args.TargetBoardId` against `_moveTargetId`, or a generation counter) if it's ever noticed in practice. Also worth double-checking outside this diff: `MoveCardToBoardAsync`'s landing-column max-position computation (`db.Cards.Where(...).MaxAsync() ?? 0.0`, then `+1.0`) is a read-then-write without a transaction/lock, so two concurrent moves into the same column could compute the same max and collide on position. If that's an accepted, pre-existing pattern elsewhere in `CardService` (e.g. `MoveCardAsync`), this is just consistent with it and not a new risk introduced here. --- ⏱ 2m 45s · 3 turns · tokens: 58.6k in / 14.7k out (+168.5k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-127/run-309.md) <!-- claude-verdict:approve --> <!-- claude-reviewed-head:8b26379a8bb2298227a475e780f9173dc64792bd -->
Grandiras deleted branch claude/issue-111 2026-08-19 19:32:53 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Grandiras/Ponente!127
No description provided.