Preserve id for reminders in sectioned lists#53
Merged
ajrosen merged 1 commit intoajrosen:mainfrom Apr 26, 2026
Merged
Conversation
`Reminder#initialize` deleted the CloudKit identifier (`zckIdentifier`, exposed as `id`) from any reminder that belonged to a sectioned list. The deletion was inside the `if @self['members']` block alongside a necessary `delete('members')`, but the section lookup only needs `id` locally — there's no reason to strip it from the output. In practice this affected ~85% of reminders on a typical user's data, because every Apple grocery list uses sections, plus any user list with sections configured. Downstream consumers that need the identifier for joins or deduplication silently received `null`. Add a regression test under test/ using stdlib minitest.
Owner
|
I don't remember why I stripped it out, but you're right, there's no reason not to keep it in. And the I'll also add an alias for it called I'm curious what kinds of joins or dedups are being done from the output? |
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
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.
Summary
Reminder#initializedeletes the CloudKit identifier (zckIdentifier, exposed asid) from any reminder that belongs to a sectioned list.The deletion lives in the
if @self['members']block alongside a necessarydelete('members'), but the section lookup only needsidlocally — there's no reason to strip it from the output.In practice this affects ~85% of reminders on a typical user's data, because every Apple grocery list uses sections, plus any user list with sections configured by the user. Downstream consumers that need the identifier for joins or deduplication silently receive
null.The fix
One-line change in
lib/reminder.rb: drop the@self.delete('id')call inside the section-handling block.membersis still deleted (it's an internal JSON membership table). A short comment is added to discourage anyone re-adding the deletion.Tests
Adds
test/reminder_id_test.rb— 4 cases using stdlib minitest:idis preserved when the reminder belongs to a sectioned list (the regression case)idis preserved when the reminder is not in a sectioned list (control)membersis still dropped from output (preserves existing behavior)idinternally; restoringidto output doesn't interfere)Run with:
I confirmed the suite catches the regression — reverting the one-line fix produces 2 failures (cases 1 and 4).
Notes
.rubocop.yml.