[CSM Portal] bundled session fixes: case comment ownership guard, auto-acknowledge, dashboard widget config and filter resolution, entity-service date/filter fixes - #1392
Conversation
The comment guard on POST /cases/{id}/comments only checked case state
(work_in_progress/ongoing), not who was posting. Any authenticated engineer
could add a public, customer-visible comment on a case assigned to someone
else. Add an ownership check alongside the existing state check, scoped to
public comments only; work notes are unaffected.
📝 WalkthroughWalkthroughThe pull request adds assignee checks for public case comments, expands dashboard presets and list widgets, moves placeholder resolution to the frontend, adds case-search filters, and supports alternate ServiceNow timestamp formats. ChangesCase controls
Dashboard backend
Dashboard frontend
Case-search service
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/backend/internal/handler/cases.go`:
- Around line 248-251: Update CreateCaseComment so assignment authorization is
enforced atomically within the upstream comment-creation operation, rather than
relying only on the GetCase preflight check around AssignedEngineer. Pass the
expected assignee or case version into the create request and reject the write
if it no longer matches, ensuring a user whose assignment changes cannot create
a public comment.
🪄 Autofix
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: a342d2d9-bbd3-4437-8439-faee0facc60d
📒 Files selected for processing (3)
apps/csm-portal/backend/internal/handler/cases.goapps/csm-portal/backend/internal/handler/cases_test.goapps/csm-portal/backend/internal/handler/response.go
The public-comment composer only gated on case state, not on whether the signed-in engineer is the case's assignee, so a non-assignee could type and send a public reply and only find out it was rejected after the fact (BFF now returns 403). Extend publicCommentGateReason to check assigneeIsMe first, locking the composer with a clear reason before the request is even sent, consistent with the existing state-gate UX.
An engineer starting work on an unacknowledged, acknowledgeable case has implicitly claimed it, but previously still had to separately click "Acknowledge". Fire the acknowledge PATCH as a best-effort side effect of startWork, right after the state PATCH (the two can't be combined server side). Covers both the direct "Start work" action and "Assign to me", which share startWork.
… sort
Generalizes the patch-list widget ask (show ETA columns at a glance) into a
reusable dashboard capability: a shape "list" widget's config can now set
"columns" (an ordered set of {path, label, format} entries, path supporting
dot-notation into nested search-response objects to arbitrary depth) and
"sortBy" (opaque, forwarded verbatim into that resourceType's own /search
request, same passthrough philosophy as "query"/filters).
Backend: both fields are opaque config on WidgetTemplate/dashboardWidgetView,
never validated or interpreted server-side, mirroring how Query/PieSlice.Query
already work.
Frontend: a widget with "columns" set renders through a new generic,
resourceType-agnostic renderer (GenericColumnList) that resolves each
column's path against every response item and formats it (plain text or
date), instead of the hardcoded WIDGET_LIST_RENDERERS[resourceType]. A widget
with no "columns" is unaffected -- the existing hardcoded renderer still
applies exactly as before. "sortBy" is forwarded into useWidgetData's search
request for shape "list" only.
Case search's bestCaseFixEta/mostLikelyFixEta/worstCaseFixEta and project.key
are not yet returned by the backing ServiceNow integration (confirmed via the
entity-service's own snCase field doc comments), so no dashboard config was
changed to reference them -- tracked as a separate follow-on once that data
is available. The abt-engineer dashboard's two patch-list widgets did pick up
a default sortBy (updatedOn asc) in the companion planning-repo config
change.
SN occasionally returns updatedOn/createdOn/resolvedOn as MM-DD-YYYY HH:MM:SS instead of the expected ISO format, which previously caused a hard parse failure and a 500 on the underlying request even though the SN write itself had already succeeded (e.g. resuming a paused case, or adding a work note). parseSNDateTime now tries the canonical layout first and falls back to the alternate one, logging a warning only when the fallback is what matched so the drift stays visible.
…s, drop server-side current_user resolution
Adds a filterPresets mechanism (dashboard-local and shared via
DASHBOARD_PRESETS_FILE, dashboard-local wins on collision) resolved once at
directory-load time via {"preset": "key"} references, so dashboard configs
can stop pasting the same filter fragment into dozens of widgets.
Splits the single "case" widget resourceType into five case-table values
(case, service_request, security_report_analysis, announcement, engagement)
matching the case-search "type" enum, and auto-injects the implied "type"
filter for each at load time when a widget doesn't already carry one
explicitly (existing explicit ones are left alone but logged as redundant).
Removes the server-side __current_user__ substitution mechanism
(CurrentUserPlaceholder/ResolveFilters/ResolveSliceFilters and the handler's
entity-service GET /users/me round trip): confirmed via a repo-wide grep that
dashboard widgets were the only consumer. GET /dashboards/{id} now returns
Query/Slices verbatim, including any unresolved __current_user__/
__current_team__ placeholder, mirroring how __current_team__ already worked
client-side only.
…field SN's own "Resolved Today" report filters on resolved_at, a distinct field from closed_at (which the existing closedOn filter already covers) -- a case is marked Resolved before it's later separately auto-closed. This platform had no way to filter by resolved_at at all. Mirrors closedOn exactly: caseFilterFieldSet, ParsedCaseFilters, snCaseFilters wire-shape, and the OR-group exclusion (a date-range field doesn't make sense ORed against other conditions). Requires a corresponding digiops-cs change (CaseSearchFilters resolvedStartDate/resolvedEndDate, pure pass-through) and a ServiceNow CaseUtils.searchCases addition (resolved_at range query) to actually take effect end to end; both already done separately.
…browser-local time Dashboard widget filters like createdOn/gte/__today__ were sent as-is to the entity-service, which resolved them against UTC. ServiceNow's own equivalent "Today" reports resolve against the support team's session timezone (Colombo, UTC+5:30), so Created/Resolved Today widgets undercounted by the cases falling in the ~5.5h gap between UTC and Colombo midnight. Add resolveRelativeDateFilters, covering the full placeholder set (__today__, __daysAgo:N__, __startOfMonth:N__, __endOfMonth:N__, __startOfQuarter:N__, __endOfQuarter:N__), resolved against the viewer's browser-local clock instead of a hardcoded timezone. Wired into the two choke points every widget's data fetch already funnels through (useWidgetData, useWidgetPieData), right where resolveTeamPlaceholder already ran. Literal (non-placeholder) filter values pass through unchanged. Not wired into DashboardWidgetTile's click-through links (the /cases deep-link, pie/bar slice navigation): resolving there would freeze "today" into the destination URL at link-build time, breaking bookmark semantics. Those still rely on the entity-service's UTC fallback for now.
…urrent_user__ client-side
WIDGET_RESOURCE_CONFIG and WIDGET_LIST_RENDERERS gain service_request,
security_report_analysis, announcement, and engagement, matching the
backend's split of the case-table resourceType (see backend commit
6dadf8019). All five route to POST /cases/search and reuse case's own
CasesList rendering/translateCaseDashboardFilters verbatim; only the icon
and click-through destination differ per type.
Also moves __current_user__ filter placeholder resolution to the frontend,
mirroring the existing client-side __current_team__ pattern
(teamFilterPlaceholder.ts), since GET /dashboards/{id} now returns widget
Query/Slices verbatim instead of pre-resolving the placeholder server-side.
The new currentUserFilterPlaceholder.ts walks a widget's filters generically
by value (both the case-search field/op/values DSL and every other
resourceType's flat filter shape), mirroring the backend's own removed
substituteCurrentUser rather than hardcoding a single field name, since a
widget can carry "the signed-in user" in more than one field across
resourceTypes. Threaded through the same points team-placeholder resolution
already runs: useWidgetData/useWidgetPieData's internal resolution and every
click-through href DashboardWidgetTile builds.
…ot id The backing data source now matches a case's parent project type by the project type's readable name (e.g. "Subscription") instead of by id, so sending converted ids silently matched nothing. Pass the caller's projectType values through verbatim, mirroring the existing product filter: drop the UUID validation and id conversion, rename the parsed field to ProjectTypeNames, and update the filter descriptions in both published API contracts. integrationCsTeam is unchanged and still id-based.
… service The backing data source now returns an additional, unfiltered list of a case's linked change requests alongside the existing (state-filtered) one. Point the case mapping at the unfiltered field so change requests in early workflow states are no longer silently dropped before they reach callers. The entity-service's own outward field name and shape (`linkedChangeRequests` on the case response) are unchanged; this is an internal swap of which upstream field is read.
The "related" tab (Linked Items) label lacked an entry count, unlike every other case-detail tab. ChildCasesWidget still can't contribute a count without an extra fetch, but LinkedChangeRequestsWidget and LinkedServiceRequestsWidget are already fed from c.linkedChangeRequests and c.linkedServiceRequests on the fetched case-detail object, so sum those two into the tab badge.
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/csm-portal/backend/internal/dashboard/widgets_test.go (1)
122-150: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftResolve
__current_user__in the backend and restore matching assertions.These tests now require the literal placeholder to reach widget query data. This conflicts with the dashboard contract. The backend must resolve
__current_user__before widget data reaches the frontend.
apps/csm-portal/backend/internal/dashboard/widgets_test.go#L122-L150: Assert the resolved assigned-user value in the widget query.apps/csm-portal/backend/internal/dashboard/widgets_test.go#L196-L206: Assert the resolved assigned-user value in the slice query.apps/csm-portal/backend/internal/dashboard/widgets_test.go#L302-L328: Assert the resolved assigned-user value after legacy-key migration.As per coding guidelines, “The backend resolves
__current_user__before widget data reaches the frontend.”🤖 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/backend/internal/dashboard/widgets_test.go` around lines 122 - 150, Restore backend placeholder resolution assertions in apps/csm-portal/backend/internal/dashboard/widgets_test.go at lines 122-150, 196-206, and 302-328: update the tests around the widget query, slice query, and legacy-key migration to expect the resolved assigned-user value rather than the literal "__current_user__" placeholder, preserving the existing query-shape checks.Source: Coding guidelines
apps/csm-portal/backend/internal/handler/dashboards_test.go (1)
400-402: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the stale reference to
substituteCurrentUser.The comment explains the assertion in terms of
substituteCurrentUser, but this PR removes that substitution from the handler. The handler now forwardsQueryverbatim. Restate the reason:recent-caseshas noassignedUserIdentry in its template, and nothing adds one.📝 Proposed comment update
- // recent-cases has no assignedUserId filter entry in its template and - // must not gain one during substitution: substituteCurrentUser only - // rewrites values already present, it never adds entries. + // recent-cases has no assignedUserId filter entry in its template. + // The handler forwards Query verbatim, so the response must not + // carry one either.🤖 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/backend/internal/handler/dashboards_test.go` around lines 400 - 402, Update the comment near the recent-cases assertion to remove the stale substituteCurrentUser reference and state that the template has no assignedUserId entry and nothing adds one because the handler forwards Query verbatim.apps/csm-portal/backend/openapi.yaml (1)
5471-5483: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDocument the auto-injected
typefilter in thequerydescription.For the five case-table resource types, the backend injects
{"field":"type","op":"in","values":["<resourceType>"]}intoquery.filtersat load time (seeinjectImpliedTypeFiltersininternal/dashboard/widgets.go). The spec describesqueryonly as "exactly as configured". A client reading this spec cannot tell that the returnedqueryalready scopes the search by type, and may add a duplicate or conflictingtypefilter of its own.Add one sentence stating that
queryalready carries the resource type's owntypefilter for those resource types.🤖 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/backend/openapi.yaml` around lines 5471 - 5483, Update the query description in the OpenAPI schema to state that, for the five case-table resource types, the returned query already includes the resource type’s injected type filter in query.filters. Clarify that clients should not add a duplicate or conflicting type filter, while preserving the existing placeholder and slice-merging guidance.
🧹 Nitpick comments (2)
apps/csm-portal/webapp/src/features/csm-cases/pages/CsmCaseDetailPage.tsx (1)
822-845: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftAdd automated coverage for the acknowledgement follow-up.
The new flow performs two ordered mutations. The acknowledgement failure path is intentionally ignored. Add a test that verifies the state PATCH occurs first and a rejected acknowledgement still continues to ongoing-case conflict resolution without an error banner.
🤖 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/webapp/src/features/csm-cases/pages/CsmCaseDetailPage.tsx` around lines 822 - 845, Add automated coverage for the start-work flow around the existing case-detail test setup, verifying the state PATCH mutation completes before the acknowledgement PATCH is attempted. Make the acknowledgement mutation reject, then assert ongoing-case conflict resolution still runs and no error banner is displayed, preserving the intentional best-effort error handling in the acknowledgement follow-up.apps/csm-portal/backend/internal/dashboard/registry_test.go (1)
467-499: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a hot-reload test for the shared presets file.
NewDirRegistry's doc comment states that the presets file is re-read on every load, so hot-reload mode picks up an edited presets file without a restart. No test covers that claim: every call site passes"".TestRegistry_HotReloadPicksUpChangesis the natural place to extend, by constructing the registry with a presets path and editing that file betweenDashboards()calls.🤖 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/backend/internal/dashboard/registry_test.go` around lines 467 - 499, Extend TestRegistry_HotReloadPicksUpChanges to create a shared presets file, pass its path to NewDirRegistry instead of an empty presets argument, then edit the presets file between Dashboards() calls and assert the reloaded dashboard reflects the updated preset. Preserve the existing definition add, edit, and removal checks.
🤖 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/backend/internal/dashboard/widgets.go`:
- Around line 443-466: Update injectTypeFilter to detect when query["filters"]
exists but is not a []any, emit a warning identifying the malformed filters
value, and avoid silently replacing the configured value without visibility.
Preserve the existing behavior for missing or valid filter arrays and continue
using the current type-filter injection flow.
In
`@apps/csm-portal/webapp/src/features/csm-dashboard/components/DashboardWidgetTile.tsx`:
- Around line 127-134: The resolvePlaceholders helper in DashboardWidgetTile
must also call resolveRelativeDateFilters before navigation filters are
generated, matching useWidgetData and useWidgetPieData behavior and preventing
__today__ from remaining unresolved. Preserve the existing team and current-user
placeholder resolution, and add a navigation test covering a relative-date
filter.
In
`@apps/csm-portal/webapp/src/features/csm-dashboard/config/widgetResourceConfig.ts`:
- Around line 401-457: Add the same case detailHref callback used by the
existing case resource to the service_request, security_report_analysis,
announcement, and engagement configurations in the widget resource config. Keep
each callback mapped to the corresponding case row so GenericColumnList can
navigate to the case detail page.
In
`@apps/csm-portal/webapp/src/features/csm-dashboard/utils/currentUserFilterPlaceholder.ts`:
- Around line 56-64: Update the current-user placeholder handling around
currentUserId so __current_user__ remains available for backend resolution
instead of removing the complete filter when the identity is unavailable.
Preserve all literal values in mixed filters, and only defer requests or
navigation as a temporary fallback until currentUserId exists; do not broaden
user-scoped queries.
In
`@apps/csm-portal/webapp/src/features/csm-dashboard/utils/resolveRelativeDateFilters.test.ts`:
- Around line 173-183: Update the test case “defaults to real wall-clock time
when no reference instant is passed” to freeze Vitest’s system clock before
calling resolveRelativeDateFilters, use that fixed date for the expected local
midnight assertion, and restore real timers after the test.
In `@entity-service/internal/service/case_service.go`:
- Line 448: Before the s.repo.SearchCases call in the case-search flow, validate
that resolvedOn.lte is not earlier than resolvedOn.gte, matching the equivalent
ServiceNow validation. Return a *apierror.ValidationError for this invalid range
and preserve the existing repository call for valid bounds.
In `@entity-service/internal/service/sn_project_service.go`:
- Around line 137-139: Update the fallback warning in the SN date parsing flow
to remove the raw value attribute from the slog.WarnContext call. Keep the
callSite and field identifiers, along with the short fallback summary, without
logging the upstream datetime value.
In `@entity-service/openapi.yaml`:
- Around line 4847-4848: Add resolvedOn to the CaseFieldFilter.field schema and
include it in the date-range filter documentation alongside createdOn,
updatedOn, and closedOn. Keep the OpenAPI definition synchronized with the
runtime-accepted filter fields so generated clients can discover and validate
resolvedOn.
---
Outside diff comments:
In `@apps/csm-portal/backend/internal/dashboard/widgets_test.go`:
- Around line 122-150: Restore backend placeholder resolution assertions in
apps/csm-portal/backend/internal/dashboard/widgets_test.go at lines 122-150,
196-206, and 302-328: update the tests around the widget query, slice query, and
legacy-key migration to expect the resolved assigned-user value rather than the
literal "__current_user__" placeholder, preserving the existing query-shape
checks.
In `@apps/csm-portal/backend/internal/handler/dashboards_test.go`:
- Around line 400-402: Update the comment near the recent-cases assertion to
remove the stale substituteCurrentUser reference and state that the template has
no assignedUserId entry and nothing adds one because the handler forwards Query
verbatim.
In `@apps/csm-portal/backend/openapi.yaml`:
- Around line 5471-5483: Update the query description in the OpenAPI schema to
state that, for the five case-table resource types, the returned query already
includes the resource type’s injected type filter in query.filters. Clarify that
clients should not add a duplicate or conflicting type filter, while preserving
the existing placeholder and slice-merging guidance.
---
Nitpick comments:
In `@apps/csm-portal/backend/internal/dashboard/registry_test.go`:
- Around line 467-499: Extend TestRegistry_HotReloadPicksUpChanges to create a
shared presets file, pass its path to NewDirRegistry instead of an empty presets
argument, then edit the presets file between Dashboards() calls and assert the
reloaded dashboard reflects the updated preset. Preserve the existing definition
add, edit, and removal checks.
In `@apps/csm-portal/webapp/src/features/csm-cases/pages/CsmCaseDetailPage.tsx`:
- Around line 822-845: Add automated coverage for the start-work flow around the
existing case-detail test setup, verifying the state PATCH mutation completes
before the acknowledgement PATCH is attempted. Make the acknowledgement mutation
reject, then assert ongoing-case conflict resolution still runs and no error
banner is displayed, preserving the intentional best-effort error handling in
the acknowledgement follow-up.
🪄 Autofix
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: d1560c9e-4272-476f-8147-bb5adda3577e
📒 Files selected for processing (41)
apps/csm-portal/backend/.env.exampleapps/csm-portal/backend/cmd/server/main.goapps/csm-portal/backend/internal/dashboard/filter_presets_test.goapps/csm-portal/backend/internal/dashboard/registry.goapps/csm-portal/backend/internal/dashboard/registry_test.goapps/csm-portal/backend/internal/dashboard/widgets.goapps/csm-portal/backend/internal/dashboard/widgets_test.goapps/csm-portal/backend/internal/handler/dashboards.goapps/csm-portal/backend/internal/handler/dashboards_test.goapps/csm-portal/backend/openapi.yamlapps/csm-portal/webapp/src/api/backend/types.tsapps/csm-portal/webapp/src/features/csm-cases/components/CaseActionBar.tsxapps/csm-portal/webapp/src/features/csm-cases/pages/CsmCaseDetailPage.tsxapps/csm-portal/webapp/src/features/csm-cases/utils/caseWorkState.test.tsapps/csm-portal/webapp/src/features/csm-cases/utils/caseWorkState.tsapps/csm-portal/webapp/src/features/csm-dashboard/api/useWidgetData.tsapps/csm-portal/webapp/src/features/csm-dashboard/api/useWidgetPieData.test.tsxapps/csm-portal/webapp/src/features/csm-dashboard/api/useWidgetPieData.tsapps/csm-portal/webapp/src/features/csm-dashboard/components/AgentsLandingPagePilot.tsxapps/csm-portal/webapp/src/features/csm-dashboard/components/DashboardWidgetTile.test.tsxapps/csm-portal/webapp/src/features/csm-dashboard/components/DashboardWidgetTile.tsxapps/csm-portal/webapp/src/features/csm-dashboard/components/GenericColumnList.tsxapps/csm-portal/webapp/src/features/csm-dashboard/config/widgetListConfig.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/utils/currentUserFilterPlaceholder.test.tsapps/csm-portal/webapp/src/features/csm-dashboard/utils/currentUserFilterPlaceholder.tsapps/csm-portal/webapp/src/features/csm-dashboard/utils/resolveRelativeDateFilters.test.tsapps/csm-portal/webapp/src/features/csm-dashboard/utils/resolveRelativeDateFilters.tsapps/csm-portal/webapp/src/features/csm-dashboard/utils/resolveWidgetColumn.test.tsapps/csm-portal/webapp/src/features/csm-dashboard/utils/resolveWidgetColumn.tsentity-service/internal/domain/entity.goentity-service/internal/service/case_filters.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/internal/service/sn_comment_service.goentity-service/internal/service/sn_date_parse_test.goentity-service/internal/service/sn_project_service.goentity-service/openapi.yaml
… search The relational schema has no resolved-on column and caseRepo.SearchCases models no predicate for it, so a resolvedOn bound was parsed, carried into the request and then dropped: the caller got a 200 listing every case instead of the resolved-in-range ones asked for. Reject it the same way every other predicate this data source cannot express is rejected, rather than only validating its bounds -- validating the bounds of a predicate that is subsequently ignored still answers with the wrong result set.
…parse warn The alternate-format warn logged the upstream datetime value verbatim. callSite and field already identify where the drift is; the value itself adds nothing to the diagnosis and logs are limited to ids and sanitized summaries.
…chema The case search accepts resolvedOn but the CaseFieldFilter field enum and the date-range notes did not list it, so generated clients could neither discover nor validate it. Also records the bounds-ordering rule and that the relational data source rejects this field.
…array A definition setting "filters" to an object failed the type assertion, and the configured value was then overwritten by the auto-injected type filter. The widget searched with criteria the definition never stated, silently. Say so at load time, where a malformed definition can still be fixed.
…ock default resolveRelativeDateFilters reads new Date() during resolution and the test read it again to build the expectation, so a run crossing local midnight compared two different calendar days. Freeze the clock on the same fixed instant every other assertion in the file uses.
…e widgets service_request, security_report_analysis, announcement and engagement all return case rows from /cases/search, but carried no detailHref -- and row navigation in a columns-configured list goes through detailHref and nothing else, so their rows could not be opened. Lifted the callback out of the case entry into a shared caseDetailHref so all five case-row resourceTypes stay on one definition.
…e identity loads CurrentUserProvider does not gate its children on GET /users/me, so a widget first renders while the profile is still in flight. The resolver dropped any filter entry carrying __current_user__ in that window, so a widget whose only filter was assignedUserId in [__current_user__] issued a completely unfiltered search and painted every engineer records into a tile labelled as the viewer own. A mixed [__current_user__, <uuid>] filter lost its literal value the same way. The resolver now fails closed: with no id available it returns the filters untouched, literals intact. hasCurrentUserPlaceholder lets callers detect that state, and useWidgetData/useWidgetPieData hold their requests until the profile lands, so nothing is sent rather than something too broad. The tile and the pie/bar hook report the wait as loading so a deferred query does not paint a 0 or an empty chart. Kept the resolution client-side rather than restoring the server-side substitution this series removed: widgetPreviewUrl already uses the same "return unchanged, caller holds off" convention for its @me sentinel, so this makes the two consistent instead of splitting resolution across two layers again.
…k-through hrefs useWidgetData and useWidgetPieData resolve __today__/__daysAgo:N__ and the rest against browser-local time before searching, but the tile built its navigation filters through team and current-user resolution only. The placeholder therefore reached the destination list page verbatim, was forwarded to the backing service and resolved there against UTC -- so the number on the tile and the list behind it disagreed by the offset between UTC midnight and the viewer own, which is exactly the discrepancy the browser-local resolution exists to close. The tile now runs the same three resolvers, in the same order, for both the count click-through and the View more preview href. This does freeze the resolved instant into the href. That is the right trade here: every other value in these hrefs (team ids, states, the viewer own user id) is already a snapshot of the moment of the click, and the destination URL scheme is the cases list own concrete filter encoding -- carrying dashboard placeholder vocabulary into it would make __today__ a value users can type into, and hand-edit out of, a shared list-page URL.
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/webapp/src/features/csm-dashboard/utils/currentUserFilterPlaceholder.test.ts`:
- Around line 72-74: In the type assertion for the resolved value in
currentUserFilterPlaceholder tests, remove the duplicate filters property
declaration and retain a single filters field typed as an array of objects with
string values.
🪄 Autofix
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: dd1bd45a-1aa4-449f-b737-e4b7b990e237
📒 Files selected for processing (14)
apps/csm-portal/backend/internal/dashboard/widgets.goapps/csm-portal/webapp/src/features/csm-dashboard/api/useWidgetData.tsapps/csm-portal/webapp/src/features/csm-dashboard/api/useWidgetPieData.test.tsxapps/csm-portal/webapp/src/features/csm-dashboard/api/useWidgetPieData.tsapps/csm-portal/webapp/src/features/csm-dashboard/components/DashboardWidgetTile.test.tsxapps/csm-portal/webapp/src/features/csm-dashboard/components/DashboardWidgetTile.tsxapps/csm-portal/webapp/src/features/csm-dashboard/config/widgetResourceConfig.tsapps/csm-portal/webapp/src/features/csm-dashboard/utils/currentUserFilterPlaceholder.test.tsapps/csm-portal/webapp/src/features/csm-dashboard/utils/currentUserFilterPlaceholder.tsapps/csm-portal/webapp/src/features/csm-dashboard/utils/resolveRelativeDateFilters.test.tsentity-service/internal/service/case_service.goentity-service/internal/service/case_service_test.goentity-service/internal/service/sn_project_service.goentity-service/openapi.yaml
🚧 Files skipped from review as they are similar to previous changes (8)
- entity-service/openapi.yaml
- entity-service/internal/service/case_service_test.go
- apps/csm-portal/webapp/src/features/csm-dashboard/api/useWidgetData.ts
- entity-service/internal/service/sn_project_service.go
- apps/csm-portal/webapp/src/features/csm-dashboard/api/useWidgetPieData.ts
- apps/csm-portal/webapp/src/features/csm-dashboard/utils/resolveRelativeDateFilters.test.ts
- apps/csm-portal/webapp/src/features/csm-dashboard/components/DashboardWidgetTile.test.tsx
- apps/csm-portal/backend/internal/dashboard/widgets.go
Purpose
A bundle of unrelated fixes and small capabilities found while working cases and dashboards in the same session. They are grouped into one PR only because they were found together; there is no single unifying theme. Grouped by area:
Case workflow
POST /cases/{id}/commentsonly checked case state, not who was posting — any engineer could add a public, customer-visible comment on a case owned by someone else.Entity-service / backing data source
5. The backing data source occasionally returns
updatedOn/createdOn/resolvedOnin an alternateMM-DD-YYYY HH:MM:SSformat instead of the canonical one. A hard parse failure turned that into a500on requests whose upstream write had already succeeded (resuming a paused case, adding a work note).6. There was no way to filter case search by resolved date at all. Resolved and closed are distinct: a case is marked Resolved and only later separately auto-closed, so the existing
closedOnfilter cannot answer "resolved today".7. The
projectTypecase-search filter converted its values to ids, but the backing data source matches a case's parent project type by the project type's readable name, so the filter silently matched nothing.8. A case's linked change requests were read from an upstream field that the backing service filters by change-request state, so change requests in early workflow states were silently dropped before reaching callers.
Dashboards
9. Dashboard
listwidgets could only render one hardcoded layout per resourceType, with no way to surface additional fields or control ordering.10. Dashboard configs repeated the same filter fragment across dozens of widgets, with no way to share it.
11. The single
casewidget resourceType could not express the distinct case types the case-searchtypeenum already carries.12. Relative-date widget filters (
__today__and friends) were resolved server-side against UTC, so "Created/Resolved Today" widgets undercounted for a support team whose working day is UTC+5:30 — the equivalent reports in the previous system resolve against the team's own timezone.13.
__current_user__was substituted server-side by the dashboards handler, requiring a per-request identity round trip, while__current_team__was already resolved client-side — two mechanisms for the same idea.No linked issues; everything here was found and fixed in the same session.
Known issues (deliberately shipped as-is, follow-up already scoped)
Two changes in this PR are known to be incomplete. They are called out here so the description does not oversell them; both are fixed in a follow-up PR that will be stacked on this one after it merges.
projectType-by-name change now fails validation instead of returning an empty result. The names are sent on a filter key whose downstream contract still constrains values to 32-hex ids, so requests carryingprojectTypeare now rejected upstream rather than silently matching nothing. Strictly speaking this trades one broken behaviour for a louder one; the contract change that makes it work is part of the follow-up.Goals
403unless the requester is the case's assigned engineer.PATCHas a best-effort side effect of starting work.resolvedOncase-search filter alongside the existingclosedOn.projectTypefilter values through as names.listwidget declare its own columns and sort order in config.caseresourceType into the five case-table types, with the impliedtypefilter injected automatically.__current_user__client-side, matching__current_team__, and drop the server-side mechanism.Approach
Case workflow
CreateCaseComment(apps/csm-portal/backend/internal/handler/cases.go) already fetches the case for the state guard; it now also decodesassignedEngineer.idfrom that same payload (no extra upstream call) and rejects with a newErrMsgCommentNotOwnCaseconstant when it does not match the requester. Scoped to public comments only — work notes are unchanged. See Known issue 1 on the id-space mismatch.publicCommentGateReason(webapp/src/features/csm-cases/utils/caseWorkState.ts) takes a newassigneeIsMeparameter and checks it first, unconditionally — a non-assignee always sees the ownership reason, never a resumable-sounding state reason, consistent withcanResumeToUnlockPublicReply's existing assignee-only quick fix.startWork()(CsmCaseDetailPage.tsx) checkscanAcknowledge(now exported fromCaseActionBar.tsxinstead of duplicated) against the pre-PATCHcase data, then fires a separate{ acknowledge: true }PATCHafter the statePATCHsucceeds — separate because the entity-service rejects combiningstateandacknowledgein one request. Best effort: its own try/catch, does not block the rest of start-work, no extra toast; a failure just leaves the Acknowledge button visible as a safe fallback. Covers both "Start work" and "Assign to me", which sharestartWork.linkedChangeRequests.length + linkedServiceRequests.lengthfrom the already-fetched case-detail object. Child cases are still excluded: that widget runs its own scoped query and would need an extra fetch to contribute a count.Entity-service
parseSNDateTimehelper tries the canonical layout first (the fast path) and only falls back to the alternateMM-DD-YYYYlayout on failure, logging aWARNnaming the call site and field only when the fallback is what matched, so the upstream format drift stays visible rather than silently masked. A genuinely malformed value still fails with the original error. Wired into every case and comment datetime parse. Root cause is upstream (a display-formatted read instead of a canonical one) and lives in a layer shared with the live customer portal, so it is worked around here rather than changed there.resolvedOnmirrorsclosedOnexactly: filter field set, parsed filter struct (ResolvedStartDate/ResolvedEndDate), the outbound filter wire shape, the start-after-end validation, and the OR-group exclusion (a date-range field does not make sense ORed against other conditions). Taking effect end to end also needs a corresponding entity-service-side integration change and a backing-data-source query addition, both tracked separately and already done.ProjectTypeIDsis renamedProjectTypeNames, the UUID validation and id conversion are dropped, and values pass through verbatim — mirroring how the existingproductfilter already behaves. The outbound wire key is unchanged.integrationCsTeamis untouched and still id-based. Both published API contracts (apps/csm-portal/backend/openapi.yaml,entity-service/openapi.yaml) have theirprojectTypefilter description updated. See Known issue 2.linkedChangeRequests) are unchanged; this is purely an internal swap of which upstream field is read. The old field is retained on the response struct, marked deprecated, and read by nothing.Dashboards (backend)
listwidget's config can now carrycolumns(an ordered set of{path, label, format}entries,pathsupporting dot notation into nested search-response objects to arbitrary depth) andsortBy. Both are opaque config onWidgetTemplate/dashboardWidgetView— never validated or interpreted server-side, mirroring howQuery/PieSlice.Queryalready work.filterPresetscan be declared per dashboard, or shared across the whole directory via a newDASHBOARD_PRESETS_FILE(documented in.env.example; dashboard-local wins on collision).{"preset": "key"}references are expanded once, at directory-load time, anywhere a literal filter object can appear.DASHBOARDS_DIRnow skips_-prefixed.jsonfiles so the shared presets file can live beside the dashboards.caseresourceType splits into five case-table values (case,service_request,security_report_analysis,announcement,engagement) matching the case-searchtypeenum. The impliedtypefilter is auto-injected at load time when a widget does not already carry one; an existing explicit one is left alone but logged as redundant.__current_user__substitution (CurrentUserPlaceholder,ResolveFilters,ResolveSliceFilters, and the handler's identity round trip) is removed after a repo-wide grep confirmed dashboard widgets were its only consumer.GET /dashboards/{id}now returnsQuery/Slicesverbatim, including unresolved placeholders, exactly as__current_team__already worked.Dashboards (frontend)
columnsset renders through a new generic, resourceType-agnosticGenericColumnListthat resolves each column path against every response item and formats it (plain text or date), instead of the hardcoded per-resourceType renderer. A widget withoutcolumnsis completely unaffected.sortByis forwarded into the search request for shapelistonly.WIDGET_RESOURCE_CONFIGandWIDGET_LIST_RENDERERSgain the four new resourceTypes. All five route toPOST /cases/searchand reuse the existing case list rendering and filter translation verbatim; only the icon and click-through destination differ per type.resolveRelativeDateFilterscovers the full placeholder set (__today__,__daysAgo:N__,__startOfMonth:N__,__endOfMonth:N__,__startOfQuarter:N__,__endOfQuarter:N__) against the viewer's browser-local clock. Wired into the two choke points every widget fetch already funnels through (useWidgetData,useWidgetPieData), right where team-placeholder resolution already ran. Literal values pass through unchanged. Deliberately not wired into click-through links: resolving there would freeze "today" into the destination URL at link-build time and break bookmark semantics, so those still rely on the server-side UTC fallback.currentUserFilterPlaceholder.tswalks a widget's filters generically by value (both the case-search field/op/values DSL and every other resourceType's flat filter shape), mirroring the removed server-side substitution rather than hardcoding one field name, since a widget can carry "the signed-in user" in more than one field across resourceTypes. Threaded through the same points team-placeholder resolution already runs, including every click-through href.User stories
Release note
resolvedOncase-search filter, distinct fromclosedOn.projectTypefilter now matches by project type name instead of id.Documentation
N/A — internal API/UI behaviour; no external-facing docs describe these rules. New dashboard config options are documented inline in
.env.exampleand in the published API contract, which is where dashboard authors already look.Training
N/A — no training content covers these endpoints or flows.
Certification
N/A — not exam-relevant.
Marketing
N/A — internal fixes and internal dashboard configuration capability, not user-facing feature announcements.
Automation tests
Security checks
go vet ./...andeslintran clean instead.env.examplegains a commented-out file path only, no valuesSamples
N/A — no sample code affected.
Related PRs
A follow-up PR fixing the two Known issues above (the comment guard's id-space mismatch and the
projectTypefilter contract) will be stacked on this branch after it merges; it does not exist yet.Taking
resolvedOnand theprojectTypename matching into effect end to end also depends on corresponding entity-service integration and backing-data-source changes, tracked separately outside this repo.Migrations (if applicable)
N/A — no schema or data migration.
DASHBOARD_PRESETS_FILEis a new optional environment variable; unset means no shared presets, and every existing dashboard config keeps working unchanged.Test environment
Go (repo toolchain version) + Node/pnpm (repo-pinned versions), macOS. Backend and entity-service verified with
go build ./...,go test ./...,go vet ./.... Frontend verified withnpx tsc -b,npx eslint, andnpx vitest runon the affected test files. The comment guard and theprojectTypefilter were additionally exercised against a running environment, which is how the two Known issues were found.Learning
Two lessons, both from the Known issues: an ownership check is only as good as the id space it compares in, and a filter value change is not done at the layer that parses it — the downstream contract that validates it has to move first, or the change turns a wrong-but-quiet result into a hard rejection. Both should have been verified end to end against a real request before the commit, not after.
Summary by CodeRabbit
New Features
resolvedOnfiltering and name-based project-type filtering.Bug Fixes