[Customer Portal] Add cases search and remove unused methods - #11
Conversation
Rashmika998
added
Type/Improvement
📝 WalkthroughWalkthroughThis pull request consolidates case and project retrieval functionality in the customer portal backend. It removes separate endpoints for project details, overview, case filters, and individual case details, replacing them with a unified Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes 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
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 review |
✅ Actions performedReview triggered.
|
![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
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/customer-portal/backend/modules/entity/entity.balapps/customer-portal/backend/modules/entity/types.balapps/customer-portal/backend/service.balapps/customer-portal/backend/tests/service_test.bal
🔇 Additional comments (6)
apps/customer-portal/backend/modules/entity/types.bal (3)
104-134: LGTM! Well-structured Case record with proper field documentation.The restructured
Caserecord with canonical fields (caseId,projectId,caseType,state,stateLabel) and optional detail fields (priority,priorityLabel,deployment,deploymentId) provides a clean, unified model.
136-144: Consider consistency in filter field types.
statusandseverityareintwhiledeploymentisstring. This is likely intentional based on backend data model, but verify that this aligns with the actual filter values expected by the downstream service.
146-172: LGTM! Clean composition using record spreading.The use of
*PaginationinCasesResponseand*CaseSearchPayloadinCaseRequestBodyprovides clean type composition. Optionalpaginationfield allows clients to rely on defaults.apps/customer-portal/backend/modules/entity/entity.bal (1)
35-52: LGTM! Clean refactoring of case search functionality.The
searchCasesfunction properly:
- Accepts
CaseSearchPayloadas input, providing a clean API for callers- Internally constructs
CaseRequestBodywithprojectIdsarray wrapping the single project ID- Spreads all payload fields (
caseTypes,pagination,filters,sortBy) into the request bodyThe endpoint change from
/projects/[projectId]/cases/search.postto/cases/search.postwithprojectIdsin the body aligns with a more flexible multi-project search architecture.apps/customer-portal/backend/service.bal (1)
140-178: LGTM! Well-implemented case search endpoint with proper validation.The implementation correctly:
- Validates authorization before processing
- Validates project ID is not empty or whitespace (line 157-165)
- Delegates to
entity:searchCaseswith proper parameters- Follows consistent error handling pattern with other endpoints
apps/customer-portal/backend/tests/service_test.bal (1)
36-45: LGTM!The formatting adjustment improves readability.
3969fa5 to
f022abc
Compare
48d98ac to
2377f24
Compare
c519185 to
df5f5b3
Compare
0fb9d99 to
1451eef
Compare
| select { | ||
| id: case.id, | ||
| projectId: case.projectId, | ||
| 'type: case.'type, | ||
| number: case.number, | ||
| createdOn: case.createdOn, | ||
| assignedEngineer: case.assignedEngineer, | ||
| title: case.title, | ||
| description: case.description, | ||
| severity: case.severity, | ||
| status: case.state, | ||
| deploymentId: case.deploymentId | ||
| }; |
There was a problem hiding this comment.
Checked with Ballerina team if we can get this simplified. FYI.
https://discord.com/channels/957996897782616114/1461387904168366244
There was a problem hiding this comment.
@shayanmalinda, I can't access the Discord conversation, but I can share my thoughts on the current code pattern!
The query expression you're using performs explicit field mapping from entity:Case to Case. While this approach is clear and type-safe, it's quite verbose since most fields are direct copies, with only one transformation (state → status).
Potential simplification approaches:
- Record spread/rest pattern - If Ballerina supports it, you might use spread operators to copy matching fields and only specify differences
- Type compatibility - If the types are structurally similar, direct assignment or casting might work
- Helper function - Extract the mapping to a reusable conversion function
- Type unification - If possible, align the
entity:CaseandCasetypes more closely to reduce mapping
Current code benefits:
- Explicit field mapping is maintainable
- Easy to see all transformations
- Type-safe at compile time
Could you share what the Ballerina team suggested? That would help me provide more specific feedback on whether the simplification is worth pursuing or if there are any trade-offs to consider.
(/◕ヮ◕)/
There was a problem hiding this comment.
@coderabbitai Does Ballerina support the spread operator (...) inside query expression select clauses to copy all fields and override specific ones?
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Ballerina spread operator query expression select clause
💡 Result:
Yes — you can use the spread operator (...expr) inside a query expression's select clause to expand a record/mapping or list into the constructed value. The select clause simply evaluates an expression for each frame; that expression may be a mapping/list/record constructor that uses the spread operator. See the language docs for the spread operator and for query select semantics. [1][2]
Example (record spread in select):
type Person record {|
string first;
string last;
int age;
|};
Person[] people = [{first:"A", last:"A", age:30}, {first:"B", last:"B", age:25}];
// Project and spread existing record fields plus add a new field
var res = from var p in people
select {...p, fullName: p.first + " " + p.last};
res will be a Person[]-compatible list of records where each record contains all fields from p plus fullName. [1][2]
References:
- Spread operator (record/list/mapping) in Ballerina. [1]
- Query expressions — select clause semantics. [2]
@shayanmalinda, yes! Ballerina does support the spread operator (...) inside query expression select clauses. You can spread a record and override specific fields.
Here's how you could simplify your code:
Current verbose approach:
Case[] cases = from entity:Case case in casesResponse.cases
select {
id: case.id,
projectId: case.projectId,
'type: case.'type,
number: case.number,
createdOn: case.createdOn,
assignedEngineer: case.assignedEngineer,
title: case.title,
description: case.description,
severity: case.severity,
status: case.state,
deploymentId: case.deploymentId
};Simplified with spread operator:
Case[] cases = from entity:Case case in casesResponse.cases
select {
...case,
status: case.state
};This spreads all fields from case and overrides just the status field with the value from case.state. Much cleaner! 🎉
Note: This assumes that entity:Case and Case have compatible field names (except for the state/status difference). If entity:Case has a state field that you want to exclude, you'll need to ensure your Case type doesn't include it, or the spread will copy both state and status.
There was a problem hiding this comment.
Are you sure this will work? Can you point me out to a official documentation where this is being mentioned?
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Ballerina official documentation spread operator query select
💡 Result:
Short answer: Ballerina query expressions use a select clause that evaluates an expression per frame and emits values — there is no language-level "spread operator" for expanding arrays into multiple select outputs in Ballerina queries (unlike some SQL engines that have a spread/expansion operator). See the Ballerina language specification for the select-clause semantics. [1][2]
Sources:
- Ballerina Language Specification — Select clause / Query expression semantics. [1]
- Ballerina language spec (alternate release page showing query expression overview). [2]
(For comparison, some SQL engines (e.g., Snowflake) provide a spread/expansion operator; see Snowflake docs.) [3]
References:
[1] Ballerina Language Specification — Select clause.
[2] Ballerina Language Specification — Query expression overview.
[3] Snowflake: Expansion (spread) operator.
@shayanmalinda, I apologize for my overconfidence in my previous response. After searching more thoroughly, I need to correct myself.
What IS documented:
The spread operator (...x) is supported in mapping constructors (record construction), and the expression in a select clause can be a mapping constructor expression.
What I CANNOT find:
I cannot find official documentation or examples that explicitly demonstrate using the spread operator pattern {...record, field: value} inside a query expression's select clause specifically. All the examples I found show either:
- Spread operator in regular mapping constructors (outside queries)
- Query select clauses with explicit field mapping
- Query select clauses with simple record constructors
Language spec considerations:
The Ballerina spec notes that "static typing of mapping constructor expr will need to be refined to work as expected in the select-clause", which suggests there may be nuances or limitations in how mapping constructors behave within select clauses.
My recommendation:
Given that you've already reached out to the Ballerina team on Discord, I'd suggest:
- Test it - Try the simplified pattern in your code to see if it compiles and works as expected
- Wait for the team's response - They can provide authoritative guidance on whether this pattern is supported
I apologize for the confusion. I should have been more careful before suggesting this as a definitive solution.
shayanmalinda
left a comment
shayanmalinda
left a comment
There was a problem hiding this comment.
Let's address the mapping logic improvement later on as per the feedback of the Ballerina team.
26b735e
into
wso2-open-operations:customer-portal-milestone-1
Description
Closes #34, #37
Related PRs
Summary by CodeRabbit
Release Notes
Breaking Changes
API Updates
✏️ Tip: You can customize this high-level summary in your review settings.