[CSM Portal] SRA attachments, name display, project filtering, parentCase routing - #1283
Conversation
Security Report Analysis creation bundled attachments into the same POST /cases request as everything else about the report, so a failed attachment upload sank the whole call -- no case created, no navigation, the same false-negative bug the customer portal already fixed for its case-creation flow (PR wso2-open-operations#1254). This looked backend-mandated (entity-service rejected a zero-attachment SRA request), but live-verified against wso2sndev: ServiceNow itself never required attachments for SRA creation -- that was purely an over-restrictive assumption in entity-service's own Go validation, mirrored by a corresponding ServiceNow change removing 'attachments' from SRA's required fields on Create Case (needed because entity-service's omitempty tag drops the attachments key entirely when there are none, which SN's required-fields check does reject). - entity-service: dropped the 'at least one attachment' rejection for security_report_analysis; added a test asserting a zero-attachment request succeeds with no attachments key in the outbound payload. - CreateSecurityReportPage.tsx: now creates the case first, uploads attachments afterward via uploadAttachmentsToCase (same pattern already used by CsmCaseCreatePage/CreateServiceRequestPage), with a non-blocking warning on partial upload failure and unconditional navigation to the created report.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe changes add ServiceNow account-filtered project searches, enrich case and attachment metadata, route parent-case links by type, support public Fix ETA updates, remove ETA cells from the case metadata band, and move security report attachments to a post-creation upload flow. ChangesProject account filtering
Case metadata and update contracts
Security report attachment workflow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CreateSecurityReportPage
participant CaseAPI
participant AttachmentAPI
User->>CreateSecurityReportPage: submit report
CreateSecurityReportPage->>CaseAPI: create security report
CaseAPI-->>CreateSecurityReportPage: return case id
CreateSecurityReportPage->>AttachmentAPI: upload attachments for case
AttachmentAPI-->>CreateSecurityReportPage: return upload results
CreateSecurityReportPage-->>User: show result and navigate to case
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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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 |
Being replaced by the Set/Share Fix ETA form redesign; the overview band isn't the right place to surface these values.
…er-side account-project filter
Case creator and attachment uploader still showed raw emails on the FE
despite the earlier SN fix adding createdByFullName -- entity-service never
actually wired the field through: snCase and snAttachment (the case-detail
and attachment-search wire structs) never had a CreatedByFullName field at
all, so it was silently dropped before reaching the FE. Added the missing
field to both wire structs and threaded it into domain.CaseView.CreatedByDetails
and domain.Attachment; the FE case-detail mapper already preferred name over
email (useGetCsmCaseDetail.ts) so it now picks it up automatically, and
uiAttachmentFromBe now does the same for attachments.
Also: POST /projects/search gained a real server-side accountId filter
(ServiceNow: new addQuery('account', ...) in ProjectUtils, cache key updated
to include it; entity-service: new AccountID field threaded through with
UUID validation). Rewrote useAccountProjects.ts to use this instead of
scanning + client-side filtering the whole project catalogue, removing the
isFilterSupported workaround and its 'not available' message entirely.
Fixed a related shape bug: entity-service returns the account reference as
a nested account: {id, name} object, but the FE's Project/BeProject types
declared a flat accountId string that never matched the wire -- so the
account filter could never have worked even after the field was added.
Updated both types and the account-detail page's ProjectsSection.
…for non-case parents
digiops-cs#2568 (parentCase null for an incident parent) was already fixed
by an earlier session-2 change: _mapCaseDetails previously never set
parentCase on GET /cases/{id} at all (only the list view and the immediate
PATCH response did). Live-verified fresh against a real case-to-incident
link that this already works end to end.
This adds the follow-up the issue also flagged: parentCase now carries a
type (case/incident/change_request/problem, resolved from the parent
record's actual sys_class_name on the ServiceNow side), threaded through
entity-service (CaseNumberRef.Type) and the FE. The case detail page's
'Parent: ...' chip previously assumed every parent was a case and always
navigated to /cases/{id} -- now routes to the correct section for an
incident/change-request/problem parent.
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
apps/csm-portal/webapp/src/features/csm-accounts/api/useAccountProjects.ts (1)
29-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider surfacing
total/hasMoreso truncation is visible.The comment acknowledges results are silently capped at
BE_MAX_PAGE_LIMIT, but the hook discardstotal/hasMorefrom the response, so the Projects section can't even hint that rows are missing. Returning them lets the table show a "showing first N of M" note until a pager exists.♻️ Proposed change
-): UseQueryResult<{ projects: Project[] }, Error> { +): UseQueryResult<{ projects: Project[]; total: number; hasMore: boolean }, Error> {and return
{ projects: data.projects ?? [], total: data.total ?? 0, hasMore: data.hasMore ?? false }.🤖 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-accounts/api/useAccountProjects.ts` around lines 29 - 40, Update useAccountProjects to preserve and expose the response metadata alongside the projects array: return projects with total and hasMore, defaulting missing values to an empty array, zero, and false respectively. Update the hook’s UseQueryResult type so consumers can access all three fields, while keeping the existing account filtering and page limit behavior unchanged.
🤖 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/api/backend/mappers.ts`:
- Line 212: Update the uploader-name expression in the attachment mapper so the
createdBy fallback is trimmed before applying the "Unknown" default; preserve
the existing priority of createdByFullName, then the cleaned createdBy
identifier, then "Unknown".
In
`@apps/csm-portal/webapp/src/features/csm-security-center/pages/CreateSecurityReportPage.tsx`:
- Around line 321-338: Update the AttachmentsField usage in
CreateSecurityReportPage so attachment additions and removals are disabled
whenever submitting is true, matching the locked editor state and preserving the
handleSubmit attachment snapshot.
In `@entity-service/internal/domain/entity.go`:
- Around line 1619-1630: Change Attachment.CreatedByFullName from string to
*string and update SearchCaseAttachments to trim the source name, assign a
pointer only when non-empty, and leave it nil when unresolved. Preserve the
existing frontend fallback behavior and update any affected mappings or type
usages to handle the nullable field.
- Around line 978-982: The CaseNumberRef.Type value is passed through without
mapping and can expose unsupported ServiceNow task types. Add a shared domain
enum/mapper for parent-case task types that maps only case, incident,
change_request, and problem values and returns nil for unknown values; update
GetCaseByID to use it instead of assigning c.ParentCase.Type directly, and align
the ServiceNow and OpenAPI CaseNumberRef.type definitions with the mapped
response.
In `@entity-service/internal/service/sn_case_service.go`:
- Around line 139-141: Update the UpdateCase response builder to populate
snCaseRef.Type from the updated parent record, matching the existing GetCaseByID
mapping. Apply the same Type propagation to the corresponding response
construction at the other referenced location, preserving the discriminator for
incident, change request, and problem parent links.
---
Nitpick comments:
In `@apps/csm-portal/webapp/src/features/csm-accounts/api/useAccountProjects.ts`:
- Around line 29-40: Update useAccountProjects to preserve and expose the
response metadata alongside the projects array: return projects with total and
hasMore, defaulting missing values to an empty array, zero, and false
respectively. Update the hook’s UseQueryResult type so consumers can access all
three fields, while keeping the existing account filtering and page limit
behavior unchanged.
🪄 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: 4579eb4f-d373-4bc2-b1b4-1329981f4346
📒 Files selected for processing (17)
apps/csm-portal/webapp/src/api/backend/mappers.tsapps/csm-portal/webapp/src/api/backend/types.tsapps/csm-portal/webapp/src/features/csm-accounts/api/useAccountProjects.test.tsxapps/csm-portal/webapp/src/features/csm-accounts/api/useAccountProjects.tsapps/csm-portal/webapp/src/features/csm-accounts/pages/CsmAccountDetailPage.tsxapps/csm-portal/webapp/src/features/csm-cases/api/uploadAttachmentsToCase.tsapps/csm-portal/webapp/src/features/csm-cases/api/useGetCsmCaseDetail.tsapps/csm-portal/webapp/src/features/csm-cases/components/CaseMetaBand.tsxapps/csm-portal/webapp/src/features/csm-cases/pages/CsmCaseDetailPage.tsxapps/csm-portal/webapp/src/features/csm-cases/types/csmCases.tsapps/csm-portal/webapp/src/features/csm-projects/types/csmProjects.tsapps/csm-portal/webapp/src/features/csm-security-center/pages/CreateSecurityReportPage.tsxentity-service/internal/domain/entity.goentity-service/internal/service/case_service.goentity-service/internal/service/sn_case_service.goentity-service/internal/service/sn_case_service_create_test.goentity-service/internal/service/sn_project_service.go
💤 Files with no reviewable changes (1)
- apps/csm-portal/webapp/src/features/csm-cases/components/CaseMetaBand.tsx
…parent-case type
- Attachment.createdBy is now a UserRef object ({id, name, email}), matching
the convention case createdBy already uses, instead of two flat
createdBy/createdByFullName strings.
- Map ServiceNow's raw parent-case type value (e.g. "default_case") to the
public case/incident/change_request/problem enum instead of passing it
through unmapped, and propagate it on UpdateCase responses too, not just
GetCaseByID.
- Disable AttachmentsField's add/remove controls while a security report
create is submitting, so the attachment list can't drift from the
snapshot handleSubmit uploads.
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
Purpose
Follow-on fixes made after PR #1274 merged:
POST /casesrequest as the rest of the report, so a failed attachment upload sank the whole call — no case created, no navigation. The customer portal already fixed this exact pattern for its case-creation flow (PR fix(customer-portal): don't block case creation on attachment uploads #1254); CSM portal's other two create flows (CsmCaseCreatePage,CreateServiceRequestPage) already had it too, but SRA didn't.createdByFullNameto ServiceNow's response — entity-service was silently dropping the field before it ever reached the FE.accountIdfilter.parentCaseon the case detail page assumed every parent was another case; ServiceNow'sparentIdcan point at a case, incident, change request, or problem (digiops-cs#2568).Goals
accountIdfilter to project search.typediscriminator toparentCaseso the FE can route to the correct page for any parent type.Approach
attachments: []already succeeded); the "at least one attachment" rejection was purely an over-restrictive assumption in entity-service's own Go validation. Removed it (case_service.go), added a test asserting a zero-attachment request succeeds.CreateSecurityReportPage.tsxnow creates the case first viapostCase.mutateAsync(noattachmentsin the payload), then uploads any selected files afterward viauploadAttachmentsToCasewith a non-blocking warning on partial failure and unconditional navigation to the created report.CaseMetaBand.tsx: removed the best/most-likely/worst-case fix-ETA cells and the now-unusedformatDateOnlyForDisplayimport.sn_case_service.go's wire structs (snCase,snAttachment) never had aCreatedByFullNamefield at all, so ServiceNow's already-fixed response data was silently dropped before reachingdomain.CaseView.CreatedByDetails/domain.Attachment. Added the field to both, threaded through to the domain types the FE mappers already knew how to prefer (useGetCsmCaseDetail.ts,uiAttachmentFromBe).filters.accountIdto ServiceNow'sProjectUtils.searchProjects(both the query and the cache key),domain.SearchProjectsRequest.AccountIDin entity-service (UUID-validated, converted to sysid), and rewroteuseAccountProjects.tsto send the filter directly instead of scanning + filtering the whole project catalogue client-side. Also fixed a real shape bug found along the way:BeProject/Projectdeclared a flataccountId?: stringthat never matched the wire (entity-service returns a nestedaccount: {id, name}object) — fixed both types to the real shape.CaseUtils._parentTypeLabelhelper mapping a parent'ssys_class_name(sn_customerservice_case/incident/change_request/problem) to a domain string, added to_mapCaseDetails'sparentCase. Threaded throughdomain.CaseNumberRef.Typein entity-service andBeCaseNumberRef.type/CsmCaseDetail.parentCase.typeon the FE.CsmCaseDetailPage.tsx's "Parent: …" chip now routes via a newparentCasePath()helper instead of always assuming/cases/{id}.User stories
Release note
Documentation
N/A — internal CSM portal UI/workflow changes, no external-facing documentation to update.
Training
N/A — no training content covers this internal portal.
Certification
N/A — no certification exam covers this internal portal.
Marketing
N/A — internal tooling change, not customer/market facing.
Automation tests
Security checks
go vetandeslintclean on the changed files instead)Samples
N/A
Related PRs
Follow-on to #1274 (merged) — same phase-1/ServiceNow-vertical base branch (
main).Migrations (if applicable)
N/A — no schema or data migration involved.
Test environment
Verified with
go build ./...,go test ./...,go vet ./...(entity-service),pnpm build,pnpm lint,npx tsc --noEmit(webapp), all on macOS/Node/Go local dev environment. End-to-end verified against a local stack (real Asgardeo + real ServiceNow DEV tenant).Learning
N/A — implementation followed existing patterns already established in this codebase.
Summary by CodeRabbit