[Customer Portal[BE] Add stats/conversations endpoint, fix ongoing cases count, and update time-cards endpoint - #254
Conversation
📝 WalkthroughWalkthroughRefactors stats to use state-id mappings and state-based Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Service as Service (service.bal)
participant Utils as Utils (utils.bal)
participant Entity as Entity (modules/entity/...)
Client->>Service: GET /projects/{id}/stats/conversations
Service->>Entity: fetch ProjectConversationStats (stateCount)
Entity-->>Service: ProjectConversationStats (stateCount)
Service->>Utils: getConversationStats(ProjectConversationStats)
Utils-->>Service: OverallConversationStats (open/active/resolved/converted/abandoned)
Service-->>Client: 200 OK with ConversationStats
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 3
♻️ Duplicate comments (2)
apps/customer-portal/backend/utils.bal (1)
23-24: Same exported spelling issue is propagated here (Converstaion,abandonded).This is the downstream usage of the public typo already flagged in
apps/customer-portal/backend/modules/types/types.bal.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/backend/utils.bal` around lines 23 - 24, The configurable declaration uses misspelled exported symbols—rename the type and keys to correct spellings: change ConverstaionStateIds to ConversationStateIds and fix keys like "abandonded" to "abandoned" in the configurable variable conversationStateIds; update any downstream references to the type name and to the keys (e.g., conversationStateIds, ConversationStateIds) across the codebase to match the corrected identifiers to avoid compile/import errors.apps/customer-portal/backend/service.bal (1)
767-770:sessionChatsnull behavior here is a downstream effect of the utils mapping bug.Root cause is already flagged in
apps/customer-portal/backend/utils.bal(getConversationStatsnot populatingsessionCounton success).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/backend/service.bal` around lines 767 - 770, The null sessionChats comes from getConversationStats not populating sessionCount; open apps/customer-portal/backend/utils.bal, find the getConversationStats function and ensure the success path sets mappedConversationStats.sessionCount (or a default 0) when constructing the returned stats object so mappedConversationStats.sessionCount is never undefined; then re-run tests and callers (e.g., where sessionChats = mappedConversationStats.sessionCount in service.bal) to confirm sessionChats is populated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/customer-portal/backend/modules/types/types.bal`:
- Around line 998-1009: The exported record type ConverstaionStateIds and its
field abandonded are misspelled; rename the type to ConversationStateIds and the
field to abandoned, then update all references to the old names (e.g., any
Converstaion* identifiers and abandonded usages) across the codebase —
specifically replace ConverstaionStateIds -> ConversationStateIds and abandonded
-> abandoned in the definitions and in callers (look for usages in utils.bal and
service.bal) ensuring imports/exports and any documentation/comments are updated
to the corrected identifiers.
In `@apps/customer-portal/backend/service.bal`:
- Around line 2839-2841: The route was changed to projects/{id}/stats/time-cards
which breaks clients using the old projects/{id}/time-cards/stats path; add a
backward-compatible alias by either adding a new resource function matching the
old route that delegates to the existing resource function (resource function
get projects/[string id]/time-cards/stats(...) { return this->get
projects/[string id]/stats/time-cards(...); }) or configure the router to map
the old path to the same handler so both projects/[string id]/stats/time-cards
and projects/[string id]/time-cards/stats resolve to the same logic (reference
the existing resource function get projects/[string id]/stats/time-cards to
avoid duplicating business logic).
In `@apps/customer-portal/backend/utils.bal`:
- Around line 597-621: In getConversationStats, the success path maps state
counts to various *_Count fields but never sets sessionCount; add a sessionCases
array by filtering mappedConversationStats for id ==
conversationStateIds.session.toString() (similar to openCases/activeCases/etc.)
and include sessionCount: sessionCases.length() > 0 ? sessionCases[0].count : ()
in the returned OverallConversationStats so sessionCount is populated when state
counts are present.
---
Duplicate comments:
In `@apps/customer-portal/backend/service.bal`:
- Around line 767-770: The null sessionChats comes from getConversationStats not
populating sessionCount; open apps/customer-portal/backend/utils.bal, find the
getConversationStats function and ensure the success path sets
mappedConversationStats.sessionCount (or a default 0) when constructing the
returned stats object so mappedConversationStats.sessionCount is never
undefined; then re-run tests and callers (e.g., where sessionChats =
mappedConversationStats.sessionCount in service.bal) to confirm sessionChats is
populated.
In `@apps/customer-portal/backend/utils.bal`:
- Around line 23-24: The configurable declaration uses misspelled exported
symbols—rename the type and keys to correct spellings: change
ConverstaionStateIds to ConversationStateIds and fix keys like "abandonded" to
"abandoned" in the configurable variable conversationStateIds; update any
downstream references to the type name and to the keys (e.g.,
conversationStateIds, ConversationStateIds) across the codebase to match the
corrected identifiers to avoid compile/import errors.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/customer-portal/backend/modules/entity/constants.balapps/customer-portal/backend/modules/entity/types.balapps/customer-portal/backend/modules/entity/utils.balapps/customer-portal/backend/modules/types/types.balapps/customer-portal/backend/service.balapps/customer-portal/backend/utils.bal
💤 Files with no reviewable changes (1)
- apps/customer-portal/backend/modules/entity/constants.bal
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
apps/customer-portal/backend/modules/entity/utils.bal (1)
18-28:⚠️ Potential issue | 🟠 MajorUsing
configurablefor business-logic state IDs is risky — preferfinalconstants.
configurablelets any deployment override these IDs viaConfig.toml. BecausecaseStateIdsdrivesvalidateCaseUpdatePayloadandconversationStateIdsdrives stats aggregation, a misconfiguration silently allows wrong state transitions or produces incorrect counts with no error at startup. These should be module-levelfinalconstants (or at leastisolatedconstants) rather than externally mutable configurables.🔧 Proposed fix
-public configurable CaseStateIds & readonly caseStateIds = { +public final CaseStateIds & readonly caseStateIds = { open: 1, closed: 3, waitingOnWso2: 1003, reopened: 1006, awaitingInfo: 18, solutionProposed: 6, workInProgress: 10 }; -public configurable ConverstaionStateIds & readonly conversationStateIds = - {open: 1, active: 2, resolved: 3, converted: 4, abandonded: 5}; +public final ConversationStateIds & readonly conversationStateIds = + {open: 1, active: 2, resolved: 3, converted: 4, abandoned: 5};Additionally, line 27-28 carry over the typos
ConverstaionStateIds(type name) andabandonded(field value key) from the entity types definition — both of which are part of the public API.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/backend/modules/entity/utils.bal` around lines 18 - 28, Replace the module-level configurable state ID definitions with immutable module constants: change public configurable CaseStateIds & readonly caseStateIds and public configurable ConverstaionStateIds & readonly conversationStateIds to final (or isolated) constant declarations so runtime config cannot override them, and update any references (e.g., validateCaseUpdatePayload) to use the new constants; also fix the typos in the type/name and key by renaming ConverstaionStateIds → ConversationStateIds and the field abandonded → abandoned so the public API and aggregation logic use the correct identifiers.apps/customer-portal/backend/modules/entity/types.bal (1)
1312-1323:⚠️ Potential issue | 🟠 MajorFix two misspelled public identifiers:
ConverstaionStateIdsandabandonded.These are exported types that form a permanent API surface. The same typos were previously flagged and corrected in
modules/types/types.bal, but the entity-layer definitions were not updated.✏️ Proposed fix
-# Conversation state IDs. -public type ConverstaionStateIds record {| +# Conversation state IDs. +public type ConversationStateIds record {| # Open state ID int open; # Active state ID int active; # Converted state ID int converted; # Resolved state ID int resolved; - # Abandoned state ID - int abandonded; + # Abandoned state ID + int abandoned; |};Also update
entity/utils.balline 27 (type annotationConverstaionStateIds→ConversationStateIds) and line 28 (field keyabandonded→abandoned), and any other references inutils.balorservice.balthat access.abandonded.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/backend/modules/entity/types.bal` around lines 1312 - 1323, Rename the misspelled exported type ConverstaionStateIds to ConversationStateIds and the field key abondonded/abandonded to abandoned in the entity type record, then update all references: change the type annotation ConverstaionStateIds → ConversationStateIds in entity/utils.bal (and any other files) and rename any property access like .abandonded → .abandoned in entity/utils.bal, service.bal, and any other modules that consume this entity; ensure imports/exports and any documentation/comments reflect the corrected identifiers so the API surface matches modules/types/types.bal.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/customer-portal/backend/modules/entity/utils.bal`:
- Around line 157-158: validateCaseUpdatePayload is checking payload.stateKey
against runtime-configurable caseStateIds (closed, reopened, waitingOnWso2),
which can be overwritten and let invalid transitions slip; change the code so it
compares payload.stateKey to canonical, final IDs resolved at startup (e.g.,
resolveCaseStateIds or getFinalStateId('closed')) or to internal enum constants
instead of reading caseStateIds directly at runtime, and add an
initialization-time validation that ensures the resolved IDs are present and
immutable (throw/fail fast if missing) so validateCaseUpdatePayload uses stable
identifiers for comparisons.
In `@apps/customer-portal/backend/service.bal`:
- Around line 680-729: The resource function get projects/[entity:IdString
id]/stats/conversations currently accepts a query parameter caseTypes but never
uses it; either remove the unused caseTypes parameter or forward it to the
entity layer by updating the call to
entity:getConversationStatsForProject(userInfo.idToken, id, caseTypes) (and
adjust the entity:getConversationStatsForProject signature to accept string[]?
if it doesn't already), ensuring any downstream functions (e.g.,
getConversationStats) handle the filtered results appropriately and update
related error handling or tests as needed.
- Around line 2839-2841: The resource function declaration for "get
projects/[string id]/stats/time-cards" is using an unconstrained path param
([string id]) while other stats endpoints use the UUID-validated type
entity:IdString; update the resource function signature to use [entity:IdString
id] so the framework enforces UUID-format validation consistently (modify the
resource function named get projects/[string id]/stats/time-cards to use
entity:IdString id and adjust any references to `id` inside that function
accordingly); if this endpoint intentionally bypasses validation, leave as-is
and add a clarifying comment.
---
Duplicate comments:
In `@apps/customer-portal/backend/modules/entity/types.bal`:
- Around line 1312-1323: Rename the misspelled exported type
ConverstaionStateIds to ConversationStateIds and the field key
abondonded/abandonded to abandoned in the entity type record, then update all
references: change the type annotation ConverstaionStateIds →
ConversationStateIds in entity/utils.bal (and any other files) and rename any
property access like .abandonded → .abandoned in entity/utils.bal, service.bal,
and any other modules that consume this entity; ensure imports/exports and any
documentation/comments reflect the corrected identifiers so the API surface
matches modules/types/types.bal.
In `@apps/customer-portal/backend/modules/entity/utils.bal`:
- Around line 18-28: Replace the module-level configurable state ID definitions
with immutable module constants: change public configurable CaseStateIds &
readonly caseStateIds and public configurable ConverstaionStateIds & readonly
conversationStateIds to final (or isolated) constant declarations so runtime
config cannot override them, and update any references (e.g.,
validateCaseUpdatePayload) to use the new constants; also fix the typos in the
type/name and key by renaming ConverstaionStateIds → ConversationStateIds and
the field abandonded → abandoned so the public API and aggregation logic use the
correct identifiers.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/customer-portal/backend/modules/entity/types.balapps/customer-portal/backend/modules/entity/utils.balapps/customer-portal/backend/modules/types/types.balapps/customer-portal/backend/service.balapps/customer-portal/backend/utils.bal
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/customer-portal/backend/utils.bal
| types:OverallConversationStats mappedConversationStats = getConversationStats(conversationStats); | ||
|
|
||
| return { | ||
| openCount: mappedConversationStats.openCount, | ||
| resolvedCount: mappedConversationStats.resolvedCount, | ||
| abandonedCount: mappedConversationStats.abandonedCount | ||
| }; |
There was a problem hiding this comment.
| types:OverallConversationStats mappedConversationStats = getConversationStats(conversationStats); | |
| return { | |
| openCount: mappedConversationStats.openCount, | |
| resolvedCount: mappedConversationStats.resolvedCount, | |
| abandonedCount: mappedConversationStats.abandonedCount | |
| }; | |
| types:OverallConversationStats { openCount, resolvedCount, abandonedCount} | |
| = getConversationStats(conversationStats); | |
| return { openCount, resolvedCount, abandonedCount }; |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/customer-portal/backend/service.bal (1)
2767-2768: Alignidpath-param typing across time-card endpoints for consistency.Line 2767 uses
[entity:IdString id], but Line 2835 still uses[string id]. Consider making bothentity:IdString(or document whystats/time-cardsintentionally differs) to avoid inconsistent validation behavior.Based on learnings: Time-card endpoints intentionally skip local project access pre-check (`entity:getProject`) and rely on upstream ServiceNow authorization; this suggestion is only about path-param validation consistency.♻️ Proposed alignment
-resource function get projects/[string id]/stats/time-cards(http:RequestContext ctx, entity:Date? startDate, +resource function get projects/[entity:IdString id]/stats/time-cards(http:RequestContext ctx, entity:Date? startDate, entity:Date? endDate)Also applies to: 2835-2836
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/backend/service.bal` around lines 2767 - 2768, The path-param type for the project id is inconsistent between the time-card endpoints: change the route declaration that uses [string id] (the stats/time-cards endpoint) to use the same typed path-param [entity:IdString id] as in resource function post projects/[entity:IdString id]/time-cards/search so both endpoints (e.g., the resource functions handling time-cards/search and stats/time-cards) perform the same validation; update the route signature and any references to the path param within the stats handler to use id as an entity:IdString.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/customer-portal/backend/service.bal`:
- Around line 2767-2768: The path-param type for the project id is inconsistent
between the time-card endpoints: change the route declaration that uses [string
id] (the stats/time-cards endpoint) to use the same typed path-param
[entity:IdString id] as in resource function post projects/[entity:IdString
id]/time-cards/search so both endpoints (e.g., the resource functions handling
time-cards/search and stats/time-cards) perform the same validation; update the
route signature and any references to the path param within the stats handler to
use id as an entity:IdString.
03e057f
into
wso2-open-operations:customer-portal-milestone-1
Description
This PR includes the following updates:
stats/conversationsendpointChanges
1️⃣ Add
stats/conversationsEndpoint2️⃣ Fix Ongoing Cases Count Issue
3️⃣ Update Time-Cards Endpoint
Reason
These changes improve reporting accuracy and overall API reliability.
Testing
Impact
Summary by CodeRabbit
New Features
Bug Fixes
Chores