Conversation
…cation in workspace/members settings
…umns and introduce new login medium labels
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
WalkthroughThis change introduces type-safe login medium constants and types across the codebase. It defines core and extended login medium types, creates corresponding label mappings in the constants package, reorganizes the TLoginMediums type definition, and updates the workspace member type to use the new type-safe TLoginMediums instead of string. The component is updated to render login medium labels from the new constants mapping. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/types/src/users.ts (1)
3-3: Add.tsextension to import type statement.Per the coding guidelines, import type statements should include file extensions.
Apply this diff:
-import type { TLoginMediums } from "./instance"; +import type { TLoginMediums } from "./instance/index.ts";Note: This is the same issue flagged in
packages/types/src/workspace.ts. Consider addressing both consistently.
🧹 Nitpick comments (6)
packages/types/src/instance/index.ts (1)
3-3: Add.tsextension to export statement.Per the coding guidelines, module specifiers in export statements should include file extensions.
Apply this diff:
-export * from "./auth-ee"; +export * from "./auth-ee.ts";Note: Consider updating all export statements in this file (lines 1-2, 4-7) to include
.tsextensions for consistency.packages/types/src/instance/base.ts (2)
2-9: Add.tsextension to import type statement.Per the coding guidelines, the import from the index module should include the file extension.
Apply this diff:
import type { TInstanceAIConfigurationKeys, TInstanceEmailConfigurationKeys, TInstanceImageConfigurationKeys, TInstanceAuthenticationKeys, TInstanceWorkspaceConfigurationKeys, TCoreLoginMediums, -} from "./"; +} from "./index.ts";
10-10: Add.tsextension to import type statement.Per the coding guidelines, import type statements should include file extensions.
Apply this diff:
-import type { TExtendedLoginMediums } from "./auth-ee"; +import type { TExtendedLoginMediums } from "./auth-ee.ts";packages/constants/src/auth/index.ts (1)
2-3: Add.tsextensions to import statements.Per the coding guidelines, relative imports should include file extensions.
Apply this diff:
-import { CORE_LOGIN_MEDIUM_LABELS } from "./core"; -import { EXTENDED_LOGIN_MEDIUM_LABELS } from "./extended"; +import { CORE_LOGIN_MEDIUM_LABELS } from "./core.ts"; +import { EXTENDED_LOGIN_MEDIUM_LABELS } from "./extended.ts";apps/web/ce/components/workspace/settings/useMemberColumns.tsx (1)
3-3: Consider a defensive fallback for unknown login mediumsThe new Authentication column logic (skip suspended / missing mediums, map via
LOGIN_MEDIUM_LABELS) looks good and is much clearer than string munging. One small hardening you might consider: if the backend ever introduces a newlast_login_mediumbefore the constants/types are updated,LOGIN_MEDIUM_LABELS[loginMedium]will renderundefined. A simple fallback likeLOGIN_MEDIUM_LABELS[loginMedium] ?? loginMedium(or an explicit"Unknown"label) would make this more robust without changing current behavior.Also applies to: 111-116
packages/constants/src/auth/core.ts (1)
1-10: Usesatisfiesfor the label map to avoid unnecessary wideningThe mapping looks correct and exhaustive for
TCoreLoginMediums, and usingimport typeis spot on. To better align with modern TS patterns and the repo’s guidelines, you can prefersatisfiesover a top-levelRecord<…>annotation so you keep literal value types while still enforcing key coverage:import type { TCoreLoginMediums } from "@plane/types"; export const CORE_LOGIN_MEDIUM_LABELS = { email: "Email", "magic-code": "Magic code", github: "GitHub", gitlab: "GitLab", google: "Google", gitea: "Gitea", } as const satisfies Record<TCoreLoginMediums, string>;Please confirm your TypeScript version comfortably supports
satisfies(TS 4.9+) and that this matches the project’s preferred typing style.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/web/ce/components/workspace/settings/useMemberColumns.tsx(4 hunks)packages/constants/src/auth/core.ts(1 hunks)packages/constants/src/auth/extended.ts(1 hunks)packages/constants/src/auth/index.ts(2 hunks)packages/types/src/instance/auth-ee.ts(1 hunks)packages/types/src/instance/auth.ts(1 hunks)packages/types/src/instance/base.ts(2 hunks)packages/types/src/instance/index.ts(1 hunks)packages/types/src/users.ts(1 hunks)packages/types/src/workspace.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(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 legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport 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 newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.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 viasuperin classes (TypeScript 5....
Files:
packages/types/src/instance/index.tspackages/constants/src/auth/extended.tsapps/web/ce/components/workspace/settings/useMemberColumns.tsxpackages/types/src/instance/auth.tspackages/constants/src/auth/index.tspackages/types/src/workspace.tspackages/constants/src/auth/core.tspackages/types/src/instance/base.tspackages/types/src/users.tspackages/types/src/instance/auth-ee.ts
🧠 Learnings (4)
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Use `.ts`, `.mts`, `.cts` extensions in `import type` statements (TypeScript 5.2+)
Applied to files:
packages/constants/src/auth/extended.ts
📚 Learning: 2025-10-21T17:22:05.204Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7989
File: apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx:45-46
Timestamp: 2025-10-21T17:22:05.204Z
Learning: In the makeplane/plane repository, the refactor from useParams() to params prop is specifically scoped to page.tsx and layout.tsx files in apps/web/app (Next.js App Router pattern). Other components (hooks, regular client components, utilities) should continue using the useParams() hook as that is the correct pattern for non-route components.
Applied to files:
apps/web/ce/components/workspace/settings/useMemberColumns.tsx
📚 Learning: 2025-10-09T20:42:31.843Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T20:42:31.843Z
Learning: In the makeplane/plane repository, React types are globally available through TypeScript configuration. Type annotations like React.FC, React.ReactNode, etc. can be used without explicitly importing the React namespace. The codebase uses the modern JSX transform, so React imports are not required for JSX or type references.
Applied to files:
apps/web/ce/components/workspace/settings/useMemberColumns.tsx
📚 Learning: 2025-05-28T09:53:44.635Z
Learnt from: prateekshourya29
Repo: makeplane/plane PR: 7094
File: web/core/store/user/base-permissions.store.ts:196-201
Timestamp: 2025-05-28T09:53:44.635Z
Learning: All role enums in this codebase (EUserPermissions, EUserWorkspaceRoles, EUserProjectRoles) use the same consistent numeric values: ADMIN = 20, MEMBER = 15, GUEST = 5. None of these enums have a value of 0, so truthy checks work correctly with these enum values.
Applied to files:
packages/types/src/workspace.ts
🧬 Code graph analysis (5)
packages/constants/src/auth/extended.ts (1)
packages/types/src/instance/auth-ee.ts (1)
TExtendedLoginMediums(1-1)
packages/constants/src/auth/index.ts (3)
packages/types/src/instance/base.ts (1)
TLoginMediums(104-104)packages/constants/src/auth/core.ts (1)
CORE_LOGIN_MEDIUM_LABELS(3-10)packages/constants/src/auth/extended.ts (1)
EXTENDED_LOGIN_MEDIUM_LABELS(3-3)
packages/types/src/workspace.ts (1)
packages/types/src/instance/base.ts (1)
TLoginMediums(104-104)
packages/constants/src/auth/core.ts (1)
packages/types/src/instance/auth.ts (1)
TCoreLoginMediums(47-47)
packages/types/src/instance/base.ts (2)
packages/types/src/instance/auth.ts (1)
TCoreLoginMediums(47-47)packages/types/src/instance/auth-ee.ts (1)
TExtendedLoginMediums(1-1)
⏰ 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: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (8)
packages/constants/src/auth/extended.ts (1)
1-3: LGTM! Good extensibility pattern.The empty EXTENDED_LOGIN_MEDIUM_LABELS with
Record<TExtendedLoginMediums, string>whereTExtendedLoginMediums = neveris a clean placeholder pattern that allows enterprise editions to override with additional login mediums without modifying core code.packages/types/src/workspace.ts (2)
86-86: Type safety improvement looks good.Changing
last_login_mediumfromstringtoTLoginMediumsprovides better type safety and enables IDE autocomplete for valid login medium values.
6-6: Verify the correct import path before applying the extension.The
.tsextension should be added toimport typestatements per TypeScript 5.2+ guidelines, but the target file must be verified first. The current suggestion of"./instance/index.ts"may be incorrect if the actual file is./instance.ts.Correct the import to use the proper file extension:
- If the target file is
instance.ts, use:import type { TLoginMediums } from "./instance.ts";- If the target is an
index.tsfile in aninstance/directory, then use:import type { TLoginMediums } from "./instance/index.ts";packages/types/src/instance/auth-ee.ts (1)
1-1: LGTM! Clean extensibility pattern.Defining
TExtendedLoginMediums = neveras a placeholder allows enterprise editions to augment the type without modifying core files. When unioned withTCoreLoginMediums, thenevertype is eliminated, keeping the type clean in CE builds.packages/types/src/instance/base.ts (1)
104-104: Type union correctly implements extensibility.The union
TLoginMediums = TCoreLoginMediums | TExtendedLoginMediumscleanly combines core and extended login mediums. SinceTExtendedLoginMediums = neverin CE, the union simplifies to justTCoreLoginMediums, with zero runtime overhead.packages/constants/src/auth/index.ts (1)
164-167: LOGIN_MEDIUM_LABELS implementation is type-safe and extensible.The composite mapping correctly combines core and extended labels using the spread operator. The type annotation
Record<TLoginMediums, string>ensures type safety, and theas constassertion provides readonly protection. This enables UI components to render labels for all login mediums through a single source of truth.packages/types/src/instance/auth.ts (1)
47-47: These types serve different purposes and should not be directly aligned.
TInstanceAuthenticationMethodKeysdefines configuration keys (UPPERCASE format) for toggling authentication methods in settings, whileTCoreLoginMediumsdefines the login method identifiers (lowercase) presented to users. The values correspond semantically (e.g., "email" ↔ "ENABLE_EMAIL_PASSWORD", "github" ↔ "IS_GITHUB_ENABLED"), but they intentionally use different naming conventions for their distinct roles. No changes needed.Likely an incorrect or invalid review comment.
apps/web/ce/components/workspace/settings/useMemberColumns.tsx (1)
17-17: Good call passingworkspaceSlugwithout castingUsing the
workspaceSlugfromuseParams()directly instead of forcing astringcast makes the typing more honest and keeps this hook consistent with other non-route components that correctly useuseParams. This aligns with the repo preference of only switching toparamsprops inpage.tsx/layout.tsxfiles.Also applies to: 53-53, 105-105
4 participants
Description
Type of Change
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.