[WEB-4882]feat: suspended users #7844
Conversation
…ane into feat-project_member_filters
…ane into feat-suspended_users
WalkthroughAdds suspension-aware rendering and header-based filtering for members, implements member filtering/sorting utilities and workspace store hooks for suspension and filtered IDs, and extends design system with a SuspendedUserIcon and an XS Pill size. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Members Table Header
participant Store as useMember (filtersStore)
participant WS as WorkspaceMemberStore
participant Utils as member/utils
participant List as Members List
UI->>Store: updateFilters(change)
Store-->>WS: getFilteredWorkspaceMemberIds()
WS->>WS: collect non-bot member IDs
WS->>Utils: sortWorkspaceMembers(members, detailsMap, getKey, filters)
Utils-->>WS: filtered + sorted IDs
WS-->>Store: IDs
Store-->>List: filtered IDs
List->>List: render rows (active first)
sequenceDiagram
participant Row as Row Renderer
participant WS as WorkspaceMemberStore
participant DS as Design System
Row->>WS: isUserSuspended(userId)
alt Suspended
Row->>DS: SuspendedUserIcon + Pill("Suspended")
Row-->>Row: hide/disable edit controls, muted text
else Active
Row-->>Row: render Avatar, editable role UI
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on October 20. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/core/components/project/member-list.tsx (1)
42-45: Guard against undefined display_name (runtime crash).display_name can be null/undefined; toLowerCase() would throw.
- const displayName = memberDetails?.member.display_name.toLowerCase(); + const displayName = memberDetails?.member.display_name?.toLowerCase() ?? "";
🧹 Nitpick comments (22)
packages/propel/src/icons/suspended-user.tsx (1)
5-14: Add basic accessibility attributes to SVG.Mark presentational by default to keep it out of the a11y tree.
export const SuspendedUserIcon: React.FC<ISvgIcons> = ({ className, ...rest }) => ( <svg width="16" height="17" viewBox="0 0 16 17" fill="none" xmlns="http://www.w3.org/2000/svg" className={className} + focusable="false" + aria-hidden="true" {...rest} >apps/web/core/components/project/dropdowns/filters/member-list.tsx (2)
47-51: Localize UI strings (“Filters”, “Roles”, role labels).Replace hardcoded labels with i18n keys (e.g.,
t("common.filters"),t("project_members.role"), and role names). Ensures consistency and translation coverage.Also applies to: 89-96, 22-26, 28-33
16-20: Minor typing/ergonomics: default filters to empty array.Use
string[]instead ofstring[] | nulland default[]from the parent. Simplifies checks (includes, counts) and avoids nullish coalescing noise.Also applies to: 41-44, 56-66, 87-88
apps/web/core/components/workspace/settings/members-list.tsx (2)
53-55: Search currently bypasses filters/order; intersect to preserve both.When searchQuery is present, the code ignores filteredMemberIds (including order_by and role filters). Intersect search results with filteredMemberIds to keep filters and ordering intact.
Apply this diff:
- const searchedMemberIds = searchQuery ? getSearchedWorkspaceMemberIds(searchQuery) : filteredMemberIds; + const searchedMemberIds = searchQuery + ? filteredMemberIds.filter((id) => (getSearchedWorkspaceMemberIds(searchQuery) ?? []).includes(id)) + : filteredMemberIds;
57-63: Avoid relying on engine sort stability; partition instead.The comparator returns 0 for ties, depending on stable sort to preserve prior ordering. Prefer explicit partitioning to keep active members first while preserving internal order.
Apply this diff:
- const memberDetails = searchedMemberIds - ?.map((memberId) => getWorkspaceMemberDetails(memberId)) - .sort((a, b) => { - if (a?.is_active && !b?.is_active) return -1; - if (!a?.is_active && b?.is_active) return 1; - return 0; - }); + const memberDetailsUnsorted = searchedMemberIds?.map((memberId) => getWorkspaceMemberDetails(memberId)) ?? []; + const activeMembers = memberDetailsUnsorted.filter((m) => m?.is_active); + const inactiveMembers = memberDetailsUnsorted.filter((m) => !m?.is_active); + const memberDetails = [...activeMembers, ...inactiveMembers];apps/web/core/components/project/member-header-column.tsx (2)
39-44: A11y: make the custom button focusable and identifiable.customButtonTabIndex is -1, which prevents keyboard focus. Set to 0 and add a button role/label to improve accessibility.
Apply this diff:
- customButtonTabIndex={-1} + customButtonTabIndex={0} className="!w-full" customButton={ - <div className="flex w-full cursor-pointer items-center justify-between gap-1.5 py-2 text-sm text-custom-text-200 hover:text-custom-text-100"> + <div + className="flex w-full cursor-pointer items-center justify-between gap-1.5 py-2 text-sm text-custom-text-200 hover:text-custom-text-100" + role="button" + aria-label={`${t(propertyDetails.i18n_title)} sorting`} + >
35-38: Avoid empty menus when sorting isn’t allowed.If sorting is not allowed for a property, return a plain label instead of rendering CustomMenu.
Apply this diff:
if (!propertyDetails) return null; + if (!propertyDetails.isSortingAllowed) { + return <span className="py-2 text-sm text-custom-text-200">{t(propertyDetails.i18n_title)}</span>; + } + return ( <CustomMenuapps/web/core/components/workspace/settings/member-columns.tsx (3)
68-76: Align avatar placeholder size with image avatar.The no-avatar placeholder uses h-4 w-4 while the image avatar container is h-6 w-6, causing jank. Match sizes for consistency.
Apply this diff:
- <span - className={cn( - "relative flex h-4 w-4 text-xs items-center justify-center rounded-full capitalize text-white", - isSuspended ? "bg-custom-background-80" : "bg-gray-700" - )} - > + <span + className={cn( + "relative flex h-6 w-6 text-xs items-center justify-center rounded-full capitalize text-white", + isSuspended ? "bg-custom-background-80" : "bg-gray-700" + )} + >
82-84: Avoid “undefined undefined” for missing names.Fallback to display_name/email when first_name/last_name are absent.
Apply this diff:
- <span className={isSuspended ? "text-custom-text-400" : ""}> - {first_name} {last_name} - </span> + <span className={isSuspended ? "text-custom-text-400" : ""}> + {first_name || last_name ? `${first_name ?? ""} ${last_name ?? ""}`.trim() : display_name ?? email} + </span>
134-140: Localize “Suspended”.Use i18n instead of a hardcoded string.
Apply this diff:
+ // i18n + import { useTranslation } from "@plane/i18n"; @@ export const AccountTypeColumn: React.FC<AccountTypeProps> = observer((props) => { const { rowData, workspaceSlug } = props; + const { t } = useTranslation(); @@ - {isSuspended ? ( + {isSuspended ? ( <div className="w-32 flex "> <Pill variant={EPillVariant.DEFAULT} size={EPillSize.SM} className="border-none"> - Suspended + {t("suspended")} </Pill> </div>Confirm the correct translation key (e.g., "suspended" or a scoped key) exists in your i18n catalog.
apps/web/core/components/dropdowns/member/member-options.tsx (2)
91-111: Compute suspension once per option to avoid repeated store lookups.Minor perf/readability improvement.
Apply this diff:
- const options = memberIds - ?.map((userId) => { - const userDetails = getUserDetails(userId); + const options = memberIds + ?.map((userId) => { + const userDetails = getUserDetails(userId); + const userSuspended = isSuspended(userId); return { value: userId, query: `${userDetails?.display_name} ${userDetails?.first_name} ${userDetails?.last_name}`, content: ( <div className="flex items-center gap-2"> - <div className="w-4"> - {isSuspended(userId) ? ( + <div className="w-4"> + {userSuspended ? ( <SuspendedUserIcon className="h-3.5 w-3.5 text-custom-text-400" /> ) : ( <Avatar name={userDetails?.display_name} src={getFileURL(userDetails?.avatar_url ?? "")} /> )} </div> - <span className={cn("flex-grow truncate", isSuspended(userId) ? "text-custom-text-400" : "")}> + <span className={cn("flex-grow truncate", userSuspended ? "text-custom-text-400" : "")}> {currentUser?.id === userId ? t("you") : userDetails?.display_name} </span> </div> ), }; })
167-170: Localize “Suspended”.Replace the hardcoded label with a translated string.
Apply this diff:
- {isSuspended(option.value) && ( - <Pill variant={EPillVariant.DEFAULT} size={EPillSize.XS} className="border-none"> - Suspended - </Pill> - )} + {isSuspended(option.value) && ( + <Pill variant={EPillVariant.DEFAULT} size={EPillSize.XS} className="border-none"> + {t("suspended")} + </Pill> + )}Confirm the right i18n key for “Suspended” in your locale files.
packages/constants/src/members.ts (2)
70-78: Confirm role sort semantics match UI titles."Guest → Admin" titles imply a specific role order. Ensure your comparator for order_by: "role"/"-role" sorts by role precedence (not lexicographic labels or enum numeric values that might not match the intended order). If needed, map roles to an explicit rank.
23-79: Consider i18n for sort titles (“A/Z”, “Old/New”, “Guest/Admin”).These are currently hardcoded strings. If surfaced in UI, prefer i18n keys to avoid mixed-language menus.
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/(workspace)/members/page.tsx (1)
132-159: Add accessible label for the search input.Placeholder-only inputs are not accessible. Associate a visually-hidden label or use aria-label.
Example minimal tweak:
-<input +<input + aria-label={t("search")} className="w-full max-w-[234px] border-none bg-transparent text-sm outline-none placeholder:text-custom-text-400" placeholder={`${t("search")}...`}apps/web/core/store/member/workspace/workspace-member.store.ts (1)
140-142: Stale comment.Comment says “filter out bots and inactive members” but code only filters bots. Update comment to avoid confusion.
- //filter out bots and inactive members - members = members.filter((m) => !this.memberRoot?.memberMap?.[m.member]?.is_bot); + // filter out bots + members = members.filter((m) => !this.memberRoot?.memberMap?.[m.member]?.is_bot);apps/web/core/components/project/member-list.tsx (1)
87-87: Use i18n for the search placeholder.Align with other screens using t("search").
- placeholder="Search" + placeholder={t("search")}apps/web/core/store/member/project/base-project-member.store.ts (2)
111-132: Preserve “current user first” ordering and ensure deterministic default sort.The previous implementation pinned the current user to the top; the new path loses that behavior. Also, if filters.order_by is unset, the current code may return backend insertion order. Recommend pinning current user first and returning IDs after a default name sort (utils can now default to display_name).
Apply this diff to pin the current user first while preserving the computed order:
const sortedMembers = sortProjectMembers( members, this.memberRoot?.memberMap || {}, (member) => member.member, currentFilters ); - - return sortedMembers.map((member) => member.member); + const ids = sortedMembers.map((member) => member.member); + const currentUserId = this.userStore.data?.id; + if (currentUserId) { + const idx = ids.indexOf(currentUserId); + if (idx > 0) { + const [self] = ids.splice(idx, 1); + ids.unshift(self); + } + } + return ids;
221-254: Minor: avoid O(n) includes for per‑member checks.If called frequently, prefer a Set for filtered IDs to get O(1) lookups.
apps/web/core/store/member/project/project-member-filters.store.ts (1)
43-57: computedFn args hamper memoization; consider keying by projectId only.Passing arrays by value limits computedFn caching. Fetch members/memberDetailsMap inside the computed to leverage MobX tracking and better memoization.
apps/web/core/store/member/workspace/workspace-member-filters.store.ts (1)
10-15: Avoid local type duplication.Prefer importing IWorkspaceMembership from the workspace member store to prevent drift.
apps/web/core/store/member/utils.ts (1)
103-106: Default to a deterministic name sort when order_by is unset.Returning unsorted arrays degrades UX and diverges from prior behavior. Use the existing parseOrderKey default ("display_name" asc).
Apply these diffs:
@@ export const sortMembers = <T>( @@ -): T[] => { - if (!orderBy) return members; - - const { field, direction } = parseOrderKey(orderBy); +): T[] => { + const { field, direction } = parseOrderKey(orderBy);@@ - // If no order_by filter, return filtered members - if (!filters?.order_by) return filteredMembers;@@ - // If no order_by filter, return filtered members - if (!filters?.order_by) return filteredMembers;Also applies to: 150-151, 172-174
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/(workspace)/members/page.tsx(4 hunks)apps/web/ce/components/projects/settings/useProjectColumns.tsx(5 hunks)apps/web/ce/components/workspace/settings/useMemberColumns.tsx(3 hunks)apps/web/ce/store/member/project-member.store.ts(1 hunks)apps/web/core/components/dropdowns/member/member-options.tsx(5 hunks)apps/web/core/components/pages/version/editor.tsx(1 hunks)apps/web/core/components/project/dropdowns/filters/member-list.tsx(1 hunks)apps/web/core/components/project/member-header-column.tsx(1 hunks)apps/web/core/components/project/member-list-item.tsx(1 hunks)apps/web/core/components/project/member-list.tsx(4 hunks)apps/web/core/components/workspace/settings/member-columns.tsx(5 hunks)apps/web/core/components/workspace/settings/members-list.tsx(2 hunks)apps/web/core/store/issue/root.store.ts(1 hunks)apps/web/core/store/member/index.ts(1 hunks)apps/web/core/store/member/project/base-project-member.store.ts(5 hunks)apps/web/core/store/member/project/project-member-filters.store.ts(1 hunks)apps/web/core/store/member/utils.ts(1 hunks)apps/web/core/store/member/workspace/workspace-member-filters.store.ts(1 hunks)apps/web/core/store/member/workspace/workspace-member.store.ts(6 hunks)packages/constants/src/index.ts(1 hunks)packages/constants/src/members.ts(1 hunks)packages/i18n/src/locales/en/translations.json(1 hunks)packages/propel/src/icons/index.ts(1 hunks)packages/propel/src/icons/suspended-user.tsx(1 hunks)packages/propel/src/pill/index.ts(1 hunks)packages/propel/src/pill/pill.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (13)
apps/web/core/components/project/member-header-column.tsx (3)
packages/constants/src/members.ts (3)
IProjectMemberDisplayProperties(15-21)MEMBER_PROPERTY_DETAILS(23-79)TMemberOrderByOptions(3-13)apps/web/core/store/member/utils.ts (1)
IMemberFilters(5-8)packages/i18n/src/hooks/use-translation.ts (1)
useTranslation(23-35)
apps/web/core/components/workspace/settings/member-columns.tsx (1)
packages/propel/src/icons/suspended-user.tsx (1)
SuspendedUserIcon(5-40)
apps/web/core/components/dropdowns/member/member-options.tsx (2)
apps/space/core/hooks/store/use-member.ts (1)
useMember(7-11)packages/propel/src/icons/suspended-user.tsx (1)
SuspendedUserIcon(5-40)
apps/web/ce/components/projects/settings/useProjectColumns.tsx (3)
apps/space/core/hooks/store/use-member.ts (1)
useMember(7-11)apps/web/core/store/member/utils.ts (1)
IMemberFilters(5-8)apps/web/core/components/project/member-header-column.tsx (1)
MemberHeaderColumn(18-114)
apps/web/core/store/member/utils.ts (4)
packages/constants/src/members.ts (1)
TMemberOrderByOptions(3-13)packages/types/src/users.ts (1)
IUserLite(20-29)apps/space/core/store/members.store.ts (1)
members(36-38)packages/types/src/project/projects.ts (1)
TProjectMembership(93-107)
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/(workspace)/members/page.tsx (2)
apps/web/core/components/project/dropdowns/filters/member-list.tsx (1)
MemberListFiltersDropdown(84-107)packages/constants/src/event-tracker/core.ts (1)
MEMBER_TRACKER_ELEMENTS(249-258)
apps/web/core/components/workspace/settings/members-list.tsx (1)
apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)
apps/web/core/store/member/project/project-member-filters.store.ts (3)
apps/web/core/store/member/utils.ts (2)
IMemberFilters(5-8)sortProjectMembers(140-161)packages/types/src/project/projects.ts (1)
TProjectMembership(93-107)packages/types/src/users.ts (1)
IUserLite(20-29)
apps/web/ce/components/workspace/settings/useMemberColumns.tsx (4)
apps/space/core/hooks/store/use-member.ts (1)
useMember(7-11)apps/web/core/components/workspace/settings/member-columns.tsx (2)
RowData(21-25)AccountTypeColumn(111-188)apps/web/core/store/member/utils.ts (1)
IMemberFilters(5-8)apps/web/core/components/project/member-header-column.tsx (1)
MemberHeaderColumn(18-114)
apps/web/core/components/project/member-list.tsx (4)
apps/space/core/hooks/store/use-member.ts (1)
useMember(7-11)apps/web/core/store/member/project/base-project-member.store.ts (1)
projectMemberIds(113-132)apps/web/core/components/project/dropdowns/filters/member-list.tsx (1)
MemberListFiltersDropdown(84-107)packages/constants/src/event-tracker/core.ts (1)
MEMBER_TRACKER_ELEMENTS(249-258)
apps/web/core/store/member/workspace/workspace-member-filters.store.ts (3)
apps/web/core/store/member/workspace/workspace-member.store.ts (1)
IWorkspaceMembership(19-24)apps/web/core/store/member/utils.ts (2)
IMemberFilters(5-8)sortWorkspaceMembers(163-183)packages/types/src/users.ts (1)
IUserLite(20-29)
apps/web/core/store/member/project/base-project-member.store.ts (2)
apps/web/core/store/member/project/project-member-filters.store.ts (2)
IProjectMemberFiltersStore(8-21)ProjectMemberFiltersStore(23-70)apps/web/core/store/member/utils.ts (1)
sortProjectMembers(140-161)
apps/web/core/store/member/workspace/workspace-member.store.ts (1)
apps/web/core/store/member/workspace/workspace-member-filters.store.ts (2)
IWorkspaceMemberFiltersStore(17-28)WorkspaceMemberFiltersStore(30-71)
⏰ 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: Build and lint web apps
🔇 Additional comments (25)
apps/web/core/components/pages/version/editor.tsx (1)
7-7: LGTM on import reorderNo functional change. Ensure linters/prettier rules are happy with the new position.
packages/propel/src/icons/index.ts (1)
59-59: LGTM: new icon is exposed via the barrel.packages/constants/src/index.ts (1)
18-18: LGTM: re-exporting members constants.Enables typed member property use across the app.
packages/propel/src/pill/index.ts (1)
1-1: LGTM: expose EPillVariant and EPillSize.Matches downstream usage patterns.
apps/web/core/store/issue/root.store.ts (2)
32-32: LGTM: import path update aligns with store reorg.
229-232: Assign workspaceMemberMap to workSpaceMemberRolesMap (not memberMap)Condition checks workspaceMemberMap but assigns memberMap — likely incorrect; change to assign workspaceMemberMap.
File: apps/web/core/store/issue/root.store.ts (lines ~229-232)
- if (!isEmpty(rootStore?.memberRoot?.workspace?.workspaceMemberMap)) - this.workSpaceMemberRolesMap = rootStore?.memberRoot?.workspace?.memberMap || undefined; + if (!isEmpty(rootStore?.memberRoot?.workspace?.workspaceMemberMap)) + this.workSpaceMemberRolesMap = rootStore?.memberRoot?.workspace?.workspaceMemberMap || undefined;Confirm workspace.workspaceMemberMap exists and the types align with workSpaceMemberRolesMap.
apps/web/core/store/member/index.ts (1)
9-9: LGTM — import path & exports verified.
apps/web/core/store/member/workspace/workspace-member.store.ts exports IWorkspaceMemberStore and WorkspaceMemberStore; the import in apps/web/core/store/member/index.ts is correct.apps/web/core/components/project/dropdowns/filters/member-list.tsx (1)
28-33: OK to keep "suspended" — enum constant not present and store expects itEUserWorkspaceRoles only defines ADMIN/MEMBER/GUEST (packages/types/src/workspace.ts); member filter logic checks for "suspended" (apps/web/core/store/member/utils.ts) and the dropdown uses that literal (apps/web/core/components/project/dropdowns/filters/member-list.tsx). Leave as-is.
apps/web/core/components/project/member-list-item.tsx (1)
16-16: Import path update aligns with new project member store.Looks correct and consistent with the new project-scoped base store.
Please confirm the alias "@/store/member/project/base-project-member.store" is exported in tsconfig paths across all apps to avoid IDE/build drift.
apps/web/ce/store/member/project-member.store.ts (1)
8-8: Consistent re-path to project base store.Matches the new project-scoped member store structure. No other changes required here.
apps/web/core/components/workspace/settings/members-list.tsx (1)
31-35: Good: filtered member IDs wiring.Using getFilteredWorkspaceMemberIds is the right entry-point for respecting current filters.
packages/propel/src/pill/pill.tsx (3)
17-17: XS size addition looks good.Enum and size map updates are consistent; default remains MD.
27-27: Type union correctly updated.TPillSize includes XS; no breaking changes.
46-46: Padding/typography for XS is consistent.Matches the design scale relative to other sizes.
apps/web/ce/components/projects/settings/useProjectColumns.tsx (2)
31-35: Filters integration is correctly wired.Using getFilters(projectId) and updateFilters(projectId, …) exposes sorting to headers without leaking store concerns into cells.
Confirm getFilters(projectId) returns an observable object to ensure MobX reactivity for header state.
Also applies to: 46-51
58-65: Header-based sorting hook-up looks solid.MemberHeaderColumn usage for full_name, display_name, email, role, and joining_date is consistent and respects displayFilters.
Also applies to: 79-84, 88-98, 102-108, 121-129
apps/web/core/components/workspace/settings/member-columns.tsx (1)
24-24: New RowData.is_active dependency.Ensure callers populate is_active for each row; otherwise suspended visuals won’t render.
apps/web/core/components/dropdowns/member/member-options.tsx (1)
153-161: Disable + cursor state are correct.Combobox.Option disabled pairs with cursor-not-allowed; good for UX and a11y.
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/(workspace)/members/page.tsx (1)
92-101: Role filter toggle logic looks good.Toggles roles and clears when empty; aligns with store API.
apps/web/core/components/project/member-list.tsx (1)
54-68: Filter wiring looks correct.Project-level role toggle persists via filters.getFilters/updateFilters and powers the dropdown state.
If roles are enum values, confirm MemberListFilters emits the same value shape expected by store filters (string vs enum).
apps/web/core/store/member/project/base-project-member.store.ts (4)
20-23: Imports look correct and cohesive.Utility and filters store imports are appropriate for the new filtering flow.
36-38: Good addition: per‑project filters on the store.Public API surface looks right and aligns with downstream usage.
75-77: Good: filters store field on class.Keeps the API symmetrical with the interface.
104-104: Initialization of filters store looks good.No concerns.
apps/web/core/store/member/workspace/workspace-member-filters.store.ts (1)
49-62: LGTM: workspace filtered IDs derivation.Wires into shared utils cleanly; behavior will improve with utils fixes below.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/web/core/components/dropdowns/member/member-options.tsx (3)
52-54: Avoid potential runtime crash on missing workspace.Nested destructuring will throw if workspace is ever undefined. Prefer a safe accessor + fallback.
- const { - workspace: { isUserSuspended }, - } = useMember(); + const memberStore = useMember(); + const isUserSuspended = (userId: string) => + memberStore.workspace?.isUserSuspended?.(userId) ?? false;
93-101: A11y: mark decorative icon hidden and avoid repeated store calls.
- Add aria-hidden to the SuspendedUserIcon.
- Minor: compute suspended once per user to avoid repeated reads.
- <div className="w-4"> - {isUserSuspended(userId) ? ( - <SuspendedUserIcon className="h-3.5 w-3.5 text-custom-text-400" /> + <div className="w-4"> + {isUserSuspended(userId) ? ( + <SuspendedUserIcon className="h-3.5 w-3.5 text-custom-text-400" aria-hidden="true" /> ) : ( <Avatar name={userDetails?.display_name} src={getFileURL(userDetails?.avatar_url ?? "")} /> )} </div> - <span className={cn("flex-grow truncate", isUserSuspended(userId) ? "text-custom-text-400" : "")}> + <span className={cn("flex-grow truncate", isUserSuspended(userId) ? "text-custom-text-400" : "")}> {currentUser?.id === userId ? t("you") : userDetails?.display_name} </span>Optional (outside this range): precompute
const suspended = isUserSuspended(userId);and reuse suspended instead of calling the function twice.
148-156: Ensure suspended users are placed at the bottom (per PR objective).This component doesn’t reorder options; suspended users may appear intermixed. If upstream doesn’t already sort, add a stable sort here.
Outside this range, adjust filteredOptions:
// after computing `filteredOptions`: const sortedOptions = filteredOptions?.slice().sort((a, b) => { const aSusp = isUserSuspended(a.value) ? 1 : 0; const bSusp = isUserSuspended(b.value) ? 1 : 0; return aSusp - bSusp; // non-suspended first });Then iterate sortedOptions instead of filteredOptions. If memberIds are already pre-sorted upstream, ignore this.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/core/components/dropdowns/member/member-options.tsx(4 hunks)apps/web/core/store/member/workspace/workspace-member.store.ts(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/web/core/store/member/workspace/workspace-member.store.ts (1)
apps/web/core/store/member/workspace/workspace-member-filters.store.ts (2)
IWorkspaceMemberFiltersStore(17-28)WorkspaceMemberFiltersStore(30-71)
apps/web/core/components/dropdowns/member/member-options.tsx (2)
apps/space/core/hooks/store/use-member.ts (1)
useMember(7-11)packages/propel/src/icons/suspended-user.tsx (1)
SuspendedUserIcon(5-40)
⏰ 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 (7)
apps/web/core/store/member/workspace/workspace-member.store.ts (5)
16-16: Remove .ts extension in type import.TypeScript disallows importing with .ts extensions by default unless allowImportingTsExtensions is enabled. This flag is only allowed when --noEmit or --emitDeclarationOnly is enabled, since these import paths would not be resolvable at runtime.
Apply this diff to fix the import:
-import type { IMemberRootStore } from "../index.ts"; +import type { IMemberRootStore } from "../index";
31-31: LGTM! Well-integrated filters store.The filtersStore property is properly declared in the interface, initialized in the constructor, and correctly typed. This provides a clean separation of filtering concerns.
Also applies to: 67-67, 94-94
38-38: LGTM! Filtered member IDs integration is well-implemented.The new
getFilteredWorkspaceMemberIdsmethod properly:
- Filters out bots and delegates to the filters store
- Uses the established pattern with computedFn for performance
- Provides a clean interface for filtered member access
Also applies to: 140-153
162-164: LGTM! Search integration updated correctly.The search functionality now properly uses the filtered member IDs as the base set, maintaining the expected filtering behavior in search results.
57-57: LGTM! Suspended user detection is correctly implemented.The
isUserSuspendedmethod properly:
- Uses computedFn for performance optimization
- Checks workspace member's
is_activestatus correctly- Returns false when workspace context is unavailable
Also applies to: 91-91, 355-360
apps/web/core/components/dropdowns/member/member-options.tsx (2)
12-13: LGTM on new visual imports for suspension state.Bringing in SuspendedUserIcon and Pill variants is appropriate for the UX you’re adding.
18-18: Confirmed: useMember hook exposes workspace.isUserSuspended (web)
apps/web/core/hooks/store/use-member.ts returns IMemberRootStore; workspace.isUserSuspended is implemented in apps/web/core/store/member/workspace/workspace-member.store.ts — no runtime mismatch.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/web/core/store/member/utils.ts (4)
41-48: Fix joining_date sentinel (current comment is incorrect; Date(0) sorts first on asc).Return an invalid date and handle nulls-last in the comparator instead, so missing/invalid dates deterministically fall to the bottom.
- case "joining_date": { - if (!memberDetails.joining_date) { - // Return a very old date for missing dates to sort them last - return new Date(0); - } - const date = new Date(memberDetails.joining_date); - // Return a very old date for invalid dates to sort them last - return isNaN(date.getTime()) ? new Date(0) : date; - } + case "joining_date": { + if (!memberDetails.joining_date) { + // Return an invalid date; comparator will place invalid/missing at the bottom (nulls-last) + return new Date(NaN); + } + const date = new Date(memberDetails.joining_date); + // If invalid, return invalid; comparator will handle it + return Number.isNaN(date.getTime()) ? new Date(NaN) : date; + }
112-113: Push entries with missing memberDetails to the bottom (deterministic order).Returning 0 for missing details can cause unstable ordering. Push them to the end.
- if (!aMemberDetails || !bMemberDetails) return 0; + if (!aMemberDetails && !bMemberDetails) return 0; + if (!aMemberDetails) return 1; + if (!bMemberDetails) return -1;
56-67: Optional: Make project-role filtering case-insensitive.If UI tokens differ in case from stored roles, includes() may fail. Normalize both sides to lower-case.
export const filterProjectMembersByRole = ( members: TProjectMembership[], roleFilters: string[] ): TProjectMembership[] => { if (roleFilters.length === 0) return members; - return members.filter((member) => { - const memberRole = String(member.role ?? member.original_role ?? ""); - return roleFilters.includes(memberRole); - }); + const roleSet = new Set(roleFilters.map((r) => r.toLowerCase())); + return members.filter((member) => { + const memberRole = String(member.role ?? member.original_role ?? "").toLowerCase(); + return roleSet.has(memberRole); + }); };
28-54: Optional: Role sort may be inconsistent if EUserPermissions is numeric.If role is a numeric enum, string sorting (“10” < “2”) is undesirable. Consider sorting by a user-facing label or an explicit precedence map.
Can you confirm whether role values are strings (labels) or numeric enums in workspace/project contexts?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/core/store/member/utils.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/core/store/member/utils.ts (3)
packages/constants/src/members.ts (1)
TMemberOrderByOptions(3-13)packages/types/src/users.ts (1)
IUserLite(20-29)packages/types/src/project/projects.ts (1)
TProjectMembership(93-107)
⏰ 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: Build and lint web apps
🔇 Additional comments (3)
apps/web/core/store/member/utils.ts (3)
69-92: Role filter union semantics for “suspended” look correct.Suspended users are included only when “suspended” is selected, and active users are filtered by the remaining roles (OR semantics). LGTM.
122-139: Make date comparator nulls-last and robust to invalid/missing dates.Avoid treating invalid dates as epoch (which floats them to the top). Push invalid/missing to the bottom on asc (and top on desc via final direction flip).
- if (field === "joining_date") { - // For dates, we need to handle Date objects and ensure they're valid - const aDate = aValue instanceof Date ? aValue : new Date(aValue); - const bDate = bValue instanceof Date ? bValue : new Date(bValue); - - // Handle invalid dates by treating them as very old dates - const aTime = isNaN(aDate.getTime()) ? 0 : aDate.getTime(); - const bTime = isNaN(bDate.getTime()) ? 0 : bDate.getTime(); - - comparison = aTime - bTime; - } else { - // For strings, use localeCompare for proper alphabetical sorting - const aStr = String(aValue); - const bStr = String(bValue); - comparison = aStr.localeCompare(bStr); - } + if (field === "joining_date") { + // For dates, handle invalid/missing as nulls-last + const aDate = aValue instanceof Date ? aValue : new Date(aValue as any); + const bDate = bValue instanceof Date ? bValue : new Date(bValue as any); + const aTime = aDate.getTime(); + const bTime = bDate.getTime(); + const aInvalid = Number.isNaN(aTime); + const bInvalid = Number.isNaN(bTime); + if (aInvalid && bInvalid) comparison = 0; + else if (aInvalid) comparison = 1; // push invalid/missing to bottom on asc + else if (bInvalid) comparison = -1; + else comparison = aTime - bTime; + } else { + // For strings, use localeCompare for proper alphabetical sorting + const aStr = String(aValue ?? ""); + const bStr = String(bValue ?? ""); + comparison = aStr.localeCompare(bStr); + }
167-187: Guarantee suspended users are always listed last (partition before sort).Currently, suspended users can appear anywhere when order_by is applied. Partition into active/suspended, sort each partition, then concatenate to keep suspended at the bottom (per PR objective).
export const sortWorkspaceMembers = <T extends { role: string | EUserPermissions; is_active?: boolean }>( members: T[], memberDetailsMap: Record<string, IUserLite>, getMemberKey: (member: T) => string, filters?: IMemberFilters ): T[] => { - const filteredMembers = - filters?.roles && filters.roles.length > 0 ? filterWorkspaceMembersByRole(members, filters.roles) : members; - - // If no order_by filter, return filtered members - if (!filters?.order_by) return filteredMembers; - - // Apply sorting - return sortMembers( - filteredMembers, - memberDetailsMap, - getMemberKey, - (member) => String(member.role ?? ""), - filters.order_by - ); + const filteredMembers = + filters?.roles && filters.roles.length > 0 ? filterWorkspaceMembersByRole(members, filters.roles) : members; + + // Partition to keep suspended at the bottom + const activeMembers = filteredMembers.filter((m) => m?.is_active !== false); + const suspendedMembers = filteredMembers.filter((m) => m?.is_active === false); + + const sortedActive = sortMembers( + activeMembers, + memberDetailsMap, + getMemberKey, + (member) => String(member.role ?? ""), + filters?.order_by + ); + + const sortedSuspended = sortMembers( + suspendedMembers, + memberDetailsMap, + getMemberKey, + (member) => String(member.role ?? ""), + filters?.order_by + ); + + return [...sortedActive, ...sortedSuspended]; }
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/core/store/member/workspace/workspace-member.store.ts(5 hunks)
⏰ 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 (5)
apps/web/core/store/member/workspace/workspace-member.store.ts (5)
56-56: API addition looks good.isUserSuspended in the store interface is clear and useful.
131-132: Bots-only filter: LGTM.This now keeps suspended users in the list as intended.
139-152: Verify suspended members are sorted to the bottom.Delegation to filtersStore is fine, but ensure final ordering deprioritizes is_active === false. This was flagged earlier; please confirm implementation in utils/filters.
#!/bin/bash # Inspect sorting implementation for suspended ordering rg -n -C3 "sortWorkspaceMembers|sortMembers" apps/web/core/store/member/utils.ts rg -n -C3 "(is_active.*false|suspended).*sort|sort.*(is_active|suspended)" apps/web/core/store/member/utils.ts
353-359: Implementation of isUserSuspended: LGTM, but align annotation.Logic is correct and safe; just remove the action annotation per comment on Line 90.
15-15: Remove .ts extension in type import (TS error unless allowImportingTsExtensions=true).Use module path without the .ts suffix.
-import type { IMemberRootStore } from "../index.ts"; +import type { IMemberRootStore } from "../index";
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/web/core/store/member/workspace/workspace-member.store.ts (3)
138-151: Also ensure filtered IDs keep suspended at the bottom.filtersStore returns IDs without suspension-aware ordering. Reorder here to keep UX consistent across lists and dropdowns.
getFilteredWorkspaceMemberIds = computedFn((workspaceSlug: string) => { let members = Object.values(this.workspaceMemberMap?.[workspaceSlug] ?? {}); //filter out bots and inactive members members = members.filter((m) => !this.memberRoot?.memberMap?.[m.member]?.is_bot); // Use filters store to get filtered member ids - const memberIds = this.filtersStore.getFilteredMemberIds( + const memberIds = this.filtersStore.getFilteredMemberIds( members, this.memberRoot?.memberMap || {}, (member) => member.member ); - return memberIds; + // suspended users at the bottom (stable sort) + return sortBy(memberIds, (userId) => (this.isUserSuspended(userId, workspaceSlug) ? 1 : 0)); });
134-151: Include suspended members at the end of workspace member lists
- getFilteredWorkspaceMemberIds (apps/web/core/store/member/workspace/workspace-member.store.ts) currently filters out inactive members—update it to append suspended users to the bottom instead of removing them.
- Verify filterWorkspaceMembersByRole, sortWorkspaceMembers (and underlying sortMembers), and all UI components consume this suspended-last ordering.
123-132: Add suspended-last sorting while preserving current user first and alphabetical ordermembers = sortBy(members, [ // current user first m => m.member !== this.userStore?.data?.id, + // suspended users last m => this.isUserSuspended(m.member, workspaceSlug) ? 1 : 0, // alphabetical within each group m => this.memberRoot?.memberMap?.[m.member]?.display_name?.toLowerCase(), ]);
🧹 Nitpick comments (3)
apps/web/core/components/dropdowns/member/member-options.tsx (1)
96-107: Avoid repeated isUserSuspended calls; compute once per option.You call the computed 4x per option. Cache at map-time and reuse to cut work and avoid reactive churn.
- const options = memberIds - ?.map((userId) => { - const userDetails = getUserDetails(userId); + const ws = workspaceSlug?.toString(); + const options = memberIds + ?.map((userId) => { + const suspended = isUserSuspended(userId, ws); + const userDetails = getUserDetails(userId); return { - value: userId, + value: userId, + suspended, query: `${userDetails?.display_name} ${userDetails?.first_name} ${userDetails?.last_name}`, content: ( <div className="flex items-center gap-2"> - <div className="w-4"> - {isUserSuspended(userId, workspaceSlug?.toString()) ? ( + <div className="w-4"> + {suspended ? ( <SuspendedUserIcon className="h-3.5 w-3.5 text-custom-text-400" /> ) : ( <Avatar name={userDetails?.display_name} src={getFileURL(userDetails?.avatar_url ?? "")} /> )} </div> <span - className={cn( - "flex-grow truncate", - isUserSuspended(userId, workspaceSlug?.toString()) ? "text-custom-text-400" : "" - )} + className={cn("flex-grow truncate", suspended && "text-custom-text-400")} > {currentUser?.id === userId ? t("you") : userDetails?.display_name} </span> </div> ), }; })- className={({ active, selected }) => + className={({ active, selected }) => cn( "flex w-full select-none items-center justify-between gap-2 truncate rounded px-1 py-1.5", active && "bg-custom-background-80", selected ? "text-custom-text-100" : "text-custom-text-200", - isUserSuspended(option.value, workspaceSlug?.toString()) - ? "cursor-not-allowed" - : "cursor-pointer" + option.suspended ? "cursor-not-allowed" : "cursor-pointer" ) } - disabled={isUserSuspended(option.value, workspaceSlug?.toString())} + disabled={option.suspended}- {isUserSuspended(option.value, workspaceSlug?.toString()) && ( + {option.suspended && ( <Pill variant={EPillVariant.DEFAULT} size={EPillSize.XS} className="border-none"> - {t("suspended")} + {t("suspended")} </Pill> )}Also applies to: 155-166, 171-175
apps/web/core/store/member/workspace/workspace-member.store.ts (2)
25-41: Param naming mismatch may confuse callers.Interface says workspaceMemberId but implementation expects userId (key into workspaceMemberMap). Align the name.
- getWorkspaceMemberDetails: (workspaceMemberId: string) => IWorkspaceMember | null; + getWorkspaceMemberDetails: (userId: string) => IWorkspaceMember | null;Optionally update JSDoc param name at Lines 191–199 accordingly.
Also applies to: 195-208
344-346: Unnecessary toString on workspaceSlug.Method already receives a string. Remove for consistency.
- deleteMemberInvitation = async (workspaceSlug: string, invitationId: string) => - await this.workspaceService.deleteWorkspaceInvitations(workspaceSlug.toString(), invitationId).then(() => { + deleteMemberInvitation = async (workspaceSlug: string, invitationId: string) => + await this.workspaceService.deleteWorkspaceInvitations(workspaceSlug, invitationId).then(() => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/core/components/dropdowns/member/member-options.tsx(5 hunks)apps/web/core/store/member/workspace/workspace-member.store.ts(4 hunks)packages/propel/src/icons/suspended-user.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/propel/src/icons/suspended-user.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
apps/web/core/components/dropdowns/member/member-options.tsx (2)
apps/space/core/hooks/store/use-member.ts (1)
useMember(7-11)packages/propel/src/icons/suspended-user.tsx (1)
SuspendedUserIcon(5-32)
apps/web/core/store/member/workspace/workspace-member.store.ts (1)
apps/space/core/store/members.store.ts (1)
members(36-38)
⏰ 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 (4)
apps/web/core/components/dropdowns/member/member-options.tsx (2)
171-175: Localize “Suspended”.Replace the hard-coded string with a translation key to avoid breaking i18n.
- {isUserSuspended(option.value, workspaceSlug?.toString()) && ( - <Pill variant={EPillVariant.DEFAULT} size={EPillSize.XS} className="border-none"> - Suspended - </Pill> - )} + {isUserSuspended(option.value, workspaceSlug?.toString()) && ( + <Pill variant={EPillVariant.DEFAULT} size={EPillSize.XS} className="border-none"> + {t("suspended")} + </Pill> + )}
155-166: LGTM on disabling suspended options.Using HeadlessUI’s disabled prop together with a not-allowed cursor gives clear UX.
apps/web/core/store/member/workspace/workspace-member.store.ts (2)
15-15: Remove .ts extension from import (TS error unless explicitly allowed).TypeScript disallows importing with .ts extensions by default. Drop the suffix.
-import type { IMemberRootStore } from "../index.ts"; +import type { IMemberRootStore } from "../index";
353-357: Good: computedFn for isUserSuspended (no action annotation).Pure derivation from workspaceMemberMap; MobX semantics preserved.
Description
This update adds suspended users to the workspace members list. In dropdowns, suspended users are also included but positioned at the bottom for better clarity and organization.
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit
New Features
Style