[Customer Portal][FE][Web] Standardize Assigned Engineer Data Model and UI Display Logic - #159
Conversation
Replace inline logic that derived avatar initials and displayed the assigned engineer with getInitials and formatValue from @utils/support. This simplifies CasesList, centralizes formatting behavior for engineer names/initials, and improves readability by removing duplicated string-manipulation code (added import for the new helpers).
Replace the inline logic that extracted the assignedEngineer label with a call to getAssignedEngineerLabel. Added the helper import and removed the temporary raw variable and type checks, simplifying and centralizing the label formatting logic.
Import AssignedEngineerValue type and update the helper to accept it. Rename the existing string test for clarity and add tests covering object shapes ({ id, label } and { id, name }) to verify label/name rendering and computed initials. Keeps the existing null case test intact.
Add a test to ensure the engineer section is hidden when assignedEngineer is null/undefined and verify Manage case status and status label remain visible. Rename/adjust the existing loading test to cover the isLoading state with an assigned engineer (checks skeletons while retaining action elements). Minor test cleanup and rendering updates with ThemeProvider.
Change AssignedEngineerDisplay prop to use AssignedEngineerValue from @utils/support for stronger typing. Update the CaseDetailsDetailsPanel test: rename the test to reflect hiding the Assigned Engineer when null and add an assertion that the "Assigned Engineer" label is not rendered when the value is null.
Guard rendering of Assigned Engineer UI using getAssignedEngineerLabel. Adds conditional checks in CaseDetailsDetailsPanel (both blocks) to skip the Assigned Engineer section when no engineer is set. In CaseDetailsActionRow, the assignedEngineer prop type was updated to AssignedEngineerValue and hasEngineer is derived via getAssignedEngineerLabel; the avatar, name, role caption and divider are now rendered only if an engineer exists while still showing loading skeletons as needed. This avoids empty/placeholder UI when no engineer is assigned.
Replace inline assignedEngineer parsing in OutstandingCasesList with getAssignedEngineerLabel and import it. Update unit test and mock data to use a consistent assignedEngineer object ({id, label}) across CaseListItem entries, and set mockCaseDetails.assignedEngineer to an object as well. These changes standardize the assignedEngineer shape and simplify rendering/tooltip logic.
Add unit tests to cover object-shaped inputs for support utils: formatValue now has tests for { id, label } and { id, name } shapes (including empty label/name -> "--"). getInitials gains tests to derive initials from those same object shapes. Also minor formatting cleanup in richTextEditor.test.ts (reflow expect call).
Update types and utilities to handle assignedEngineer being either a string or an object ({ id, label?, name? }).
- models/responses.ts: broaden assignedEngineer types in CaseListItem and CaseDetails to accept objects with optional label or name.
- utils/support.ts: add AssignedEngineerValue type, getAssignedEngineerDisplayValue helper, and getAssignedEngineerLabel. Update formatValue and getInitials to extract display text from assignedEngineer objects and fall back to "--" for empty values.
This ensures robust display/initials extraction for API responses that return either strings or {id,label} / {id,name} objects.
📝 WalkthroughWalkthroughThis PR refactors how the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
dileepapeiris
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/customer-portal/webapp/src/components/support/support-overview-cards/__tests__/OutstandingCasesList.test.tsx (1)
22-25:⚠️ Potential issue | 🔴 CriticalIncomplete mock —
getAssignedEngineerLabelandresolveColorFromThemeare missing.The
vi.mock("@utils/support", ...)factory only providesformatRelativeTimeandstripHtml. The component also imports and callsgetAssignedEngineerLabel(line 161) andresolveColorFromTheme(line 77), which will beundefinedunder this mock, causing the test to fail at runtime.Either include all used exports in the mock factory, or use
importOriginalto preserve un-mocked exports:🐛 Proposed fix using importOriginal
-vi.mock("@utils/support", () => ({ - formatRelativeTime: vi.fn(() => "2 hours ago"), - stripHtml: vi.fn((html) => html), -})); +vi.mock("@utils/support", async (importOriginal) => { + const actual = await importOriginal<typeof import("@utils/support")>(); + return { + ...actual, + formatRelativeTime: vi.fn(() => "2 hours ago"), + stripHtml: vi.fn((html: string) => html), + }; +});
🧹 Nitpick comments (5)
apps/customer-portal/webapp/src/utils/__tests__/support.test.ts (1)
431-442: Consider adding a direct test forgetAssignedEngineerLabel.
getAssignedEngineerLabelis exported and used by multiple components (e.g.,CaseDetailsActionRow,AllCasesList,OutstandingCasesList) but has no direct test coverage here — it's only indirectly exercised throughformatValueandgetInitials. A few targeted assertions (string input, object with label, object with name only, null) would strengthen confidence.apps/customer-portal/webapp/src/models/mockData.ts (1)
1462-1465:mockCaseDetailsuses{ id, name }shape — consistent with CaseDetails API.Note that
name: "dileepapp@wso2.com"will causegetInitialsto return"D"(single initial from email). This is fine for mock/demo purposes, but worth noting that if the real API returns emails as names, the initials display may not be user-friendly (e.g., avatar shows "D" instead of meaningful initials).apps/customer-portal/webapp/src/components/support/case-details/details-tab/__tests__/AssignedEngineerDisplay.test.tsx (1)
38-48: Good coverage of the new object shapes.The new tests for
{ id, label }and{ id, name }variants align well with theAssignedEngineerValuetype. Consider also adding a test for the edge case{ id: "eng-1" }(object with neitherlabelnorname) to verify fallback behavior.apps/customer-portal/webapp/src/components/support/case-details/header/CaseDetailsActionRow.tsx (2)
36-41: Merge type and value imports from the same module.Minor nit: the type import and value imports from
@utils/supportcan be consolidated into a single import statement.Proposed merge
-import type { AssignedEngineerValue } from "@utils/support"; -import { - formatValue, - getAssignedEngineerLabel, - getAvailableCaseActions, -} from "@utils/support"; +import { + type AssignedEngineerValue, + formatValue, + getAssignedEngineerLabel, + getAvailableCaseActions, +} from "@utils/support";
66-68: Remove redundantengineerInitialsprop and compute internally.The caller (CaseDetailsContent.tsx) already computes
engineerInitials = getInitials(assignedEngineer)before passing it. SincegetInitialscan accept the sameAssignedEngineerValuetype, derive initials internally within the component instead of accepting it as a prop. This prevents inconsistency and reduces prop surface area. Will require importinggetInitialsfrom@utils/supportand updating tests.
There was a problem hiding this comment.
Pull request overview
This PR standardizes how “assigned engineer” data coming from the support/cases APIs is represented and rendered across the customer portal UI by introducing a shared AssignedEngineerValue type plus helper utilities for label/initial extraction, then refactoring components and tests to use them.
Changes:
- Added
AssignedEngineerValuealong withgetAssignedEngineerLabel, and expandedformatValue/getInitialsto support assigned-engineer objects. - Refactored multiple UI surfaces (case lists + case details) to consistently display/hide assigned engineer information based on the derived label.
- Updated mock data and tests to use the new assigned engineer object shape and added/updated test cases accordingly.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/customer-portal/webapp/src/utils/support.ts | Introduces AssignedEngineerValue and shared helpers for label/initials + extends formatValue/getInitials. |
| apps/customer-portal/webapp/src/utils/tests/support.test.ts | Adds tests for assigned engineer object handling in formatValue/getInitials. |
| apps/customer-portal/webapp/src/utils/tests/richTextEditor.test.ts | Minor test formatting change. |
| apps/customer-portal/webapp/src/models/responses.ts | Updates response typings for assignedEngineer to allow { id, label?, name? }. |
| apps/customer-portal/webapp/src/models/mockData.ts | Updates mock case data to use the assigned engineer object shape. |
| apps/customer-portal/webapp/src/components/support/support-overview-cards/OutstandingCasesList.tsx | Uses getAssignedEngineerLabel to render “Assigned to …” consistently. |
| apps/customer-portal/webapp/src/components/support/support-overview-cards/tests/OutstandingCasesList.test.tsx | Updates mocks to the assigned engineer object shape. |
| apps/customer-portal/webapp/src/components/support/case-details/header/CaseDetailsActionRow.tsx | Hides the engineer section when no engineer label is present; adopts AssignedEngineerValue. |
| apps/customer-portal/webapp/src/components/support/case-details/header/tests/CaseDetailsActionRow.test.tsx | Updates expectations to match new hide/show behavior. |
| apps/customer-portal/webapp/src/components/support/case-details/details-tab/CaseDetailsDetailsPanel.tsx | Conditionally renders “Assigned Engineer” blocks using getAssignedEngineerLabel. |
| apps/customer-portal/webapp/src/components/support/case-details/details-tab/tests/CaseDetailsDetailsPanel.test.tsx | Updates assertions to validate assigned engineer section hiding. |
| apps/customer-portal/webapp/src/components/support/case-details/details-tab/AssignedEngineerDisplay.tsx | Updates prop type to AssignedEngineerValue and relies on shared formatting/initials helpers. |
| apps/customer-portal/webapp/src/components/support/case-details/details-tab/tests/AssignedEngineerDisplay.test.tsx | Adds coverage for string vs {id,label} vs {id,name} assigned engineer inputs. |
| apps/customer-portal/webapp/src/components/support/all-cases/AllCasesList.tsx | Uses getAssignedEngineerLabel instead of ad-hoc parsing. |
| apps/customer-portal/webapp/src/components/dashboard/cases-table/CasesList.tsx | Uses getInitials/formatValue for assigned engineer rendering in the dashboard table. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dileepapeiris
added
Type/New Feature
| * @param value - Assigned engineer from API. | ||
| * @returns {string} Display label or empty string if null/undefined. | ||
| */ | ||
| export function getAssignedEngineerLabel(value: AssignedEngineerValue): string { |
There was a problem hiding this comment.
@dileepapeiris is getAssignedEngineerLabel used to render components? If so are you using an empty string comparison (e.g., getAssignedEngineerLabel(someValue) === "" && <Component />)?
| * @returns {string} Display string. | ||
| */ | ||
| export function formatValue(value: string | number | null | undefined): string { | ||
| export function formatValue( |
There was a problem hiding this comment.
I think we can make this:
export function formatValue(
value: AssignedEngineerValue | number,
): string {Check and verify this later. Safer if and when the AssignedEngineerValue object structure changes
| const initials = name | ||
| export function getInitials( | ||
| name: | ||
| | string |
There was a problem hiding this comment.
ditto, use the predefined object if applicable
|
Those changes will be addressed in upcoming PRs |
v15a1
left a comment
v15a1
left a comment
There was a problem hiding this comment.
As discussed offline, the suggested changes will be addressed in upcoming PRs. LGTM
efeb64c
into
wso2-open-operations:customer-portal-milestone-1
Description
This pull request standardizes how assigned engineer data is handled and displayed throughout the customer portal app. It introduces a new
AssignedEngineerValuetype and utility functions for extracting and formatting engineer names and initials. This results in more consistent UI behavior, improved handling of null/undefined values, and cleaner code in components and tests.Key changes include:
Refactoring and Type Improvements:
Introduced the
AssignedEngineerValuetype and utility functions likegetAssignedEngineerLabelandgetInitialsin@utils/support, and updated all relevant components to use these for displaying assigned engineer information. [1] [2] [3]Updated the mock data in
mockData.tsand test files to use the new assigned engineer object structure instead of plain strings, ensuring consistency in test and demo data. [1] [2] [3] [4] [5] [6] [7]UI and Logic Consistency:
CasesList,AllCasesList,OutstandingCasesList,CaseDetailsDetailsPanel,CaseDetailsActionRow, etc.) to use the new utility functions for extracting and displaying engineer names and initials, and improved logic to hide the engineer section when no engineer is assigned. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]Testing Enhancements:
Component API Changes:
AssignedEngineerValueinstead of just strings, enforcing type safety and consistency across the codebase. [1] [2]These changes improve code maintainability, UI consistency, and type safety throughout the application.
Summary by CodeRabbit
Release Notes
Bug Fixes
Refactor