Conversation
…Groups/Permissions) Mirrors the webapp's admin Settings panel (CsmAdminLayout + CsmUsersPage): a Users tab with search/role/status filtering backed by POST /users/search, plus Roles/Groups/Permissions as WIP tabs reusing the existing ComingSoonPage. Reached from More > Settings, last in the list to match CSM_NAV_ITEMS. The Settings component is self-contained (owns its own data fetching and state) so it can be relocated to a different page without rewiring.
|
Warning Review limit reached
Next review available in: 11 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds dashboard widgets for assigned cases and case composition, plus a routed Settings page with admin-user search, filtering, infinite pagination, and normalized user models. Profile editing is removed, leaving read-only profile content. ChangesAdmin settings users
Home dashboard widgets
Profile and shared layout cleanup
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant HomePage
participant dashboard
participant getAllCases
participant CasesEndpoint
HomePage->>dashboard: request dashboard data
dashboard->>getAllCases: run filtered case searches
getAllCases->>CasesEndpoint: fetch counts or assigned cases
CasesEndpoint-->>getAllCases: return case data and totals
getAllCases-->>dashboard: provide query results
dashboard-->>HomePage: render widgets
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/csm-portal/microapp/src/pages/ProfilePage.tsx (1)
78-80: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winSkeleton renders 3 placeholder rows but content now has only 1 field.
ProfileSkeletonstill shows threeSkeletonrows matching the old form (email, phone, time zone). The simplifiedProfileContentrenders only a single disabled emailTextField. This mismatch causes a visible layout jump when data resolves. Reduce the skeleton to match the new minimal layout.♻️ Proposed fix
<Skeleton variant="text" width={140} height={28} /> </Stack> - <Skeleton variant="rounded" height={40} /> - <Skeleton variant="rounded" height={40} /> <Skeleton variant="rounded" height={40} /> </Stack>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/csm-portal/microapp/src/pages/ProfilePage.tsx` around lines 78 - 80, Update ProfileSkeleton to render a single rounded Skeleton row matching the lone disabled email TextField in ProfileContent, removing the two obsolete placeholder rows so loading and resolved layouts have the same height.
🧹 Nitpick comments (3)
apps/csm-portal/microapp/src/types/adminUser.model.ts (1)
38-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd unit tests for
isSnUserandtoAdminUserThese pure functions handle the critical data-source distinction and normalization. Unit tests would catch regressions if DTO shapes change or if the type guard's property checks drift.
🧪 Suggested test cases
// isSnUser // - returns true for SnUserDto (has name, active, roles) // - returns false for UserDto (has firstName, lastName, userType) // toAdminUser — SnUserDto branch // - normalizes name, timeZone→timezone, active, roles // - trims whitespace-only name to "" // toAdminUser — UserDto branch // - concatenates firstName + lastName into name // - maps timezone, userType // - handles missing optional timezone (null)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/csm-portal/microapp/src/types/adminUser.model.ts` around lines 38 - 62, Add unit tests covering isSnUser and toAdminUser for both SnUserDto and UserDto shapes. Verify type discrimination, trimmed and whitespace-only names, SnUserDto timeZone-to-timezone mapping with active and roles, and UserDto firstName/lastName concatenation, timezone fallback to null, and userType mapping.apps/csm-portal/microapp/src/components/settings/Settings.tsx (1)
87-101: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid recreating the IntersectionObserver on every page fetch.
The observer is torn down and rebuilt whenever
isFetchingNextPagetoggles (after every page load), causing unnecessary churn. Using refs forhasNextPageandisFetchingNextPagelets the observer live for the component's lifetime while the callback always reads current values. AddingrootMarginalso enables pre-fetching before the sentinel enters the viewport, smoothing the scroll experience.♻️ Proposed refactor: ref-based observer with rootMargin
export default function Settings() { const [tab, setTab] = useState<AdminTabId>("users"); const activeTab = ADMIN_TABS.find((t) => t.id === tab) ?? ADMIN_TABS[0]; @@ function UsersTab() { const [search, setSearch] = useState(""); const debouncedSearch = useDebouncedValue(search, 300); const [filters, setFilters] = useState<UsersFilters>(EMPTY_USERS_FILTERS); const [filtersOpen, setFiltersOpen] = useState(false); const activeFilterCount = countActiveUsersFilters(filters); const { data, isLoading, isError, refetch, fetchNextPage, hasNextPage, isFetchingNextPage } = useInfiniteQuery( adminUsers.infinite(toUserSearchFilters(debouncedSearch, filters)), ); + // Refs let the observer callback read live values without recreating the observer. + const hasNextPageRef = useRef(hasNextPage); + const isFetchingNextPageRef = useRef(isFetchingNextPage); + hasNextPageRef.current = hasNextPage; + isFetchingNextPageRef.current = isFetchingNextPage; + const sentinelRef = useRef<HTMLDivElement>(null); useEffect(() => { const el = sentinelRef.current; if (!el) return; const observer = new IntersectionObserver( ([entry]) => { - if (entry?.isIntersecting && hasNextPage && !isFetchingNextPage) { + if (entry?.isIntersecting && hasNextPageRef.current && !isFetchingNextPageRef.current) { void fetchNextPage(); } }, - { threshold: 0.1 }, + { threshold: 0, rootMargin: "200px" }, ); observer.observe(el); return () => observer.disconnect(); - }, [hasNextPage, isFetchingNextPage, fetchNextPage]); + }, [fetchNextPage]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/csm-portal/microapp/src/components/settings/Settings.tsx` around lines 87 - 101, The IntersectionObserver in the settings component is recreated whenever pagination state changes. Update the useEffect around sentinelRef and the observer callback to read current hasNextPage and isFetchingNextPage through refs, limit the effect dependencies to stable values so the observer persists for the component lifetime, and configure a rootMargin to prefetch before the sentinel enters the viewport.apps/csm-portal/microapp/src/components/settings/UserCard.tsx (1)
50-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
USER_ROLE_LABELSfor role chip labels to match the filter sheet.Role chips render raw backend strings (e.g.,
customer_admin) whileUsersFiltersSheetrenders human-readable labels viaUSER_ROLE_LABELS(e.g.,Customer admin). A safe lookup with fallback keeps both surfaces consistent.♻️ Proposed refactor: label lookup with fallback
import { Card, Chip, Skeleton, Stack, Typography } from "`@wso2/oxygen-ui`"; import type { AdminUser } from "`@src/types`"; -import { INTERNAL_USER_ROLES } from "./config"; +import { INTERNAL_USER_ROLES, USER_ROLE_LABELS } from "./config"; + +const ROLE_LABELS = USER_ROLE_LABELS as Record<string, string>;{user.roles && user.roles.length > 0 ? ( user.roles.map((role) => ( <Chip key={role} size="small" - label={role} + label={ROLE_LABELS[role] ?? role} color={(INTERNAL_USER_ROLES as string[]).includes(role) ? "primary" : "default"} variant="outlined" /> ))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/csm-portal/microapp/src/components/settings/UserCard.tsx` around lines 50 - 58, Update the role chip label in the user roles mapping to use USER_ROLE_LABELS[role], falling back to the raw role value when no label exists. Keep the existing role key, color, size, and variant behavior unchanged so UserCard matches UsersFiltersSheet.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/csm-portal/microapp/src/components/settings/UsersFiltersSheet.tsx`:
- Around line 79-113: Add explicit accessibility state to the chips in the roles
and status sections. In the ALL_USER_ROLES map, add aria-pressed based on
isSelected to each clickable role Chip; in the STATUS_OPTIONS map, expose the
mutually exclusive selection using a radiogroup container and radio semantics,
including the selected state for each status Chip while preserving existing
selection behavior.
---
Outside diff comments:
In `@apps/csm-portal/microapp/src/pages/ProfilePage.tsx`:
- Around line 78-80: Update ProfileSkeleton to render a single rounded Skeleton
row matching the lone disabled email TextField in ProfileContent, removing the
two obsolete placeholder rows so loading and resolved layouts have the same
height.
---
Nitpick comments:
In `@apps/csm-portal/microapp/src/components/settings/Settings.tsx`:
- Around line 87-101: The IntersectionObserver in the settings component is
recreated whenever pagination state changes. Update the useEffect around
sentinelRef and the observer callback to read current hasNextPage and
isFetchingNextPage through refs, limit the effect dependencies to stable values
so the observer persists for the component lifetime, and configure a rootMargin
to prefetch before the sentinel enters the viewport.
In `@apps/csm-portal/microapp/src/components/settings/UserCard.tsx`:
- Around line 50-58: Update the role chip label in the user roles mapping to use
USER_ROLE_LABELS[role], falling back to the raw role value when no label exists.
Keep the existing role key, color, size, and variant behavior unchanged so
UserCard matches UsersFiltersSheet.
In `@apps/csm-portal/microapp/src/types/adminUser.model.ts`:
- Around line 38-62: Add unit tests covering isSnUser and toAdminUser for both
SnUserDto and UserDto shapes. Verify type discrimination, trimmed and
whitespace-only names, SnUserDto timeZone-to-timezone mapping with active and
roles, and UserDto firstName/lastName concatenation, timezone fallback to null,
and userType mapping.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 38ceee97-4bb9-4b87-8735-4f36ca70e9d5
📒 Files selected for processing (16)
apps/csm-portal/microapp/src/App.tsxapps/csm-portal/microapp/src/components/common/ComingSoonPage.tsxapps/csm-portal/microapp/src/components/layout/TopBar.tsxapps/csm-portal/microapp/src/components/settings/Settings.tsxapps/csm-portal/microapp/src/components/settings/UserCard.tsxapps/csm-portal/microapp/src/components/settings/UsersFiltersSheet.tsxapps/csm-portal/microapp/src/components/settings/config.tsapps/csm-portal/microapp/src/components/settings/filters.tsapps/csm-portal/microapp/src/config/endpoints.tsapps/csm-portal/microapp/src/pages/MorePage.tsxapps/csm-portal/microapp/src/pages/ProfilePage.tsxapps/csm-portal/microapp/src/pages/SettingsPage.tsxapps/csm-portal/microapp/src/services/adminUsers.tsapps/csm-portal/microapp/src/types/adminUser.dto.tsapps/csm-portal/microapp/src/types/adminUser.model.tsapps/csm-portal/microapp/src/types/index.ts
💤 Files with no reviewable changes (1)
- apps/csm-portal/microapp/src/components/layout/TopBar.tsx
…hips Role chips get aria-pressed (multi-select toggle semantics); status chips are grouped under role="radiogroup" with role="radio" and aria-checked per option (mutually-exclusive selection). No behavior change.
Two live widgets: "Assigned to me" (the signed-in user's non-closed cases, capped to a 5-item preview) and case composition donuts (severity + state breakdown of active cases), both ported from the webapp's csm-dashboard feature. Donut styling/behavior mirrors the webapp's CompositionDonut directly. The severity×state matrix table and the ABT-scope header are left out: the matrix costs 25 more fan-out requests (no aggregation endpoint exists) on top of the donuts' 11, and doesn't fit a mobile screen well; the header is feature-flagged off in the webapp itself and only ever shows a static fallback. getAllCases is now exported from cases.ts so the composition query can fan out count-only searches directly, same as the webapp's useCaseComposition bypasses its own query-options wrapper for the same reason.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/csm-portal/microapp/src/services/dashboard.ts (3)
36-64: 🧹 Nitpick | 🔵 Trivial11 parallel count-only requests per composition fetch.
fetchCompositionfires 11 requests in parallel (5 severity + 5 state + 1 closed). ThestaleTime: 60_000oncomposition()mitigates refetch-on-revisit, but there's no aggregation endpoint, so each dashboard view after staleness expiry re-fires all 11. Consider monitoring request volume and backend impact as user count grows. If the backend supports batched count queries or an aggregation endpoint in the future, this is the primary candidate for optimization.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/csm-portal/microapp/src/services/dashboard.ts` around lines 36 - 64, Monitor the request volume and backend impact of fetchComposition, which issues 11 parallel count-only requests per composition fetch. Preserve the current behavior for now, and prioritize replacing the fan-out with batched count queries or an aggregation endpoint if backend support becomes available.
78-90: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider adding
staleTimetoassignedToMe.Unlike
composition(),assignedToMehas nostaleTime, so it refetches on every mount/revisit. For a 5-item preview that changes infrequently, a shortstaleTime(e.g. 30s–60s) would reduce unnecessary network traffic without meaningfully degrading freshness.♻️ Proposed addition
assignedToMe: (userId: string | null) => queryOptions({ queryKey: ["dashboard", "assigned-to-me", userId], queryFn: () => userId ? getAllCases({ filters: { types: ["case"], assignedUserIds: [userId], states: ASSIGNED_TO_ME_STATES }, sortBy: { field: "updatedOn", order: "desc" }, pagination: { limit: 5 }, }) : Promise.resolve(EMPTY_RESULT), enabled: !!userId, + staleTime: 30_000, }),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/csm-portal/microapp/src/services/dashboard.ts` around lines 78 - 90, Add a short staleTime, such as 30–60 seconds, to the queryOptions configuration returned by assignedToMe, matching the caching approach used by composition while preserving the existing query key, query function, and enabled behavior.
26-32: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winMake
byStatea sparse map
COMPOSITION_STATESintentionally omitsclosedandreopened, soObject.fromEntries(stateCounts)only returns the active-state keys.Partial<Record<CaseState, number>>matches the runtime shape, and the current consumer already falls back to0.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/csm-portal/microapp/src/services/dashboard.ts` around lines 26 - 32, Update the CaseComposition.byState property to use Partial<Record<CaseState, number>> instead of requiring every CaseState key, matching the sparse map produced by Object.fromEntries(stateCounts). Preserve the existing consumer fallback to 0.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/csm-portal/microapp/src/components/home/AssignedToMeSection.tsx`:
- Around line 29-33: Use the assigned-to-me query’s isPending state instead of
isLoading when deciding whether to render the loading skeleton in
AssignedToMeSection. This must keep the empty state from rendering while
currentUserId is unresolved and the dependent dashboard.assignedToMe query is
disabled, while preserving the existing empty state once the query has completed
without items.
---
Nitpick comments:
In `@apps/csm-portal/microapp/src/services/dashboard.ts`:
- Around line 36-64: Monitor the request volume and backend impact of
fetchComposition, which issues 11 parallel count-only requests per composition
fetch. Preserve the current behavior for now, and prioritize replacing the
fan-out with batched count queries or an aggregation endpoint if backend support
becomes available.
- Around line 78-90: Add a short staleTime, such as 30–60 seconds, to the
queryOptions configuration returned by assignedToMe, matching the caching
approach used by composition while preserving the existing query key, query
function, and enabled behavior.
- Around line 26-32: Update the CaseComposition.byState property to use
Partial<Record<CaseState, number>> instead of requiring every CaseState key,
matching the sparse map produced by Object.fromEntries(stateCounts). Preserve
the existing consumer fallback to 0.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5fec3950-24ce-4350-aff4-f72ed4068447
📒 Files selected for processing (7)
apps/csm-portal/microapp/src/components/home/AssignedToMeSection.tsxapps/csm-portal/microapp/src/components/home/CaseCompositionSection.tsxapps/csm-portal/microapp/src/components/home/CompositionDonut.tsxapps/csm-portal/microapp/src/components/home/config.tsapps/csm-portal/microapp/src/pages/HomePage.tsxapps/csm-portal/microapp/src/services/cases.tsapps/csm-portal/microapp/src/services/dashboard.ts
…on.tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
12ced32
into
wso2-open-operations:dev-app-csm-portal
…resolution Two undefined-reference bugs landed on dev-app-csm-portal from an incomplete resolution of the #1134 merge conflict: - MorePage.tsx used the Settings icon but the merge dropped it from the import list. - AssignedToMeSection.tsx destructured isPending (renamed from isLoading by a partial suggestion commit) but the JSX below still referenced the old isLoading name. Both are one-line fixes; verified with eslint, tsc --noEmit, and vite build.
Summary
CsmAdminLayout+CsmUsersPage): a Users tab with search/role/status filtering backed byPOST /users/search, plus Roles/Groups/Permissions as WIP tabs reusing the existingComingSoonPage(same copy as the webapp'sCsmComingSoonPagefor those sections).CSM_NAV_ITEMSorder.csmUsers.tsfield-for-field (theoneOfpostgres/ServiceNow user shapes, normalized into one shape), paged via infinite scroll — the mobile equivalent of the webapp's table +TablePagination.Settingscomponent is deliberately self-contained (owns its own tab state, filters, and data fetching) so it can be relocated to a different page later without threading props through.blockedOnprop to the sharedComingSoonPagefor parity with the webapp's version.Test plan
eslintcleanvite buildcleantsc --noEmitclean (worked around the pre-existing, unrelatedignoreDeprecationsTS-version mismatch to confirm no real type errors)Summary by CodeRabbit