[CSM Portal] fix Task/Problem follow-ups: null parent-case guard, On-suffix rename, keyboard accessibility - #1195
Conversation
…rence TaskDetail.parentCase can be null when ServiceNow's parent-case lookup comes back empty (the entity-service/BE layers already model it as optional), but the webapp typed it as required and read task.parentCase.number directly, which throws when the field is actually absent.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughProblem detail timestamps were renamed from ChangesProblem detail and task interaction updates
Estimated code review effort: 3 (Moderate) | ~20 minutes 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 |
…dIncidents nullable Renames ProblemDetail.resolvedAt/openedAt/closedAt to resolvedOn/openedOn/closedOn across the entity-service domain type, the SN mapping layer, both openapi specs, and the CSM webapp FE type/page/test, matching this codebase's On-suffix timestamp convention. Also marks linkedIncidents nullable in entity-service/openapi.yaml to reflect that an empty collection serializes as JSON null.
Add role="button", tabIndex={0}, onKeyDown (Enter/Space), and an
aria-label to the clickable problem row in ProblemsTab, mirroring the
pattern already used in ProductVulnerabilitiesTab. Keyboard-only users
can now reach and activate a row to open its detail page.
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/csm-portal/webapp/src/api/backend/types.ts (1)
1971-1971: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign type with
nullable: truein the OpenAPI schema.The backend OpenAPI schema explicitly marks
linkedIncidentsasnullable: true, and the API returnsnullrather than an empty array when no incidents are linked. Adding| nullhere ensures the TypeScript definition accurately reflects the payload.♻️ Proposed refactor
- linkedIncidents?: BeProblemRef[]; + linkedIncidents?: BeProblemRef[] | null;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/csm-portal/webapp/src/api/backend/types.ts` at line 1971, Update the linkedIncidents property type to allow null values in addition to BeProblemRef arrays, matching the nullable OpenAPI payload behavior.
🤖 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-operations/components/ProblemsTab.tsx`:
- Line 154: Remove the role="button" prop from the interactive TableRow in
ProblemsTab.tsx, preserving its native table-row semantics. Keep tabIndex,
aria-label, onClick, and onKeyDown unchanged so keyboard interaction remains
supported.
---
Nitpick comments:
In `@apps/csm-portal/webapp/src/api/backend/types.ts`:
- Line 1971: Update the linkedIncidents property type to allow null values in
addition to BeProblemRef arrays, matching the nullable OpenAPI payload behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9ee70f9f-a40f-4801-a642-841923cb537b
📒 Files selected for processing (10)
apps/csm-portal/backend/openapi.yamlapps/csm-portal/webapp/src/api/backend/types.tsapps/csm-portal/webapp/src/features/csm-cases/components/TaskDetailDialog.tsxapps/csm-portal/webapp/src/features/csm-cases/components/TasksWidget.test.tsxapps/csm-portal/webapp/src/features/csm-operations/components/ProblemsTab.tsxapps/csm-portal/webapp/src/features/csm-operations/pages/ProblemDetailPage.test.tsxapps/csm-portal/webapp/src/features/csm-operations/pages/ProblemDetailPage.tsxentity-service/internal/domain/entity.goentity-service/internal/service/sn_problem_service.goentity-service/openapi.yaml
role="button" on a TableRow replaces its implicit "row" role, breaking grid navigation and column association for screen-reader users. Keep tabIndex/onKeyDown/aria-label for keyboard activation without overriding native table semantics.
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
Purpose
Follow-up fixes for two features merged earlier this session (Task tab on the Case detail page, Problem management): a null-safety crash in the task detail dialog, a naming-convention/schema-accuracy fix on Problem detail, and a keyboard-accessibility gap on the Problems list. No related issue link (internal tracking only).
Goals
ProblemDetail's timestamp field names with this codebase'sOnsuffix convention, and marklinkedIncidentsnullable to match the actual wire format.Approach
BeTaskDetail.parentCasewidened toBeCaseNumberRef | null, matching the entity-service/BE schema'snullable: true.TaskDetailDialognow readstask.parentCase?.number ?? task.parentCase?.id ?? "—", consistent with howassignedTo/productalready handle optionality in the same component.ProblemDetail.ResolvedAt/OpenedAt/ClosedAttoResolvedOn/OpenedOn/ClosedOn(struct fields, JSON tags, and the SN-response mapping) across the entity-service, bothopenapi.yamlspecs, and the webapp'sBeProblemDetailtype +ProblemDetailPage. MarkedlinkedIncidentsnullable: truein the entity-service OpenAPI schema to match the Go implementation's actualnil-slice-serializes-to-nullbehavior.ProblemsTabgainedrole="button",tabIndex={0}, anonKeyDownhandler for Enter/Space, and anaria-label, mirroring the existing accessible-row pattern already used inProductVulnerabilitiesTab.User stories
Release note
linkedIncidents.Documentation
N/A — internal CS-engineer portal fixes; no external/product docs impacted.
Training
N/A — no training content impact.
Certification
N/A — no certification exam impact.
Marketing
N/A — internal tooling, not customer-facing.
Automation tests
TasksWidget.test.tsxcovers a task detail withparentCase: null, asserting the dialog renders without throwing.ProblemDetailPage.test.tsxupdated for the renamed fields (10/10 passing).Security checks
go vetandeslintran clean instead.Samples
N/A — no new samples.
Related PRs
Stacked on top of #1194 (Tasks tab) and #1193 (Problem management), both already merged — this PR is a consolidated follow-up covering fixes found during CodeRabbit review and post-merge testing of both.
Migrations (if applicable)
N/A — no schema/data migration; type/naming-level fixes only.
Test environment
Verified locally:
go build/go vet/go testclean (entity-service),go build/go vetclean (backend),tsc --noEmit/eslintclean and relevant vitest suites passing (webapp), on macOS.Learning
Consolidated three small follow-up fixes into one PR rather than opening a separate PR per fix, per reviewer preference.
Summary by CodeRabbit