Skip to content

[CSM Portal] minor fixes: strip internal error tags, gate CR approval on assigned team, split watchers into own tab - #1264

Merged
Rashmika998 merged 4 commits into
wso2-open-operations:mainfrom
rksk:three-fixes-consolidated
Jul 27, 2026
Merged

[CSM Portal] minor fixes: strip internal error tags, gate CR approval on assigned team, split watchers into own tab#1264
Rashmika998 merged 4 commits into
wso2-open-operations:mainfrom
rksk:three-fixes-consolidated

Conversation

@rksk

@rksk rksk commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Purpose

Consolidates three independent, unrelated-but-small fixes into one PR (each was originally opened separately as #1259/#1260/#1261; those are closed in favor of this one) to reduce review overhead. Each commit is self-contained and independently revertable, spanning entity-service (Go) and the CSM webapp (React).

Goals

  • Stop leaking internal [SOMETHING_ERROR]-style tags (e.g. [SERVICENOW_ERROR]) from the backing data source into user-facing error messages — confirmed live in production traffic on a change-request approval rejection.
  • Stop "Request approval" on a change request from round-tripping to the backing data source and failing when the request lacks an assigned team — the existing state-based gate didn't check for this.
  • Give case watchers their own detail tab instead of sharing the "Related" tab with child cases, matching the existing SLAs/Attachments/Time tracking tab pattern.

Approach

Three independent commits, each addressing one issue in its own files with its own tests — see individual commit messages for detail. No shared code between them; a review can evaluate each commit in isolation.

Release note

  • Error messages from the backing data source no longer show an internal tag prefix.
  • "Request approval" is now disabled (with an explanatory tooltip) instead of failing after a round trip, when a change request has no assigned team.
  • Case watchers now live on their own tab.

Documentation

N/A — internal bug fixes and a UX improvement, no contract/shape changes.

Security checks

  • Secure coding standards followed: yes
  • FindSecurityBugs / static analysis: N/A for Go/TS — go vet ./... and eslint/tsc clean
  • No secrets committed: yes

Related PRs

Supersedes #1259, #1260, #1261 (closed in favor of this consolidated PR).

Summary by CodeRabbit

  • New Features
    • Added a dedicated Watchers tab to case details, including a live watcher count.
    • Watchers management now navigates directly to the Watchers tab.
  • Bug Fixes
    • “Request approval” is now disabled unless the legal next state allows it and an assigned team exists, with an explanation shown via tooltip.
    • ServiceNow client errors no longer leak internal error tags and are sanitized for safer, clearer client-facing messaging.
  • Tests
    • Added coverage for ServiceNow error tag stripping and fallback behavior.

rksk added 3 commits July 27, 2026 17:08
… no assigned team

The client-side gate for the New -> Assess transition only checked the
generic state-machine transition list, but the backing data source also
requires an assigned team before that transition is accepted. Without
this, clicking Request approval could still round-trip to the backend
and fail there. The button now also disables (with an explanatory
tooltip) when the transition is otherwise legal but no team is assigned.
Watchers is now split into its own tab, positioned between Related and
SLAs, with a dedicated Eye icon and a live count of watchers in the
tab label. The Related tab keeps only the child-cases list and drops
to a single-column layout now that it's a single widget. The action
bar's "Manage watchers" item jumps to the new tab. No changes to
WatchersWidget or the inline add/remove behavior.
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c1184f6e-9de1-4491-8050-88d2822e4761

📥 Commits

Reviewing files that changed from the base of the PR and between ef95d36 and 57fdfc2.

📒 Files selected for processing (4)
  • apps/csm-portal/webapp/src/features/csm-operations/pages/CsmChangeRequestDetailPage.test.tsx
  • apps/csm-portal/webapp/src/features/csm-operations/pages/CsmChangeRequestDetailPage.tsx
  • entity-service/internal/servicenow-integration-service/client.go
  • entity-service/internal/servicenow-integration-service/client_test.go
🚧 Files skipped from review as they are similar to previous changes (4)
  • entity-service/internal/servicenow-integration-service/client_test.go
  • apps/csm-portal/webapp/src/features/csm-operations/pages/CsmChangeRequestDetailPage.test.tsx
  • entity-service/internal/servicenow-integration-service/client.go
  • apps/csm-portal/webapp/src/features/csm-operations/pages/CsmChangeRequestDetailPage.tsx

📝 Walkthrough

Walkthrough

This PR adds a dedicated Watchers tab to case details, restricts change request approval when no team is assigned, and strips internal tags from ServiceNow client-facing errors while sanitizing logs.

Changes

Case watchers tab

Layer / File(s) Summary
Watchers tab navigation
apps/csm-portal/webapp/src/features/csm-cases/pages/CsmCaseDetailPage.tsx
Adds the watchers tab, Eye icon, announcement fallback, and navigation from manage_watchers.
Watchers tab rendering
apps/csm-portal/webapp/src/features/csm-cases/pages/CsmCaseDetailPage.tsx
Adds announcement filtering, watcher counts, and dedicated WatchersWidget rendering.

Change request approval gating

Layer / File(s) Summary
Approval eligibility and blocked action
apps/csm-portal/webapp/src/features/csm-operations/pages/CsmChangeRequestDetailPage.tsx, apps/csm-portal/webapp/src/features/csm-operations/pages/CsmChangeRequestDetailPage.test.tsx
Approval now requires the assess legal next state and an assigned team; missing assignment disables the action with a tooltip, with tests for blocked, accessible, and enabled states.

ServiceNow error handling

Layer / File(s) Summary
Error tag stripping and safe logging
entity-service/internal/servicenow-integration-service/client.go
Strips leading internal tags from downstream messages and sanitizes logged values.
Sanitization and client error tests
entity-service/internal/servicenow-integration-service/client_test.go
Tests tag extraction, client-facing ConflictError sanitization, and tag-only fallback behavior.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  actor User
  participant CsmCaseDetailPage
  participant WatchersWidget
  User->>CsmCaseDetailPage: Select watchers tab
  CsmCaseDetailPage->>WatchersWidget: Render watcher list
  User->>CsmCaseDetailPage: Choose manage_watchers
  CsmCaseDetailPage->>WatchersWidget: Navigate to watchers tab
Loading

Possibly related PRs

Suggested labels: Type/Bug, Type/New Feature

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the three fixes: error-tag stripping, approval gating, and watchers tab split.
Description check ✅ Passed The description covers purpose, goals, approach, release note, docs, security, and related PRs, with only minor template sections omitted.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@rksk rksk changed the title [CSM Portal] three fixes: strip internal error tags, gate CR approval on assigned team, split watchers into own tab [CSM Portal] minor fixes: strip internal error tags, gate CR approval on assigned team, split watchers into own tab Jul 27, 2026
@rksk

rksk commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai resume

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Reviews resumed.

@rksk

rksk commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai resume

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Reviews resumed.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 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/pages/CsmChangeRequestDetailPage.tsx`:
- Around line 289-302: Update the Tooltip wrapper around the disabled “Request
approval” Button to be keyboard-focusable with tabIndex={0} and an aria-label
combining “Request approval” with requestApprovalBlockedReason; add a
focus-based test verifying keyboard users can access the tooltip reason.

In `@entity-service/internal/servicenow-integration-service/client.go`:
- Around line 341-345: Update the error-message handling around
stripInternalErrorTag so that when stripping a tagged payload yields an empty
clientMsg, the function returns defaultMsg instead. Preserve the existing
sanitized logging for non-empty tagged messages and the current behavior for
untagged payloads.
- Around line 341-345: Update the logging in the stripInternalErrorTag flow to
retain the sanitized tag but replace the downstream-controlled clientMsg with a
fixed, non-user-controlled error summary. Keep returning clientMsg unchanged,
and preserve the existing conditional logging behavior when tag is present.
🪄 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: 6939ba64-2814-411b-84db-cf0ab13fd238

📥 Commits

Reviewing files that changed from the base of the PR and between 18b8f83 and ef95d36.

📒 Files selected for processing (5)
  • apps/csm-portal/webapp/src/features/csm-cases/pages/CsmCaseDetailPage.tsx
  • apps/csm-portal/webapp/src/features/csm-operations/pages/CsmChangeRequestDetailPage.test.tsx
  • apps/csm-portal/webapp/src/features/csm-operations/pages/CsmChangeRequestDetailPage.tsx
  • entity-service/internal/servicenow-integration-service/client.go
  • entity-service/internal/servicenow-integration-service/client_test.go

Comment thread entity-service/internal/servicenow-integration-service/client.go
…ck, log sanitization

- CsmChangeRequestDetailPage.tsx: make the disabled "Request approval"
  tooltip reachable by keyboard (tabIndex, aria-label on the wrapper), with
  a focus-target test.
- client.go: fall back to defaultMsg when stripping an internal error tag
  leaves an empty client-facing message (e.g. a downstream body whose
  message is nothing but the tag).
- client.go: stop logging the raw downstream-controlled message text; log
  only the sanitized tag, per this repo's "log IDs and sanitized summaries
  only" guideline.
@rksk

rksk commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai resume

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Reviews resumed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants