An imported character sheet keeps the old server's pack and record ids in its content rows #1095

Closed
opened 2026-10-05 04:51:50 +00:00 by claude-bot · 3 comments
Collaborator

Found while building #1070 (PR for it open). It dates from #1053, which added character sheets to the export.

What

A character sheet's content rows can point at a rules record: entries[].ref.{pack_id, entry_id}, held in JSON, not as a foreign key. The export writes those ids as they are, and the import stores them as they are. On another server they are the old server's ids.

sheet_service.resolve_entries looks records up by that stored entry_id. So:

  • Normally: the id matches nothing on the new server, and the row stands without its rule. That is the same loss #1070 fixes for shelf cards, prep items and pages.
  • An id from a file is honoured. If the id happens to be, or is made to be, the id of a record on this server, the sheet resolves to it. The lookup is behind _usable, and a UUID has to be known or guessed.

Read from the code by the delegate; not reproduced. The first thing to do is confirm what _usable allows: in particular whether a record in another campaign's own pack could be reached this way.

Intended approach

  • On import, rewrite each row's reference with #1070's resolver (pack key, kind and slug), so a row links to this server's record or to nothing. Never keep an id that came from a file.
  • On export, write the stable reference and not the ids. A new export schema version.
  • Check what the sheet evaluator keys on before changing the shape of ref: that is why #1070 left it alone.
  • Tests: an imported sheet row re-links by key and slug; a row whose pack is absent stands alone; an id from the file is never stored or resolved; a record in another campaign's pack cannot be reached.

#1070, #1053, #958.

Found while building #1070 (PR for it open). It dates from #1053, which added character sheets to the export. ## What A character sheet's content rows can point at a rules record: `entries[].ref.{pack_id, entry_id}`, held in JSON, not as a foreign key. The export writes those ids as they are, and the import stores them as they are. On another server they are the old server's ids. `sheet_service.resolve_entries` looks records up by that stored `entry_id`. So: - **Normally:** the id matches nothing on the new server, and the row stands without its rule. That is the same loss #1070 fixes for shelf cards, prep items and pages. - **An id from a file is honoured.** If the id happens to be, or is made to be, the id of a record on this server, the sheet resolves to it. The lookup is behind `_usable`, and a UUID has to be known or guessed. Read from the code by the delegate; not reproduced. The first thing to do is confirm what `_usable` allows: in particular whether a record in another campaign's own pack could be reached this way. ## Intended approach - On import, rewrite each row's reference with #1070's resolver (pack key, kind and slug), so a row links to this server's record or to nothing. Never keep an id that came from a file. - On export, write the stable reference and not the ids. A new export schema version. - Check what the sheet evaluator keys on before changing the shape of `ref`: that is why #1070 left it alone. - Tests: an imported sheet row re-links by key and slug; a row whose pack is absent stands alone; an id from the file is never stored or resolved; a record in another campaign's pack cannot be reached. ## Related #1070, #1053, #958.
Author
Collaborator

The guard checked, by reading: this is a lost link, not a way to reach another campaign's content.

sheet_service._usable allows a record only when its pack is active and is either an instance pack or this campaign's own:

pack is not None
and pack.status == "active"
and (pack.scope == "instance" or pack.campaign_id == campaign_id)

Every lookup in resolve_entries goes through it. So an id taken from an import file can resolve only to a record the campaign could already use on this server. A record in another campaign's pack is refused, and so is one in a disabled pack.

What is left of this issue: an imported sheet's rows lose their rule on a new server, because the stored ids match nothing there. That is the same loss #1070 fixes for cards, prep items and pages. The approach above still stands; the "never keep an id from a file" part is tidiness, not a fix for an exposure.

Not confirmed by running. The test in the plan (a record in another campaign's pack cannot be reached) should still be written.

**The guard checked, by reading: this is a lost link, not a way to reach another campaign's content.** `sheet_service._usable` allows a record only when its pack is active and is either an instance pack or this campaign's own: ```python pack is not None and pack.status == "active" and (pack.scope == "instance" or pack.campaign_id == campaign_id) ``` Every lookup in `resolve_entries` goes through it. So an id taken from an import file can resolve only to a record the campaign could already use on this server. A record in another campaign's pack is refused, and so is one in a disabled pack. **What is left of this issue:** an imported sheet's rows lose their rule on a new server, because the stored ids match nothing there. That is the same loss #1070 fixes for cards, prep items and pages. The approach above still stands; the "never keep an id from a file" part is tidiness, not a fix for an exposure. Not confirmed by running. The test in the plan (a record in another campaign's pack cannot be reached) should still be written.
Author
Collaborator

Picking this up, on feat/1095-sheet-rows-relink, now that #1094 is merged.

First, the facts: exactly what a sheet row's ref holds, what reads each field, and whether sheet versions hold the same references. If the sheet evaluator's key makes the change unsafe or large, nothing is built and I report the smallest safe design.

If it is contained (export schema 20):

  • A sheet row's rule is written by pack key, kind and slug, as #1094 writes a card's. No ids in the file.
  • On import the row gets this server's ids, or the link is cleared and the row stands alone. An id from a file is never stored.
  • An older file's rows come in with their foreign ids cleared.
  • A sheet must evaluate identically after a round trip with the same pack.
  • Sheet rows are counted in the import's existing rule-links summary.
**Picking this up**, on `feat/1095-sheet-rows-relink`, now that #1094 is merged. **First, the facts:** exactly what a sheet row's `ref` holds, what reads each field, and whether sheet versions hold the same references. If the sheet evaluator's key makes the change unsafe or large, nothing is built and I report the smallest safe design. **If it is contained** (export schema 20): - A sheet row's rule is written by pack key, kind and slug, as #1094 writes a card's. No ids in the file. - On import the row gets this server's ids, or the link is cleared and the row stands alone. An id from a file is never stored. - An older file's rows come in with their foreign ids cleared. - A sheet must evaluate identically after a round trip with the same pack. - Sheet rows are counted in the import's existing rule-links summary.
Author
Collaborator

Fixed in PR #1101, merged to main (export schema 20). CI was green on the PR's head.

  • In the file: a sheet's rules are named by pack key, kind and slug in all four places a sheet held ids. No id is written.
  • On import: each is linked to this server's record, or cleared so the row stands on its snapshot. No id from a file is stored.
  • A file of schema 19 or less: its ids are dropped on the way in.
  • Evaluation: identical after a round trip with the same pack, and the same numbers from the snapshot when the pack is absent.

Known and accepted: an export made before this, imported on the server it came from, loses its sheet links. Exporting again fixes it.

Not done: sheets imported before this fix still hold foreign ids. They are harmless, since _usable refuses them and a test proves it; cleaning them up would need a data migration.

Reaches dev with v4.5.0-rc6.

**Fixed in PR #1101, merged to main (export schema 20). CI was green on the PR's head.** - **In the file:** a sheet's rules are named by pack key, kind and slug in all four places a sheet held ids. No id is written. - **On import:** each is linked to this server's record, or cleared so the row stands on its snapshot. No id from a file is stored. - **A file of schema 19 or less:** its ids are dropped on the way in. - **Evaluation:** identical after a round trip with the same pack, and the same numbers from the snapshot when the pack is absent. **Known and accepted:** an export made before this, imported on the server it came from, loses its sheet links. Exporting again fixes it. **Not done:** sheets imported before this fix still hold foreign ids. They are harmless, since `_usable` refuses them and a test proves it; cleaning them up would need a data migration. Reaches dev with `v4.5.0-rc6`.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
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
rbrooks/Quest-Board#1095
No description provided.