fix: ensure unique item IDs when importing checklists (#501)#534
Merged
Conversation
The client-side parseChecklistContent generated fallback item IDs as
`${id}-${currentItemIndex}` and reset currentItemIndex to 0 in each
recursive call for nested children. Group headers and their nested
items therefore shared the same numeric range, producing duplicate IDs
across siblings and groups. Clicking one item toggled every other item
with the same ID.
The server-side parser (parseMarkdown in checklist-utils.ts) already
avoided this by using a monotonic counter combined with level
(`${id}-${level}-${counter}`). Align the client parser with the same
scheme so a file dropped into the importer is assigned the same IDs
that the server would produce.
Also add a post-parse dedup pass so already-imported files with broken
duplicate IDs heal themselves on the next save: the first occurrence
keeps its ID, every later collision is reassigned a fresh unique ID.
Stored metadata IDs are still honoured.
Tests cover a 4-group, 5-children-each fresh parse (all IDs unique),
preservation of stored item-metadata IDs, and dedup of pre-broken
imported content.
Owner
|
Works! thank you for the assist ❤️ |
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.
Problem
Fixes #501. When a checklist is imported via file-drop (without per-item
metadata), several items get identical IDs. Since checkbox toggling is keyed on
the item ID, checking one box toggles every item that shares that ID — e.g.
clicking "Item D1" also toggled Group A, Item A1, B1 and C1.
Root cause
Two markdown parsers assign item IDs:
app/_utils/checklist-utils.ts(parseMarkdown) generates${id}-${level}-${counter}with a global counter → unique.app/_utils/client-parser-utils.ts(parseChecklistContent→buildNestedItems) generated${id}-${currentItemIndex}with no levelsegment and reset the index to
0for each nested group. Group headers andchild items therefore shared the same number space and collided
(
<file>-0appeared 5× in the repro).The IDs that actually get persisted on file-drop come from the client parser,
so the earlier server-side change in #509 didn't cover this path.
Fix (
client-parser-utils.ts)generateItemId(level)(
${id}-${level}-${globalCounter++}), matching the scheme already used by theserver parser. Guarantees uniqueness regardless of nesting.
imported before this fix), the first occurrence is kept and later duplicates
get a fresh unique ID. Already-broken checklists self-heal on the next save.
Tests
Added
tests/utils/client-parser-utils.test.ts:yarn lint,tsc --noEmitandyarn test:runall pass.