[Customer Portal] Add stats endpoints - #42
shayanmalinda merged 12 commits into
Conversation
Rashmika998
added
Type/Improvement
📝 WalkthroughWalkthroughAdds four new project statistics retrieval functions to the entity module and three corresponding REST endpoints in the service layer that fetch and aggregate case, chat, deployment, and activity statistics for a project. Includes new types for stats responses and updates ID validation logic across endpoints. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Service as Service Layer
participant Entity as Entity Module
participant Backend as Backend APIs
Client->>Service: GET /projects/[id]/stats
Service->>Service: Validate idToken & ID
Service->>Service: Check user access to project
Service->>Entity: getCaseStatsForProject(idToken, id)
Entity->>Backend: GET /projects/[id]/cases/stats
Backend-->>Entity: ProjectCaseStatsResponse
Service->>Entity: getChatStatsForProject(idToken, id)
Entity->>Backend: GET /projects/[id]/chats/stats
Backend-->>Entity: ProjectChatStatsResponse
Service->>Entity: getDeploymentStatsForProject(idToken, id)
Entity->>Backend: GET /projects/[id]/deployments/stats
Backend-->>Entity: ProjectDeploymentStatsResponse
Service->>Entity: getProjectActivityStats(idToken, id)
Entity->>Backend: GET /projects/[id]/stats
Backend-->>Entity: ProjectStatsResponse
Service->>Service: Aggregate stats into response
Service-->>Client: ProjectStatsResponse (with projectStats + recentActivity)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
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. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/customer-portal/backend/service.bal`:
- Around line 216-219: The cache lookup using getCaseStatsFromCache(id) (and the
similar cache calls around lines 269-273 and 364-371) returns project-scoped
data without verifying the requesting user's access; update the cache key and
helper signatures to include the effective userId/tenantId (or session context)
and/or perform an authorization check before returning cached data so that
cachedCaseStats cannot be returned for a different user/tenant. Locate and
modify getCaseStatsFromCache (and the support/health cache helpers) to accept
the user/tenant identifier, update callers to pass that context, and ensure you
either scope cache keys by user/tenant or validate project access after a cache
hit before returning the cached ProjectCaseStats.
🧹 Nitpick comments (1)
apps/customer-portal/backend/types.bal (1)
121-158: Fix typo in public type name (ProjectHealthtStats).This name will leak into public schemas; better to correct now to avoid a breaking rename later. Please rename to
ProjectHealthStatsand update usages (e.g., the stats/health endpoint return type).♻️ Proposed rename
-public type ProjectHealthtStats record {| +public type ProjectHealthStats record {| # Open cases count int openCases; # Active chats count int activeChats; # Deployments count int deployments; |};
6e1547c to
4e9db6b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/customer-portal/backend/utils.bal`:
- Around line 236-241: The logForbiddenProjectAccess function currently logs raw
email (variable email) via log:printWarn, which exposes PII; change it to avoid
raw emails by replacing email with a non-PII identifier (e.g., a masked version
or a stable hash) before logging. Locate logForbiddenProjectAccess and update
the log:printWarn call to use a helper (e.g., maskEmail or hashEmail) that
either redacts local-part (userpart → ****@domain) or returns a deterministic
hash, or else log only a user id if available; ensure the helper is implemented
and tested so the warning includes project id (id) and the masked/hashed
identifier instead of the raw email.
🧹 Nitpick comments (2)
apps/customer-portal/backend/service.bal (2)
327-337: Warm support cache on derived responseWhen case stats are cached, you return support stats but skip writing support cache, so repeated calls still re-fetch chat stats. Consider caching before returning.
♻️ Suggested tweak
if cachedCaseStats is ProjectCaseStats { + _ = updateSupportStatsCache(id, chatStats, cachedCaseStats.totalCases); return { totalCases: cachedCaseStats.totalCases, activeChats: chatStats.activeCount, sessionChats: chatStats.sessionCount, resolvedChats: chatStats.resolvedCount }; }
431-458: Warm health cache on early returnsWhen you compute health stats from cached case/support data, the health cache isn’t populated, reducing cache hit rate. Consider updating it before returning.
♻️ Suggested tweak
if cachedCaseStats is ProjectCaseStats { if cachedSupportStats is ProjectSupportStats { + _ = updateProjectHealthStatsCache(id, cachedCaseStats.openCases, + cachedSupportStats.activeChats, deploymentStats); return { openCases: cachedCaseStats.openCases, activeChats: cachedSupportStats.activeChats, deployments: deploymentStats.totalCount }; } @@ _ = updateSupportStatsCache(id, chatStats, cachedCaseStats.totalCases); + _ = updateProjectHealthStatsCache(id, cachedCaseStats.openCases, + chatStats.activeCount, deploymentStats); return { openCases: cachedCaseStats.openCases, activeChats: chatStats.activeCount, deployments: deploymentStats.totalCount }; }
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/customer-portal/backend/service.bal`:
- Around line 431-458: The cached-return branches in the function (when
cachedCaseStats is ProjectCaseStats and cachedSupportStats is
ProjectSupportStats, and the later branch after fetching chatStats) do not call
updateProjectHealthStatsCache, so project health is not recorded on partial
cache-hits; modify both return paths to call updateProjectHealthStatsCache with
the combined health object (openCases from cachedCaseStats.openCases,
activeChats from cachedSupportStats.activeChats or chatStats.activeCount, and
deployments from deploymentStats.totalCount) before returning, and in the branch
that fetches chat stats also ensure updateSupportStatsCache(id, chatStats,
cachedCaseStats.totalCases) is preserved as currently implemented.
- Around line 328-337: When cachedCaseStats is present (result of
getCaseStatsFromCache), the code returns early and never populates the
support-stats cache; modify the early-return branch so it calls
updateSupportStatsCache with the same identifiers/values used for live caching
(e.g., pass id and the computed chatStats/support payload) before returning.
Locate the cachedCaseStats check and ensure updateSupportStatsCache(...) is
invoked with chatStats (or the constructed support stats object) and the project
id prior to the return so subsequent requests hit the support-stats cache.
♻️ Duplicate comments (1)
apps/customer-portal/backend/utils.bal (1)
236-241: Avoid logging raw email (PII).This logs the user's email address, which can violate privacy/retention policies. Consider masking or using a non-PII identifier.
🧹 Nitpick comments (2)
apps/customer-portal/backend/utils.bal (1)
136-150: Redundantreturn;statement.The explicit
return;at line 149 is unnecessary in Ballerina for functions with no return type. The same applies to the other cache update functions (lines 186, 223).♻️ Suggested cleanup
if cacheError is error { log:printWarn(string `Error writing case stats of project: ${projectId} to cache`, cacheError); } - return; }apps/customer-portal/backend/service.bal (1)
196-263: Consider extracting common authorization and validation logic.The three new stats endpoints share identical patterns for:
- Extracting and validating
userInfofrom context- Validating the project ID
- Verifying project access with forbidden/error handling
This duplication could be reduced by extracting a helper function that returns the validated
userInfoor an appropriate error response.♻️ Example helper signature
# Validate request context and project access. # # + ctx - HTTP request context # + projectId - Project ID to validate # + return - UserDataPayload on success, or appropriate error response isolated function validateProjectAccess(http:RequestContext ctx, string projectId) returns authorization:UserDataPayload|http:BadRequest|http:Forbidden|http:InternalServerError { // Extract userInfo, validate ID, verify project access // Return userInfo on success or appropriate HTTP error }
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/authorization/types.bal (1)
18-25: Schema validation returns 500 on missinguseridclaim—should handle as 401/403.This file is new, so there are no existing tokens to break. However, the required
useridfield will cause JWT validation to fail with a 500 error ("Malformed JWT payload!") if the claim is missing from any token, rather than returning a proper 401/403 authentication error. Makeuseridoptional and enforce presence at the interceptor boundary to distinguish between malformed vs. unauthorized requests.Suggested change
- string userid; + string userid?;Also note the naming inconsistency:
CustomJwtPayloadusesuseridwhileUserDataPayloadusesuserId(camelCase). Consider standardizing to one convention across both types.
🤖 Fix all issues with AI agents
In `@apps/customer-portal/backend/modules/authorization/authorization.bal`:
- Around line 57-62: The code currently assumes payloadData.userid exists when
constructing UserDataPayload; confirm the IdP claim name (e.g., "userid" vs
"sub" vs "user_id") and update the code that builds UserDataPayload (the
payloadData access and the UserDataPayload initializer) to first check for the
presence of an identifier claim, prefer the canonical claim (try
payloadData.userid, then payloadData.sub, then payloadData.user_id), and if none
exist return an appropriate error (401/403) rather than proceeding; ensure any
downstream call that uses UserDataPayload (including cloneWithType) only runs
after this validation so you never propagate an empty identifier.
fa4bc2e to
c7bbbef
Compare
73e6ae2 to
29f1521
Compare
7915fa5 to
655e8c3
Compare
00b0c52 to
ff4468c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/customer-portal/backend/utils.bal`:
- Around line 83-87: The documentation for isEmptyId is misleading: it currently
says "True if valid" but the function returns true when the ID is
empty/whitespace. Update the comment above public isolated function
isEmptyId(string id) to state that the function returns true if the ID is empty
or contains only whitespace (i.e., is invalid) and false otherwise; also adjust
the short description to reflect that it checks for an empty ID.
🧹 Nitpick comments (1)
apps/customer-portal/backend/service.bal (1)
184-189: Use ERR_MSG_PROJECT_ID_EMPTY for consistency across project endpoints.These branches still return hardcoded strings while new endpoints use the constant. Aligning them simplifies maintenance and keeps messages consistent.
♻️ Suggested change (apply to all shown occurrences)
- return <http:BadRequest>{ - body: { - message: "Project ID cannot be empty or whitespace" - } - }; + return <http:BadRequest>{ + body: { + message: ERR_MSG_PROJECT_ID_EMPTY + } + };Also applies to: 528-534, 566-572
1a93776 to
dfd7faa
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/customer-portal/backend/utils.bal (1)
98-106: Fix copy-paste error in doc comment.The doc comment says "Get HTTP status code" but the function is
extractErrorMessage. This appears to be copied fromgetStatusCode.✏️ Suggested fix
-# Get HTTP status code from the given error. +# Extract the error message from the given error. # # + err - Error to handle # + return - Error message public isolated function extractErrorMessage(error err) returns string {apps/customer-portal/backend/service.bal (1)
266-267: Fix incomplete log message - TODO is stale.The TODO comment says "Will log the UUID once the PR
#42is merged" but this IS PR#42anduserInfo.userIdis already available. The log message is incomplete.✏️ Suggested fix
if getStatusCode(projectResponse) == http:STATUS_FORBIDDEN { - // TODO: Will log the UUID once the PR `#42` is merged - log:printWarn(string `Access to project ID: ${id} is forbidden for user:`); + logForbiddenProjectAccess(id, userInfo.userId); return <http:Forbidden>{
🧹 Nitpick comments (1)
apps/customer-portal/backend/service.bal (1)
330-377: Consider parallel fetching of independent stats.The
/statsendpoint makes four sequential backend calls (getCaseStatsForProject,getChatStatsForProject,getDeploymentStatsForProject,getProjectActivityStats) that are independent of each other. This could increase latency, especially if the backend services have high response times.Ballerina supports concurrent execution patterns that could fetch these in parallel to reduce overall response time. This is optional but could improve dashboard performance.
7f4a9fd
into
wso2-open-operations:customer-portal-milestone-1
github-project-automation
Bot
moved this from In Progress
to Done
in Customer Portal Development
3 participants
Description
This PR introduces dedicated, read-only statistics endpoints to support the Dashboard, Support, and Project Details pages in the Customer Portal web app and microapp.
These endpoints provide aggregated, project-level insights such as case metrics, chat activity, deployments, and overall project health, enabling the frontend to render summary views efficiently.
All APIs follow REST best practices and are optimized for performance.
New Endpoints
The following endpoints are implemented:
Scope
Each endpoint aggregates data from relevant internal services and entities (cases, chats, deployments, etc.) and returns summarized metrics required by the frontend.
These endpoints are read-only and optimized for dashboard-style consumption.
Metrics Provided
/stats/cases/stats/supportSLA-related statistics
/stats/healthAll the new endpoints' project access has been validated before returning the responses to the user.
Related Issues
Related PRs
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.