[Customer Portal][BE] Enhance /projects/{id}/stats/cases endpoint to include additional metrics and cases trend - #182
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded two error constants; extended getCaseStatsForProject and service endpoints to accept optional caseTypes; replaced single-value status/state filters with array variants and added caseType filters; restructured case-statistics types to aggregated counts and trends; added mapping helpers and made some stat retrievals non-fatal. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Service as Service (HTTP)
participant Entity as Entity Module
participant Search as Search/DB
Client->>Service: GET /projects/:id/stats?caseTypes=[...]
Service->>Entity: getCaseStatsForProject(idToken, id, caseTypes)
Entity->>Search: query with filters (caseTypeIds?, statusIds?/stateKeys?)
Search-->>Entity: ProjectCaseStatsResponse (stateCount, severityCount, caseTypeCount, casesTrend)
Entity-->>Service: ProjectCaseStatsResponse
Service->>Service: mapCaseStats(...) / getOpenCasesCountFromProjectCasesStats(...)
Service-->>Client: HTTP 200 with composed stats (counts, trends) or partial data + logged errors
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/customer-portal/backend/service.bal (1)
490-496:⚠️ Potential issue | 🔴 Critical
map.get(STATE_OPEN)will panic if the key is absent.In Ballerina,
map.get(key)panics at runtime when the key does not exist. If the entity service returns astateCountmap without an"Open"entry (e.g., when filtered bycaseTypesthat have no open cases), this line will crash the request.Use member access (
stateCount[STATE_OPEN]) which returns a nilable type, and provide a fallback:🐛 Proposed fix
- openCases: caseStats.stateCount.get(STATE_OPEN).count, + openCases: caseStats.stateCount[STATE_OPEN]?.count ?: 0,🤖 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 490 - 496, The code uses caseStats.stateCount.get(STATE_OPEN) which will panic if STATE_OPEN is missing; update the assignment to projectStats.openCases to use member access (caseStats.stateCount[STATE_OPEN]) that returns a nilable value and provide a safe fallback (e.g., treat missing entry as 0) before reading .count so the handler cannot crash; change the expression referencing caseStats.stateCount.get(STATE_OPEN).count to a nil-safe expression that checks caseStats.stateCount[STATE_OPEN] and uses a default count when nil.
🧹 Nitpick comments (1)
apps/customer-portal/backend/modules/entity/types.bal (1)
332-356: Remove unusedActiveCaseCountandOutstandingCasesCounttypes.These types are no longer referenced anywhere in the codebase after the refactor to map-based
stateCountandoutstandingSeverityCountfields. Removing them will eliminate dead code.🤖 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 332 - 356, Remove the dead types ActiveCaseCount and OutstandingCasesCount from the module to eliminate unused code: locate the public type declarations named ActiveCaseCount and OutstandingCasesCount in types.bal and delete their definitions (including their comment blocks) so the codebase relies solely on the map-based stateCount and outstandingSeverityCount structures; ensure no other symbols reference these type names before removing.
🤖 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/service.bal`:
- Around line 563-572: The return block uses unsafe map.get access on
caseStats.stateCount for STATE_OPEN which can panic if the key is missing;
replace the direct get expression with a safe optional access on
caseStats.stateCount for STATE_OPEN and provide a fallback (e.g., 0) when the
entry or its count is nil — i.e., use the map bracket access with optional
chaining to read the count from caseStats.stateCount[STATE_OPEN] and default to
0, updating the openCases field accordingly.
---
Outside diff comments:
In `@apps/customer-portal/backend/service.bal`:
- Around line 490-496: The code uses caseStats.stateCount.get(STATE_OPEN) which
will panic if STATE_OPEN is missing; update the assignment to
projectStats.openCases to use member access (caseStats.stateCount[STATE_OPEN])
that returns a nilable value and provide a safe fallback (e.g., treat missing
entry as 0) before reading .count so the handler cannot crash; change the
expression referencing caseStats.stateCount.get(STATE_OPEN).count to a nil-safe
expression that checks caseStats.stateCount[STATE_OPEN] and uses a default count
when nil.
---
Nitpick comments:
In `@apps/customer-portal/backend/modules/entity/types.bal`:
- Around line 332-356: Remove the dead types ActiveCaseCount and
OutstandingCasesCount from the module to eliminate unused code: locate the
public type declarations named ActiveCaseCount and OutstandingCasesCount in
types.bal and delete their definitions (including their comment blocks) so the
codebase relies solely on the map-based stateCount and outstandingSeverityCount
structures; ensure no other symbols reference these type names before removing.
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 (1)
apps/customer-portal/backend/modules/entity/types.bal (1)
332-356:⚠️ Potential issue | 🟡 MinorRemove
ActiveCaseCountandOutstandingCasesCount— these types are dead code.Both types have no references anywhere in the codebase and can be safely deleted to reduce clutter and avoid confusion.
🤖 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 332 - 356, Delete the unused record type declarations ActiveCaseCount and OutstandingCasesCount from the codebase: remove the entire public type definitions (including their fields like workInProgress, waitingOnClient, waitingOnWso2, total, and medium/high/critical/total with json... rest) so no dead type definitions remain; also search for any unused imports or comments that referenced these exact type names (ActiveCaseCount, OutstandingCasesCount) and remove them to avoid leftover clutter.
🧹 Nitpick comments (2)
apps/customer-portal/backend/service.bal (2)
583-592: ReturningstateCountin the response leaks theSTATE_OPENentry thatopenCasesalready surfaces.
openCasesis derived fromstateCount.get(STATE_OPEN).count(line 585), and then the fullstateCountmap is also returned (line 588). This means the "Open" count appears twice in the response under different shapes. If that's intentional for flexibility, it's fine — just flagging it for confirmation.🤖 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 583 - 592, Response currently includes both openCases (derived from stateCount.get(STATE_OPEN).count) and the full stateCount map, causing the "Open" count to appear twice; to fix, remove stateCount from the returned object (or alternatively, if the full map must be returned, delete the STATE_OPEN entry from stateCount before returning) — update the return block that constructs the response object (the code around stateCount and openCases) so only one representation of the "Open" count is included.
441-442:STATE_OPENguard runs before chat/deployment stats are fetched, which is good for early exit. However, note that the guard returns anInternalServerErrorto the client for what is essentially a data-quality issue (missing "Open" key). Consider whether a0default would be more resilient — an absent "Open" key could simply mean zero open cases — rather than failing the entire/statscall.This is a design choice, so just flagging for consideration.
Also applies to: 453-461, 500-513
🤖 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 441 - 442, The code currently treats a missing "Open" key as an InternalServerError when processing case stats (e.g., around entity:getCaseStatsForProject and ProjectCaseStatsResponse handling and the STATE_OPEN guard); change the guard logic to treat a missing "Open" entry as 0 instead of failing: when reading the map/record that should contain "Open" (the STATE_OPEN branch/guard), coerce/lookup the value with a safe-default (0) and continue normal processing rather than returning an InternalServerError, and apply the same defaulting approach to the other similar blocks noted (the other STATE_OPEN/empty-key guards around the chat/deployment stats handling).
🤖 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/types.bal`:
- Line 390: Fix the typo in the doc comment that currently reads "Cases trend by
quarter and quarter" by updating it to a clear phrase such as "Cases trend by
year and quarter" (or the intended wording); locate the comment string "# Cases
trend by quarter and quarter" in
apps/customer-portal/backend/modules/entity/types.bal and replace it with the
corrected comment to reflect year and quarter.
---
Outside diff comments:
In `@apps/customer-portal/backend/modules/entity/types.bal`:
- Around line 332-356: Delete the unused record type declarations
ActiveCaseCount and OutstandingCasesCount from the codebase: remove the entire
public type definitions (including their fields like workInProgress,
waitingOnClient, waitingOnWso2, total, and medium/high/critical/total with
json... rest) so no dead type definitions remain; also search for any unused
imports or comments that referenced these exact type names (ActiveCaseCount,
OutstandingCasesCount) and remove them to avoid leftover clutter.
---
Nitpick comments:
In `@apps/customer-portal/backend/service.bal`:
- Around line 583-592: Response currently includes both openCases (derived from
stateCount.get(STATE_OPEN).count) and the full stateCount map, causing the
"Open" count to appear twice; to fix, remove stateCount from the returned object
(or alternatively, if the full map must be returned, delete the STATE_OPEN entry
from stateCount before returning) — update the return block that constructs the
response object (the code around stateCount and openCases) so only one
representation of the "Open" count is included.
- Around line 441-442: The code currently treats a missing "Open" key as an
InternalServerError when processing case stats (e.g., around
entity:getCaseStatsForProject and ProjectCaseStatsResponse handling and the
STATE_OPEN guard); change the guard logic to treat a missing "Open" entry as 0
instead of failing: when reading the map/record that should contain "Open" (the
STATE_OPEN branch/guard), coerce/lookup the value with a safe-default (0) and
continue normal processing rather than returning an InternalServerError, and
apply the same defaulting approach to the other similar blocks noted (the other
STATE_OPEN/empty-key guards around the chat/deployment stats handling).
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
apps/customer-portal/backend/utils.bal (2)
330-351:mapCaseStatspassesresolvedCasesandcasesTrendfrom the entity layer without mapping.Lines 344 and 349 assign entity-layer types directly. Other fields in this function (stateCount, severityCount, etc.) are explicitly mapped from entity types to public types. For consistency and to maintain the entity/public API boundary,
resolvedCountandcasesTrendshould also be mapped to portal-level types.This is the root cause of the type-level concern noted in
types.balwhereProjectCaseStatsreferencesentity:ResolvedCaseCountandentity:CasesTrend[].🤖 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 330 - 351, The mapCaseStats function is exposing entity-layer types for resolvedCases and casesTrend; update mapCaseStats to convert response.resolvedCount (entity:ResolvedCaseCount) into the portal-level resolvedCases type and map each element of response.casesTrend (entity:CasesTrend[]) into the portal-level casesTrend element type before returning. Locate mapCaseStats and add mapping logic similar to stateCount/severityCount: create a mapped resolvedCases value (transform fields on response.resolvedCount into the types expected by ProjectCaseStats) and create a mapped casesTrend array (map each entity:CasesTrend item to the public trend item shape), then return those mapped variables instead of passing response.resolvedCount and response.casesTrend directly.
353-357: Redundant full mapping and fragilecountaccess.Two concerns:
Efficiency:
getOpenCasesCountFromProjectCasesStatscallsmapCaseStats(performing full mapping of all fields) just to look up one entry. You could searchresponse.stateCountdirectly instead.Fragile nil handling: If a
stateCountentry withid == STATE_OPEN_IDexists but itscountfield is()(optional), this returns()— the caller inservice.bal(Line 453) treats()as "missing statistics" and returns a 500 error. This may be overly strict if the open state legitimately has zero cases but the upstream omits the count field.♻️ Proposed refactor — search entity response directly
public isolated function getOpenCasesCountFromProjectCasesStats(entity:ProjectCaseStatsResponse response) returns int? { - types:ProjectCaseStats stats = mapCaseStats(response); - types:ReferenceItem[] openCases = stats.stateCount.filter(stat => stat.id == STATE_OPEN_ID); - return openCases.length() > 0 ? openCases[0].count : (); + entity:ChoiceListItem[] openStates = response.stateCount.filter(item => item.id.toString() == STATE_OPEN_ID); + return openStates.length() > 0 ? openStates[0].count : (); }🤖 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 353 - 357, Replace the full mapping in getOpenCasesCountFromProjectCasesStats by directly searching the incoming ProjectCaseStatsResponse.stateCount for an entry with id == STATE_OPEN_ID (avoid calling mapCaseStats); if an entry is found, treat a missing or optional count as zero (return 0) instead of propagating () so callers in service.bal don't treat it as an error, and if no entry exists also return 0 (or otherwise a sensible default) so the function no longer returns () for a legitimate zero count.apps/customer-portal/backend/constants.bal (1)
37-38: Hard-codedSTATE_OPEN_ID = "1"— consider documenting or sourcing from metadata.This couples the backend to a specific entity-layer state ID. If the upstream system ever changes the "Open" state identifier, this silently breaks. A brief doc comment explaining the source of this value (e.g., ServiceNow choice list) would help future maintainers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/backend/constants.bal` around lines 37 - 38, The constant STATE_OPEN_ID = "1" is hard-coded and should be documented or loaded from metadata; add a clear doc comment above the public const STATE_OPEN_ID explaining its origin (e.g., ServiceNow choice list value for "Open" and any expected immutability), and refactor to read the value from configuration/metadata where available (e.g., from an environment variable or a shared metadata loader used elsewhere) so callers can fall back to the constant only if runtime metadata is unavailable; update any references to STATE_OPEN_ID to handle the configurable source.apps/customer-portal/backend/modules/types/types.bal (1)
168-177:entity:CasesTrend[]andentity:ResolvedCaseCountleak entity-layer types into the public API.
ProjectCaseStatsis a public-facing type, yetcasesTrendandresolvedCasesreference entity module types directly. Other aggregate fields (stateCount,severityCount, etc.) are correctly mapped totypes:ReferenceItem[]. For consistency and to decouple the public API from the entity contract, consider defining portal-level equivalents forCasesTrendandResolvedCaseCount.Based on learnings, the Customer Portal backend ensures public API types are intentionally decoupled from entity types (e.g., Comment type omits referenceId). The same principle should apply here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/backend/modules/types/types.bal` around lines 168 - 177, ProjectCaseStats currently leaks entity-layer types via the fields casesTrend (entity:CasesTrend[]) and resolvedCases (entity:ResolvedCaseCount); replace those references with portal-level equivalents (e.g., define PortalCasesTrend and PortalResolvedCaseCount in this module or the public types module) and map/populate them from the entity types at the service/mapper layer, keeping other aggregates as types:ReferenceItem[] unchanged; update ProjectCaseStats to use PortalCasesTrend[] and PortalResolvedCaseCount (or similarly named public types) so the public API no longer depends on entity:CasesTrend or entity:ResolvedCaseCount.apps/customer-portal/backend/service.bal (1)
578-578: Verify thatcaseTypesis meaningful for the/stats/supportendpoint.The
caseTypesparameter filterscaseStats, but this endpoint only usescaseStats.totalCount(Line 645). The chat stats (activeChats,sessionChats,resolvedChats) are fetched separately without any case-type filtering. Ensure the caller understands thatcaseTypesonly affectstotalCaseshere, not the chat metrics — or document this in the endpoint's doc comment.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/backend/service.bal` at line 578, The caseTypes parameter in resource function get projects/[string id]/stats/support currently only filters caseStats.totalCount while chat metrics (activeChats, sessionChats, resolvedChats) are computed without case-type filtering; either remove caseTypes from the signature, or update the endpoint doc comment to explicitly state that caseTypes only affects totalCases and not chat metrics, or apply the same case-type filter when computing chat stats by modifying the chat-fetching logic (the code paths producing activeChats, sessionChats, resolvedChats) to accept and use caseTypes so all returned metrics are consistently filtered.
🤖 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/constants.bal`:
- Around line 28-29: Replace the inline log literal "Open cases count is missing
in the project case statistics response!" in service.bal with the existing
constant ERR_MSG_OPEN_CASES_MISSING from constants.bal so the code references
the defined symbol instead of a duplicated string; alternatively, if you prefer
to keep the inline message, remove the unused constant
ERR_MSG_OPEN_CASES_MISSING from constants.bal to avoid dead code. Ensure you
import or reference the constant the same way other constants are used in
service.bal so compilation is unchanged.
In `@apps/customer-portal/backend/service.bal`:
- Around line 452-460: The log and response use mismatched messages when
openCasesCount is missing: replace the inline string in the log and the generic
ERR_MSG_CASES_STATISTICS_MISSING in the returned body with the dedicated
constant ERR_MSG_OPEN_CASES_MISSING so both the log (log:printError call) and
the returned http:InternalServerError body use ERR_MSG_OPEN_CASES_MISSING;
locate the check around getOpenCasesCountFromProjectCasesStats and the
openCasesCount variable to update the two places accordingly.
---
Duplicate comments:
In `@apps/customer-portal/backend/modules/entity/types.bal`:
- Around line 370-377: The review note is a duplicate and no code change is
required for the CasesTrend record; leave the type definition (public type
CasesTrend record {| string period; ChoiceListItem[] severities; json...; |};)
as-is and remove the duplicate review marker/placeholder ([duplicate_comment])
from the PR or review metadata so only the resolved comment remains.
---
Nitpick comments:
In `@apps/customer-portal/backend/constants.bal`:
- Around line 37-38: The constant STATE_OPEN_ID = "1" is hard-coded and should
be documented or loaded from metadata; add a clear doc comment above the public
const STATE_OPEN_ID explaining its origin (e.g., ServiceNow choice list value
for "Open" and any expected immutability), and refactor to read the value from
configuration/metadata where available (e.g., from an environment variable or a
shared metadata loader used elsewhere) so callers can fall back to the constant
only if runtime metadata is unavailable; update any references to STATE_OPEN_ID
to handle the configurable source.
In `@apps/customer-portal/backend/modules/types/types.bal`:
- Around line 168-177: ProjectCaseStats currently leaks entity-layer types via
the fields casesTrend (entity:CasesTrend[]) and resolvedCases
(entity:ResolvedCaseCount); replace those references with portal-level
equivalents (e.g., define PortalCasesTrend and PortalResolvedCaseCount in this
module or the public types module) and map/populate them from the entity types
at the service/mapper layer, keeping other aggregates as types:ReferenceItem[]
unchanged; update ProjectCaseStats to use PortalCasesTrend[] and
PortalResolvedCaseCount (or similarly named public types) so the public API no
longer depends on entity:CasesTrend or entity:ResolvedCaseCount.
In `@apps/customer-portal/backend/service.bal`:
- Line 578: The caseTypes parameter in resource function get projects/[string
id]/stats/support currently only filters caseStats.totalCount while chat metrics
(activeChats, sessionChats, resolvedChats) are computed without case-type
filtering; either remove caseTypes from the signature, or update the endpoint
doc comment to explicitly state that caseTypes only affects totalCases and not
chat metrics, or apply the same case-type filter when computing chat stats by
modifying the chat-fetching logic (the code paths producing activeChats,
sessionChats, resolvedChats) to accept and use caseTypes so all returned metrics
are consistently filtered.
In `@apps/customer-portal/backend/utils.bal`:
- Around line 330-351: The mapCaseStats function is exposing entity-layer types
for resolvedCases and casesTrend; update mapCaseStats to convert
response.resolvedCount (entity:ResolvedCaseCount) into the portal-level
resolvedCases type and map each element of response.casesTrend
(entity:CasesTrend[]) into the portal-level casesTrend element type before
returning. Locate mapCaseStats and add mapping logic similar to
stateCount/severityCount: create a mapped resolvedCases value (transform fields
on response.resolvedCount into the types expected by ProjectCaseStats) and
create a mapped casesTrend array (map each entity:CasesTrend item to the public
trend item shape), then return those mapped variables instead of passing
response.resolvedCount and response.casesTrend directly.
- Around line 353-357: Replace the full mapping in
getOpenCasesCountFromProjectCasesStats by directly searching the incoming
ProjectCaseStatsResponse.stateCount for an entry with id == STATE_OPEN_ID (avoid
calling mapCaseStats); if an entry is found, treat a missing or optional count
as zero (return 0) instead of propagating () so callers in service.bal don't
treat it as an error, and if no entry exists also return 0 (or otherwise a
sensible default) so the function no longer returns () for a legitimate zero
count.
4394de1 to
1332164
Compare
76100c1 to
acdb8c9
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (4)
apps/customer-portal/backend/service.bal (2)
534-544: UseERR_MSG_CASES_STATISTICS_MISSINGconstant instead of the inline duplicate.Line 537 declares an inline string
"Failed to retrieve project case statistics."that is identical to theERR_MSG_CASES_STATISTICS_MISSINGconstant defined inconstants.balline 28 and already used at line 444.♻️ Proposed fix
- if caseStats is error { - string customError = "Failed to retrieve project case statistics."; - log:printError(customError, caseStats); + if caseStats is error { + log:printError(ERR_MSG_CASES_STATISTICS_MISSING, caseStats); return <http:InternalServerError>{ body: { - message: customError + message: ERR_MSG_CASES_STATISTICS_MISSING } }; }🤖 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 534 - 544, Replace the inline error message string used when getCaseStatsForProject returns an error with the existing ERR_MSG_CASES_STATISTICS_MISSING constant; specifically, in the block handling the result of entity:getCaseStatsForProject (variable caseStats), remove the duplicate literal "Failed to retrieve project case statistics." and use ERR_MSG_CASES_STATISTICS_MISSING for the customError (and in the response body message) so the code reuses the constant already defined in constants.bal.
593-614: Same inline string duplication as/stats/casesendpoint.Line 598 uses the inline string
"Failed to retrieve project case statistics."— same as theERR_MSG_CASES_STATISTICS_MISSINGconstant.♻️ Proposed fix
- log:printError("Failed to retrieve project case statistics.", caseStats); + log:printError(ERR_MSG_CASES_STATISTICS_MISSING, caseStats);🤖 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 593 - 614, Replace the duplicated inline error strings in the project stats retrieval with the existing error constants: use ERR_MSG_CASES_STATISTICS_MISSING in the log:printError call that checks caseStats (identifier caseStats) instead of the literal "Failed to retrieve project case statistics.", and similarly replace the chat error literal in the chatStats error branch with the corresponding constant ERR_MSG_CHAT_STATISTICS_MISSING (identifier chatStats). Ensure the constants are in scope (add or export/import them if needed) and keep passing the error object as the second argument to log:printError.apps/customer-portal/backend/utils.bal (1)
359-362: Avoid callingmapCaseStatsjust to extractstateCount.
getOpenCasesCountFromProjectCasesStatscalls the fullmapCaseStats, which mapsseverityCount,outstandingSeverityCount,caseTypeCount, andcasesTrend— all unnecessary for extracting the open cases count. Consider filteringresponse.stateCountdirectly:♻️ Proposed refactor
public isolated function getOpenCasesCountFromProjectCasesStats(entity:ProjectCaseStatsResponse response) returns int? { - types:ProjectCaseStats stats = mapCaseStats(response); - types:ReferenceItem[] openCases = stats.stateCount.filter(stat => stat.id == stateIdOpen.toString()); - return openCases.length() > 0 ? openCases[0].count : (); + entity:ChoiceListItem[] openStates = response.stateCount.filter(item => item.id == stateIdOpen); + return openStates.length() > 0 ? openStates[0].count : (); }🤖 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 359 - 362, getOpenCasesCountFromProjectCasesStats currently calls mapCaseStats unnecessarily; instead read and filter the response.stateCount directly to avoid mapping all fields. Replace the mapCaseStats call in getOpenCasesCountFromProjectCasesStats with logic that accesses response.stateCount, filter entries where id == stateIdOpen.toString(), and return the matching count or nil; keep using types:ProjectCaseStatsResponse and stateIdOpen to locate the correct value.apps/customer-portal/backend/modules/types/types.bal (1)
168-178:casesTrendexposes entity-layer type withjson...rest fields in the public API.
entity:CasesTrend[](and its nestedentity:ChoiceListItem[]) both includejson...rest descriptors. Any unexpected fields from the upstream entity service will pass through to API consumers. The same pattern already exists forentity:ResolvedCaseCountat line 167, so this appears to be an accepted convention in this codebase — but worth being aware of if data minimization becomes a concern later.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/backend/modules/types/types.bal` around lines 168 - 178, The public type exposes upstream entity rest fields because casesTrend is declared as entity:CasesTrend[] (and its nested entity:ChoiceListItem[]), so change the API surface to use a sanitized DTO type (e.g., CasesTrendDTO and ChoiceListItemDTO) that omits the `json...` rest descriptors instead of entity:CasesTrend and entity:ChoiceListItem; update the declaration of casesTrend to use the DTO type and ensure any place that maps data from entity:CasesTrend (and entity:ChoiceListItem) performs an explicit transform/copy to the DTO to avoid leaking unexpected fields (follow the same pattern used for entity:ResolvedCaseCount if present).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/customer-portal/backend/modules/types/types.bal`:
- Around line 168-178: The public type exposes upstream entity rest fields
because casesTrend is declared as entity:CasesTrend[] (and its nested
entity:ChoiceListItem[]), so change the API surface to use a sanitized DTO type
(e.g., CasesTrendDTO and ChoiceListItemDTO) that omits the `json...` rest
descriptors instead of entity:CasesTrend and entity:ChoiceListItem; update the
declaration of casesTrend to use the DTO type and ensure any place that maps
data from entity:CasesTrend (and entity:ChoiceListItem) performs an explicit
transform/copy to the DTO to avoid leaking unexpected fields (follow the same
pattern used for entity:ResolvedCaseCount if present).
In `@apps/customer-portal/backend/service.bal`:
- Around line 534-544: Replace the inline error message string used when
getCaseStatsForProject returns an error with the existing
ERR_MSG_CASES_STATISTICS_MISSING constant; specifically, in the block handling
the result of entity:getCaseStatsForProject (variable caseStats), remove the
duplicate literal "Failed to retrieve project case statistics." and use
ERR_MSG_CASES_STATISTICS_MISSING for the customError (and in the response body
message) so the code reuses the constant already defined in constants.bal.
- Around line 593-614: Replace the duplicated inline error strings in the
project stats retrieval with the existing error constants: use
ERR_MSG_CASES_STATISTICS_MISSING in the log:printError call that checks
caseStats (identifier caseStats) instead of the literal "Failed to retrieve
project case statistics.", and similarly replace the chat error literal in the
chatStats error branch with the corresponding constant
ERR_MSG_CHAT_STATISTICS_MISSING (identifier chatStats). Ensure the constants are
in scope (add or export/import them if needed) and keep passing the error object
as the second argument to log:printError.
In `@apps/customer-portal/backend/utils.bal`:
- Around line 359-362: getOpenCasesCountFromProjectCasesStats currently calls
mapCaseStats unnecessarily; instead read and filter the response.stateCount
directly to avoid mapping all fields. Replace the mapCaseStats call in
getOpenCasesCountFromProjectCasesStats with logic that accesses
response.stateCount, filter entries where id == stateIdOpen.toString(), and
return the matching count or nil; keep using types:ProjectCaseStatsResponse and
stateIdOpen to locate the correct value.
9782176 to
83027b8
Compare
| } | ||
| }; | ||
| log:printError(ERR_MSG_CASES_STATISTICS, caseStats); | ||
| // To return other stats even if case stats retrieval fails, error will not be returned. |
There was a problem hiding this comment.
nitpick: These comments are not actually necessary. The code is already self-explanatory.
There was a problem hiding this comment.
Ack, will update this in a later PR.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
apps/customer-portal/backend/service.bal (3)
395-400: Add# + caseTypesdoc annotation for the new parameter (applies to all three changed endpoints).All three resource functions —
stats(Line 399),stats/cases(Line 493), andstats/support(Line 552) — gain thestring[]? caseTypesparameter but their Ballerina doc-comment blocks omit the corresponding# + caseTypes - ...entry. Ballerina's standard convention requires a# + paramName - <description>line for every resource function parameter.📝 Proposed doc annotation (same pattern for all three)
# Get overall project statistics by ID. # # + id - ID of the project +# + caseTypes - Optional list of case type identifiers to filter case statistics # + return - Project statistics response or error resource function get projects/[string id]/stats(http:RequestContext ctx, string[]? caseTypes)🤖 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 395 - 400, Add a Ballerina doc annotation for the new optional parameter in each updated resource function: in resource function get projects/[string id]/stats (function signature with string[]? caseTypes), resource function handling get projects/[string id]/stats/cases, and resource function get projects/[string id]/stats/support, add a line like "# + caseTypes - Optional array of case type identifiers to filter the returned statistics" to each doc-comment block so every parameter has a corresponding "# + paramName - ..." entry.
456-468: Extract deployment/activity stat error strings to constants for consistency.Lines 459 and 466 log inline strings, while the adjacent case and chat stat failures (lines 444 and 451) now use named constants. If these strings are used only once, inline is fine, but extracting them to
constants.balwould be consistent with the pattern introduced in this PR for case/chat stats.📝 Suggested additions to constants.bal
+const ERR_MSG_DEPLOYMENT_STATISTICS = "Failed to retrieve project deployment statistics."; +const ERR_MSG_PROJECT_ACTIVITY_STATISTICS = "Failed to retrieve project activity statistics.";- log:printError("Failed to retrieve project deployment statistics.", deploymentStats); + log:printError(ERR_MSG_DEPLOYMENT_STATISTICS, deploymentStats); - log:printError("Failed to retrieve project activity statistics.", projectActivityStats); + log:printError(ERR_MSG_PROJECT_ACTIVITY_STATISTICS, projectActivityStats);🤖 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 456 - 468, Extract the two inline log message strings used with log:printError when handling deploymentStats (result of entity:getDeploymentStatsForProject) and projectActivityStats (result of entity:getProjectActivityStats) into named constants in constants.bal, following the same naming/pattern used for case/chat stat errors; then replace the inline literals in the error branches that check deploymentStats and projectActivityStats with those new constants so both log:printError calls reference the constants instead of hard-coded strings.
472-473:openCases: ()conflates two distinct states — consider returning0for "no open cases" vs()for "stats unavailable".When
getOpenCasesCountFromProjectCasesStatsreturns()becauseSTATE_OPENis absent from thestateCountmap (i.e., data was fetched successfully but there are literally no open cases), the caller receives the samenullit would see when the entirecaseStatsfetch failed. If the frontend needs to distinguish between "0 open cases" and "could not retrieve stats", consider returning0as the fallback fromgetOpenCasesCountFromProjectCasesStatsinstead of().🤖 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 472 - 473, The current logic makes openCases indistinguishable between "no data" and "zero open cases" because getOpenCasesCountFromProjectCasesStats returns () when STATE_OPEN is absent; change getOpenCasesCountFromProjectCasesStats to return 0 (an int) as the fallback when the stateCount map has no STATE_OPEN entry while keeping the function return type compatible with the caller, and leave the outer conditional (openCases: caseStats is entity:ProjectCaseStatsResponse ? ...) to return () only when caseStats itself is missing/unavailable so the frontend can distinguish "0 open cases" from "stats unavailable".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/customer-portal/backend/service.bal`:
- Around line 395-400: Add a Ballerina doc annotation for the new optional
parameter in each updated resource function: in resource function get
projects/[string id]/stats (function signature with string[]? caseTypes),
resource function handling get projects/[string id]/stats/cases, and resource
function get projects/[string id]/stats/support, add a line like "# + caseTypes
- Optional array of case type identifiers to filter the returned statistics" to
each doc-comment block so every parameter has a corresponding "# + paramName -
..." entry.
- Around line 456-468: Extract the two inline log message strings used with
log:printError when handling deploymentStats (result of
entity:getDeploymentStatsForProject) and projectActivityStats (result of
entity:getProjectActivityStats) into named constants in constants.bal, following
the same naming/pattern used for case/chat stat errors; then replace the inline
literals in the error branches that check deploymentStats and
projectActivityStats with those new constants so both log:printError calls
reference the constants instead of hard-coded strings.
- Around line 472-473: The current logic makes openCases indistinguishable
between "no data" and "zero open cases" because
getOpenCasesCountFromProjectCasesStats returns () when STATE_OPEN is absent;
change getOpenCasesCountFromProjectCasesStats to return 0 (an int) as the
fallback when the stateCount map has no STATE_OPEN entry while keeping the
function return type compatible with the caller, and leave the outer conditional
(openCases: caseStats is entity:ProjectCaseStatsResponse ? ...) to return ()
only when caseStats itself is missing/unavailable so the frontend can
distinguish "0 open cases" from "stats unavailable".
3ef87da
into
wso2-open-operations:customer-portal-milestone-1
Summary
This PR enhances the
/projects/{id}/stats/casesendpoint to return additional statistical data, including case trends.Changes
Reason
The existing endpoint returned limited statistical information. The UI and reporting views require:
By extending this endpoint, we reduce the need for multiple API calls and centralize case statistics under a single endpoint.
Testing
Impact
Related PRs
Summary by CodeRabbit
New Features
Bug Fixes
Chores