-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
fix: Fixed issue where deeply nested keys were having incorrect values #9720
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: alpha
Are you sure you want to change the base?
fix: Fixed issue where deeply nested keys were having incorrect values #9720
Conversation
I will reformat the title to use the proper commit message syntax. |
🚀 Thanks for opening this pull request! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## alpha #9720 +/- ##
=======================================
Coverage 93.00% 93.00%
=======================================
Files 187 187
Lines 15082 15073 -9
Branches 174 174
=======================================
- Hits 14027 14019 -8
+ Misses 1043 1042 -1
Partials 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Looks like it’s failing in Postgres, getting recursive coalesces (dot notation) working would be a challenge |
I'll try to work on this part, do you have any insights on where to look into? I'm not a Postgres expert |
Here you go, it might work I'll try as well, might be a week parse-server/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js Lines 185 to 187 in ed69e03
|
@RahulLanjewar93 We can make this test Mongo only as PG Dot notation support is limited already. |
Ok I'll make changes accordingly then |
I will reformat the title to use the proper commit message syntax. |
📝 WalkthroughWalkthroughThe changes simplify the handling of keys containing dots in the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant afterSaveTrigger
Client->>Server: Save GameScore object with nested keys
Server->>afterSaveTrigger: Invoke afterSave with saved object
afterSaveTrigger->>Server: Validate nested keys, increment counter
Server-->>Client: Respond to save request
Note over Server,afterSaveTrigger: This sequence repeats for each save with modifications to nested keys.
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code Graph Analysis (1)spec/ParseObject.spec.js (3)
🪛 ESLintspec/ParseObject.spec.js[error] 2175-2175: 'it_only_db' is not defined. (no-undef) [error] 2179-2179: 'expect' is not defined. (no-undef) [error] 2183-2183: 'expect' is not defined. (no-undef) [error] 2184-2184: 'expect' is not defined. (no-undef) [error] 2187-2187: 'expect' is not defined. (no-undef) [error] 2188-2188: 'expect' is not defined. (no-undef) [error] 2191-2191: 'expect' is not defined. (no-undef) [error] 2192-2192: 'expect' is not defined. (no-undef) [error] 2195-2195: 'expect' is not defined. (no-undef) [error] 2196-2196: 'expect' is not defined. (no-undef) [error] 2199-2199: 'expect' is not defined. (no-undef) [error] 2200-2200: 'expect' is not defined. (no-undef) [error] 2224-2224: 'expect' is not defined. (no-undef) ⏰ Context from checks skipped due to timeout of 90000ms (12)
🔇 Additional comments (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
What's the state of this PR? |
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) |
I have to change the test to mongo instead of generic test. Have been occupied with something will do it sometime this week |
Ready for review/merging |
parentVal[splittedKey[1]] = data[key]; | ||
updatedObject.set(parentProp, parentVal); | ||
} | ||
updatedObject.set(key, data[key]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line replaces a bunch of code. Why was that code not needed, can it just be removed? Looking at this check for example:
if (typeof data[key].__op === 'string') {
if (!readOnlyAttributes.includes(key)) {
updatedObject.set(key, data[key]);
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the parse js sdk handles it as per @dplewis comment here #7385 (comment)
This pr is just a duplicate for #7385 since its in draft mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, maybe @dplewis wants to chime in here an approve this since he already looked into this.
refer #7385 (comment)
Summary by CodeRabbit
Bug Fixes
Tests