Skip to content

Conversation

@coratgerl
Copy link
Contributor

@coratgerl coratgerl commented Dec 6, 2025

Pull Request

Issue

Fixes: #9329

  • Add tests
  • Add changes to documentation (guides, repository pages, code comments)
  • Add security check
  • Add new Parse Error codes to Parse JS SDK

Summary by CodeRabbit

  • New Features

    • Added logEvents configuration option to customize log levels for specific server events.
    • Implemented per-event log level control for username-taken signup failures (configurable as 'error', 'warn', or 'silent').
  • Tests

    • Added tests validating logging behavior for duplicate username signup attempts.

✏️ Tip: You can customize this high-level summary in your review settings.

@parse-github-assistant
Copy link

🚀 Thanks for opening this pull request!

@coderabbitai
Copy link

coderabbitai bot commented Dec 6, 2025

📝 Walkthrough

Walkthrough

These changes introduce a new logEvents configuration option enabling customization of log levels for specific events. The initial implementation allows users to configure (or silence) logging for the "username already exists" error during signup, previously logged unconditionally.

Changes

Cohort / File(s) Summary
Configuration option definitions
src/Options/Definitions.js, src/Options/docs.js, src/Options/index.js, types/Options/index.d.ts
Adds new logEvents option with usernameAlreadyExists field to ParseServerOptions, with corresponding TypeScript definitions and documentation. Defaults to 'error' level.
Configuration validation
src/Config.js
Introduces validateLogEvents() static method to validate configured log events against valid log levels, integrating logEvents into Config.validateOptions() flow.
Logging implementation
src/middlewares.js
Makes USERNAME_TAKEN error logging conditional based on logEvents.usernameAlreadyExists config; supports 'silent' to suppress, or specific log level functions.
Build configuration
resources/buildConfigDefinitions.js
Registers LogEvents as a nested option type and adds corresponding environment variable prefix mapping.
Test coverage
spec/ParseUser.spec.js
Adds two tests validating logging behavior for USERNAME_TAKEN error with logEvents.usernameAlreadyExists set to 'warn' and 'silent'.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • src/Config.js — New validation method must correctly validate log event levels; verify consistency with LogLevels handling
  • src/middlewares.js — Conditional logging logic is critical; ensure graceful fallback and that 'silent' mode properly suppresses logs
  • Option definition files (src/Options/*, types/Options/index.d.ts) — Verify all definition files (Definitions.js, docs.js, index.js, TypeScript types) remain consistent in structure and defaults

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding an option to configure log levels for the username already exists message.
Description check ✅ Passed The description references the linked issue #9329 and marks 'Add tests' as complete, but lacks implementation details and approach explanation.
Linked Issues check ✅ Passed The PR implements the requested feature to add logEvents.usernameAlreadyExists option allowing control over log levels including silencing.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing the logEvents.usernameAlreadyExists feature and its tests; no out-of-scope modifications detected.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e78e58d and f75fad7.

📒 Files selected for processing (8)
  • resources/buildConfigDefinitions.js (2 hunks)
  • spec/ParseUser.spec.js (1 hunks)
  • src/Config.js (4 hunks)
  • src/Options/Definitions.js (2 hunks)
  • src/Options/docs.js (2 hunks)
  • src/Options/index.js (2 hunks)
  • src/middlewares.js (2 hunks)
  • types/Options/index.d.ts (2 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 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.
📚 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:

  • resources/buildConfigDefinitions.js
  • types/Options/index.d.ts
  • src/Options/Definitions.js
  • src/Options/docs.js
  • src/Config.js
  • src/Options/index.js
📚 Learning: 2025-12-02T06:55:53.808Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-12-02T06:55:53.808Z
Learning: When reviewing Parse Server PRs that add or modify Parse Server options, always verify that changes are properly reflected in three files: src/Options/index.js (where changes originate), src/Options/Definitions.js, and src/Options/docs.js. The correct workflow is: make changes in index.js first, then run `npm run definitions` to automatically replicate the changes to Definitions.js and docs.js.

Applied to files:

  • src/Options/Definitions.js
  • src/Options/docs.js
  • src/Options/index.js
📚 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: 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.

Applied to files:

  • src/Options/docs.js
  • src/Options/index.js
📚 Learning: 2025-11-17T15:02:48.786Z
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.

Applied to files:

  • src/Options/index.js
📚 Learning: 2025-11-17T15:02:24.824Z
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.

Applied to files:

  • src/Options/index.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/ParseUser.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/ParseUser.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/ParseUser.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/ParseUser.spec.js
🧬 Code graph analysis (5)
src/Options/Definitions.js (2)
resources/buildConfigDefinitions.js (1)
  • parsers (12-12)
src/Adapters/AdapterLoader.js (1)
  • module (49-49)
src/Config.js (1)
types/Options/index.d.ts (1)
  • LogEvents (302-304)
src/Options/index.js (1)
types/Options/index.d.ts (1)
  • LogEvents (302-304)
src/middlewares.js (3)
src/Adapters/Storage/Mongo/MongoStorageAdapter.js (1)
  • err (523-526)
src/Adapters/Storage/Postgres/PostgresStorageAdapter.js (2)
  • err (1476-1479)
  • log (23-23)
src/ParseServerRESTController.js (1)
  • Parse (4-4)
spec/ParseUser.spec.js (1)
spec/helper.js (2)
  • reconfigureServer (180-214)
  • Parse (4-4)
🪛 Biome (2.1.2)
src/Options/index.js

[error] 798-798: Expected a statement but instead found '?'.

Expected a statement here.

(parse)


[error] 798-798: Expected a statement but instead found '}'.

Expected a statement here.

(parse)

⏰ 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 22
  • GitHub Check: Node 18
  • GitHub Check: Node 20
  • GitHub Check: MongoDB 7, ReplicaSet
  • GitHub Check: MongoDB 8, ReplicaSet
  • GitHub Check: Redis Cache
  • GitHub Check: MongoDB 6, ReplicaSet
  • GitHub Check: PostgreSQL 15, PostGIS 3.5
  • GitHub Check: PostgreSQL 18, PostGIS 3.6
  • GitHub Check: PostgreSQL 16, PostGIS 3.5
  • GitHub Check: PostgreSQL 17, PostGIS 3.5
  • GitHub Check: PostgreSQL 15, PostGIS 3.4
  • GitHub Check: PostgreSQL 15, PostGIS 3.3
  • GitHub Check: Docker Build
  • GitHub Check: Benchmarks
🔇 Additional comments (10)
resources/buildConfigDefinitions.js (1)

28-28: LGTM!

The addition of LogEvents to the nested option types and environment variable prefix mapping is consistent with existing patterns like LogLevels.

Also applies to: 43-43

src/Options/docs.js (1)

66-66: LGTM!

The documentation for the new logEvents option and LogEvents interface is clear and consistent with existing patterns. The help text appropriately describes the feature and references available log levels.

Also applies to: 334-337

types/Options/index.d.ts (1)

50-50: LGTM!

The TypeScript type declarations for logEvents and LogEvents are correctly defined and consistent with the corresponding Flow definitions.

Also applies to: 302-304

src/middlewares.js (1)

469-469: LGTM!

The conditional logging implementation for USERNAME_TAKEN errors is well-designed:

  • Respects the configured log level from logEvents.usernameAlreadyExists
  • Correctly silences logs when level is set to 'silent'
  • Includes defensive fallback to log.error for invalid log levels
  • Maintains existing behavior for all other error codes

Also applies to: 484-494

src/Options/Definitions.js (1)

355-361: LGTM!

The generated definitions for logEvents and LogEvents are correctly structured with appropriate environment variable mappings, help text, and default values. The configuration follows established patterns in the codebase.

Also applies to: 1517-1524

src/Options/index.js (1)

797-802: Verify Flow type checking passes on the LogEvents interface.

The LogEvents interface definition appears syntactically correct and consistent with other Flow interfaces in this file. Biome may be reporting a false positive since it has limited Flow syntax support. Please run npm run flow check src/Options/index.js to confirm the code compiles successfully with Flow type checking.

spec/ParseUser.spec.js (2)

84-110: LGTM! Test correctly validates configurable log level for username taken event.

The test properly:

  1. Reconfigures the server with the new logEvents.usernameAlreadyExists: 'warn' option
  2. Re-establishes spies after server reconfiguration (necessary since the module is reloaded)
  3. Verifies the warning is logged instead of an error
  4. Confirms the functional behavior (error code USERNAME_TAKEN) is preserved

Based on learnings, this test correctly uses async/await with promise-based patterns.


112-135: LGTM! Test correctly validates silent mode for username taken log event.

The test properly verifies that when logEvents.usernameAlreadyExists is set to 'silent':

  1. No warnings are logged
  2. No errors are logged
  3. The USERNAME_TAKEN error is still thrown to the client

This ensures the feature works as intended per issue #9329 - allowing administrators to silence the log event while maintaining correct error handling.

src/Config.js (2)

17-17: LGTM! Integration of logEvents validation follows established patterns.

The changes correctly:

  1. Import LogEvents from Definitions
  2. Add logEvents to the destructured parameters in validateOptions
  3. Call the validation method in the appropriate location

Also applies to: 132-132, 175-175


647-658: Add null/undefined guard to prevent crash when logEvents is not configured.

If logEvents is not provided in the server configuration, Object.keys(logEvents) will throw a TypeError. Other validators like validateDatabaseOptions handle this by returning early if the option is undefined.

Apply this diff:

 static validateLogEvents(logEvents) {
+  if (!logEvents) {
+    return;
+  }
   for (const key of Object.keys(LogEvents)) {
     if (logEvents[key]) {
       // We validate that each configured event uses a valid log *level* (same list as logLevels).
       if (validLogLevels.indexOf(logEvents[key]) === -1) {
         throw `'${key}' must be one of ${JSON.stringify(validLogLevels)}`;
       }
     } else {
       logEvents[key] = LogEvents[key].default;
     }
   }
 }

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ast-grep (0.40.0)
spec/ParseUser.spec.js

[]


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@parseplatformorg
Copy link
Contributor

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@codecov
Copy link

codecov bot commented Dec 6, 2025

Codecov Report

❌ Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 92.58%. Comparing base (e78e58d) to head (f75fad7).

Files with missing lines Patch % Lines
src/Config.js 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            alpha    #9962      +/-   ##
==========================================
+ Coverage   92.56%   92.58%   +0.01%     
==========================================
  Files         191      191              
  Lines       15544    15556      +12     
  Branches      177      177              
==========================================
+ Hits        14389    14402      +13     
+ Misses       1143     1142       -1     
  Partials       12       12              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add log option for username taken event

2 participants