⚠ This page is served via a proxy. Original site: https://github.com
This service does not collect credentials or authentication data.
Skip to content

Conversation

@JayashTripathy
Copy link
Member

@JayashTripathy JayashTripathy commented Jan 9, 2026

Description

This PR adds PASSWORD_TOO_WEAK error code and update related error handling in password change flow

Type of Change

  • Improvement (change that would cause existing functionality to not work as expected)
  • Code refactoring

Summary by CodeRabbit

  • New Features

    • Added an explicit "Password too weak" error code and exposed it in client error sets so weak-password cases are recognized and handled.
  • Bug Fixes

    • Change-password and reset flows now surface server-provided weak-password messages: shown as banner alerts and rendered inline beneath the new password field for clearer user feedback.

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 9, 2026

📝 Walkthrough

Walkthrough

Adds a new authentication error code PASSWORD_TOO_WEAK (5021) and updates backend validation, shared constants, frontend helper mappings, and UI handlers so weak-password checks emit the new code and surface field-level errors for new_password.

Changes

Cohort / File(s) Summary
Backend: error definitions & constants
apps/api/plane/authentication/adapter/error.py, packages/constants/src/auth/index.ts
Introduced PASSWORD_TOO_WEAK = "5021" to backend error codes and shared auth constants.
Backend: password validation & endpoints
apps/api/plane/authentication/adapter/base.py, apps/api/plane/authentication/views/common.py, apps/api/plane/authentication/views/app/password_management.py, apps/api/plane/authentication/views/space/password_management.py, apps/api/plane/license/api/views/admin.py
Replaced prior weak-password/invalid-password error usages with PASSWORD_TOO_WEAK (error_code and message) in password validation and reset/change/signup flows.
Frontend: helper mappings
apps/web/helpers/authentication.helper.tsx
Added EAuthenticationErrorCodes.PASSWORD_TOO_WEAK mapping, user-facing title/message, included it in banner handling, and exported passwordErrors (includes PASSWORD_TOO_WEAK).
Frontend: pages & forms
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx, apps/web/app/(all)/profile/security/page.tsx
Pulled setError from useForm; on change-password failure detect password-related codes and call setError('new_password', ...); render errors.new_password inline beneath the new-password input.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant Frontend
  participant API
  participant Constants

  User->>Frontend: Submit change-password (old, new)
  Frontend->>API: POST /change-password
  API->>API: validate_password(new) -> score
  alt score < threshold
    API->>Constants: use PASSWORD_TOO_WEAK code
    API-->>Frontend: 400 { error_code: "5021", message: "Password too weak" }
    Frontend->>Frontend: authErrorHandler -> maps code to message
    Frontend->>Frontend: setError('new_password', { type: 'manual', message })
    Frontend-->>User: display inline new_password error
  else score >= threshold
    API-->>Frontend: 200 success
    Frontend-->>User: success UI
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I nibbled bytes and hopped the stack,
A tiny code — five-oh-two-one — on my back.
Server whispers "too weak", the form gives a peep,
The password field wakes from its gentle sleep.
Hooray — I thump my foot and guard your keep! 🥕

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The description covers the main objectives but lacks detailed explanation of what was changed and why; Test Scenarios section is completely empty. Add more detailed explanation of the changes across backend and frontend files, and include test scenarios describing how the PASSWORD_TOO_WEAK error is validated.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: adding a new PASSWORD_TOO_WEAK error code to the authentication system.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f6ed777 and d110590.

📒 Files selected for processing (4)
  • apps/api/plane/authentication/adapter/base.py
  • apps/api/plane/authentication/views/app/password_management.py
  • apps/api/plane/authentication/views/space/password_management.py
  • apps/api/plane/license/api/views/admin.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-23T14:18:32.899Z
Learnt from: dheeru0198
Repo: makeplane/plane PR: 8339
File: apps/api/plane/db/models/api.py:35-35
Timestamp: 2025-12-23T14:18:32.899Z
Learning: Django REST Framework rate limit strings are flexible: only the first character of the time unit matters. Acceptable formats include: "60/s", "60/sec", "60/second" (all equivalent), "60/m", "60/min", "60/minute" (all equivalent), "60/h", "60/hr", "60/hour" (all equivalent), and "60/d", "60/day" (all equivalent). Abbreviations like "min" are valid and do not need to be changed to "minute". Apply this guidance to any Python files in the project that configure DRF throttling rules.

Applied to files:

  • apps/api/plane/license/api/views/admin.py
  • apps/api/plane/authentication/views/space/password_management.py
  • apps/api/plane/authentication/adapter/base.py
  • apps/api/plane/authentication/views/app/password_management.py
⏰ 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). (2)
  • GitHub Check: check:lint
  • GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
apps/api/plane/authentication/adapter/base.py (1)

79-88: LGTM! Core password validation now uses the unified error code.

The validate_password method correctly raises PASSWORD_TOO_WEAK when the password strength score is below 3, which is consistent with the refactoring across all password validation paths.

apps/api/plane/authentication/views/space/password_management.py (1)

134-142: LGTM! Space password reset now uses the unified error code.

The error code change for weak passwords is consistent with the app endpoint and other password validation paths in this PR.

apps/api/plane/authentication/views/app/password_management.py (1)

140-151: LGTM! App password reset now uses the unified error code.

The error code change for weak passwords is consistent with the space endpoint and base adapter changes in this PR.

apps/api/plane/license/api/views/admin.py (1)

187-204: LGTM! Error code update aligns with the refactoring objectives.

The change from INVALID_ADMIN_PASSWORD to PASSWORD_TOO_WEAK improves error specificity and aligns with the unified password strength validation error handling introduced in this PR. The error code is properly defined in AUTHENTICATION_ERROR_CODES (value 5021) and consistently applied across password validation endpoints throughout the codebase.


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.

@makeplane
Copy link

makeplane bot commented Jan 9, 2026

Linked to Plane Work Item(s)

This comment was auto-generated by Plane

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a new error code PASSWORD_TOO_WEAK (5021) to provide more specific error handling for weak passwords in the password change flow. Previously, the system used INVALID_NEW_PASSWORD for this purpose, but this change creates a distinct error code specifically for password strength validation.

Key changes:

  • Added PASSWORD_TOO_WEAK error code (5021) across frontend and backend
  • Updated password change endpoint to use the new error code for weak passwords
  • Enhanced UI error handling to display field-level validation errors for weak passwords

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
packages/constants/src/auth/index.ts Added PASSWORD_TOO_WEAK enum value (5021) to the auth error codes
apps/web/helpers/authentication.helper.tsx Added PASSWORD_TOO_WEAK error code, error message, banner alert handling, and passwordErrors array for field-level validation
apps/web/app/(all)/profile/security/page.tsx Updated error handling to display field-level validation errors for password errors using setError
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx Updated error handling to display field-level validation errors for password errors using setError
apps/api/plane/authentication/views/common.py Changed password strength validation error from INVALID_NEW_PASSWORD to PASSWORD_TOO_WEAK
apps/api/plane/authentication/adapter/error.py Added PASSWORD_TOO_WEAK error code mapping (5021) to the backend authentication error codes

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @apps/api/plane/authentication/views/common.py:
- Around line 82-83: The SetUserPasswordEndpoint currently returns
AUTHENTICATION_ERROR_CODES["INVALID_PASSWORD"] when results["score"] < 3,
causing inconsistency with ChangePasswordEndpoint; update the
SetUserPasswordEndpoint error response to use
AUTHENTICATION_ERROR_CODES["PASSWORD_TOO_WEAK"] and set error_message to
"PASSWORD_TOO_WEAK" for the same results["score"] < 3 branch so both endpoints
return the same error code/message for weak passwords.
🧹 Nitpick comments (1)
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx (1)

92-108: Verify the duplicate error messaging UX pattern.

Password-related errors will display both:

  1. A toast notification (lines 96-101)
  2. An inline field error (line 208)

This double notification might be intentional to ensure users don't miss the validation feedback, but please confirm this is the desired UX.

Optional: Line 106 - .toString() is likely redundant.

Based on the authErrorHandler implementation, errorInfo.message is already a string. The .toString() call is safe but unnecessary.

♻️ Optional simplification
       if (passwordErrors.includes(code as EAuthenticationErrorCodes)) {
         setError("new_password", {
           type: "manual",
-          message: errorInfo?.message?.toString() || t("auth.common.password.toast.error.message"),
+          message: errorInfo?.message || t("auth.common.password.toast.error.message"),
         });
       }
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8399f64 and a3023d1.

📒 Files selected for processing (6)
  • apps/api/plane/authentication/adapter/error.py
  • apps/api/plane/authentication/views/common.py
  • apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx
  • apps/web/app/(all)/profile/security/page.tsx
  • apps/web/helpers/authentication.helper.tsx
  • packages/constants/src/auth/index.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,mts,cts}

📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)

**/*.{ts,tsx,mts,cts}: Use const type parameters for more precise literal inference in TypeScript 5.0+
Use the satisfies operator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicit is return types in filter/check functions
Use NoInfer<T> utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing in switch(true) blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacy experimentalDecorators
Use using declarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Use with { type: "json" } for import attributes; avoid deprecated assert syntax (TypeScript 5.3/5.8+)
Use import type explicitly when importing types to ensure they are erased during compilation, respecting verbatimModuleSyntax flag
Use .ts, .mts, .cts extensions in import type statements (TypeScript 5.2+)
Use import type { Type } from "mod" with { "resolution-mode": "import" } for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize new Set methods like union, intersection, etc., when available (TypeScript 5.5+)
Use Object.groupBy / Map.groupBy standard methods for grouping instead of external libraries (TypeScript 5.4+)
Use Promise.withResolvers() for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted, toSpliced, with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields via super in classes (TypeScript 5....

Files:

  • packages/constants/src/auth/index.ts
  • apps/web/app/(all)/profile/security/page.tsx
  • apps/web/helpers/authentication.helper.tsx
  • apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Enable TypeScript strict mode and ensure all files are fully typed

Files:

  • packages/constants/src/auth/index.ts
  • apps/web/app/(all)/profile/security/page.tsx
  • apps/web/helpers/authentication.helper.tsx
  • apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx
**/*.{js,jsx,ts,tsx,json,css}

📄 CodeRabbit inference engine (AGENTS.md)

Use Prettier with Tailwind plugin for code formatting, run pnpm fix:format

Files:

  • packages/constants/src/auth/index.ts
  • apps/web/app/(all)/profile/security/page.tsx
  • apps/web/helpers/authentication.helper.tsx
  • apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{js,jsx,ts,tsx}: Use ESLint with shared config across packages, adhering to max warnings limits per package
Use camelCase for variable and function names, PascalCase for components and types
Use try-catch with proper error types and log errors appropriately

Files:

  • packages/constants/src/auth/index.ts
  • apps/web/app/(all)/profile/security/page.tsx
  • apps/web/helpers/authentication.helper.tsx
  • apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx
🧠 Learnings (2)
📚 Learning: 2025-07-08T13:41:19.928Z
Learnt from: prateekshourya29
Repo: makeplane/plane PR: 7363
File: apps/web/helpers/react-hook-form.helper.ts:9-19
Timestamp: 2025-07-08T13:41:19.928Z
Learning: In the apps/web/helpers/react-hook-form.helper.ts file, the getNestedError function's FieldError detection logic using "message" property check is sufficient for their use case, and more robust type checking is not required.

Applied to files:

  • apps/web/app/(all)/profile/security/page.tsx
  • apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx
📚 Learning: 2025-12-23T14:18:32.899Z
Learnt from: dheeru0198
Repo: makeplane/plane PR: 8339
File: apps/api/plane/db/models/api.py:35-35
Timestamp: 2025-12-23T14:18:32.899Z
Learning: Django REST Framework rate limit strings are flexible: only the first character of the time unit matters. Acceptable formats include: "60/s", "60/sec", "60/second" (all equivalent), "60/m", "60/min", "60/minute" (all equivalent), "60/h", "60/hr", "60/hour" (all equivalent), and "60/d", "60/day" (all equivalent). Abbreviations like "min" are valid and do not need to be changed to "minute". Apply this guidance to any Python files in the project that configure DRF throttling rules.

Applied to files:

  • apps/api/plane/authentication/views/common.py
  • apps/api/plane/authentication/adapter/error.py
🧬 Code graph analysis (2)
apps/web/app/(all)/profile/security/page.tsx (1)
apps/web/helpers/authentication.helper.tsx (1)
  • passwordErrors (435-438)
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx (1)
apps/web/helpers/authentication.helper.tsx (2)
  • authErrorHandler (369-433)
  • passwordErrors (435-438)
⏰ 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). (6)
  • GitHub Check: Lint API
  • GitHub Check: CodeQL analysis (javascript-typescript)
  • GitHub Check: CodeQL analysis (python)
  • GitHub Check: Agent
  • GitHub Check: Analyze (javascript)
  • GitHub Check: Build packages
🔇 Additional comments (7)
packages/constants/src/auth/index.ts (1)

111-111: LGTM!

The new error code is well-placed in the password strength section and follows the existing numbering convention.

apps/api/plane/authentication/adapter/error.py (1)

12-12: LGTM!

The error code mapping is correctly placed and consistent with the frontend definition.

apps/web/helpers/authentication.helper.tsx (1)

44-44: LGTM! Clean error handling additions.

The changes are well-structured:

  • Error code and message mapping follow existing patterns
  • Banner alert handling is consistent with other error codes
  • The new passwordErrors export provides a clean way to group password-related errors for field-level validation

Also applies to: 142-145, 421-421, 435-438

apps/web/app/(all)/profile/security/page.tsx (1)

17-17: LGTM! Excellent UX improvement with field-level errors.

The implementation correctly uses react-hook-form's setError to display server-side password validation errors directly on the new_password field, in addition to the toast notification. This provides immediate, contextual feedback to users.

The error handling logic is clean:

  • Checks if the error code is password-related using the passwordErrors array
  • Sets a manual field error with the appropriate message
  • Safely converts ReactNode messages to strings with a fallback

Also applies to: 56-56, 101-107, 208-210

apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx (3)

16-17: LGTM! Clean imports for the new error handling.

The imports are appropriate and necessary for the server-side password validation feedback feature.


56-56: LGTM! Proper integration of setError for programmatic field validation.

The addition of setError enables server-side validation messages to be displayed inline on the form field.


208-208: LGTM! Proper inline error display for server-side validation.

The error rendering correctly displays the manual field error set by setError, providing immediate feedback to users when password validation fails server-side.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In
@apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx:
- Around line 91-109: The catch block must guard against non-object/null thrown
values and avoid calling includes with undefined: first check that error is an
object and not null (e.g., if (typeof error !== "object" || error === null) {
set a generic toast via setToast and return; }), then safely extract code (const
err = error as Error & { error_code?: string }; const code =
err.error_code?.toString();), and only call passwordErrors.includes(...) if code
is a non-empty string (e.g., if (typeof code === "string" && code &&
passwordErrors.includes(code as EAuthenticationErrorCodes)) {
setError("new_password", ... ) }). Ensure authErrorHandler is only called with a
defined code and fall back to generic messages when code is absent.
🧹 Nitpick comments (2)
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx (2)

16-17: Consider consolidating the type import into the named import (optional).
Current approach is correct; if you want to reduce import lines, TS supports type modifiers in named imports.


208-208: Nice UX improvement: inline new_password error rendering.
Optional: consider role="alert" (or whatever your form-error convention is) so screen readers announce updates.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a3023d1 and 3c53207.

📒 Files selected for processing (1)
  • apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,mts,cts}

📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)

**/*.{ts,tsx,mts,cts}: Use const type parameters for more precise literal inference in TypeScript 5.0+
Use the satisfies operator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicit is return types in filter/check functions
Use NoInfer<T> utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing in switch(true) blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacy experimentalDecorators
Use using declarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Use with { type: "json" } for import attributes; avoid deprecated assert syntax (TypeScript 5.3/5.8+)
Use import type explicitly when importing types to ensure they are erased during compilation, respecting verbatimModuleSyntax flag
Use .ts, .mts, .cts extensions in import type statements (TypeScript 5.2+)
Use import type { Type } from "mod" with { "resolution-mode": "import" } for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize new Set methods like union, intersection, etc., when available (TypeScript 5.5+)
Use Object.groupBy / Map.groupBy standard methods for grouping instead of external libraries (TypeScript 5.4+)
Use Promise.withResolvers() for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted, toSpliced, with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields via super in classes (TypeScript 5....

Files:

  • apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Enable TypeScript strict mode and ensure all files are fully typed

Files:

  • apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx
**/*.{js,jsx,ts,tsx,json,css}

📄 CodeRabbit inference engine (AGENTS.md)

Use Prettier with Tailwind plugin for code formatting, run pnpm fix:format

Files:

  • apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{js,jsx,ts,tsx}: Use ESLint with shared config across packages, adhering to max warnings limits per package
Use camelCase for variable and function names, PascalCase for components and types
Use try-catch with proper error types and log errors appropriately

Files:

  • apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx
🧬 Code graph analysis (1)
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx (1)
apps/web/helpers/authentication.helper.tsx (2)
  • authErrorHandler (369-433)
  • passwordErrors (435-438)
⏰ 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). (1)
  • GitHub Check: check:lint
🔇 Additional comments (2)
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx (2)

52-59: Good: setError wired up for field-level server validation.
This aligns well with react-hook-form usage for server-side validation feedback.


16-17: EAuthenticationErrorCodes is an enum with string values; PASSWORD_TOO_WEAK is both mapped in authErrorHandler and included in passwordErrors.

Since PASSWORD_TOO_WEAK is in the bannerAlertErrorCodes array (line 421), authErrorHandler will always return a TAuthErrorInfo object (not undefined) for this error code. However, the code has type safety issues: code is cast as EAuthenticationErrorCodes without validation, and the .toString() call on err.error_code is redundant if it's already a string. Additionally, calling .toString() on message (a ReactNode) is problematic—it works for the string literal in PASSWORD_TOO_WEAK but would fail for complex React elements.

Consider validating the error code against known values before casting, removing the unnecessary toString() calls, and handling message as a ReactNode without conversion.

⛔ Skipped due to learnings
Learnt from: prateekshourya29
Repo: makeplane/plane PR: 7363
File: apps/web/helpers/react-hook-form.helper.ts:9-19
Timestamp: 2025-07-08T13:41:19.928Z
Learning: In the apps/web/helpers/react-hook-form.helper.ts file, the getNestedError function's FieldError detection logic using "message" property check is sufficient for their use case, and more robust type checking is not required.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx (1)

91-108: Inconsistent message handling between toast and field error.

Line 100 guards against non-string messages with typeof errorInfo?.message === "string", but line 106 uses errorInfo?.message?.toString() which could produce unexpected results if message is not a string (e.g., "[object Function]").

For consistency, use the same resolution logic:

♻️ Suggested fix
       if (code && passwordErrors.includes(code as EAuthenticationErrorCodes)) {
+        const fieldErrorMessage =
+          typeof errorInfo?.message === "string" ? errorInfo.message : t("auth.common.password.toast.error.message");
         setError("new_password", {
           type: "manual",
-          message: errorInfo?.message?.toString() || t("auth.common.password.toast.error.message"),
+          message: fieldErrorMessage,
         });
       }
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3c53207 and f6ed777.

📒 Files selected for processing (2)
  • apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx
  • apps/web/app/(all)/profile/security/page.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/web/app/(all)/profile/security/page.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,mts,cts}

📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)

**/*.{ts,tsx,mts,cts}: Use const type parameters for more precise literal inference in TypeScript 5.0+
Use the satisfies operator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicit is return types in filter/check functions
Use NoInfer<T> utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing in switch(true) blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacy experimentalDecorators
Use using declarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Use with { type: "json" } for import attributes; avoid deprecated assert syntax (TypeScript 5.3/5.8+)
Use import type explicitly when importing types to ensure they are erased during compilation, respecting verbatimModuleSyntax flag
Use .ts, .mts, .cts extensions in import type statements (TypeScript 5.2+)
Use import type { Type } from "mod" with { "resolution-mode": "import" } for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize new Set methods like union, intersection, etc., when available (TypeScript 5.5+)
Use Object.groupBy / Map.groupBy standard methods for grouping instead of external libraries (TypeScript 5.4+)
Use Promise.withResolvers() for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted, toSpliced, with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields via super in classes (TypeScript 5....

Files:

  • apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Enable TypeScript strict mode and ensure all files are fully typed

Files:

  • apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx
**/*.{js,jsx,ts,tsx,json,css}

📄 CodeRabbit inference engine (AGENTS.md)

Use Prettier with Tailwind plugin for code formatting, run pnpm fix:format

Files:

  • apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{js,jsx,ts,tsx}: Use ESLint with shared config across packages, adhering to max warnings limits per package
Use camelCase for variable and function names, PascalCase for components and types
Use try-catch with proper error types and log errors appropriately

Files:

  • apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx
🧠 Learnings (2)
📚 Learning: 2025-07-08T13:41:19.928Z
Learnt from: prateekshourya29
Repo: makeplane/plane PR: 7363
File: apps/web/helpers/react-hook-form.helper.ts:9-19
Timestamp: 2025-07-08T13:41:19.928Z
Learning: In the apps/web/helpers/react-hook-form.helper.ts file, the getNestedError function's FieldError detection logic using "message" property check is sufficient for their use case, and more robust type checking is not required.

Applied to files:

  • apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx
📚 Learning: 2025-12-12T15:20:36.542Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T15:20:36.542Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use try-catch with proper error types and log errors appropriately

Applied to files:

  • apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx
🧬 Code graph analysis (1)
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx (1)
apps/web/helpers/authentication.helper.tsx (2)
  • authErrorHandler (369-433)
  • passwordErrors (435-438)
⏰ 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). (3)
  • GitHub Check: check:lint
  • GitHub Check: check:types
  • GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/account/security/page.tsx (3)

16-17: LGTM!

The import of passwordErrors alongside authErrorHandler is appropriate and follows the existing pattern.


52-59: LGTM!

The setError destructuring from useForm is correct and follows react-hook-form conventions.


207-213: LGTM!

The inline error display for new_password follows the established pattern used for other fields and is properly positioned after the strength indicator.

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.

4 participants