feat: add batch endpoint for working days#641
Conversation
|
Warning Review limit reached
More reviews will be available in 35 minutes and 8 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR adds a batch endpoint ChangesBatch Working Days Calculation API
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/apis/places/working-days.md (1)
8-8:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStatus line is now stale after adding POST batch endpoint.
Line 8 still states availability only at
GET /v1/places/working-days, but this PR also introducesPOST /v1/places/working-days/batch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/apis/places/working-days.md` at line 8, Update the status line to reflect both endpoints by mentioning that the feature is live at GET /v1/places/working-days and also available as a batch endpoint POST /v1/places/working-days/batch; edit the status text (the line currently reading "✅ **Live** - Available now at `GET /v1/places/working-days`") to include the POST path and/or rephrase to "Available now at GET /v1/places/working-days and POST /v1/places/working-days/batch" so the documentation accurately lists both endpoints.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/services/places/working-days/transport_http.go`:
- Around line 43-55: The batch handler currently only checks item count and then
calls svc.GetWorkingDaysBatch; add per-item validation of each req.Items element
(check presence/format of from and to, enforce to >= from, and validate
country/subdivision combinations) before calling svc.GetWorkingDaysBatch. Reuse
the same validation logic used by the single-item endpoint (or extract it into a
helper like validateWorkingDayItem) and return httpx.Error(w,
http.StatusBadRequest, "invalid_request", ...) for the first invalid item
(include item index in the message) so invalid entries never reach
svc.GetWorkingDaysBatch; keep the final call to svc.GetWorkingDaysBatch and
httpx.JSON unchanged once all items are valid.
- Around line 38-41: The batch endpoint fails to accept documented "YYYY-MM-DD"
dates because json.Decoder uses time.Time's RFC3339-only unmarshaler; fix by
adding a custom date type with UnmarshalJSON that accepts both "2006-01-02" and
RFC3339 (or parses fallback layouts) and use that type for the Request's
items[].from and items[].to fields (or implement UnmarshalJSON on the Request
struct to parse those fields manually), then keep the same
json.NewDecoder(r.Body).Decode(&req) call so clients can send "YYYY-MM-DD" as
documented.
---
Outside diff comments:
In `@docs/apis/places/working-days.md`:
- Line 8: Update the status line to reflect both endpoints by mentioning that
the feature is live at GET /v1/places/working-days and also available as a batch
endpoint POST /v1/places/working-days/batch; edit the status text (the line
currently reading "✅ **Live** - Available now at `GET /v1/places/working-days`")
to include the POST path and/or rephrase to "Available now at GET
/v1/places/working-days and POST /v1/places/working-days/batch" so the
documentation accurately lists both endpoints.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d0809aa5-9efa-47a8-8957-491828e7f665
📒 Files selected for processing (8)
apps/api/services/places/working-days/service.goapps/api/services/places/working-days/service_test.goapps/api/services/places/working-days/transport_http.goapps/api/services/places/working-days/transport_http_test.goapps/api/services/places/working-days/type.goapps/dashboard/config/api_catalog.ymlapps/dashboard/config/api_docs/working-days.ymldocs/apis/places/working-days.md
|
|
||
| httpx.JSON(w, http.StatusOK, response) | ||
| }) | ||
| r.Post("/working-days/batch", func(w http.ResponseWriter, r *http.Request) { |
There was a problem hiding this comment.
If I remember correctly we have a new function in the httpx library called HandleBatch exactly for this
|
|
||
| httpx.JSON(w, http.StatusOK, response) | ||
| }) | ||
| r.Post("/working-days/batch", func(w http.ResponseWriter, r *http.Request) { |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/api/services/places/working-days/type.go (1)
57-57:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
to >= fromvalidation for batch items.Line 57 is missing
gtfield=From, so batch accepts inverted ranges unlike the single-item contract and docs.Suggested fix
type BatchItem struct { From Date `json:"from" validate:"required"` - To Date `json:"to" validate:"required"` + To Date `json:"to" validate:"required,gtfield=From"` Country string `json:"country" validate:"omitempty,iso3166_1_alpha2"` Subdivision string `json:"subdivision" validate:"omitempty,iso3166_2,required_with=Country"` }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/services/places/working-days/type.go` at line 57, The batch-item struct's To field (declared as To Date) lacks a cross-field validator, so inverted ranges slip through; update the struct tag on To to include the cross-field rule (e.g., validate:"required,gtefield=From") so To is validated as greater-than-or-equal-to From (reference the To Date field and the From field name when making the change).
🧹 Nitpick comments (1)
apps/api/services/places/working-days/transport_http_test.go (1)
103-219: ⚡ Quick winAdd a regression test for
to < fromin batch items.Current cases cover count/malformed-body bounds, but not invalid date ordering. Add one case expecting validation failure (422) when an item has
tobeforefrom.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/services/places/working-days/transport_http_test.go` around lines 103 - 219, Add a regression test function (e.g., TestWorkingDaysBatch_ToBeforeFrom) that constructs a BatchRequest with one BatchItem whose To date is earlier than its From date (use Date{Time: mustParseDate(...)} and mustMarshal for the body), POSTs it to "/working-days/batch" via newBatchRouter() and asserts the response code is http.StatusUnprocessableEntity (422); reuse the existing test setup patterns (httptest.NewRequest, req.Header.Set("Content-Type","application/json"), httptest.NewRecorder, and newBatchRouter().ServeHTTP) and mirror the other tests' style and parallelization (t.Parallel()).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@apps/api/services/places/working-days/type.go`:
- Line 57: The batch-item struct's To field (declared as To Date) lacks a
cross-field validator, so inverted ranges slip through; update the struct tag on
To to include the cross-field rule (e.g., validate:"required,gtefield=From") so
To is validated as greater-than-or-equal-to From (reference the To Date field
and the From field name when making the change).
---
Nitpick comments:
In `@apps/api/services/places/working-days/transport_http_test.go`:
- Around line 103-219: Add a regression test function (e.g.,
TestWorkingDaysBatch_ToBeforeFrom) that constructs a BatchRequest with one
BatchItem whose To date is earlier than its From date (use Date{Time:
mustParseDate(...)} and mustMarshal for the body), POSTs it to
"/working-days/batch" via newBatchRouter() and asserts the response code is
http.StatusUnprocessableEntity (422); reuse the existing test setup patterns
(httptest.NewRequest, req.Header.Set("Content-Type","application/json"),
httptest.NewRecorder, and newBatchRouter().ServeHTTP) and mirror the other
tests' style and parallelization (t.Parallel()).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b2d9a1d6-bd80-44d0-a843-cda49dc5240e
📒 Files selected for processing (5)
apps/api/services/places/working-days/transport_http.goapps/api/services/places/working-days/transport_http_test.goapps/api/services/places/working-days/type.goapps/dashboard/config/api_docs/working-days.ymldocs/apis/places/working-days.md
✅ Files skipped from review due to trivial changes (1)
- apps/dashboard/config/api_docs/working-days.yml
|
@copilot resolve the merge conflicts in this pull request |
1 similar comment
|
@copilot resolve the merge conflicts in this pull request |
321f14c to
e0209d8
Compare
| httpx.JSON(w, http.StatusOK, response) | ||
| }) | ||
|
|
||
| r.Post("/working-days/batch", httpx.HandleBatch( |
There was a problem hiding this comment.
We are not validating max length....
Check https://github.com/bobadilla-tech/requiems-api/blob/main/apps/api/services/networking/mx/transport_http.go#L15
There was a problem hiding this comment.
Oh you still got a type.go file....
| Subdivision string `json:"subdivision,omitempty"` | ||
| } | ||
|
|
||
| func (WorkingDays) IsData() {} |
There was a problem hiding this comment.
We dont support IsData anymore....
| businessdayscalculator "github.com/bobadilla-tech/business-days-calculator" | ||
| ) | ||
|
|
||
| // WorkingDays represents the response for working days calculation. |
There was a problem hiding this comment.
This is the correct location for the type
https://github.com/bobadilla-tech/requiems-api/blob/main/docs/core/adding-go-endpoints.md#1a-types--define-your-types-in-the-owning-file
e0209d8 to
bc72df2
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/api/services/places/working-days/transport_http.go (1)
60-63:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
to >= fromvalidation for each batch item.
BatchItemcurrently validates presence/format but not ordering. On Line 61,Toshould enforcegtefield=From(or equivalent) to match the documented contract and prevent invalid ranges from reaching service logic.Suggested fix
type BatchItem struct { From Date `json:"from" validate:"required"` - To Date `json:"to" validate:"required"` + To Date `json:"to" validate:"required,gtefield=From"` Country string `json:"country" validate:"omitempty,iso3166_1_alpha2"` Subdivision string `json:"subdivision" validate:"omitempty,iso3166_2,required_with=Country"` }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/services/places/working-days/transport_http.go` around lines 60 - 63, The BatchItem struct's date range isn't validated for ordering: add a validation rule to the To field so it enforces To >= From (e.g., add `validate:"required,gtefield=From"` or append `gtefield=From` to the existing tag on `To`) in the BatchItem definition in transport_http.go so each batch item fails validation when the end date precedes the start date; ensure the tag uses the same validator framework used elsewhere for `Date` fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/services/places/working-days/transport_http.go`:
- Around line 3-11: Run goimports on the file to fix the import formatting
issue: reformat the import block in transport_http.go so imports are grouped and
ordered correctly (standard libs first, then third-party, then local packages)
and remove any unnecessary blank lines; target the import block that contains
symbols like "context", "fmt", "github.com/go-chi/chi/v5", "strings", "time",
and "requiems-api/platform/httpx" so the linter warning on the import formatting
(around the current import list) is resolved.
- Line 85: The transport maps decode failures for the batch endpoint to the
wrong error code; update the HandleBatch error mapping used in the
r.Post("/working-days/batch", httpx.HandleBatch(...)) call so that
decode/malformed-JSON errors return "invalid_request" (instead of
"bad_request"), or alternatively update the batch docs/tests to expect
"bad_request" to keep the contract consistent; modify the argument or options
passed into httpx.HandleBatch to produce the "invalid_request" error code for
JSON decode failures, ensuring any relevant error-to-code mapping logic in
httpx.HandleBatch is updated accordingly.
---
Duplicate comments:
In `@apps/api/services/places/working-days/transport_http.go`:
- Around line 60-63: The BatchItem struct's date range isn't validated for
ordering: add a validation rule to the To field so it enforces To >= From (e.g.,
add `validate:"required,gtefield=From"` or append `gtefield=From` to the
existing tag on `To`) in the BatchItem definition in transport_http.go so each
batch item fails validation when the end date precedes the start date; ensure
the tag uses the same validator framework used elsewhere for `Date` fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b39cfe0d-b530-46a4-b56b-ce297689eca3
📒 Files selected for processing (4)
apps/api/services/places/working-days/service.goapps/api/services/places/working-days/transport_http.goapps/api/services/places/working-days/transport_http_test.goapps/dashboard/config/api_catalog.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/dashboard/config/api_catalog.yml
- apps/api/services/places/working-days/service.go
| }, nil | ||
| })) | ||
|
|
||
| r.Post("/working-days/batch", httpx.HandleBatch( |
There was a problem hiding this comment.
Malformed-JSON error code appears inconsistent with the endpoint docs.
At Line 85, httpx.HandleBatch maps decode failures to bad_request, while the batch docs specify invalid_request for malformed JSON. Please align either transport behavior or docs/tests so clients get a stable error contract.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/services/places/working-days/transport_http.go` at line 85, The
transport maps decode failures for the batch endpoint to the wrong error code;
update the HandleBatch error mapping used in the r.Post("/working-days/batch",
httpx.HandleBatch(...)) call so that decode/malformed-JSON errors return
"invalid_request" (instead of "bad_request"), or alternatively update the batch
docs/tests to expect "bad_request" to keep the contract consistent; modify the
argument or options passed into httpx.HandleBatch to produce the
"invalid_request" error code for JSON decode failures, ensuring any relevant
error-to-code mapping logic in httpx.HandleBatch is updated accordingly.
bc72df2 to
584d3c2
Compare
584d3c2 to
d19733b
Compare
Batch working-days calculation for up to 50 date ranges. Pure in-memory computation uses sequential loop per batch-apis.md. Fixes from review: - Remove unused BatchResponse type from transport_http.go - Remove GetWorkingDaysBatch from service (took HTTP-binding Request type); inline loop directly in the transport handler - Date custom type supports both YYYY-MM-DD and RFC3339 input formats Co-Authored-By: OneLastChanc3 <OneLastChanc3@users.noreply.github.com>
Co-Authored-By: OneLastChanc3 <OneLastChanc3@users.noreply.github.com>
Closed because it landed directly on main! 🚀
|
3 participants
Summary by CodeRabbit
Release Notes