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.
  • Introduced configurable logging for "username already exists" signup errors—set to 'warn', 'error', or 'silent' to control verbosity.

Tests

  • Added test coverage for username already exists logging behavior with different log level configurations.

✏️ 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

This PR introduces a new logEvents configuration option that allows customizing log levels for specific parse server events. Initially supporting usernameAlreadyExists, the feature enables operators to silence or adjust logging severity when signup fails due to duplicate username, while defaulting to error level logging.

Changes

Cohort / File(s) Change Summary
Configuration & Validation
src/Config.js, src/Options/Definitions.js
Adds logEvents configuration option with validation. Config.validateLogEvents() validates log event levels against allowed levels. Definitions.js exports logEvents option schema (env: PARSE_SERVER_LOG_EVENTS) and extends LogLevels with usernameAlreadyExists field (env: PARSE_SERVER_LOG_LEVELS_USERNAME_ALREADY_EXISTS, default: 'error').
TypeScript Type Definitions
src/Options/docs.js, src/Options/index.js, types/Options/index.d.ts
Extends ParseServerOptions interface with logEvents property and LogLevels interface with usernameAlreadyExists field across Flow, JSDoc, and TypeScript definition files.
Runtime Logging
src/middlewares.js
Implements logEvents behavior in handleParseErrors middleware. When error code is USERNAME_TAKEN, applies configured log level from logEvents.usernameAlreadyExists (defaulting to 'info'); silences logging if level is 'silent'.
Tests
spec/ParseUser.spec.js
Adds two test cases validating logEvents.usernameAlreadyExists behavior: one verifies 'warn' level logs a warning without error, another verifies 'silent' level suppresses all logs while still throwing USERNAME_TAKEN error.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • src/middlewares.js: Verify log level resolution, fallback to error, and 'silent' suppression logic handles all edge cases correctly
  • src/Config.js: Ensure validateLogEvents properly validates against LogLevels and applies defaults consistently
  • spec/ParseUser.spec.js: Confirm test coverage adequately validates both log and silent modes with duplicate signup attempts

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 PR title accurately describes the main feature: adding an option to change log levels for the username already exists message.
Description check ✅ Passed The PR description includes the required template structure with issue reference, but lacks implementation details in the Approach section.
Linked Issues check ✅ Passed All coding requirements from issue #9329 are met: logEvents.usernameAlreadyExists option added [#9329], customizable log levels implemented [#9329], and tests added validating behavior [#9329].
Out of Scope Changes check ✅ Passed All changes are directly scoped to implementing the logEvents.usernameAlreadyExists feature; no unrelated or extraneous 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 f75fad7 and 4526b0a.

📒 Files selected for processing (6)
  • src/Config.js (3 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)
🚧 Files skipped from review as they are similar to previous changes (4)
  • types/Options/index.d.ts
  • src/Config.js
  • src/Options/Definitions.js
  • src/Options/docs.js
🧰 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: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:

  • 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/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/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
🧬 Code graph analysis (1)
src/Options/index.js (1)
types/Options/index.d.ts (1)
  • LogLevels (295-302)
🪛 Biome (2.1.2)
src/Options/index.js

[error] 795-795: 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: PostgreSQL 15, PostGIS 3.4
  • GitHub Check: Node 22
  • GitHub Check: Node 18
  • GitHub Check: MongoDB 6, ReplicaSet
  • GitHub Check: Node 20
  • GitHub Check: MongoDB 8, ReplicaSet
  • GitHub Check: Redis Cache
  • 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.5
  • GitHub Check: PostgreSQL 18, PostGIS 3.6
  • GitHub Check: PostgreSQL 15, PostGIS 3.3
  • GitHub Check: Docker Build
  • GitHub Check: Benchmarks
🔇 Additional comments (2)
src/Options/index.js (2)

101-103: New logEvents option is well‑placed; verify downstream docs are generated

Adding logEvents: ?LogLevels; next to logLevels follows the existing pattern and keeps all logging‑related options together, which is good.

Given this is a new Parse Server option, just confirm that:

  • src/Options/Definitions.js has the corresponding logEvents entry (with PARSE_SERVER_LOG_EVENTS if intended), and
  • npm run definitions has been run so src/Options/docs.js and types/Options/index.d.ts are in sync.

This avoids drift between runtime behavior, types, and the generated docs. Based on learnings, …


795-798: Default value documentation for usernameAlreadyExists conflicts with implementation

Here the comment and :DEFAULT: error annotation state that usernameAlreadyExists defaults to 'error', but handleParseErrors currently defaults to 'info' when logEvents.usernameAlreadyExists is unset.

To keep behavior and documentation aligned, consider:

-  /* Log level used when a sign-up fails because the username already exists. Default is `error`. See [LogLevel](LogLevel.html) for available values.
-  :DEFAULT: error
+  /* Log level used when a sign-up fails because the username already exists. Default is `info`. See [LogLevel](LogLevel.html) for available values.
+  :DEFAULT: info
   */
   usernameAlreadyExists: ?string;

or adjust the middleware default back to 'error' if that is the intended behavior.

Also, the Biome parse error about ? is a false positive here; this file is Flow‑typed and already uses the same ?string syntax for other fields like triggerAfter, so no change is needed for that.


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

parseplatformorg commented Dec 6, 2025

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.

coderabbitai[bot]
coderabbitai bot previously approved these changes Dec 6, 2025
@codecov
Copy link

codecov bot commented Dec 6, 2025

Codecov Report

❌ Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 92.15%. Comparing base (e78e58d) to head (4526b0a).

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.15%   -0.42%     
==========================================
  Files         191      191              
  Lines       15544    15555      +11     
  Branches      177      177              
==========================================
- Hits        14389    14335      -54     
- Misses       1143     1204      +61     
- Partials       12       16       +4     

☔ 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.

@mtrezza
Copy link
Member

mtrezza commented Dec 7, 2025

I think we can also the default level for this to info. It's not really an error or warning.

Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LogEvents is still all over the code

@coratgerl
Copy link
Contributor Author

LogEvents is still all over the code

Do you mean this object ?
image

This is because in the issue you mentionned this kind of object:
image

Do you want I delete the object and pass directly the option ?

@mtrezza
Copy link
Member

mtrezza commented Dec 8, 2025

I see where you got this from. May have been a typo. logLevels does the same for Cloud Function and trigger events:

logLevels: {
  env: 'PARSE_SERVER_LOG_LEVELS',
  help: '(Optional) Overrides the log levels used internally by Parse Server to log events.',
  action: parsers.objectParser,
  type: 'LogLevels',
  default: {},
},

Let's add there, unless you can think of a reason not to? They are grouped by prefix, cloudFunction..., trigger..., so maybe add a prefix like signup... since this is a sign-up event and name it signupUsernameTaken, which is the same wording as ParseError.USERNAME_TAKEN = 202, so we have a nice mapping.

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

3 participants