[CSM Portal] fix post-merge CodeRabbit findings; case-list Updated bug, dashboard polish - #1328
Merged
cloby99 merged 15 commits intoAug 2, 2026
Merged
Conversation
New DASHBOARDS_CONFIG entry (`abt`, `isTeamBased: true`) modeling a subset of an existing ServiceNow production landing page's widget set, using only filter capabilities this stack already supports end to end: My Work section (assignedUserId-scoped counts/lists) and an Overall section (team-wide counts), covering Open Incident/Query, Patches, Pending Closure, Reminders, Discussions, On Going Cases, Unassigned Cases, and a couple of tag-scoped team widgets (WOW P1, IAM Open Cases). Two real gaps in the current DSL meant some source widgets couldn't be replicated exactly: - No `notIn` op on fields besides `tag` — any "X not in (small closed set)" condition (e.g. state exclusions) is expressed instead as the enumerated complement of that closed enum, which is behaviorally identical. - No server-side "current team" resolution (only `__current_user__` is substituted) — so, matching the existing `team_performance` dashboard's own precedent, `isTeamBased` stays UI-only metadata here; no widget filters on `integrationCsTeamIds` yet, since there's no safe placeholder to put there (an unresolved one would 400 against `validateUUIDs`). Out of scope, same as the original gap analysis this task's earlier phases were scoped against: Task-SLA joins, an Escalation entity, and aggregate/group-by (donut) shapes — none of those exist in this stack yet. Test registry (`dashboards_test.go`) and its doc comments updated from 5 to 6 dashboards; `TestGetDashboards`'s team-based-count assertion updated to expect two (`team_performance`, `abt`) instead of one.
Replace the pilot's real dashboard content in .env.example (real tag taxonomy, real team names, a dashboard modeled directly on a real ServiceNow production landing page) with a short, clearly-dummy 2-dashboard example. This is a public repo; illustrating the config schema does not require documenting real internal structure. The real deployment's DASHBOARDS_CONFIG value is set directly in its own environment, not committed here.
…of the named day A YYYY-MM-DD lte filter parsed to UTC midnight, so it excluded the whole named day except the 00:00:00Z instant, while both openapi specs document lte as "on or before" that date. CodeRabbit finding on cs-tools#1325.
…stgres backend ParseCaseFieldFilters accepts every field in caseFilterFieldSet, but the Postgres-backed SearchCases path only ever reads 11 of them plus the date bounds. tag/parentId/product/projectOnboardingStatus/projectType/ integrationCsTeam/assignedUserId-isEmpty/resolutionNotes-isEmpty were silently dropped, returning 200 with a broader-than-requested result set. These fields dot-walk into SN-only concepts with no Postgres schema equivalent and no repository query support today, so reject them outright instead of inventing partial support. CodeRabbit finding on cs-tools#1325.
…h updatedOn Two fixes to the ServiceNow case-search path, landed together since both touch sn_case_service.go's SearchCases: - Add missing enum validation for state/severity/issueType/engagementType filters (CodeRabbit finding on cs-tools#1325). Only type and workState were validated; the other four were converted straight to SN numeric ids via domainStatesToSNIDs/domainSeveritiesToSNIDs/domainIssueTypesToSNIDs/ domainEngagementTypesToSNIDs, which silently skip unrecognized values, producing an empty key slice that omitempty then dropped from the SN payload -- an unrecognized value widened the result set instead of erroring. Validated against the same validCaseState/validCaseSeverity/ validCaseIssueType/validEngagementType maps the Postgres backend already uses; confirmed those maps cover the exact same value sets as snStateIDMap/snSeverityIDMap/snIssueTypeIDMap/snEngagementTypeIDMap, so this rejects no value that previously reached SN. - Add domain.SearchCaseView.UpdatedOn (was missing entirely -- only CreatedOn existed), and populate it on both backends: the SN path from the same c.UpdatedOn field GetCaseByID already uses (falling back to createdOn when SN omits it, matching GetCaseByID's own convention), and the Postgres path from cases.updated_at, matching how created_at is already selected. Fixes a real bug: the case list's "Updated" column always showed the created date because /cases/search never carried an update timestamp.
…am content dashboards_test.go's inline fixture mirrored real WSO2 CS-team dashboard structure (tag taxonomy, team names, an actual SN production dashboard's widget layout). Replace it with the dummy 2-dashboard fixture already documented in .env.example's DASHBOARDS_CONFIG example, extended with two small dummy widgets (an empty-filter incident widget, a flat-filter change_request widget) so resourceType-diversity and scalar-filter test coverage isn't lost. widgets_test.go's independent fixture is genericized the same way. All prior test behavior/coverage is preserved under new dummy names.
An empty `values: []` DSL entry for state/severity/type/product was truthy under a plain `if (x)` check, so it set an explicit empty-array filter on the generated cases-list link instead of leaving the field unset (matching the existing `assignedUserIds` guard). Add tests covering both the empty and populated cases.
…column The cases list's Updated column always rendered "Created <relative time>" because the backend's /cases/search response never actually returned updatedOn, so every row's updatedAt silently fell back to createdOn. The fallback in caseSearchPayload/useGetMyAssignedOpenCases is a genuine safety net (a row with no updatedOn at all should still show something) and stays, but per explicit instruction the "Created " prefix is removed entirely — the column always renders unprefixed now, and the now-fully-unused updatedAtIsCreatedFallback flag is removed.
The dashboard tile's big number (shape: "count") rendered at h5, too small to read as the tile's primary content. Bump it to h4 and add noWrap plus an ellipsis fallback so a realistic 5-digit total doesn't wrap or clip in a narrow (3/12) grid column; the label above it also gets noWrap for the same reason.
The dashboard switcher and (for isTeamBased dashboards) the team selector were both local component state, so a shared link always landed back on the BE default. CsmDashboardPage now reads/writes ?dashboard= and ?team= via useSearchParams (replace, matching casesFiltersUrl's convention so switching doesn't spam history), falls back to the BE isDefault entry when the URL names an unknown/absent dashboard, and clears a stale team param when switching to a non-team-based dashboard. selectedTeamId/onTeamChange are lifted out of AbtDashboardHeader into controlled props from the page, which owns the URL-synced state.
…enapi.yaml
Found while genericizing the dashboard test fixtures: this one description
string still referenced the real internal dashboard id ("agents_pilot")
directly rather than a dummy example.
Contributor
|
Warning Review limit reached
Next review available in: 53 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 Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe change updates dashboard fixtures, URL-backed dashboard and team selection, widget filter translation, count-tile rendering, case timestamp propagation, date filtering, and case-search validation. ChangesDashboard experience
Case search contract
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant CsmDashboardPage
participant AbtDashboardHeader
Browser->>CsmDashboardPage: Provide dashboard and team query parameters
CsmDashboardPage->>AbtDashboardHeader: Provide selected dashboard and team values
AbtDashboardHeader->>CsmDashboardPage: Report dashboard or team selection
CsmDashboardPage->>Browser: Replace dashboard and team query parameters
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
github-actions
Bot
added
App/CSM Portal
Area/Backend
Area/Frontend
Entity Service
Platform/Web
labels
…bold) Still too small at h4 per user feedback after previewing.
Dropped the MUI variant preset in favor of an explicit fontSize (4rem) so the size is unambiguous rather than tied to a theme's h3 definition, which apparently wasn't as big as expected.
rksk
marked this pull request as ready for review
August 2, 2026 16:47
Contributor
Author
|
@coderabbitai resume |
Contributor
✅ Action performedReviews resumed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
entity-service/internal/service/sn_case_service_test.go (1)
1107-1154: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the
createdOnfallback.This test only covers a ServiceNow response with
updatedOn. Add a case that omitsupdatedOnor returns an empty value. Verify thatSearchCasesreturnsCreatedOnasUpdatedOn. An implementation that returns an empty timestamp currently passes this test.🤖 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 `@entity-service/internal/service/sn_case_service_test.go` around lines 1107 - 1154, Extend TestSNCaseService_SearchCases_PopulatesUpdatedOn with a ServiceNow case whose updatedOn field is omitted or empty, then assert SearchCases returns that case’s CreatedOn value in UpdatedOn. Keep the existing non-empty updatedOn assertions and add coverage that the fallback is applied rather than returning an empty timestamp.entity-service/internal/service/case_service_test.go (1)
114-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover all supported date fields.
This table claims to cover every supported field. The repository also supports
closedOnandupdatedOn, but the table only includescreatedOn. Add representative filters for both fields so this regression test covers the complete Postgres date-filter contract.🤖 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 `@entity-service/internal/service/case_service_test.go` around lines 114 - 129, Extend the filter table in the case service test to include representative gte filters for both supported date fields, closedOn and updatedOn, alongside createdOn. Use the same date-filter structure and ensure the cases validate the complete Postgres date-filter contract.entity-service/internal/service/sn_case_service.go (1)
2027-2037: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for both fallback conditions.
The code falls back to
CreatedOnwhenUpdatedOnis nil or empty. The shown test covers only a non-emptyUpdatedOn. Add cases for both fallback inputs and assert thatUpdatedOnequalsCreatedOn.🤖 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 `@entity-service/internal/service/sn_case_service.go` around lines 2027 - 2037, Add test cases for the SearchCaseView construction covering both UpdatedOn fallback conditions: a nil UpdatedOn and a non-nil empty UpdatedOn. In each case, assert that the resulting SearchCaseView.UpdatedOn equals the case’s CreatedOn, while preserving the existing non-empty UpdatedOn coverage.
🤖 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 `@entity-service/internal/service/case_filters_test.go`:
- Around line 191-213: Update
TestParseCaseFieldFilters_DateOnlyLteBoundIncludesWholeDay to assert that
EndCreatedDate equals the exact end-of-day boundary
2026-01-31T23:59:59.999999999Z, while retaining the existing next-day exclusion
check.
---
Nitpick comments:
In `@entity-service/internal/service/case_service_test.go`:
- Around line 114-129: Extend the filter table in the case service test to
include representative gte filters for both supported date fields, closedOn and
updatedOn, alongside createdOn. Use the same date-filter structure and ensure
the cases validate the complete Postgres date-filter contract.
In `@entity-service/internal/service/sn_case_service_test.go`:
- Around line 1107-1154: Extend TestSNCaseService_SearchCases_PopulatesUpdatedOn
with a ServiceNow case whose updatedOn field is omitted or empty, then assert
SearchCases returns that case’s CreatedOn value in UpdatedOn. Keep the existing
non-empty updatedOn assertions and add coverage that the fallback is applied
rather than returning an empty timestamp.
In `@entity-service/internal/service/sn_case_service.go`:
- Around line 2027-2037: Add test cases for the SearchCaseView construction
covering both UpdatedOn fallback conditions: a nil UpdatedOn and a non-nil empty
UpdatedOn. In each case, assert that the resulting SearchCaseView.UpdatedOn
equals the case’s CreatedOn, while preserving the existing non-empty UpdatedOn
coverage.
🪄 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 Plus
Run ID: 54f526af-63a3-493a-98a8-87dfb652bb31
📒 Files selected for processing (24)
apps/csm-portal/backend/.env.exampleapps/csm-portal/backend/internal/dashboard/widgets_test.goapps/csm-portal/backend/internal/handler/dashboards_test.goapps/csm-portal/backend/openapi.yamlapps/csm-portal/webapp/src/features/csm-cases/components/CasesList.tsxapps/csm-portal/webapp/src/features/csm-cases/types/csmCases.tsapps/csm-portal/webapp/src/features/csm-cases/utils/caseSearchPayload.tsapps/csm-portal/webapp/src/features/csm-dashboard/api/useGetMyAssignedOpenCases.tsapps/csm-portal/webapp/src/features/csm-dashboard/components/AbtDashboardHeader.test.tsxapps/csm-portal/webapp/src/features/csm-dashboard/components/AbtDashboardHeader.tsxapps/csm-portal/webapp/src/features/csm-dashboard/components/DashboardWidgetTile.tsxapps/csm-portal/webapp/src/features/csm-dashboard/config/widgetResourceConfig.test.tsapps/csm-portal/webapp/src/features/csm-dashboard/config/widgetResourceConfig.tsapps/csm-portal/webapp/src/features/csm-dashboard/pages/CsmDashboardPage.test.tsxapps/csm-portal/webapp/src/features/csm-dashboard/pages/CsmDashboardPage.tsxentity-service/internal/domain/entity.goentity-service/internal/repository/case_repo.goentity-service/internal/service/case_filters.goentity-service/internal/service/case_filters_test.goentity-service/internal/service/case_service.goentity-service/internal/service/case_service_test.goentity-service/internal/service/sn_case_service.goentity-service/internal/service/sn_case_service_test.goentity-service/openapi.yaml
💤 Files with no reviewable changes (3)
- apps/csm-portal/webapp/src/features/csm-dashboard/api/useGetMyAssignedOpenCases.ts
- apps/csm-portal/webapp/src/features/csm-cases/components/CasesList.tsx
- apps/csm-portal/webapp/src/features/csm-cases/utils/caseSearchPayload.ts
…loose one The prior assertion accepted 23:59:59Z, which would also pass if the one-nanosecond adjustment were silently missing (23:59:59Z < any time in that last second). Assert equality with the exact 23:59:59.999999999Z boundary the fix actually produces.
cloby99
approved these changes
Aug 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Goals
apps/csm-portal/backend/.env.example'sDASHBOARDS_CONFIGexample and its mirrored test fixtures: this is a public repo, and the example previously mirrored real internal CS-team dashboard content (real tag taxonomy, real team names, a dashboard modeled directly on a real ServiceNow production landing page). Replaced with a short, clearly-dummy 2-dashboard example; the real deployment's actualDASHBOARDS_CONFIGvalue is set directly in its own environment, not committed here./cases/search's response never actually carried anupdatedOnfield.count-shape widget numbers were too small to read as the tile's primary content.Approach
CodeRabbit fixes (from #1325's post-merge review)
ltebound was exclusive of the named day (entity-service/internal/service/case_filters.go) — aYYYY-MM-DDvalue parsed to UTC midnight, so anltebound excluded the whole named day except the instant00:00:00Z, while both OpenAPI docs describelteas inclusive "on or before." Fixed by advancing a date-onlyltebound to one nanosecond before the next midnight.entity-service/internal/service/case_service.go) —Tags,ExcludeTags,ParentID,ProductNames,ProjectOnboardingStatuses,ProjectTypeIDs,IntegrationCsTeamIDs,Unassigned,ResolutionNotesEmptyare genuinely ServiceNow-only (they dot-walk into SN-specific concepts with no Postgres-schema equivalent), so a caller sending any of them against the Postgres-backed data source got a200with a broader-than-requested result set and no signal the predicate was dropped. Now rejected explicitly with a validation error naming the field.entity-service/internal/service/sn_case_service.go) —States,Severities,IssueTypes,EngagementTypesweren't validated before conversion to SN keys, so an unrecognized value silently widened the result set instead of erroring (the Postgres backend already validated these). Verified the SN key maps cover exactly the same value sets as the validation maps before applying the fix (a mismatch would have newly rejected previously-accepted values) — no mismatch found. Now aligned with the Postgres backend's validation.apps/csm-portal/webapp/.../widgetResourceConfig.ts) —states,severities,types,productNamesused a plain truthy check instead ofassignedUserIds'.length > 0guard, so an emptyvalues: []DSL entry set an explicit empty filter instead of leaving the field unset. Aligned all four.updatedOnbugdomain.SearchCaseViewhad noUpdatedOnfield at all — onlyCreatedOn. Added it, populated from the same source field the single-case detail path already uses (SN and Postgres both), and addedupdatedOnto the response schema in bothopenapi.yamlfiles. On the frontend, removed the{c.updatedAtIsCreatedFallback && "Created "}prefix on the case list's Updated column (kept the?? c.createdOnfallback as a genuine safety net for a case with truly no updatedOn, but the visible "Created 3 days ago" wording is gone — it's just "3 days ago" now, matching what the column header already correctly said).Dashboard polish
variant="h5"→h4→h3→ an explicitfontSize: "4rem"(dropped the MUI variant preset entirely partway through, since it wasn't reading as big as the underlying theme'sh3/h4sizing suggested — went through several rounds against live preview feedback), withnoWrapand an ellipsis-overflow fallback for a realistic 5-digit total in a narrow grid column.?dashboard=<id>&team=<id>query params (replace, notpush, so switching doesn't spam browser history), read on load and cleared appropriately when switching to a non-team-based dashboard.User stories
Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning
Summary by CodeRabbit
New Features
Bug Fixes