Internalize category race condition fix - #476
Open
nabalone wants to merge 9 commits into
Open
Conversation
nabalone
commented
Jul 24, 2026
Collaborator
Author
There was a problem hiding this comment.
Copying from the Previous PR, Sarah wrote: "We shouldn't ever allow a duplicated name...is this covering a possible temporary state?"
nabalone
force-pushed
the
internalization_categoryfix2
branch
from
July 24, 2026 15:31
6b1dc76 to
6ce8867
Compare
React 19 types collapse the two ref shapes into a single writable RefObject<T>, deprecating MutableRefObject. Swap the category commit-ref prop types accordingly; useRef call sites are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The recording path reaches finish() via afterUploadCb rather than uploadMedia, so the beforeUpload commit never ran and a newly typed category was silently dropped (resource saved with no category, category never created). Commit deferred metaData edits in afterUploadCb too, guarded on mediaId so no orphan category is created when nothing was recorded. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Deferring new-category creation to commit() means a brand-new typed name never fires onCategoryChange, so consumers that gate their Save button on a `changed` flag (ResourceOverview, DiscussionCard) could never enable it when the new category was the only edit — a regression vs the old create-on-blur behavior. Add a lightweight onNewDraft(hasDraft) callback to SelectArtifactCategory that fires as the user types a new (allowNew) name matching no existing category. ResourceCategory and DiscussionCard use it to mark themselves dirty. Kept the single commitRef function ref as the creation channel (no second ref / draft-name machinery); a ref alone can't reactively re-enable a button, so one small callback is the minimal complement. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Autocomplete options were the raw localized names with no dedupe, and a render-index was baked into the React key to avoid collisions when two categories shared a localized name — i.e. it masked the duplicate rather than preventing it. New-category creation already blocks duplicate localized names, but two distinct slugs can still collapse to the same localized string (e.g. created under different languages). Deduplicate the options by localized name (keeping the first occurrence, matching how resolveExisting/commit resolve a typed name to an id), so a name can never appear twice. The key can then be the unique category id and the index workaround is removed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
commit() ran on every save and re-resolved the field text to an id by name. When the field is unchanged that round-trip could (a) drop the category — if its localized name is filtered out of the options the field shows blank, so commit() saw '' and cleared it — or (b) switch it to a different id when two slugs share the same localized name (commit picks the first match, not necessarily the committed one). Short-circuit commit() when the trimmed input equals the committed category's name (including empty === empty), returning the existing categoryId untouched. New/changed names still resolve and create as before. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This reverts commit 6ce8867.
nabalone
force-pushed
the
internalization_categoryfix2
branch
from
July 28, 2026 17:32
bdc5790 to
1ded511
Compare
nabalone
changed the title
Collaborator
Author
|
Claude says test failure is preexisting, I haven't looked into it |
nabalone
marked this pull request as ready for review
July 28, 2026 21:36
sarahentzel
approved these changes
Jul 28, 2026
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
2 participants
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.
The artifact-category deferred-commit work. Now targets develop (PR #469 "Internalize changes part 1" has been squash-merged into develop, so this rebases to just the category delta on top of it).
What changed
Autocomplete. Blur/change only resolve against existing categories (resolveExisting); a brand-new typed name is created at form submission via a singlecommit()handle exposed throughcommitRef. Avoids leaving an orphaned category behind when the user navigates away without saving.commitRefso creation happens at save:ResourceOverview/ResourceCategory,DiscussionCard, andPassageDetailArtifacts(+mobile) for both the edit and add dialogs (two refs because two instances can be mounted).finish()viaafterUploadCbrather thanuploadMedia, so the upload-pathbeforeUploadcommit never ran and a newly typed category was silently dropped. The record path now commits the category too (guarded onmediaId).RefObjectinstead of deprecatedMutableRefObject(React 19).Review follow-ups included
onCategoryChange, so forms gating Save on achangedflag (ResourceOverview,DiscussionCard) could never enable it. Added a lightweightonNewDraft(hasDraft)callback that marks those forms dirty as the user types a new name. Kept the singlecommitReffor creation — a ref can't reactively re-enable a button, so one small callback is the minimal complement.Test plan
🤖 Generated with Claude Code