feat(webapp): campaign ownership — owner may be a player, owner-set GMs, transfer with acceptance (#644) #648

Merged

claude-bot merged 1 commit from feat/644-campaign-ownership into main 2026-09-14 06:13:44 +00:00

Conversation 0 Commits 1 Files changed 73 +3794 -304

claude-bot commented 2026-09-14 05:55:39 +00:00

Collaborator

Copy link

Implements #644. A campaign gets an owner (campaigns.owner_id), separate from its GMs: the one account accountable for the campaign, and the anchor for hosted billing (#642). The owner is always a member, as a player or a GM, and owning grants no content access. An owner who is a player still gets 403 on beat notes, GM notes and GM transcript views.

⚠️ Before deploying to prod: run this read-only check

The migration fails loudly (it raises and makes no change) if any campaign has no GM to become its owner. Run this against prod first:

SELECT count(*) FROM campaigns c
WHERE NOT EXISTS (SELECT 1 FROM campaign_members m WHERE m.campaign_id = c.id AND m.role = 'gm');

If it's non-zero, add a GM to each of those campaigns first. Dev: 3 campaigns, 0 without a GM (checked 2026-09-14).

Behaviour changes for self-hosters

• Only the owner can:
  • archive and un-archive; un-archive also stays open to admins, and was admin-only before
  • trash a campaign, and restore it from the trash
  • make a member a GM or remove the GM role
  • transfer ownership
• Existing campaigns: the owner is the earliest-joined GM.
• A GM can now step down, and a campaign may have no GM. The owner sees "This campaign has no GM. Assign one." The "last GM can't leave" rule is gone.
• Account deletion is blocked by campaigns you own, including trashed ones, rather than by being the only GM. A sole GM who isn't the owner can delete their account. This fixes the old message that told you to "transfer GM" when no transfer existed.
• The trash list (/trash/mine) shows campaigns you own, since only the owner can restore them.

Design

• ownership_service, which never commits; each route commits once, after its audit entry.
  • Transfer is an offer the recipient accepts. The table is campaign_ownership_offers, with a partial unique index allowing one open offer per campaign.
  • Expiry is lazy: after 7 days an offer is resolved as expired the next time it's read. There's no Celery job.
  • Accept locks the offer row (FOR UPDATE with populate_existing), so two accepts can't both succeed. It also re-checks that the recipient is still a member, and that the offer came from the current owner (an admin may have reassigned ownership meanwhile). Either failure cancels the offer with a 409.
  • A refusal that resolves an offer (expired, stale) is committed before the error is returned, so the resolution sticks.
  • A comment marks where #642's "recipient needs a free slot" check will go. Nothing billing-related is built.
• Permissions:
  • require_owner and require_owner_or_admin dependencies.
  • The owner half of invite_service.can_manage_membership is filled in, so a player who owns the campaign can manage invites and join requests.
  • Removing members:
    • GMs can remove players.
    • Only the owner can remove a GM.
    • Nobody can remove the owner.
    • The owner can't leave while they own the campaign.
  • Leaving or being removed cancels any offer made to you.
  • Adding a member directly (POST /members, /members/search) is also open to the owner, since it's the same decision as approving a join request.
• Audit log: every role change (only when the role actually changes), offer, cancellation, transfer and admin reassignment is recorded.
• Admin recovery: PUT /api/admin/campaigns/{id}/owner (the target must be a member; any open offer is cancelled). API only, because there's no admin campaign page to put a control on.
• Creation and import: the creating or importing user becomes owner and GM.
• Pending offer: available from its own GET …/ownership/offer, which returns the offer to the owner and the recipient and null to everyone else.

Migration e4f5a6b7c8da (on d3e4f5a6b7c8)

• The issue named e4f5a6b7c8d9, which already exists (chain #21, lore proposal targets), so the id's last character was bumped, following the precedent f4a5b6c7d8ea set for #405.
• Steps:
  1. Backfill from the earliest-joined GM, tie-broken by user_id so reruns are deterministic.
  2. A DO $$ … RAISE EXCEPTION guard names every campaign that couldn't get an owner.
  3. Set NOT NULL.
  4. Add the FK with ON DELETE RESTRICT.
  5. Create the offers table with CHECK constraints.
• Round trip, on a scratch DB:
  • The two-GM campaign got its earliest GM as owner, not the first row inserted.
  • Downgrade then upgrade gave the same owners.
  • A campaign with only a player made the upgrade exit 1 with the named id, leaving the schema at d3e4f5a6b7c8.
• The migration's real upgrade() is also exercised by test_campaign_ownership.py.
• The head pin moved, and webapp/CLAUDE.md's migration chain lists the new revision.

Also fixed (found in passing)

to_campaign_response never copied transcription_language (#419). The edit form always showed "detect", and saving the form wiped the setting. There's a CHANGELOG "Fixed" entry.

Bot

The only role-dependent bot call is /record authorisation, via GET /api/bot/guilds/{id}/gms. With no GM that returns an empty list, so the bot refuses unless the user has Manage Server, which is the intended no-GM behaviour. Nothing else depends on GM roles, so no contract change and no BOT_CONTRACT_VERSION bump.

Verification

• About 40 guards mutation-checked, each seen to fail and then restored. They include every owner dependency, the owner half of can_manage_membership, the offer lock (a real two-connection race test), lazy expiry, the stale-offer re-checks, the RESTRICT backstop on account deletion, and the migration's RAISE and tiebreak.
• Backend full suite, rebased onto main (with #646 and #647): 3606 passed, 13 skipped.
• Frontend: 905 passed, eslint clean.
• Lint and versions: ruff 0.4.4 and version-sync clean.

Not fixed here

• docs/API.md still describes PATCH /members/{user_id} as character_name only, and the members response fields are out of date.
• The _reject_if_trashed docstring still says "the four dependencies".

Closes #644

🤖 Generated with Claude Code

Implements #644. A campaign gets an **owner** (`campaigns.owner_id`), separate from its GMs: the one account accountable for the campaign, and the anchor for hosted billing (#642). The owner is always a member, as a player or a GM, and **owning grants no content access**. An owner who is a player still gets 403 on beat notes, GM notes and GM transcript views. ## ⚠️ Before deploying to prod: run this read-only check The migration **fails loudly** (it raises and makes no change) if any campaign has no GM to become its owner. Run this against prod first: ```sql SELECT count(*) FROM campaigns c WHERE NOT EXISTS (SELECT 1 FROM campaign_members m WHERE m.campaign_id = c.id AND m.role = 'gm'); ``` If it's non-zero, add a GM to each of those campaigns first. **Dev: 3 campaigns, 0 without a GM** (checked 2026-09-14). ## Behaviour changes for self-hosters - **Only the owner can:** - archive and un-archive; un-archive also stays open to admins, and was admin-only before - trash a campaign, and restore it from the trash - make a member a GM or remove the GM role - transfer ownership - **Existing campaigns:** the owner is the earliest-joined GM. - **A GM can now step down**, and a campaign may have no GM. The owner sees "This campaign has no GM. Assign one." The "last GM can't leave" rule is gone. - **Account deletion is blocked by campaigns you *own*,** including trashed ones, rather than by being the only GM. A sole GM who isn't the owner can delete their account. This fixes the old message that told you to "transfer GM" when no transfer existed. - **The trash list (`/trash/mine`) shows campaigns you own**, since only the owner can restore them. ## Design - **`ownership_service`**, which never commits; each route commits once, after its audit entry. - **Transfer is an offer the recipient accepts.** The table is `campaign_ownership_offers`, with a partial unique index allowing one open offer per campaign. - **Expiry is lazy:** after 7 days an offer is resolved as `expired` the next time it's read. There's no Celery job. - **Accept locks the offer row** (`FOR UPDATE` with `populate_existing`), so two accepts can't both succeed. It also re-checks that the recipient is still a member, and that the offer came from the *current* owner (an admin may have reassigned ownership meanwhile). Either failure cancels the offer with a 409. - A refusal that resolves an offer (expired, stale) is committed before the error is returned, so the resolution sticks. - A comment marks where #642's "recipient needs a free slot" check will go. Nothing billing-related is built. - **Permissions:** - `require_owner` and `require_owner_or_admin` dependencies. - The owner half of `invite_service.can_manage_membership` is filled in, so a player who owns the campaign can manage invites and join requests. - **Removing members:** - GMs can remove players. - Only the owner can remove a GM. - Nobody can remove the owner. - The owner can't leave while they own the campaign. - Leaving or being removed cancels any offer made to you. - Adding a member directly (`POST /members`, `/members/search`) is also open to the owner, since it's the same decision as approving a join request. - **Audit log:** every role change (only when the role actually changes), offer, cancellation, transfer and admin reassignment is recorded. - **Admin recovery:** `PUT /api/admin/campaigns/{id}/owner` (the target must be a member; any open offer is cancelled). API only, because there's no admin campaign page to put a control on. - **Creation and import:** the creating or importing user becomes owner and GM. - **Pending offer:** available from its own `GET …/ownership/offer`, which returns the offer to the owner and the recipient and `null` to everyone else. ## Migration `e4f5a6b7c8da` (on `d3e4f5a6b7c8`) - The issue named `e4f5a6b7c8d9`, which **already exists** (chain #21, lore proposal targets), so the id's last character was bumped, following the precedent `f4a5b6c7d8ea` set for #405. - **Steps:** 1. Backfill from the earliest-joined GM, tie-broken by `user_id` so reruns are deterministic. 2. A `DO $$ … RAISE EXCEPTION` guard names every campaign that couldn't get an owner. 3. Set NOT NULL. 4. Add the FK with **ON DELETE RESTRICT**. 5. Create the offers table with CHECK constraints. - **Round trip, on a scratch DB:** - The two-GM campaign got its earliest GM as owner, not the first row inserted. - Downgrade then upgrade gave the same owners. - A campaign with only a player made the upgrade exit 1 with the named id, leaving the schema at `d3e4f5a6b7c8`. - The migration's real `upgrade()` is also exercised by `test_campaign_ownership.py`. - The head pin moved, and `webapp/CLAUDE.md`'s migration chain lists the new revision. ## Also fixed (found in passing) `to_campaign_response` never copied `transcription_language` (#419). The edit form always showed "detect", and saving the form wiped the setting. There's a CHANGELOG "Fixed" entry. ## Bot The only role-dependent bot call is `/record` authorisation, via `GET /api/bot/guilds/{id}/gms`. With no GM that returns an empty list, so the bot refuses unless the user has Manage Server, which is the intended no-GM behaviour. Nothing else depends on GM roles, so **no contract change and no `BOT_CONTRACT_VERSION` bump**. ## Verification - **About 40 guards mutation-checked,** each seen to fail and then restored. They include every owner dependency, the owner half of `can_manage_membership`, the offer lock (a real two-connection race test), lazy expiry, the stale-offer re-checks, the RESTRICT backstop on account deletion, and the migration's RAISE and tiebreak. - **Backend full suite, rebased onto main** (with #646 and #647): **3606 passed, 13 skipped**. - **Frontend: 905 passed**, eslint clean. - **Lint and versions:** ruff 0.4.4 and version-sync clean. ## Not fixed here - `docs/API.md` still describes `PATCH /members/{user_id}` as `character_name` only, and the members response fields are out of date. - The `_reject_if_trashed` docstring still says "the four dependencies". Closes #644 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 1 commit 2026-09-14 05:55:39 +00:00

feat(webapp): campaign ownership — owner may be a player, owner-set GMs, transfer with acceptance (#644) ... 14ca23e78b

"GM" meant both "runs the game" and "is responsible for the campaign": any
GM could archive or trash, nothing could make anyone a GM, the last GM could
never leave, and account deletion told a sole GM to "transfer GM" with no way
to. Ownership is now a property of the campaign.

Model and migration
- campaigns.owner_id NOT NULL, FK users ON DELETE RESTRICT. The owner is
  always a member (player or GM) and owning grants no content access.
- campaign_ownership_offers: TEXT + CHECK outcome
  (accepted|declined|cancelled|expired), partial unique index for one open
  offer per campaign, CASCADE on campaign and both users.
- Migration e4f5a6b7c8da (down d3e4f5a6b7c8), raw SQL. Backfills from the
  earliest-joined GM (joined_at, then user_id) and RAISEs, naming the
  campaigns, if any has no GM. Revision id is not e4f5a6b7c8d9: that id
  already exists (#21, lore proposal targets) and a duplicate gives two heads.

Permissions
- require_owner / require_owner_or_admin in auth/dependencies.py.
- Owner only: PUT /members/{user_id}/role, archive, trash, transfer offer and
  cancel. Un-archive (/restore) and untrash (/undelete): owner or admin.
- can_manage_membership gains the owner half; POST /members and
  /members/search move onto it too ("GM or owner").
- Removal: the owner removes anyone else; a GM removes players only (403 on
  another GM); nobody removes the owner. The owner cannot leave. Both
  last-GM counts are gone: a campaign may have no GM. CampaignResponse
  carries owner, is_owner and has_gm.
- Trash list is scoped to the owner, since only the owner can untrash.

Transfer
- POST/DELETE/GET /ownership/offer, POST /ownership/accept and /decline.
  Accept locks the offer row (FOR UPDATE, populate_existing) and sets
  owner_id in the same transaction; re-checks membership and that the offer
  came from the current owner. Expiry is lazy (7 days), resolved on read.
  A leaving or removed member's open offer is cancelled. Placeholder comment
  for #642's hosted free-slot check in accept_offer; no billing built.
- Every role change, offer, cancel, decline, transfer and admin reassignment
  writes an audit entry.

Account deletion and admin
- Deletion blocks on owned campaigns (trashed included, since owner_id is
  RESTRICT); a sole GM who is not the owner can delete. 409 body shape kept.
- PUT /api/admin/campaigns/{id}/owner (target must be a member; cancels any
  open offer). Admin inventory lists the owner. API only, no admin UI.

Frontend
- CampaignDetail: Owner badge, owner-only Make GM / Remove GM, Transfer
  ownership, pending offer with Cancel, Accept/Decline for the recipient,
  no-GM banner; Archive/Delete/Restore for the owner; invites and join
  requests for GM or owner; Leave for everyone but the owner.
- Profile and Admin deletion screens list owned campaigns.

Also fixed: to_campaign_response never copied transcription_language, so the
edit form always read "detect" and saving it wiped the setting (#419).

Bot: /bot/guilds/{id}/gms still returns GMs; an empty list denies /record
unless the invoker has Manage Server, which is the intended no-GM behaviour.
No contract change, no BOT_CONTRACT_VERSION bump.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

claude-bot merged commit 0c759ce92c into main 2026-09-14 06:13:44 +00:00

claude-bot referenced this pull request from a commit 2026-09-14 06:13:45 +00:00

Merge PR #648: feat(webapp): campaign ownership — owner may be a player, owner-set GMs, transfer with acceptance (#644)

claude-bot deleted branch feat/644-campaign-ownership 2026-09-14 06:13:46 +00:00

claude-bot referenced this pull request 2026-09-14 06:13:47 +00:00

[Backend+Frontend] Campaign ownership: an owner who may be a player, owner-set GMs, transfer with acceptance #644

claude-bot referenced this pull request 2026-09-15 18:21:01 +00:00

[UX] Redesign coverage: where every shipped and planned feature lives in Design A #656

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

backend

Issue related to the backend

  

bot

Issue related to the bot

  

bug

Something is not working

  

duplicate

This issue or pull request already exists

  

enhancement

New feature

  

frontend

Issue related to the frontend

  

help wanted

Need some help

  

invalid

Something is wrong

  

question

More information is needed

  

recording

Issue related to the recording system

  

security

Security hardening / vulnerability work

  

wontfix

This won't be fixed

No labels

backend

bot

bug

duplicate

enhancement

frontend

help wanted

invalid

question

recording

security

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

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

rbrooks/Quest-Board!648

No description provided.