fix(catalog): show linked person in editor when credit name has stray whitespace - #1863
Open
alphatownsman wants to merge 4 commits into
Open
fix(catalog): show linked person in editor when credit name has stray whitespace#1863alphatownsman wants to merge 4 commits into
alphatownsman wants to merge 4 commits into
Conversation
added 2 commits
September 6, 2026 22:20
… whitespace Scrapers can leave surrounding whitespace in credit jsondata (Douban text nodes, e.g. "杨力州 "). sync_credits_from_metadata strips the name before creating the ItemCredit row, and _link_credits later links that row to a People without touching the metadata. The edit form then failed to match the raw value against the stripped credit name, so the editor showed the plain name instead of the /person/<uuid> link. Strip on both sides of the form lookup, and persist the stripped values in sync_credits_from_metadata, which previously set them on the instance but only saved when a person URL was substituted.
… person With stripped names now persisted, a refetch merges the scraper's unstripped copy next to the stored one because uniq() compares exactly. Drop entries in sync_credits_from_metadata that strip to a name already seen, or that resolve to a People already seen, for the same character. This also collapses items that list one person under two localized names.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1863 +/- ##
==========================================
+ Coverage 68.14% 68.23% +0.09%
==========================================
Files 520 520
Lines 50798 50844 +46
Branches 7651 7657 +6
==========================================
+ Hits 34615 34693 +78
+ Misses 13621 13588 -33
- Partials 2562 2563 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
added 2 commits
September 6, 2026 23:00
Legacy ItemCredit rows may carry surrounding whitespace. Strip them in linked_by_name and existing_by_key so a linked row keeps its link and is reused instead of being recreated and pruned.
Re-run sync_credits_from_metadata on items that hold duplicate ItemCredit rows for one role, either one People linked twice or one plain name twice. The sync now collapses both, canonicalizes linked jsondata entries to person.url and prunes the stale rows. neodb-manage catalog migrate --name resync_duplicate_credits [--dry-run]
Comment on lines
+1336
to
1346
| metadata_changed = True | ||
| elif new_values != values: | ||
| setattr(self, field_name, new_values) | ||
| metadata_changed = True | ||
|
|
||
| existing_by_key: dict[tuple[str, int | None], ItemCredit] = {} | ||
| for c in existing: | ||
| key = (c.name, c.person.pk if c.person else None) | ||
| key = (c.name.strip(), c.person.pk if c.person else None) | ||
| existing_by_key.setdefault(key, c) | ||
|
|
||
| used_pks: set[int] = set() |
There was a problem hiding this comment.
Bug: The credit sync logic uses a dynamic, localized name for lookups, which can mismatch the stored, non-localized name, causing credit duplication and deletion of the original.
Severity: MEDIUM
Suggested Fix
To ensure a stable lookup, the key for both existing and desired credits should be based on a non-dynamic identifier. Instead of using the display name for the lookup, use the original raw_name from the metadata or another stable attribute that is not subject to localization. This will prevent mismatches when person.display_name returns a localized value.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: neodb/catalog/models/item.py#L1333-L1346
Potential issue: When syncing item credits, the system may fail to reuse an existing
`ItemCredit` if a person has localized names. The lookup dictionary for existing credits
is built using `c.name.strip()`, which is a frozen snapshot of the person's name.
However, the lookup key for the incoming credit data uses `display`, which is derived
from the dynamic `person.display_name` property. This property can return a localized
name (e.g., in Chinese) that differs from the stored snapshot name (e.g., in English).
This mismatch causes the lookup to fail, leading to the creation of a duplicate
`ItemCredit` and the deletion of the original one.
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
1 participant
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.
Problem
Editing https://neodb.social/movie/3pYSPRNyGVr0FrmMszp0mQ showed the director as a plain name, while the producers on the same item showed
/person/<uuid>links.The Douban scraper stored the director as
"杨力州 "with a trailing space.sync_credits_from_metadatastrips names before creating theItemCreditrow, and_link_creditslater linked that stripped row to aPeoplewithout touching the metadata (by design).canonicalize_credit_initialsin the edit form matched raw metadata against credit names exactly, so the whitespace defeated the match and the editor fell back to the plain name.Changes
catalog/forms.py: strip both sides before matching a raw credit name to a linked credit.catalog/models/item.py: persist stripped values insync_credits_from_metadata(they were set on the instance but only saved when a person URL was substituted). Compare stored credit names stripped in the sync lookups too, so legacy rows with whitespace keep their link. Collapse entries that strip to the same name, or resolve to the same person, for the same character. Without this, a refetch would merge the scraper's unstripped copy next to the stored stripped one (uniqcompares exactly) and create a duplicate credit. This also collapses items that list one person under two localized names, which is the state of the live item above (two linked director credits, shown twice on the page).catalog/common/migrations.py: newresync_duplicate_creditsmigration that re-runs the sync on items holding duplicate credits for one role (one person linked twice, or one name twice), which collapses them and canonicalizes the jsondata.Deploy
Items with whitespace-only metadata but no duplicates are left alone; the editor now shows their links, and the next edit save, refetch or merge canonicalizes them.