[WEB-4959]chore: refactor members page #8475
Conversation
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
📝 WalkthroughWalkthroughThis PR refactors member stores by removing optimistic-activity mutation hooks ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
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: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/web/ce/store/member/project-member.store.tsapps/web/ce/store/member/workspace-member.store.tsapps/web/ce/store/workspace/index.tsapps/web/core/store/member/index.tsapps/web/core/store/member/project/base-project-member.store.tsapps/web/core/store/member/workspace/workspace-member.store.tsapps/web/core/store/root.store.tsapps/web/core/store/workspace/index.ts
💤 Files with no reviewable changes (2)
- apps/web/ce/store/member/project-member.store.ts
- apps/web/ce/store/workspace/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}: 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:
apps/web/ce/store/member/workspace-member.store.tsapps/web/core/store/member/index.tsapps/web/core/store/member/workspace/workspace-member.store.tsapps/web/core/store/root.store.tsapps/web/core/store/member/project/base-project-member.store.tsapps/web/core/store/workspace/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Enable TypeScript strict mode and ensure all files are fully typed
Files:
apps/web/ce/store/member/workspace-member.store.tsapps/web/core/store/member/index.tsapps/web/core/store/member/workspace/workspace-member.store.tsapps/web/core/store/root.store.tsapps/web/core/store/member/project/base-project-member.store.tsapps/web/core/store/workspace/index.ts
**/*.{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/ce/store/member/workspace-member.store.tsapps/web/core/store/member/index.tsapps/web/core/store/member/workspace/workspace-member.store.tsapps/web/core/store/root.store.tsapps/web/core/store/member/project/base-project-member.store.tsapps/web/core/store/workspace/index.ts
**/*.{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/ce/store/member/workspace-member.store.tsapps/web/core/store/member/index.tsapps/web/core/store/member/workspace/workspace-member.store.tsapps/web/core/store/root.store.tsapps/web/core/store/member/project/base-project-member.store.tsapps/web/core/store/workspace/index.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: makeplane/plane PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T15:20:36.542Z
Learning: Applies to packages/shared-state/**/*.{ts,tsx} : Maintain MobX stores in `packages/shared-state` using reactive patterns
📚 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 packages/shared-state/**/*.{ts,tsx} : Maintain MobX stores in `packages/shared-state` using reactive patterns
Applied to files:
apps/web/ce/store/member/workspace-member.store.tsapps/web/core/store/member/index.tsapps/web/core/store/member/workspace/workspace-member.store.tsapps/web/core/store/root.store.ts
📚 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 **/package.json : Use `workspace:*` for internal packages and `catalog:` for external dependencies in imports
Applied to files:
apps/web/core/store/root.store.ts
📚 Learning: 2025-10-06T01:44:38.472Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/collapsible/collapsible.stories.tsx:4-4
Timestamp: 2025-10-06T01:44:38.472Z
Learning: In Storybook v9, imports use bare paths instead of scoped packages. For example, `import { useArgs } from "storybook/preview-api"` is correct (not `storybook/preview-api`). This applies to other Storybook modules as well - the scoped storybook/* packages were consolidated into bare "storybook/*" imports.
Applied to files:
apps/web/core/store/root.store.ts
📚 Learning: 2025-06-16T07:23:39.497Z
Learnt from: vamsikrishnamathala
Repo: makeplane/plane PR: 7214
File: web/core/store/issue/helpers/base-issues.store.ts:117-117
Timestamp: 2025-06-16T07:23:39.497Z
Learning: In the updateIssueDates method of BaseIssuesStore (web/core/store/issue/helpers/base-issues.store.ts), the projectId parameter is intentionally made optional to support override implementations in subclasses. The base implementation requires projectId and includes an early return check, but making it optional allows derived classes to override the method with different parameter requirements.
Applied to files:
apps/web/core/store/member/project/base-project-member.store.ts
🧬 Code graph analysis (3)
apps/web/ce/store/member/workspace-member.store.ts (2)
apps/web/core/store/member/workspace/workspace-member.store.ts (1)
IBaseWorkspaceMemberStore(26-58)apps/web/core/store/member/index.ts (1)
IMemberRootStore(12-21)
apps/web/core/store/member/workspace/workspace-member.store.ts (1)
packages/types/src/workspace.ts (1)
IWorkspaceBulkInviteFormData(56-58)
apps/web/core/store/member/project/base-project-member.store.ts (1)
packages/types/src/project/projects.ts (1)
IProjectBulkAddFormData(110-112)
⏰ 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: Build packages
- GitHub Check: Cursor Bugbot
- GitHub Check: Analyze (javascript)
🔇 Additional comments (11)
apps/web/core/store/root.store.ts (1)
64-64: LGTM: Import path updated for workspace store reorganization.The import source change aligns with the broader refactoring to consolidate workspace store implementations.
apps/web/core/store/member/index.ts (1)
8-9: LGTM: Import paths updated to use absolute aliases.The import source change from relative to absolute paths aligns with the architectural shift where workspace member store implementations are now sourced from the CE layer.
apps/web/ce/store/member/workspace-member.store.ts (1)
1-13: LGTM: Valid CE concrete implementation.This follows the standard CE/EE pattern where the CE layer provides a concrete implementation of the abstract base store from core. The type alias maintains backward compatibility while the class extends the base with no additional CE-specific logic, which is expected for the baseline CE implementation.
apps/web/core/store/workspace/index.ts (1)
66-66: LGTM: Concrete workspace root store implementation.The class is now a concrete implementation of IWorkspaceRootStore with all methods properly defined. MobX decorators are correctly applied and the implementation maintains proper reactive patterns.
apps/web/core/store/member/workspace/workspace-member.store.ts (4)
26-26: LGTM: Interface rename and abstract base class declaration.The interface rename to
IBaseWorkspaceMemberStoreand class conversion toabstract BaseWorkspaceMemberStorecorrectly establish this as a base abstraction for CE/EE extension. This aligns with the architectural refactoring pattern.Also applies to: 60-60
254-271: LGTM: Method conversion from arrow function to standard async method.The
updateMembermethod has been correctly converted from an arrow function property to a standard async method. The MobX action decorator is properly applied (line 87), and the optimistic update pattern with error rollback is maintained.
278-285: LGTM: Method conversion for removeMemberFromWorkspace.The
removeMemberFromWorkspacemethod has been correctly converted to a standard async method with the MobX action decorator properly applied (line 88).
345-352: LGTM: Method conversion for deleteMemberInvitation.The
deleteMemberInvitationmethod has been correctly converted to a standard async method with proper MobX action decorator (line 91) and state updates wrapped inrunInAction.apps/web/core/store/member/project/base-project-member.store.ts (3)
307-327: LGTM: Method conversion for bulkAddMembersToProject.The
bulkAddMembersToProjectmethod has been correctly converted from an arrow function to a standard async method. The MobX action decorator is properly applied (line 119), state updates are wrapped inrunInAction, and the method now returns the service response directly. The blank line 324 indicates where the mutation hook call was removed.
348-401: LGTM: Method conversion for updateMemberRole.The
updateMemberRolemethod has been correctly converted to a standard async method with proper MobX action decorator (line 120). The implementation maintains sophisticated optimistic updates with error rollback, correctly handling both regular member updates and current user permission synchronization.
431-438: LGTM: Method conversion for removeMemberFromProject.The
removeMemberFromProjectmethod has been correctly converted to a standard async method with proper MobX action decorator (line 121). The implementation correctly delegates member cleanup to the abstractprocessMemberRemovalmethod and properly wraps state updates inrunInAction.
| async inviteMembersToWorkspace(workspaceSlug: string, data: IWorkspaceBulkInviteFormData) { | ||
| await this.workspaceService.inviteWorkspace(workspaceSlug, data); | ||
| await this.fetchWorkspaceMemberInvitations(workspaceSlug); | ||
| void this.rootStore.workspaceRoot.mutateWorkspaceMembersActivity(workspaceSlug); | ||
| }; | ||
| } |
There was a problem hiding this comment.
Missing MobX action decorator for inviteMembersToWorkspace.
The inviteMembersToWorkspace method was converted from an arrow function to a standard async method, but it's not included in the makeObservable configuration (lines 77-92). Since this method triggers state changes by calling fetchWorkspaceMemberInvitations, it should be decorated with action.
🔎 Proposed fix
Add the action decorator in the makeObservable call:
makeObservable(this, {
// observables
workspaceMemberMap: observable,
workspaceMemberInvitations: observable,
// computed
workspaceMemberIds: computed,
workspaceMemberInvitationIds: computed,
memberMap: computed,
// actions
fetchWorkspaceMembers: action,
updateMember: action,
removeMemberFromWorkspace: action,
fetchWorkspaceMemberInvitations: action,
+ inviteMembersToWorkspace: action,
updateMemberInvitation: action,
deleteMemberInvitation: action,
});Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/core/store/member/workspace/workspace-member.store.ts around lines
304-307, the async method inviteMembersToWorkspace was converted from an arrow
function but not added to the makeObservable configuration (lines 77-92), so it
lacks the MobX action decorator; update the makeObservable call to include
inviteMembersToWorkspace as an action (alongside the other action-decorated
methods) so state changes triggered by fetchWorkspaceMemberInvitations are
wrapped as a MobX action.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| throw error; | ||
| } | ||
| }; | ||
| } |
There was a problem hiding this comment.
Methods lose this binding after arrow-to-method conversion
Several methods (updateMember, removeMemberFromWorkspace, inviteMembersToWorkspace, deleteMemberInvitation, bulkAddMembersToProject, updateMemberRole, removeMemberFromProject) were converted from arrow functions to regular class methods. Throughout the codebase, these methods are destructured from stores and called as standalone functions (e.g., const { removeMemberFromProject } = useMember().project; await removeMemberFromProject(...)). Regular methods lose their this binding when destructured, causing this to be undefined at runtime. This will crash when users try to update roles, remove members, or invite members.
Description
improvements in members page
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Note
Refactors member and workspace stores, removes unused activity mutation hooks, and consolidates workspace store wiring.
mutateProjectMembersActivityandmutateWorkspaceMembersActivityfrom project/workspace member flows (bulk add, role update, remove, invite, delete invitation)BaseWorkspaceMemberStorein core with a CEWorkspaceMemberStoreimplementation; updates imports to useplane-webpathsWorkspaceRootStoreconcrete and updatesCoreRootStoreto use it; removes CEworkspace/index.tsasyncfunction syntax and internal cleanups in project/workspace member storesWritten by Cursor Bugbot for commit 1aac9ce. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.