Repository navigation
[Customer Portal][BE] Add time cards search endpoints - #219
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:
📝 WalkthroughWalkthroughAdds server-side timecard search: new DateTime/Date types and TimeCard types, a new entity function Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Service as Service Layer
participant Entity as Entity Layer
participant Utils as Utils
Client->>Service: POST /projects/{id}/time-cards/search (TimeCardSearchPayload)
Service->>Service: Extract idToken from headers
Service->>Entity: searchTimeCards(idToken, payload)
Entity->>Entity: HTTP POST /time-cards/search (Authorization header)
Entity-->>Service: TimeCardsResponse or error
Service->>Utils: mapTimeCardSearchResponse(response)
Utils-->>Service: Mapped response (timeCards, totalRecords, limit, offset)
Service-->>Client: 200 OK or 400/401/403/500
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
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: 1
🧹 Nitpick comments (1)
apps/customer-portal/backend/modules/entity/entity.bal (1)
329-329: Doc comment says "of a project" but the search is not project-scopedThe function accepts an optional
projectIdsfilter but is not inherently project-scoped. The existing# Search cases of a project.pattern uses this phrasing when a project ID is a required parameter. Consider updating to something like# Search timecards based on the given search criteria.to avoid implying the function is project-bound.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/backend/modules/entity/entity.bal` at line 329, Update the doc comment that currently reads "# Search timecards of a project." to avoid implying a required project scope; change it to a neutral description such as "# Search timecards based on the given search criteria." Locate the doc comment immediately above the timecard search function in entity.bal (the comment string "# Search timecards of a project.") and replace it so it matches the function's optional projectIds parameter behavior and the module's existing "# Search cases..." pattern.
🤖 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`:
- Around line 1088-1090: The startDate and endDate filter fields are declared as
plain string? but should use the existing Date constraint to enforce ISO8601
validation; change the types of startDate and endDate from string? to Date?
(using the Date type defined in this file around the Date regex) so invalid
dates are rejected at validation time—update any related documentation/comments
if needed and ensure CallRequestCreatePayload.utcTimes /
CallRequestUpdatePayload.utcTimes usage is consistent with this Date type.
---
Nitpick comments:
In `@apps/customer-portal/backend/modules/entity/entity.bal`:
- Line 329: Update the doc comment that currently reads "# Search timecards of a
project." to avoid implying a required project scope; change it to a neutral
description such as "# Search timecards based on the given search criteria."
Locate the doc comment immediately above the timecard search function in
entity.bal (the comment string "# Search timecards of a project.") and replace
it so it matches the function's optional projectIds parameter behavior and the
module's existing "# Search cases..." pattern.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/customer-portal/backend/modules/types/types.bal (1)
677-684: Minor: doc comment "# Date Constraint." should be updated to match the renamed type.The
@constraintannotation at line 677 was previously associated with the oldDatetype (which carried the datetime pattern). This PR renamed that type toDateTimeat line 684, but the comment was not updated. The corresponding definition inentity/types.bal(line 921) correctly reads# Date-time constraint.✏️ Proposed fix
-# Date Constraint. +# Date-time constraint. `@constraint`:String { pattern: { value: re `^\d{4}-(0[1-9]|1[0-2])-(0[1-9]|[12]\d|3[01])T...`,🤖 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 677 - 684, Update the doc comment above the constraint to reflect the renamed type: change the existing "# Date Constraint." comment to something like "# Date-time constraint." so it matches the renamed public type DateTime and the corresponding constraint annotation (the `@constraint`:String block that precedes the public type DateTime); ensure the wording mirrors the entity/types.bal comment for consistency.
🤖 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 677-684: Update the doc comment above the constraint to reflect
the renamed type: change the existing "# Date Constraint." comment to something
like "# Date-time constraint." so it matches the renamed public type DateTime
and the corresponding constraint annotation (the `@constraint`:String block that
precedes the public type DateTime); ensure the wording mirrors the
entity/types.bal comment for consistency.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
apps/customer-portal/backend/modules/entity/types.bal (2)
1133-1140:TimeCardsResponseis missingjson...;, unlike every other paginated entity response typeAll other paginated response records in this file that spread
*Pagination—CaseSearchResponse(line 283),CommentsResponse(line 488),ProductVulnerabilitySearchResponse(line 868) — include a trailingjson...;. Without it,TimeCardsResponseis a closed record; any additional top-level field returned by the ServiceNow entity service (e.g., extra metadata or new pagination fields) will fail deserialization silently or with a runtime error.♻️ Proposed fix
public type TimeCardsResponse record {| TimeCard[] timeCards; int totalRecords; *Pagination; + json...; |};🤖 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 1133 - 1140, TimeCardsResponse is currently a closed record (missing the trailing json...;) which breaks deserialization if ServiceNow returns extra top-level fields; update the TimeCardsResponse type definition (the record that spreads *Pagination and contains TimeCard[] timeCards and int totalRecords) to include a trailing json...; so it becomes an open record like the other paginated responses (e.g., CaseSearchResponse, CommentsResponse) and will accept additional fields from the service.
921-928:DateTimeandDatetypes are duplicated identically acrossentity/types.balandtypes/types.balBoth modules independently define
DateTime(same regex, same error message) andDate(same regex, same error message). Any future change to the constraint — say, tightening the seconds handling or updating the error message — must be applied in two places. Consider whether a sharedcommonmodule could own these constraint types and be imported by both.Also applies to: 1081-1088
🤖 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 921 - 928, Duplicate string constraint types DateTime and Date are defined in multiple modules; extract them into a single shared module (e.g., create a common types module that declares and exports the public type DateTime and Date with the existing regex/constraint and messages), then remove the local duplicate definitions from the current modules (remove the DateTime/Date declarations in entity/types.bal and the other types module) and update those modules to import the shared types from the new common module so all references continue to use the same public type names.apps/customer-portal/backend/modules/types/types.bal (2)
686-692:Dateconstraint error message is less descriptive thanDateTime's
DateTimereports"Invalid date provided. Please provide a valid date value."whileDateemits only"Invalid date format."— both surface directly in API error responses. Making theDatemessage consistent and actionable would improve the API consumer experience.♻️ Proposed fix
`@constraint`:String { pattern: { value: re `^\d{4}-(0[1-9]|1[0-2])-(0[1-9]|[12]\d|3[01])$`, - message: "Invalid date format." + message: "Invalid date provided. Please provide a valid date value in YYYY-MM-DD format." } }🤖 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 686 - 692, The Date constraint's error message is too terse; update the `@constraint`:String on the Date type (the pattern constraint with value re `^\d{4}-(0[1-9]|1[0-2])-(0[1-9]|[12]\d|3[01])$`) to use the same descriptive message as DateTime (e.g. "Invalid date provided. Please provide a valid date value.") so API errors are consistent and actionable.
760-764: Anonymous inlinecaserecord — consider a named type for consistency with the entity layerThe entity module defines a dedicated
CaseAssociatedWithTimeCardnamed type for this structure. Using an anonymous inline record in the public module makes it invisible in generated documentation and harder to reference if the shape ever needs reuse. Extracting it mirrors the entity layer's approach and aligns with how other similar sub-records are handled.♻️ Proposed refactor
+# Time card case reference. +public type CaseReferenceInTimeCard record {| + *ReferenceItem; + # Case number + string number; +|}; public type TimeCard record {| ... - record {| - *ReferenceItem; - # Case number - string number; - |}? case; + CaseReferenceInTimeCard? case; json...; |};🤖 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 760 - 764, The inline anonymous record assigned to the field named `case` should be extracted into a named record type (e.g., `CaseAssociatedWithTimeCard`) to match the entity layer and make the shape reusable and documented; create a new record type declaration with the same members (embedding `*ReferenceItem` and `string number`) and replace the anonymous inline record in the `case` field with that type name so the public module exposes a named type consistent with the entity module.
🤖 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`:
- Around line 972-973: CallRequestUpdatePayload.utcTimes currently allows an
empty array; add the same array constraint used on
CallRequestCreatePayload.utcTimes so empty arrays are rejected. Update the
CallRequestUpdatePayload definition to annotate the utcTimes field with
`@constraint`:Array {minLength: 1} (matching the create payload) so validations
block [] submissions before reaching the entity service.
---
Duplicate comments:
In `@apps/customer-portal/backend/modules/types/types.bal`:
- Around line 713-714: Add the missing array-length constraint to the
CallRequestUpdatePayload.utcTimes field by annotating utcTimes (the DateTime[]
utcTimes? property in CallRequestUpdatePayload) with `@constraint`:Array
{minLength: 1} so the array cannot be empty (matching the entity module fix);
update the annotation directly above utcTimes in types.bal and keep the field
optional as currently declared.
---
Nitpick comments:
In `@apps/customer-portal/backend/modules/entity/types.bal`:
- Around line 1133-1140: TimeCardsResponse is currently a closed record (missing
the trailing json...;) which breaks deserialization if ServiceNow returns extra
top-level fields; update the TimeCardsResponse type definition (the record that
spreads *Pagination and contains TimeCard[] timeCards and int totalRecords) to
include a trailing json...; so it becomes an open record like the other
paginated responses (e.g., CaseSearchResponse, CommentsResponse) and will accept
additional fields from the service.
- Around line 921-928: Duplicate string constraint types DateTime and Date are
defined in multiple modules; extract them into a single shared module (e.g.,
create a common types module that declares and exports the public type DateTime
and Date with the existing regex/constraint and messages), then remove the local
duplicate definitions from the current modules (remove the DateTime/Date
declarations in entity/types.bal and the other types module) and update those
modules to import the shared types from the new common module so all references
continue to use the same public type names.
In `@apps/customer-portal/backend/modules/types/types.bal`:
- Around line 686-692: The Date constraint's error message is too terse; update
the `@constraint`:String on the Date type (the pattern constraint with value re
`^\d{4}-(0[1-9]|1[0-2])-(0[1-9]|[12]\d|3[01])$`) to use the same descriptive
message as DateTime (e.g. "Invalid date provided. Please provide a valid date
value.") so API errors are consistent and actionable.
- Around line 760-764: The inline anonymous record assigned to the field named
`case` should be extracted into a named record type (e.g.,
`CaseAssociatedWithTimeCard`) to match the entity layer and make the shape
reusable and documented; create a new record type declaration with the same
members (embedding `*ReferenceItem` and `string number`) and replace the
anonymous inline record in the `case` field with that type name so the public
module exposes a named type consistent with the entity module.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/customer-portal/backend/modules/types/types.bal (2)
742-746: Consider extracting the inlinecaserecord to a named type.The entity module already defines a
CaseAssociatedWithTimeCardtype. Having an anonymous inline record in the public types module is inconsistent with how the AI summary characterizes the entity layer. A named public type (e.g.,TimeCardCase) would improve readability and allow reuse.♻️ Proposed refactor
Add before
TimeCard:+# Case associated with a time card. +public type TimeCardCase record {| + *ReferenceItem; + # Case number + string number; +|};Then reference it in
TimeCard:- record {| - *ReferenceItem; - # Case number - string number; - |}? case; + TimeCardCase? case;🤖 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 742 - 746, The inline anonymous record for the field named `case` inside the `TimeCard` record should be extracted into a named public type for consistency and reuse; create a new record type (e.g., `TimeCardCase`) that mirrors the current inline shape (embedding `*ReferenceItem` and `string number`) and replace the inline `record { ... }? case;` in `TimeCard` with a nullable `TimeCardCase? case;`; align naming with the existing `CaseAssociatedWithTimeCard` concept so consumers can import and reuse the new `TimeCardCase` type across the entity layer.
736-736: Consider aligningTimeCard.statewith the API's reference type pattern.
TimeCard.stateisstring, while all other state/status fields useReferenceItem(CallRequest.state,Case.status,CreatedCase.state). Additionally, other reference fields inTimeCardare transformed to structured{id, label}objects in the mapping (approvedBy,project,case), butstateis copied directly as a plain string. Either this field should useReferenceItemto match the pattern, or its handling should be documented as an intentional exception.🤖 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` at line 736, TimeCard.state is currently a plain string but should follow the API's reference pattern: change the TimeCard.type declaration from string to ReferenceItem (use the same ReferenceItem type used by CallRequest.state/Case.status/CreatedCase.state) and update the mapping that transforms TimeCard fields (see mapping for approvedBy, project, case) to produce a structured {id,label} object for state as well; alternatively, if you intend to keep it as a string, add a clear comment in the TimeCard declaration documenting this exception and update any serializers/mappers to preserve the string contract consistently.
🤖 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 742-746: The inline anonymous record for the field named `case`
inside the `TimeCard` record should be extracted into a named public type for
consistency and reuse; create a new record type (e.g., `TimeCardCase`) that
mirrors the current inline shape (embedding `*ReferenceItem` and `string
number`) and replace the inline `record { ... }? case;` in `TimeCard` with a
nullable `TimeCardCase? case;`; align naming with the existing
`CaseAssociatedWithTimeCard` concept so consumers can import and reuse the new
`TimeCardCase` type across the entity layer.
- Line 736: TimeCard.state is currently a plain string but should follow the
API's reference pattern: change the TimeCard.type declaration from string to
ReferenceItem (use the same ReferenceItem type used by
CallRequest.state/Case.status/CreatedCase.state) and update the mapping that
transforms TimeCard fields (see mapping for approvedBy, project, case) to
produce a structured {id,label} object for state as well; alternatively, if you
intend to keep it as a string, add a clear comment in the TimeCard declaration
documenting this exception and update any serializers/mappers to preserve the
string contract consistently.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/customer-portal/backend/modules/entity/types.bal (2)
1081-1088: Nit: error message differs fromDateTime's pattern-violation message.
Dateuses"Invalid date format."whileDateTime(line 925) uses"Invalid date provided. Please provide a valid date value."Both messages are readable, but aligning them makes client-side error handling consistent.✏️ Suggested alignment
- message: "Invalid date format." + message: "Invalid date provided. Please provide a valid date value."🤖 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 1081 - 1088, The Date type's `@constraint` pattern message currently reads "Invalid date format." and should be aligned with the DateTime type's message; update the pattern.message value in the `@constraint` on the Date type (the annotation above public type Date) to: "Invalid date provided. Please provide a valid date value." so both types use the same validation message.
1133-1139: Consider addingjson...;for resilience against unexpected top-level fields.
CaseSearchResponse(line 276) explicitly addsjson...;after*Pagination— in Ballerina, record inclusion (*T) does not inherit the rest descriptor, soTimeCardsResponseis currently a closed record at the top level. If the entity (ServiceNow) response ever carries an unexpected top-level key, parsing will fail at runtime.AttachmentsResponsefollows the same closed pattern and works today, so this is a soft suggestion for consistency withCaseSearchResponse.✏️ Suggested change
public type TimeCardsResponse record {| # List of time cards TimeCard[] timeCards; # Total records count int totalRecords; *Pagination; + json...; |};🤖 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 1133 - 1139, The TimeCardsResponse record is currently closed at the top level and should allow unexpected top-level fields like CaseSearchResponse does; update the TimeCardsResponse type (and consider doing the same for AttachmentsResponse) to include an open rest descriptor by adding json...; after the existing *Pagination inclusion so the record becomes resilient to extra keys from the ServiceNow response.
🤖 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/entity/types.bal`:
- Around line 1081-1088: The Date type's `@constraint` pattern message currently
reads "Invalid date format." and should be aligned with the DateTime type's
message; update the pattern.message value in the `@constraint` on the Date type
(the annotation above public type Date) to: "Invalid date provided. Please
provide a valid date value." so both types use the same validation message.
- Around line 1133-1139: The TimeCardsResponse record is currently closed at the
top level and should allow unexpected top-level fields like CaseSearchResponse
does; update the TimeCardsResponse type (and consider doing the same for
AttachmentsResponse) to include an open rest descriptor by adding json...; after
the existing *Pagination inclusion so the record becomes resilient to extra keys
from the ServiceNow response.
8982e45
into
wso2-open-operations:customer-portal-milestone-1
Description
This PR introduces API search endpoints for managing time cards and update call request payload.
Changes
Reason
Time card management functionality was not previously exposed via API. These endpoints enable:
This enhances time tracking capabilities and supports related operational processes.
Testing
Impact
Summary by CodeRabbit
New Features
API / Schema Changes