fix: Deprecation warning logged at server launch for nested Parse Server option even if option is explicitly set #9934
Conversation
|
🚀 Thanks for opening this pull request! |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
📝 WalkthroughWalkthroughThis pull request introduces support for nested option key deprecation handling. A new utility method Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10–15 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
mtrezza
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
spec/Deprecator.spec.js (1)
62-72: Remove unused Config variable.Line 68 retrieves the Config but never uses it. This appears to be leftover code from development.
Apply this diff:
spyOn(Deprecator, '_getDeprecations').and.callFake(() => deprecations); const logSpy = spyOn(Deprecator, '_logOption').and.callFake(() => {}); - const Config = require('../lib/Config'); - const config = Config.get('test'); // Directly test scanParseServerOptions with nested option set Deprecator.scanParseServerOptions({ databaseOptions: { allowPublicExplain: true } }); expect(logSpy).not.toHaveBeenCalled();Note: The direct call to
scanParseServerOptionsis a valid unit testing approach, though it differs from the pattern used in the earlier test (lines 29-36) which usesreconfigureServer. Both approaches are acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
spec/Deprecator.spec.js(1 hunks)spec/Utils.spec.js(1 hunks)src/Deprecator/Deprecator.js(2 hunks)src/Utils.js(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:24.824Z
Learning: For Parse Server PRs, always suggest an Angular-style PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion with every new commit to the PR.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: For new Parse Server options, verify that the option is documented in src/Options/index.js and that npm run definitions has been executed to reflect changes in src/Options/docs.js and src/Options/Definitions.js. README.md documentation is a bonus but not required for new options.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:48.786Z
Learning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.
📚 Learning: 2025-11-08T13:46:04.940Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: For new Parse Server options, verify that the option is documented in src/Options/index.js and that npm run definitions has been executed to reflect changes in src/Options/docs.js and src/Options/Definitions.js. README.md documentation is a bonus but not required for new options.
Applied to files:
spec/Deprecator.spec.js
📚 Learning: 2025-10-16T19:27:05.311Z
Learnt from: Moumouls
Repo: parse-community/parse-server PR: 9883
File: spec/CloudCodeLogger.spec.js:410-412
Timestamp: 2025-10-16T19:27:05.311Z
Learning: In spec/CloudCodeLogger.spec.js, the test "should log cloud function triggers using the silent log level" (around lines 383-420) is known to be flaky and requires the extra `await new Promise(resolve => setTimeout(resolve, 100))` timeout after awaiting `afterSavePromise` for reliability, even though it may appear redundant.
Applied to files:
spec/Deprecator.spec.js
📚 Learning: 2025-05-04T20:41:05.147Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1312-1338
Timestamp: 2025-05-04T20:41:05.147Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`.
Applied to files:
spec/Deprecator.spec.js
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: Tests in the parse-server repository should use promise-based approaches rather than callback patterns with `done()`. Use a pattern where a Promise is created that resolves when the event occurs, then await that promise.
Applied to files:
spec/Deprecator.spec.js
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`. The preferred pattern is to create a Promise that resolves when an expected event occurs, then await that Promise.
Applied to files:
spec/Deprecator.spec.js
🧬 Code graph analysis (3)
spec/Deprecator.spec.js (1)
spec/helper.js (1)
reconfigureServer(180-214)
src/Deprecator/Deprecator.js (1)
src/cli/utils/commander.js (1)
options(77-77)
src/Utils.js (1)
src/Controllers/DatabaseController.js (1)
path(1836-1836)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: Node 18
- GitHub Check: PostgreSQL 15, PostGIS 3.3
- GitHub Check: Docker Build
- GitHub Check: PostgreSQL 15, PostGIS 3.4
- GitHub Check: Redis Cache
- GitHub Check: Node 22
- GitHub Check: MongoDB 7, ReplicaSet
- GitHub Check: PostgreSQL 18, PostGIS 3.6
- GitHub Check: Node 20
- GitHub Check: PostgreSQL 15, PostGIS 3.5
- GitHub Check: MongoDB 6, ReplicaSet
- GitHub Check: MongoDB 8, ReplicaSet
- GitHub Check: PostgreSQL 16, PostGIS 3.5
- GitHub Check: PostgreSQL 17, PostGIS 3.5
- GitHub Check: Benchmarks
🔇 Additional comments (5)
spec/Utils.spec.js (1)
126-175: LGTM! Comprehensive test coverage for the new utility.The test suite thoroughly covers all relevant scenarios including edge cases (null objects, empty paths, falsy values). The tests follow existing patterns and properly validate the nested property access functionality.
src/Deprecator/Deprecator.js (2)
3-3: LGTM! Correct import for nested property access.The Utils import is properly added to support nested option key traversal.
25-25: LGTM! Correctly implements nested option key support.Replacing direct property access with
Utils.getNestedPropertyenables the Deprecator to correctly handle dot-notated option keys likedatabaseOptions.allowPublicExplain, fixing the issue where deprecation warnings were incorrectly logged for explicitly set nested options.src/Utils.js (1)
448-471: LGTM! Clean and well-documented utility method.The implementation correctly handles nested property traversal with proper edge case handling (null objects, non-object traversal, empty paths). The JSDoc documentation is clear and includes a helpful example.
spec/Deprecator.spec.js (1)
49-60: LGTM! Correct test for nested option deprecation.The test properly validates that deprecation warnings are logged for nested option keys when they are not explicitly set. Uses the existing test pattern with
reconfigureServer()and logger spy.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #9934 +/- ##
=======================================
Coverage 93.05% 93.06%
=======================================
Files 187 187
Lines 15243 15253 +10
Branches 177 177
=======================================
+ Hits 14185 14195 +10
Misses 1046 1046
Partials 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
# [8.5.0-alpha.11](8.5.0-alpha.10...8.5.0-alpha.11) (2025-11-17) ### Bug Fixes * Deprecation warning logged at server launch for nested Parse Server option even if option is explicitly set ([#9934](#9934)) ([c22cb0a](c22cb0a))
|
🎉 This change has been released in version 8.5.0-alpha.11 |
2 participants
Pull Request
Issue
The deprecator incorrectly handles nested option keys like
databaseOptions.allowPublicExplainas it doesn't consider the dot notation to access nested properties.Approach
Parse dot notation of deprecated option keys.