[CSM] fix: nest Comment.createdBy and map SN type strings to domain enum - #1037
Rashmika998 merged 3 commits into
Conversation
The flat createdBy/createdByFirstName/createdByLastName/createdByFullName fields are replaced with a single nested createdBy object carrying id, firstName, lastName, and fullName — matching the shape already used by CaseComment. OpenAPI schemas updated in both entity-service and csm-portal. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
SN returns plural strings ("comments", "work_notes") which are now mapped
to the canonical domain values ("comment", "work_note", "activity") via a
reverse map. Comment.Type is typed as CommentType instead of plain string.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 53 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 Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughComment author metadata is refactored from flat string/name fields into a structured object ( ChangesStructured comment author refactor
Estimated code review effort: 2 (Simple) | ~15 minutes 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)
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
entity-service/openapi.yaml (1)
4521-4529: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Comment.typestill lacks an enum and its description is stale.Unlike the sibling
CaseComment.type(Line 3190-3192,enum: [work_note, comment, activity]),Comment.typeremains an unconstrained string whose description still references the old plural SN tokenwork_notesrather than the canonical domain valuework_note. Given the domain model now maps to the canonicalCommentTypeenum (comment,work_note,activity), the schema should match for API-contract clarity and generated client type-safety.📝 Proposed fix
type: type: string - description: Comment type (e.g. comment, work_notes). + enum: [work_note, comment, activity] + description: Comment type.🤖 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 `@entity-service/openapi.yaml` around lines 4521 - 4529, Update the Comment.type schema in openapi.yaml so it matches the canonical CommentType values used by CaseComment.type and the domain model. Add the enum for comment, work_note, and activity, and revise the description to remove the stale work_notes wording and reflect the canonical token names. Locate the Comment schema near the createdOn/createdBy fields and keep the contract consistent with the sibling CaseComment.type definition.apps/csm-portal/backend/openapi.yaml (1)
4117-4134: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
CaseComment.createdByshould be a nested user ref.apps/csm-portal/backend/openapi.yaml:4117still documentscreatedByas a string, but this schema is reused by the case comment create/search responses and should match the entity-service payload (id,firstName,lastName,fullName). Update the portal schema to the object shape so the contract stays aligned.🤖 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 4117 - 4134, The CaseComment schema still defines createdBy as a string, but it should match the nested user payload used by the entity-service responses. Update the CaseComment object in openapi.yaml so createdBy uses the user reference shape with id, firstName, lastName, and fullName, and make sure any create/search response schemas that reuse CaseComment continue to align with that object structure.
♻️ Duplicate comments (1)
entity-service/internal/service/sn_comment_service.go (1)
90-90: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUnmapped SN type resolves to empty string.
snCommentTypeToCommentType[c.Type]yields the zero valuedomain.CommentType("")for anyc.Typenot exactly"comments","work_notes", or"activity"— this includes singular forms thatsn_case_service.go's equivalent switch (Line 730/732) explicitly normalizes. This can surface an invalidtypein theGET /conversations/{id}/messagesresponse.🤖 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 `@entity-service/internal/service/sn_comment_service.go` at line 90, The type mapping in snCommentService’s comment conversion is too strict and can fall back to an empty domain.CommentType for unrecognized ServiceNow values. Update the logic around snCommentTypeToCommentType usage in snComment_service.go so c.Type is normalized the same way as in sn_case_service.go (for example, singular/plural variants) before assigning commentType, and ensure any unmapped value is handled with a safe default or explicit validation instead of returning an empty string.
🧹 Nitpick comments (2)
entity-service/internal/service/sn_case_service.go (1)
680-685: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate type-mapping logic (map vs. switch).
The same SN-type-to-
CommentTypemapping is now implemented twice: once assnCommentTypeToCommentType(Lines 680-685) and once as an inlineswitch(Lines 728-738), with differing behavior (the switch handles singular forms and has a default; the map does not). Consolidating into a single helper function used by both call sites would prevent this kind of divergence.♻️ Suggested consolidation
-var snCommentTypeToCommentType = map[string]domain.CommentType{ - "comments": domain.CommentTypeComment, - "work_notes": domain.CommentTypeWorkNote, - "activity": domain.CommentTypeActivity, -} +func mapSNCommentType(snType string) domain.CommentType { + switch snType { + case "comments", "comment": + return domain.CommentTypeComment + case "work_notes", "work_note": + return domain.CommentTypeWorkNote + case "activity": + return domain.CommentTypeActivity + default: + return domain.CommentTypeComment + } +}Then replace the switch at Line 728-738 and the map lookup in
sn_comment_service.gowith calls tomapSNCommentType(c.Type).Also applies to: 728-738
🤖 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 `@entity-service/internal/service/sn_case_service.go` around lines 680 - 685, The SN-type-to-CommentType conversion is duplicated and has drifted between the map and the inline switch. Introduce a single helper like mapSNCommentType and use it from both the snCommentTypeToCommentType lookup and the switch-based path in sn_case_service.go/sn_comment_service.go so the singular/plural handling and default fallback stay consistent. Remove the separate map/switch logic after wiring both call sites to the shared helper.apps/csm-portal/backend/openapi.yaml (1)
5058-5060: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
ConversationMessage.typenot constrained to the domainCommentTypevalues.The domain
Comment.Typefield is now typed asCommentType(per the PR), but this schema still documentstypeas a free-form string without an enum. Consider addingenum: [comment, work_note, activity]to matchCaseComment.type(line 4126) and keep client-generated types accurate.🤖 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 5058 - 5060, The ConversationMessage.type schema is still a free-form string and should be constrained to the CommentType domain values. Update the ConversationMessage definition in the OpenAPI schema to add the enum values used by CaseComment.type, so generated clients reflect the actual type contract. Use the ConversationMessage and CaseComment schema entries as the references when making the change.
🤖 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 `@entity-service/internal/service/sn_case_service.go`:
- Around line 680-685: The SN comment type mapping can return the zero value for
unexpected API values, causing an empty domain comment type. Update the lookup
in the SN comment handling path that uses snCommentTypeToCommentType and add an
ok check on c.Type; when the type is unknown, default to
domain.CommentTypeComment instead of using the map result directly. Keep the fix
near the mapping/lookup symbols snCommentTypeToCommentType and the SN comment
conversion logic so it is applied consistently.
---
Outside diff comments:
In `@apps/csm-portal/backend/openapi.yaml`:
- Around line 4117-4134: The CaseComment schema still defines createdBy as a
string, but it should match the nested user payload used by the entity-service
responses. Update the CaseComment object in openapi.yaml so createdBy uses the
user reference shape with id, firstName, lastName, and fullName, and make sure
any create/search response schemas that reuse CaseComment continue to align with
that object structure.
In `@entity-service/openapi.yaml`:
- Around line 4521-4529: Update the Comment.type schema in openapi.yaml so it
matches the canonical CommentType values used by CaseComment.type and the domain
model. Add the enum for comment, work_note, and activity, and revise the
description to remove the stale work_notes wording and reflect the canonical
token names. Locate the Comment schema near the createdOn/createdBy fields and
keep the contract consistent with the sibling CaseComment.type definition.
---
Duplicate comments:
In `@entity-service/internal/service/sn_comment_service.go`:
- Line 90: The type mapping in snCommentService’s comment conversion is too
strict and can fall back to an empty domain.CommentType for unrecognized
ServiceNow values. Update the logic around snCommentTypeToCommentType usage in
snComment_service.go so c.Type is normalized the same way as in
sn_case_service.go (for example, singular/plural variants) before assigning
commentType, and ensure any unmapped value is handled with a safe default or
explicit validation instead of returning an empty string.
---
Nitpick comments:
In `@apps/csm-portal/backend/openapi.yaml`:
- Around line 5058-5060: The ConversationMessage.type schema is still a
free-form string and should be constrained to the CommentType domain values.
Update the ConversationMessage definition in the OpenAPI schema to add the enum
values used by CaseComment.type, so generated clients reflect the actual type
contract. Use the ConversationMessage and CaseComment schema entries as the
references when making the change.
In `@entity-service/internal/service/sn_case_service.go`:
- Around line 680-685: The SN-type-to-CommentType conversion is duplicated and
has drifted between the map and the inline switch. Introduce a single helper
like mapSNCommentType and use it from both the snCommentTypeToCommentType lookup
and the switch-based path in sn_case_service.go/sn_comment_service.go so the
singular/plural handling and default fallback stay consistent. Remove the
separate map/switch logic after wiring both call sites to the shared helper.
🪄 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: bfa6b1cd-af54-4377-aee0-c560bff990c1
📒 Files selected for processing (5)
apps/csm-portal/backend/openapi.yamlentity-service/internal/domain/entity.goentity-service/internal/service/sn_case_service.goentity-service/internal/service/sn_comment_service.goentity-service/openapi.yaml
An ok check guards the snCommentTypeToCommentType lookup so an unexpected value from the SN API never produces an empty type in the response. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
createdBy,createdByFirstName,createdByLastName,createdByFullNamefields onCommentwith a single nestedcreatedByobject ({ id, firstName, lastName, fullName }) — matching the shape already used byCaseCommentComment.TypeasCommentTypeinstead ofstring, and maps SN plural type strings ("comments","work_notes") back to the canonical domain values ("comment","work_note","activity") via a reverse lookup mapGET /conversations/{id}/messagesandPOST /cases/{id}/comments/searchresponses in the CSM portalTest plan
POST /comments/search(entity service) returnscreatedByas a nested object withid/firstName/lastName/fullNametypefield returns"comment"/"work_note"/"activity"(not"comments"/"work_notes")GET /conversations/{id}/messagesandPOST /cases/{id}/comments/searchboth reflect the new shapesmake test)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes