fix: _Installation update fails when clearing deviceToken - #10454
fix: _Installation update fails when clearing deviceToken#10454AdrianCurtin wants to merge 2 commits into
Conversation
handleInstallation runs before Parse __op operators are processed, so
when a client cleared deviceToken via { __op: 'Delete' } or null, the
operator object was being pushed into the Mongo $or lookup query and
rejected by transformQueryKeyValue with "You cannot use [object Object]
as a query parameter" (Parse error 107).
Detect the clearing intent and route the identification/lookup paths
through deviceTokenForLookup (undefined when clearing) while leaving
this.data.deviceToken untouched so the field is still cleared on write.
Adds regression specs covering both clearing shapes and the case where
deviceToken is cleared alongside another field update.
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. Tip
Note Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect. Caution Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement. Our CI and AI review are safeguards, not development tools. If many issues are flagged, rethink your development approach. Invest more effort in planning and design rather than using review cycles to fix low-quality code. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR updates ChangesInstallation device-token clearing
Estimated code review effort: 3 (Moderate) | ~28 minutes Merge Risk: ⚪ Minimal · up to The change narrowly fixes clearing an installation device token while preserving the existing write behavior. No actionable merge-blocking risk remains beyond normal checks and review. Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates _Installation handling so clearing deviceToken during an update no longer feeds a delete/null payload into the installation lookup path, which was causing Mongo query transformation failures.
Changes:
- Adds a
deviceTokenForLookuppath inRestWrite.handleInstallationto treatnulland{ __op: 'Delete' }as non-identifying values during installation matching. - Updates installation matching, validation, and dedup lookup branches to use the derived lookup token instead of the raw request payload.
- Adds regression tests covering
deviceTokenclearing via delete op, vianull, and while updating another field.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/RestWrite.js |
Adjusts _Installation lookup logic to avoid querying with delete-op/null deviceToken values. |
spec/ParseInstallation.spec.js |
Adds regression tests for clearing deviceToken during installation updates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Treat that as "no deviceToken to identify/match by" so we do not feed the | ||
| // operator object into the lookup query (which would fail Mongo transform). | ||
| const clearingDeviceToken = | ||
| this.data.deviceToken === null || | ||
| (typeof this.data.deviceToken === 'object' && | ||
| this.data.deviceToken !== null && | ||
| this.data.deviceToken.__op === 'Delete'); |
There was a problem hiding this comment.
Good catch, this was real. Fixed in 9042b7e.
Confirmed the behavior first: tightening the assertion to expect(results[0].deviceToken).toBeUndefined() failed with Expected null to be undefined, so the null was being persisted. Since $exists: true matches a field that is present and null, the row stayed in the push recipient set.
handleInstallation now collapses the null form onto { __op: 'Delete' } before any of the early returns, so both client forms take the delete path. That covers creates too: on create the Delete operator flattens to undefined in transformUpdateOperator, so the field is simply never set rather than stored as null.
| expect(results[0].deviceToken == null).toBeTrue(); | ||
| expect(results[0].installationId).toEqual(installId); |
There was a problem hiding this comment.
Agreed, the assertion was too loose. deviceToken == null passed for both the stored null and the absent field, which is exactly the distinction that mattered.
Tightened to toBeUndefined(), and added removes an installation cleared via null from push recipients, which builds its query with the real applyDeviceTokenExists helper from src/Push/utils.js rather than restating the filter, asserts the installation is selected while it has a token, clears it with null, and asserts it is no longer selected.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
… push-addressable
Issue
Updating an existing
_Installationto cleardeviceTokenfrom the client (iOS/Android SDKs, Dashboard, anything sending{ deviceToken: { __op: 'Delete' } }or{ deviceToken: null }) fails with:RestWrite.handleInstallationruns at step 109 ofRestWrite.execute, before the field-write loop processes__opoperators. TheDeleteoperator object is truthy, so it was being pushed into the$orlookup query as{ deviceToken: { __op: 'Delete' } }, whichtransformQueryKeyValuecannot transform into a Mongo predicate.Approach
Detect the two clearing shapes (
nulland{ __op: 'Delete' }) at the top ofhandleInstallationand derive adeviceTokenForLookupvalue (undefined when clearing). Identification paths usedeviceTokenForLookup:$orlookuporQueriespushresult.deviceTokenmatch loopdelQueryblocksthis.data.deviceTokenis left untouched, so the actual write step still applies the Delete op and the field is cleared on the row.Tasks
Add changes to documentation (guides, repository pages, code comments)Add security checkAdd new Parse Error codes to Parse JS SDKSummary by CodeRabbit
Bug Fixes
Tests