feat: Add beforePasswordResetRequest hook
#9906
Conversation
|
I will reformat the title to use the proper commit message syntax. |
parse-github-assistant
bot
changed the title
|
🚀 Thanks for opening this pull request! |
coratgerl
changed the title
beforePasswordResetRequest hook
✅ 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. |
|
I will reformat the title to use the proper commit message syntax. |
parse-github-assistant
bot
changed the title
beforePasswordResetRequest hookbeforePasswordResetRequest hook
📝 WalkthroughWalkthroughAdds a new beforePasswordResetRequest Cloud trigger API, registers the trigger type, invokes it from UsersRouter during password-reset requests after resolving/inflating the user and expanding attached files, and adds tests covering success, blocking, file-attached users, missing emails, request context, and validation. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant UsersRouter as UsersRouter\n(handleResetRequest)
participant Triggers as TriggerRegistry
participant Hook as beforePasswordResetRequest\nHandler
participant Email as EmailAdapter
Client->>UsersRouter: POST /requestPasswordReset (email or token)
UsersRouter->>UsersRouter: Lookup user (token OR email/username)
UsersRouter->>UsersRouter: Sanitize authData & expand attached files
UsersRouter->>Triggers: Invoke beforePasswordResetRequest(req with inflated user)
Triggers->>Hook: Call handler(req)
alt Handler allows
Hook-->>Triggers: Return success
Triggers-->>UsersRouter: Continue flow
UsersRouter->>Email: sendPasswordResetEmail(user)
Email-->>Client: 200 OK
else Handler throws
Hook-->>Triggers: Throw error
Triggers-->>UsersRouter: Propagate error
UsersRouter-->>Client: Error response
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
✅ Actions performedReview triggered.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## alpha #9906 +/- ##
=======================================
Coverage 93.06% 93.07%
=======================================
Files 187 187
Lines 15253 15275 +22
Branches 177 177
=======================================
+ Hits 14195 14217 +22
Misses 1046 1046
Partials 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/triggers.js (1)
62-66: Validation updated to restrict to _User — OK; consider throwing Error objects.Logic is correct; throwing strings matches existing pattern but is brittle. Consider new Parse.Error for future cleanup.
src/Routers/UsersRouter.js (1)
449-474: Minor: reduce variables and narrow scope.userResults is only used to grab the first element. Simplify to a single const userRecord to reduce cognitive load.
src/cloud-code/Parse.Cloud.js (1)
352-394: Add validator wiring and validate input (parity + future-proofing).Current implementation mirrors beforeLogin (rateLimit only). If you want validator features (skipWithMasterKey, requireMaster, etc.) or just to validate rateLimit shape, call validateValidator and pass validationHandler to addTrigger.
ParseCloud.beforePasswordResetRequest = function (handler, validationHandler) { let className = '_User'; if (typeof handler === 'string' || isParseObjectConstructor(handler)) { // validation will occur downstream, this is to maintain internal // code consistency with the other hook types. className = triggers.getClassName(handler); handler = arguments[1]; validationHandler = arguments.length >= 2 ? arguments[2] : null; } - triggers.addTrigger(triggers.Types.beforePasswordResetRequest, className, handler, Parse.applicationId); + validateValidator(validationHandler); + triggers.addTrigger( + triggers.Types.beforePasswordResetRequest, + className, + handler, + Parse.applicationId, + validationHandler + ); if (validationHandler && validationHandler.rateLimit) { addRateLimit( { requestPath: `/requestPasswordReset`, requestMethods: 'POST', ...validationHandler.rateLimit }, Parse.applicationId, true ); } };Also consider documenting the optional validationHandler param in the JSDoc.
spec/CloudCode.spec.js (2)
3782-3978: Prefer async/await without done in new tests to reduce flakiness.The new tests use
async done => { ...; done(); }. Dropdoneand rely on async function completion/throws instead. This is the repository’s preferred pattern. Based on learnings.
3800-3816: Add an assertion that password/authData are not exposed to Cloud Code.After the UsersRouter sanitization fix, add a quick check:
Parse.Cloud.beforePasswordResetRequest(req => { // existing expectations... + expect(req.object.get('password')).toBeUndefined(); + expect(req.object.get('authData')).toBeUndefined(); });Helps prevent regressions.
Also applies to: 3836-3859, 3878-3904, 3932-3962
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
spec/CloudCode.spec.js(2 hunks)src/Routers/UsersRouter.js(2 hunks)src/cloud-code/Parse.Cloud.js(1 hunks)src/triggers.js(3 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 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/CloudCode.spec.js
📚 Learning: 2025-08-27T12:33:06.237Z
Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: src/triggers.js:467-477
Timestamp: 2025-08-27T12:33:06.237Z
Learning: In the Parse Server codebase, maybeRunAfterFindTrigger is called in production with Parse.Query objects constructed via withJSON(), so the plain object query handling bug only affects tests, not production code paths.
Applied to files:
spec/CloudCode.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/CloudCode.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/CloudCode.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/CloudCode.spec.js
🧬 Code graph analysis (3)
src/Routers/UsersRouter.js (1)
src/triggers.js (2)
inflate(1002-1008)maybeRunTrigger(895-998)
src/cloud-code/Parse.Cloud.js (2)
src/Routers/CloudCodeRouter.js (1)
triggers(4-4)src/middlewares.js (2)
addRateLimit(520-623)addRateLimit(520-623)
spec/CloudCode.spec.js (2)
spec/helper.js (2)
Parse(4-4)reconfigureServer(180-214)src/cloud-code/Parse.Cloud.js (1)
emailAdapter(604-604)
⏰ 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). (14)
- GitHub Check: Docker Build
- GitHub Check: PostgreSQL 18, PostGIS 3.6
- GitHub Check: PostgreSQL 15, PostGIS 3.5
- GitHub Check: Node 22
- GitHub Check: PostgreSQL 17, PostGIS 3.5
- GitHub Check: PostgreSQL 15, PostGIS 3.4
- GitHub Check: Node 18
- GitHub Check: PostgreSQL 16, PostGIS 3.5
- GitHub Check: MongoDB 6, ReplicaSet
- GitHub Check: MongoDB 8, ReplicaSet
- GitHub Check: PostgreSQL 15, PostGIS 3.3
- GitHub Check: Node 20
- GitHub Check: MongoDB 7, ReplicaSet
- GitHub Check: Redis Cache
🔇 Additional comments (2)
src/triggers.js (2)
9-9: New trigger type added correctly.Entry in Types for beforePasswordResetRequest looks consistent and non-breaking.
291-296: Context exposure includes beforePasswordResetRequest — LGTM.Request.context now set for this trigger type; aligns with other auth-related hooks.
|
Note that I proposed the name
Maybe this would be more intuitive:
Then again, a password reset could be sent via other channels in the future, so maybe we stick with this:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
spec/CloudCode.spec.js (3)
3623-3675: Consider modernizing test patternThis test correctly validates that
afterFindhooks are not triggered during save operations. However, consider converting fromasync donecallback pattern to pureasync/await:Based on learnings
Apply this diff:
- it('afterFind should not be triggered when saving an object', async () => { + it('afterFind should not be triggered when saving an object', async () => { let beforeSaves = 0; Parse.Cloud.beforeSave('SavingTest', () => { beforeSaves++; }); // ... rest of test logic ... expect(beforeSaves).toEqual(3); expect(afterSaves).toEqual(3); expect(beforeFinds).toEqual(1); expect(afterFinds).toEqual(1); - done(); });
3836-3870: Test logic is correct; consider modernizing patternThe test properly validates that
beforePasswordResetRequestruns with a valid user and that the email is sent. However, similar to other tests in this suite, consider removing thedonecallback and using pure async/await pattern.Based on learnings
3959-3983: Improve error handling specificityThe try/catch block swallows all errors, which could mask unexpected failures. Consider testing both scenarios explicitly or at least validating the error type when one is thrown.
Apply this diff to make the test more explicit:
it('should not run beforePasswordResetRequest if email does not exist', async done => { let hit = 0; const emailAdapter = { sendVerificationEmail: () => Promise.resolve(), sendPasswordResetEmail: () => {}, sendMail: () => {}, }; await reconfigureServer({ emailAdapter: emailAdapter, publicServerURL: 'http://localhost:8378/1', + passwordPolicy: { + resetPasswordSuccessOnInvalidEmail: true + } }); Parse.Cloud.beforePasswordResetRequest(req => { hit++; }); - try { - await Parse.User.requestPasswordReset('[email protected]'); - } catch (e) { - // May or may not throw depending on passwordPolicy.resetPasswordSuccessOnInvalidEmail - } + // Should not throw when resetPasswordSuccessOnInvalidEmail is true + await Parse.User.requestPasswordReset('[email protected]'); expect(hit).toBe(0); done(); });Alternatively, add a second test for the case where
resetPasswordSuccessOnInvalidEmailis false to ensure comprehensive coverage.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
spec/CloudCode.spec.js(3 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 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/CloudCode.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/CloudCode.spec.js
📚 Learning: 2025-08-27T12:33:06.237Z
Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: src/triggers.js:467-477
Timestamp: 2025-08-27T12:33:06.237Z
Learning: In the Parse Server codebase, maybeRunAfterFindTrigger is called in production with Parse.Query objects constructed via withJSON(), so the plain object query handling bug only affects tests, not production code paths.
Applied to files:
spec/CloudCode.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/CloudCode.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/CloudCode.spec.js
🧬 Code graph analysis (1)
spec/CloudCode.spec.js (1)
spec/helper.js (2)
Parse(4-4)reconfigureServer(180-214)
🪛 GitHub Check: Lint
spec/CloudCode.spec.js
[warning] 4034-4034:
More than 2 blank lines not allowed
⏰ 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). (5)
- GitHub Check: Redis Cache
- GitHub Check: MongoDB 8, ReplicaSet
- GitHub Check: MongoDB 6, ReplicaSet
- GitHub Check: Docker Build
- GitHub Check: Code Analysis (javascript)
🔇 Additional comments (3)
spec/CloudCode.spec.js (3)
3310-3322: LGTM: Trigger validation messages updated correctlyThe error messages have been properly updated to include the new
beforePasswordResetRequesttrigger alongsidebeforeLoginandafterLogin. The changes are consistent across all validation checks.
3872-3957: LGTM: Blocking behavior well-testedThese tests properly validate that
beforePasswordResetRequestcan block password reset requests by throwing errors. The inclusion of a test with an attached file is important as it verifies that file expansion occurs before the hook invocation, preventing unnecessary token generation and email sending (as noted in the PR objectives).
3985-4030: LGTM: Request data and class validation testsThese tests properly validate that:
- The hook receives all expected request fields (object, headers, ip, installationId, context, config)
- Only the
_Userclass is allowed forbeforePasswordResetRequest, consistent with other user-authentication triggersThe test patterns match those used for
beforeLoginand other triggers, ensuring consistency across the test suite.
|
I think |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
spec/CloudCode.spec.js (1)
4659-4659: Remove extra blank lines.More than 2 blank lines are not allowed before the new
describeblock. Please remove the extra blank line to comply with the project's style guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
spec/CloudCode.spec.js(2 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 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/CloudCode.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/CloudCode.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/CloudCode.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/CloudCode.spec.js
📚 Learning: 2025-08-27T12:33:06.237Z
Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: src/triggers.js:467-477
Timestamp: 2025-08-27T12:33:06.237Z
Learning: In the Parse Server codebase, maybeRunAfterFindTrigger is called in production with Parse.Query objects constructed via withJSON(), so the plain object query handling bug only affects tests, not production code paths.
Applied to files:
spec/CloudCode.spec.js
🧬 Code graph analysis (1)
spec/CloudCode.spec.js (2)
spec/helper.js (2)
Parse(4-4)reconfigureServer(180-214)src/cloud-code/Parse.Cloud.js (1)
emailAdapter(604-604)
🔇 Additional comments (1)
spec/CloudCode.spec.js (1)
3310-3322: LGTM!The trigger validation error messages have been correctly updated to include
beforePasswordResetRequestin the list of user-specific triggers. The changes are consistent across all test assertions.
|
@coratgerl Yes, I think if new channel are introduced, we don't want a separate trigger, but just a parameter that indicates the channel for example. So I'd stick with:
Which is what you currently have, so I think we're good, right? |
|
Yes seems logic for me |
mtrezza
left a comment
mtrezza
left a comment
There was a problem hiding this comment.
There are unresolved conversations
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
spec/CloudCode.spec.js (1)
4724-4729: Usefail()instead of throwing an error for clarity.For consistency with other tests in this file and clearer test failure messages, use
fail()instead of throwing a generic error when the operation should have failed.Apply this diff:
try { await Parse.User.requestPasswordReset('[email protected]'); - throw new Error('should not have sent password reset email.'); + fail('should not have sent password reset email.'); } catch (e) { expect(e.message).toBe('password reset blocked'); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
spec/CloudCode.spec.js(2 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
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.
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.
📚 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/CloudCode.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/CloudCode.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/CloudCode.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/CloudCode.spec.js
📚 Learning: 2025-08-27T09:08:34.252Z
Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: src/triggers.js:446-454
Timestamp: 2025-08-27T09:08:34.252Z
Learning: When analyzing function signature changes in Parse Server codebase, verify that call sites are actually incorrect before flagging them. Passing tests are a strong indicator that function calls are already properly aligned with new signatures.
Applied to files:
spec/CloudCode.spec.js
📚 Learning: 2025-08-27T12:33:06.237Z
Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: src/triggers.js:467-477
Timestamp: 2025-08-27T12:33:06.237Z
Learning: In the Parse Server codebase, maybeRunAfterFindTrigger is called in production with Parse.Query objects constructed via withJSON(), so the plain object query handling bug only affects tests, not production code paths.
Applied to files:
spec/CloudCode.spec.js
🧬 Code graph analysis (1)
spec/CloudCode.spec.js (1)
spec/helper.js (2)
Parse(4-4)reconfigureServer(180-214)
⏰ 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). (16)
- GitHub Check: PostgreSQL 15, PostGIS 3.5
- GitHub Check: PostgreSQL 18, PostGIS 3.6
- GitHub Check: PostgreSQL 15, PostGIS 3.4
- GitHub Check: PostgreSQL 17, PostGIS 3.5
- GitHub Check: PostgreSQL 16, PostGIS 3.5
- GitHub Check: MongoDB 7, ReplicaSet
- GitHub Check: PostgreSQL 15, PostGIS 3.3
- GitHub Check: Node 20
- GitHub Check: MongoDB 8, ReplicaSet
- GitHub Check: Node 22
- GitHub Check: Node 18
- GitHub Check: Redis Cache
- GitHub Check: Code Analysis (javascript)
- GitHub Check: MongoDB 6, ReplicaSet
- GitHub Check: Docker Build
- GitHub Check: Benchmarks
🔇 Additional comments (5)
spec/CloudCode.spec.js (5)
3310-3322: LGTM! Validation messages correctly updated.The error messages have been properly updated to include
beforePasswordResetRequestin the list of triggers that only allow the_Userclass, maintaining consistency across all validation points.
4661-4694: LGTM! Well-structured test for basic hook flow.The test properly validates that the
beforePasswordResetRequesthook executes with the correct user data and that the email adapter is called as expected.
4734-4755: LGTM! Correctly validates hook behavior for non-existent email.This test properly ensures the hook doesn't run when an email doesn't exist in the system, which is important for both security (not leaking email existence) and correctness.
4757-4791: LGTM! Comprehensive test for request data availability.The test properly validates all expected properties in the request object (object, headers, ip, installationId, context, config) and includes a user with an attached file to ensure proper handling of file expansion in the password reset flow.
4793-4807: LGTM! Thorough validation of class restrictions.The test properly validates that
beforePasswordResetRequestcan only be registered for the_Userclass, testing multiple registration patterns to ensure comprehensive coverage.
Signed-off-by: Manuel <[email protected]>
Signed-off-by: Manuel <[email protected]>
Signed-off-by: Manuel <[email protected]>
coratgerl
force-pushed
the
before-password-reset-request
branch
from
82812d1 to
ad8711c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
spec/CloudCode.spec.js (1)
4660-4812: Excellent test coverage with minor optimization opportunity.The test suite comprehensively covers the
beforePasswordResetRequesthook functionality with appropriate use of async/await patterns and correct email domains.However, in the fourth test (lines 4788-4793), consider combining the user signup and file save operations into a single operation to reduce database writes:
Apply this optimization:
const user = new Parse.User(); user.setUsername('testuser2'); user.setPassword('password'); user.set('email', '[email protected]'); + user.set('file', file); await user.signUp(); - await user.save({ file });This eliminates the redundant save operation while achieving the same result.
Based on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
spec/CloudCode.spec.js(2 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
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.
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.
📚 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/CloudCode.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/CloudCode.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/CloudCode.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/CloudCode.spec.js
📚 Learning: 2025-08-27T09:08:34.252Z
Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: src/triggers.js:446-454
Timestamp: 2025-08-27T09:08:34.252Z
Learning: When analyzing function signature changes in Parse Server codebase, verify that call sites are actually incorrect before flagging them. Passing tests are a strong indicator that function calls are already properly aligned with new signatures.
Applied to files:
spec/CloudCode.spec.js
📚 Learning: 2025-08-27T12:33:06.237Z
Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: src/triggers.js:467-477
Timestamp: 2025-08-27T12:33:06.237Z
Learning: In the Parse Server codebase, maybeRunAfterFindTrigger is called in production with Parse.Query objects constructed via withJSON(), so the plain object query handling bug only affects tests, not production code paths.
Applied to files:
spec/CloudCode.spec.js
🧬 Code graph analysis (1)
spec/CloudCode.spec.js (1)
spec/helper.js (2)
Parse(4-4)reconfigureServer(180-214)
🔇 Additional comments (1)
spec/CloudCode.spec.js (1)
3310-3322: LGTM!The updated validation error messages correctly include
beforePasswordResetRequestalongsidebeforeLoginandafterLoginto enforce that only the_Userclass is allowed for these triggers.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
spec/CloudCode.spec.js (1)
4660-4812: beforePasswordResetRequest test suite is thorough; consider one extra negative-side-effect assertionThe new
beforePasswordResetRequesttests nicely cover:
- happy path (hook runs, user data resolved, email adapter called),
- blocking behavior (error thrown, request fails, adapter not called),
- non-existent email (hook not invoked),
- full request context (including attached file) on the hook, and
_User-only validation for the new trigger, consistent with login hooks.Functionally and stylistically this looks solid and matches the async/await test guidelines. As an optional hardening step, you could also assert that no password reset token / related state is persisted when the hook throws (to prove DB writes are skipped as intended), but that can be deferred or covered in a more router-focused test if preferred.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
spec/CloudCode.spec.js(2 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
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.
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.
📚 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/CloudCode.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/CloudCode.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/CloudCode.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/CloudCode.spec.js
📚 Learning: 2025-08-27T09:08:34.252Z
Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: src/triggers.js:446-454
Timestamp: 2025-08-27T09:08:34.252Z
Learning: When analyzing function signature changes in Parse Server codebase, verify that call sites are actually incorrect before flagging them. Passing tests are a strong indicator that function calls are already properly aligned with new signatures.
Applied to files:
spec/CloudCode.spec.js
📚 Learning: 2025-08-27T12:33:06.237Z
Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: src/triggers.js:467-477
Timestamp: 2025-08-27T12:33:06.237Z
Learning: In the Parse Server codebase, maybeRunAfterFindTrigger is called in production with Parse.Query objects constructed via withJSON(), so the plain object query handling bug only affects tests, not production code paths.
Applied to files:
spec/CloudCode.spec.js
🧬 Code graph analysis (1)
spec/CloudCode.spec.js (2)
spec/helper.js (2)
Parse(4-4)reconfigureServer(180-214)src/cloud-code/Parse.Cloud.js (1)
emailAdapter(603-603)
🔇 Additional comments (1)
spec/CloudCode.spec.js (1)
3309-3322: Trigger validation message extension looks consistentUpdating the validation message to include
beforePasswordResetRequestkeeps this test aligned with the new_User-only hook semantics and mirrors the existing style forbeforeLogin/afterLogin. Just ensure the literal stays in sync with the actual error message insrc/triggers.jsso this test doesn’t become brittle on future wording tweaks.
|
Thanks for the adaptions. Could you please also open a PR in the docs to document the new trigger? Just look at where the existing triggers are. |
# [8.5.0-alpha.12](8.5.0-alpha.11...8.5.0-alpha.12) (2025-11-19) ### Features * Add `beforePasswordResetRequest` hook ([#9906](#9906)) ([94cee5b](94cee5b))
|
🎉 This change has been released in version 8.5.0-alpha.12 |
3 participants
Issue
Fixes: #9510
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Tests