[CSM] Rework upstream error exposure, default filter panels open, dashboard widget polish - #1322
Conversation
…oard widget icons and refresh skeleton - backend: split mapUpstreamError into a PATCH-only variant that surfaces the upstream 400/409/422 reason, and mapUpstreamErrorGeneric (now the default for every other endpoint) that never echoes upstream detail to the caller; full reason is still logged server-side - webapp: rework the Incidents filter bar layout (Priority/Product/SLA violated on one row, Created from/to on their own row) and cap both date pickers at today - webapp: default the Incidents/Problems/Change requests/Time cards filter panels open (Cases already was) - webapp: dashboard widget tiles get a resource-type icon, a themed hover ring, and an info affordance (tooltip copy intentionally empty pending finalized messaging) - webapp: the widget-pilot refresh button now also invalidates every tile's own data query and shows skeletons until it resolves, instead of only refreshing the dashboard's own metadata Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 33 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 (5)
📝 WalkthroughWalkthroughThe PR adds ChangesBackend upstream error handling
Dashboard updates
Operations filter updates
Estimated code review effort: 4 (Complex) | ~45 minutes 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
apps/csm-portal/backend/internal/handler/cases_test.go (1)
77-100: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStandardize the upstream-error test helper names.
The generic test behavior is correct, but the PR uses
upstreamErrorsGenericwhile the repository convention requiresupstreamErrors(fallback). Rename the PATCH-specific matrix and useupstreamErrorsfor generic tests.
apps/csm-portal/backend/internal/handler/cases_test.go#L77-L100: rename the helper definitions and update all case-handler call sites.apps/csm-portal/backend/internal/handler/task_slas_test.go#L80-L80: useupstreamErrors(fallback)for search tests.apps/csm-portal/backend/internal/handler/tasks_test.go#L118-L118: useupstreamErrors(fallback)for case-task search tests.apps/csm-portal/backend/internal/handler/time_cards_test.go#L67-L67: useupstreamErrors(fallback)for time-card creation tests.apps/csm-portal/backend/internal/handler/updates_test.go#L72-L72: useupstreamErrors(fallback)for product-level retrieval tests.apps/csm-portal/backend/internal/handler/users_test.go#L352-L352: useupstreamErrors(fallback)for user-search tests.As per coding guidelines, handler tests must use
upstreamErrors(fallback).🤖 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/cases_test.go` around lines 77 - 100, Standardize the upstream-error test helpers by renaming the generic helper to upstreamErrors(fallback) and the PATCH-specific matrix to the repository’s expected distinct name, then update all case-handler call sites in apps/csm-portal/backend/internal/handler/cases_test.go. Replace the helper usage with upstreamErrors(fallback) in apps/csm-portal/backend/internal/handler/task_slas_test.go:80, tasks_test.go:118, time_cards_test.go:67, updates_test.go:72, and users_test.go:352.Source: Coding guidelines
apps/csm-portal/backend/internal/handler/accounts.go (1)
98-98: 🔒 Security & Privacy | 🔵 TrivialRun the required backend security scan.
Run
gosec -fmt=text ./...and resolve every finding before merge. Use#noseconly for an exact, justified suppression.Based on learnings, every backend change must run
gosec -fmt=text ./..., and exact#nosecsuppressions require justification.🤖 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/accounts.go` at line 98, Run the required backend security scan with gosec -fmt=text ./... and resolve every reported finding across the backend change, including the flow around mapUpstreamErrorGeneric. Use an exact `#nosec` suppression only where the finding is justified, and document the suppression rationale inline.Source: Learnings
apps/csm-portal/backend/internal/handler/change_requests.go (1)
232-232: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for
CreateCommentfailures.
CreateChangeRequestCommentnow maps errors from bothGetChangeRequestandCreateComment.TestCreateChangeRequestCommentonly makesgetChangeRequestFnfail. Add a case where the guard succeeds andcreateCommentFnreturns each generic error. Assert the fallback message and status.🤖 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/change_requests.go` at line 232, Extend TestCreateChangeRequestComment with cases where getChangeRequestFn succeeds and createCommentFn returns each generic error, then assert CreateChangeRequestComment produces the fallback error message and expected status. Keep the existing GetChangeRequest failure coverage unchanged.apps/csm-portal/backend/internal/handler/configuration_items_test.go (1)
80-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReconcile the generic error fixture name across handler tests.
The changed tests use
upstreamErrorsGeneric(...), while the applicable path instruction requiresupstreamErrors(fallback). Preserve the generic body-hiding assertions, but rename the fixture or update the instruction so the repository has one clear test contract.
apps/csm-portal/backend/internal/handler/configuration_items_test.go#L80-L80: Reconcile theupstreamErrorsGeneric(...)call with the mandated helper name.apps/csm-portal/backend/internal/handler/conversations_test.go#L130-L130: Reconcile the conversation-message error fixture.apps/csm-portal/backend/internal/handler/conversations_test.go#L246-L246: Reconcile the conversation-search error fixture.apps/csm-portal/backend/internal/handler/deployments_test.go#L86-L86: Reconcile the deployment-creation error fixture.apps/csm-portal/backend/internal/handler/deployments_test.go#L163-L163: Reconcile the deployment-search error fixture.apps/csm-portal/backend/internal/handler/deployments_test.go#L371-L371: Reconcile the deployed-product search error fixture.apps/csm-portal/backend/internal/handler/deployments_test.go#L473-L473: Reconcile the deployed-product creation error fixture.apps/csm-portal/backend/internal/handler/groups_test.go#L80-L80: Reconcile the group-search error fixture.As per coding guidelines, handler tests must use
upstreamErrors(fallback),withUser(), anddecodeJSON[T]().🤖 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/configuration_items_test.go` at line 80, Standardize the generic upstream-error fixture across handler tests by replacing each upstreamErrorsGeneric(...) usage with the mandated upstreamErrors(fallback) contract in apps/csm-portal/backend/internal/handler/configuration_items_test.go:80, conversations_test.go:130 and 246, deployments_test.go:86, 163, 371, and 473, and groups_test.go:80. Preserve the existing generic body-hiding assertions and ensure these tests continue using withUser() and decodeJSON[T]().Source: Coding guidelines
apps/csm-portal/backend/internal/handler/projects.go (1)
179-179: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for
GetProjectContact.The changed handler uses
mapUpstreamErrorGeneric, but the suppliedapps/csm-portal/backend/internal/handler/projects_test.gocontext has noTestGetProjectContact. Add generic upstream-error cases with fallback"Failed to fetch the project contact."and assert that the upstream body is not returned.🤖 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/projects.go` at line 179, Add a TestGetProjectContact test covering generic upstream-error responses from the handler, including the fallback message "Failed to fetch the project contact.". Assert each response uses the mapped generic error and does not expose the upstream response body.apps/csm-portal/backend/internal/handler/problems_test.go (1)
86-86: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument
upstreamErrorsGenericin the handler-test convention.The calls here are valid for non-PATCH handlers using
mapUpstreamErrorGeneric, but the guideline still points only atupstreamErrors(fallback). Update that line to requireupstreamErrorsGeneric(fallback)for the default generic upstream error table, or alias the generic table under that name as the documented convention.🤖 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/problems_test.go` at line 86, Update the handler-test convention to document upstreamErrorsGeneric(fallback) as the default generic upstream error table, or alias the existing generic table under that name. Apply this consistently at apps/csm-portal/backend/internal/handler/problems_test.go:86, 166, and 244, and apps/csm-portal/backend/internal/handler/projects_test.go:75, 153, and 264, preserving the existing non-PATCH handler behavior.Source: Coding guidelines
apps/csm-portal/backend/internal/handler/problems.go (1)
115-115: 🔒 Security & Privacy | 🔵 TrivialRun the backend gosec scan before merge.
gosecis required for each backend change and must report zero issues. Installgosecviago install github.com/securego/gosec/v2/cmd/gosec@latestand rungosec -fmt=text ./...fromapps/csm-portal/backend. Resolve any finding rather than suppressing it unless a#nosecannotation with justification already covers that exact case.🤖 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/problems.go` at line 115, Run the required gosec scan from apps/csm-portal/backend using gosec -fmt=text ./..., after installing it with the specified go install command. Resolve every reported security finding in the affected backend code, including the flow around mapUpstreamErrorGeneric, and ensure the scan completes with zero issues without adding unjustified suppression annotations.Source: Learnings
🤖 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/response.go`:
- Around line 118-125: The response.go documentation must not endorse logging
full upstream error bodies through apierror.Error.Error(); revise it to require
sanitized status and operation summaries while preserving the fixed
client-facing fallback behavior. In
apps/csm-portal/backend/internal/handler/response.go lines 118-125, update the
mapUpstreamErrorGeneric documentation accordingly; in
apps/csm-portal/backend/CLAUDE.md line 69, remove the requirement to log the
full reason and state that backend logs contain only identifiers and sanitized
error summaries.
In
`@apps/csm-portal/webapp/src/features/csm-dashboard/components/DashboardWidgetTile.tsx`:
- Around line 101-118: Remove the empty Tooltip and its Info icon from
DashboardWidgetTile; do not leave an inactive affordance inside the navigable
card. Keep the tile layout and link behavior unchanged until meaningful tooltip
content and separate interaction handling are available.
In
`@apps/csm-portal/webapp/src/features/csm-operations/components/IncidentsFilterBar.tsx`:
- Around line 111-115: Update the today value in the filter-bar component so it
is recalculated after UTC midnight instead of being permanently cached by the
empty-dependency useMemo. Prefer deriving todayUTCDateOnly() during render,
while preserving the existing createdEndDate and fromMaxDate behavior.
- Around line 252-253: Update the two date-picker onChange handlers in
IncidentsFilterBar to validate typed dates before invoking onChange: ensure the
start date respects the existing opposite bound and the end date respects the
start bound and today, while preserving valid selections. Add focused tests
covering typed out-of-range values for both handlers.
---
Nitpick comments:
In `@apps/csm-portal/backend/internal/handler/accounts.go`:
- Line 98: Run the required backend security scan with gosec -fmt=text ./... and
resolve every reported finding across the backend change, including the flow
around mapUpstreamErrorGeneric. Use an exact `#nosec` suppression only where the
finding is justified, and document the suppression rationale inline.
In `@apps/csm-portal/backend/internal/handler/cases_test.go`:
- Around line 77-100: Standardize the upstream-error test helpers by renaming
the generic helper to upstreamErrors(fallback) and the PATCH-specific matrix to
the repository’s expected distinct name, then update all case-handler call sites
in apps/csm-portal/backend/internal/handler/cases_test.go. Replace the helper
usage with upstreamErrors(fallback) in
apps/csm-portal/backend/internal/handler/task_slas_test.go:80,
tasks_test.go:118, time_cards_test.go:67, updates_test.go:72, and
users_test.go:352.
In `@apps/csm-portal/backend/internal/handler/change_requests.go`:
- Line 232: Extend TestCreateChangeRequestComment with cases where
getChangeRequestFn succeeds and createCommentFn returns each generic error, then
assert CreateChangeRequestComment produces the fallback error message and
expected status. Keep the existing GetChangeRequest failure coverage unchanged.
In `@apps/csm-portal/backend/internal/handler/configuration_items_test.go`:
- Line 80: Standardize the generic upstream-error fixture across handler tests
by replacing each upstreamErrorsGeneric(...) usage with the mandated
upstreamErrors(fallback) contract in
apps/csm-portal/backend/internal/handler/configuration_items_test.go:80,
conversations_test.go:130 and 246, deployments_test.go:86, 163, 371, and 473,
and groups_test.go:80. Preserve the existing generic body-hiding assertions and
ensure these tests continue using withUser() and decodeJSON[T]().
In `@apps/csm-portal/backend/internal/handler/problems_test.go`:
- Line 86: Update the handler-test convention to document
upstreamErrorsGeneric(fallback) as the default generic upstream error table, or
alias the existing generic table under that name. Apply this consistently at
apps/csm-portal/backend/internal/handler/problems_test.go:86, 166, and 244, and
apps/csm-portal/backend/internal/handler/projects_test.go:75, 153, and 264,
preserving the existing non-PATCH handler behavior.
In `@apps/csm-portal/backend/internal/handler/problems.go`:
- Line 115: Run the required gosec scan from apps/csm-portal/backend using gosec
-fmt=text ./..., after installing it with the specified go install command.
Resolve every reported security finding in the affected backend code, including
the flow around mapUpstreamErrorGeneric, and ensure the scan completes with zero
issues without adding unjustified suppression annotations.
In `@apps/csm-portal/backend/internal/handler/projects.go`:
- Line 179: Add a TestGetProjectContact test covering generic upstream-error
responses from the handler, including the fallback message "Failed to fetch the
project contact.". Assert each response uses the mapped generic error and does
not expose the upstream response body.
🪄 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: 81dc234b-26c4-4fc1-b605-6cdb13f59242
📒 Files selected for processing (52)
apps/csm-portal/backend/CLAUDE.mdapps/csm-portal/backend/internal/handler/accounts.goapps/csm-portal/backend/internal/handler/accounts_test.goapps/csm-portal/backend/internal/handler/cases.goapps/csm-portal/backend/internal/handler/cases_test.goapps/csm-portal/backend/internal/handler/catalogs.goapps/csm-portal/backend/internal/handler/change_requests.goapps/csm-portal/backend/internal/handler/change_requests_test.goapps/csm-portal/backend/internal/handler/configuration_items.goapps/csm-portal/backend/internal/handler/configuration_items_test.goapps/csm-portal/backend/internal/handler/conversations.goapps/csm-portal/backend/internal/handler/conversations_test.goapps/csm-portal/backend/internal/handler/deployments.goapps/csm-portal/backend/internal/handler/deployments_test.goapps/csm-portal/backend/internal/handler/groups.goapps/csm-portal/backend/internal/handler/groups_test.goapps/csm-portal/backend/internal/handler/incidents.goapps/csm-portal/backend/internal/handler/incidents_test.goapps/csm-portal/backend/internal/handler/notifications.goapps/csm-portal/backend/internal/handler/problems.goapps/csm-portal/backend/internal/handler/problems_test.goapps/csm-portal/backend/internal/handler/product_vulnerabilities.goapps/csm-portal/backend/internal/handler/products.goapps/csm-portal/backend/internal/handler/products_test.goapps/csm-portal/backend/internal/handler/projects.goapps/csm-portal/backend/internal/handler/projects_test.goapps/csm-portal/backend/internal/handler/reference.goapps/csm-portal/backend/internal/handler/response.goapps/csm-portal/backend/internal/handler/service_offerings.goapps/csm-portal/backend/internal/handler/service_offerings_test.goapps/csm-portal/backend/internal/handler/services.goapps/csm-portal/backend/internal/handler/services_test.goapps/csm-portal/backend/internal/handler/task_slas.goapps/csm-portal/backend/internal/handler/task_slas_test.goapps/csm-portal/backend/internal/handler/tasks.goapps/csm-portal/backend/internal/handler/tasks_test.goapps/csm-portal/backend/internal/handler/time_cards.goapps/csm-portal/backend/internal/handler/time_cards_test.goapps/csm-portal/backend/internal/handler/updates.goapps/csm-portal/backend/internal/handler/updates_test.goapps/csm-portal/backend/internal/handler/users.goapps/csm-portal/backend/internal/handler/users_test.goapps/csm-portal/webapp/src/features/csm-dashboard/components/AgentsLandingPagePilot.test.tsxapps/csm-portal/webapp/src/features/csm-dashboard/components/AgentsLandingPagePilot.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-operations/components/ChangeRequestsTab.tsxapps/csm-portal/webapp/src/features/csm-operations/components/IncidentsFilterBar.tsxapps/csm-portal/webapp/src/features/csm-operations/components/IncidentsTab.tsxapps/csm-portal/webapp/src/features/csm-operations/components/ProblemsTab.tsxapps/csm-portal/webapp/src/features/csm-operations/pages/OperationsTabFiltersUrl.test.tsxapps/csm-portal/webapp/src/features/csm-timecards/pages/CsmTimeCardsPage.tsx
| // mapUpstreamErrorGeneric is mapUpstreamError's counterpart for every | ||
| // non-PATCH endpoint: 401/403/404 still translate to the fixed messages, but | ||
| // every other case — 400, 409, 422, 5xx, and unmapped statuses alike — falls | ||
| // back to fallbackMsg instead of echoing the upstream body. The real reason | ||
| // is still available: the caller always logs it via | ||
| // slog.ErrorContext(ctx, ..., "err", err) before calling this, and | ||
| // apierror.Error.Error() includes both the status code and the body, so nothing | ||
| // is lost — it just isn't returned to the browser. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Do not log raw upstream error bodies.
apierror.Error.Error() includes Body, and the documented callers pass err to slog.ErrorContext. This conflicts with the backend logging rule and can retain upstream PII or internal details.
apps/csm-portal/backend/internal/handler/response.go#L118-L125: document sanitized status and operation summaries instead of retaining the full upstream body.apps/csm-portal/backend/CLAUDE.md#L69-L69: remove the requirement to log the full reason.
As per coding guidelines, backend logs must contain only identifiers and error summaries.
📍 Affects 2 files
apps/csm-portal/backend/internal/handler/response.go#L118-L125(this comment)apps/csm-portal/backend/CLAUDE.md#L69-L69
🤖 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/response.go` around lines 118 - 125,
The response.go documentation must not endorse logging full upstream error
bodies through apierror.Error.Error(); revise it to require sanitized status and
operation summaries while preserving the fixed client-facing fallback behavior.
In apps/csm-portal/backend/internal/handler/response.go lines 118-125, update
the mapUpstreamErrorGeneric documentation accordingly; in
apps/csm-portal/backend/CLAUDE.md line 69, remove the requirement to log the
full reason and state that backend logs contain only identifiers and sanitized
error summaries.
Source: Coding guidelines
There was a problem hiding this comment.
Fixed the documentation, but kept the actual logging behavior as-is per an explicit product decision: full upstream detail (status + body) is intended to stay in server-side logs — those are operator-facing, not caller-facing, so withholding it there wasn't the goal. Only the HTTP response to the client is meant to stay generic, which mapUpstreamErrorGeneric already enforces.
Reworded response.go's doc comment and CLAUDE.md to state this explicitly (full reason in logs is intentional, not an accidental leak) rather than reading as if mapUpstreamErrorGeneric were somehow responsible for sanitizing logs — it isn't, and never was.
Also worth noting: the slog.ErrorContext(ctx, ..., "err", err) pattern this flags predates this PR — it's the existing convention at all ~85 call sites, unchanged here. Not something introduced by this diff.
Comment added by Claude on behalf of @Rashmika998.
| {/* Tooltip copy is intentionally empty until the per-widget messages | ||
| are finalized — the icon renders now so the layout/interaction is | ||
| in place ahead of that content. */} | ||
| <Tooltip title=""> | ||
| <Box | ||
| component="span" | ||
| sx={{ | ||
| position: "absolute", | ||
| top: 8, | ||
| right: 8, | ||
| zIndex: 1, | ||
| display: "inline-flex", | ||
| color: "text.secondary", | ||
| }} | ||
| > | ||
| <Info size={14} /> | ||
| </Box> | ||
| </Tooltip> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Remove the inactive information affordance.
Tooltip title="" provides no information. The icon is inside the link card, so selecting it navigates to the tile target.
Remove the icon until tooltip content is available. Alternatively, provide nonempty tooltip text and separate interaction behavior.
🤖 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-dashboard/components/DashboardWidgetTile.tsx`
around lines 101 - 118, Remove the empty Tooltip and its Info icon from
DashboardWidgetTile; do not leave an inactive affordance inside the navigable
card. Keep the tile layout and link behavior unchanged until meaningful tooltip
content and separate interaction handling are available.
There was a problem hiding this comment.
Declining this one — the empty tooltip is intentional, not an oversight. This came directly from the requester: render the info icon now (so the layout/interaction is in place), but leave the tooltip copy empty for the time being since the per-widget messaging hasn't been finalized yet. It'll be filled in once that copy is decided, in a follow-up change.
Comment added by Claude on behalf of @Rashmika998.
…t-of-range dates, add missing error-mapping test coverage - IncidentsFilterBar: compute today's UTC date fresh each render instead of memoizing it for the component's lifetime (was frozen past a UTC midnight for a long-lived session); reject a manually-typed out-of-range date in both Created-date onChange handlers, since minDate/maxDate only constrain the calendar popup, not direct text entry - response.go/CLAUDE.md: reword mapUpstreamErrorGeneric's doc comment to stop reading as an endorsement of logging raw upstream bodies; full upstream detail in server logs remains intentional (operator-facing, not caller-facing) per explicit product decision - CLAUDE.md: document upstreamErrors vs. upstreamErrorsGeneric as two distinct, intentional test tables rather than a naming inconsistency - change_requests_test.go: add the missing CreateComment-failure case to TestCreateChangeRequestComment (previously only GetChangeRequest failure was covered) - projects_test.go: add TestGetProjectContact, which had no coverage at all Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Addressed the remaining nitpicks from the review (no separate inline thread for these, so replying here):
The 4 actionable inline comments (raw-body-logging docs, the empty tooltip, the stale UTC-today bound, and typed out-of-range dates) were replied to individually on their own threads. Comment added by Claude on behalf of @Rashmika998. |
2 participants
Summary
mapUpstreamErrorGeneric(now the default for every endpoint — never echoes an upstream 4xx/5xx reason to the caller) andmapUpstreamError(kept only for the 10 PATCH/update handlers, where the upstream reason like "Invalid state transition" is genuinely caller-actionable). The full reason is still logged server-side viaslog.ErrorContexteither way.ListStatGridpattern), and an info-icon affordance — tooltip copy intentionally left empty pending finalized per-widget messaging.Test plan
go build ./...,go vet ./...,go test ./...(backend, all packages)gosec -fmt=text ./...— 0 issuestsc -b(webapp) — no type errorseslinton changed webapp files — cleanvitest run(webapp) — 767 passing; the 9 failures inCaseActionBar.test.tsxandCsmAnnouncementsPage.test.tsxare pre-existing and unrelated (confirmed against a cleanmaincheckout before this branch's changes)AgentsLandingPagePilot.test.tsx(refresh re-fetches + skeleton),cases_test.go/incidents_test.go(new generic vs. detail-surfacing error-mapping tables)Note: this backend change hasn't been deployed to the shared staging environment yet — testing against it will still show old behavior until it's redeployed.
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
New Features