Conversation
The day is now derived directly from dueDate, making dayOfMonth redundant. Removes the field from domain entities, input/output DTOs, MongoDB model, validation logic, error constants, swagger docs, and all related tests. X-Lerian-Ref: 0x1
Compares dates at day granularity instead of exact timestamp, so a dueDate set to today is accepted. Only dates before today are rejected. X-Lerian-Ref: 0x1
WalkthroughThis pull request removes the 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
Comment |
|
Consider updating CHANGELOG.md to document this change. If this change doesn't need a changelog entry, add the |
🔒 Security Scan Results —
|
📊 Unit Test Coverage Report:
|
| Metric | Value |
|---|---|
| Overall Coverage | 88.2% ✅ PASS |
| Threshold | 85% |
Coverage by Package
| Package | Coverage |
|---|---|
github.com/LerianStudio/reporter/components/manager/internal/adapters/http/in |
89.2% |
github.com/LerianStudio/reporter/components/manager/internal/services |
89.2% |
Generated by Go PR Analysis workflow
📊 Unit Test Coverage Report:
|
| Metric | Value |
|---|---|
| Overall Coverage | 90.7% ✅ PASS |
| Threshold | 85% |
Coverage by Package
| Package | Coverage |
|---|---|
github.com/LerianStudio/reporter/components/worker/internal/services |
92.6% |
Generated by Go PR Analysis workflow
![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 (3)
components/manager/internal/adapters/http/in/notification_test.go (1)
364-399: 🧹 Nitpick | 🔵 TrivialMissing loop variable capture in parallel subtest.
The test range loop captures
ttbut the capture statementtt := ttis missing before the parallel subtest, which can cause race conditions in Go versions prior to 1.22. While Go 1.22+ fixed this behavior, it's still a best practice to include the capture for compatibility and clarity.♻️ Suggested fix
for _, tt := range tests { + tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel()Based on learnings: "In Go tests that use parallel subtests, avoid patterns that can cause race conditions when subtests call t.Parallel()."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/manager/internal/adapters/http/in/notification_test.go` around lines 364 - 399, The subtest loop captures the loop variable `tt` while launching parallel subtests with `t.Run(..., func(t *testing.T) { t.Parallel() ... })`, which can cause races; fix it by adding a per-iteration capture (e.g. `tt := tt`) immediately inside the `for _, tt := range tests {` body before calling `t.Run`, so each subtest closure gets its own copy of `tt` (affects the loop over `tests`, the `tt` var used in `t.Run` and subsequent calls like `tt.mockSetup`, `tt.expectedStatus`, and `tt.checkBody`).components/manager/internal/services/deadline-service_test.go (2)
33-40:⚠️ Potential issue | 🟠 MajorAdd explicit boundary tests for dueDate at “today” vs “before today”.
This PR states day-granularity validation was fixed, but this table currently does not assert that a
dueDateon the same calendar day is accepted while a date before today is rejected. That leaves the core fix unguarded against regression.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/manager/internal/services/deadline-service_test.go` around lines 33 - 40, Add two explicit table-driven tests inside the tests slice in deadline-service_test.go to guard the day-granularity fix: one case where input is a *deadline.CreateDeadlineInput with DueDate set to today’s date (use time.Now().Truncate(24*time.Hour) or equivalent) which should have expectErr=false and expectedResult=true, and one case with DueDate set to yesterday (today minus 24h) which should have expectErr=true and errContains matching the validation message; ensure both cases provide a mockSetup (returning a *UseCase) consistent with other entries so the test exercises the same validation path in the CreateDeadline use-case.
215-227:⚠️ Potential issue | 🟠 MajorResolve inconsistency between validation and domain fallback logic for annual/semiannual frequencies.
The test correctly expects
ErrMonthsOfYearRequiredat line 227, butpkg/mongodb/deadline/deadline.go(NextDueDate, lines 250-254) has explicit fallback logic for emptymonthsOfYear:
semiannual: adds 6 months if emptyannual: adds 1 year if emptyThis contradicts the validation requirement. Decide whether
monthsOfYearshould be:
- Mandatory (current validation): Remove the fallback logic in
NextDueDatesince it will never be reached.- Optional (fallback-enabled): Relax validation to allow empty
monthsOfYearand update tests accordingly.Currently, the validation enforces requirement but the domain provides fallback—align both directions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/manager/internal/services/deadline-service_test.go` around lines 215 - 227, The test expects monthsOfYear to be mandatory, so remove the implicit fallback behavior in the domain: update pkg/mongodb/deadline/deadline.go by editing the NextDueDate function to delete the branch logic that defaults semiannual to +6 months and annual to +1 year when MonthsOfYear is empty; ensure NextDueDate relies on validation (and surfaces constant.ErrMonthsOfYearRequired when MonthsOfYear is missing) so validation and domain behavior are consistent with the CreateDeadlineInput requirement.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/manager/internal/adapters/http/in/template-builder.go`:
- Line 164: The Swagger annotation change adding/removing the `@Param` for
template_builder.ValidateBlocksInput is unrelated to the deadline removal work;
either revert this annotation edit or add a short comment in the commit/PR
explaining why it is needed here. Locate the Swagger tag in template-builder.go
that references ValidateBlocksInput and either remove the modified annotation
line or restore it to its original state, or if it must remain, update the PR
description and add an inline code comment near the annotation explaining the
dependency between this change and the deadline/dayOfMonth refactor so reviewers
understand the rationale.
In `@pkg/mongodb/deadline/deadline.go`:
- Around line 157-160: The date comparison uses time.Now().Truncate(24 *
time.Hour) while dueDate may be in a different timezone, causing incorrect
"past" detection; normalize both sides to the same location (UTC) before
truncation — e.g., convert dueDate = dueDate.UTC() and use now :=
time.Now().UTC().Truncate(24*time.Hour) (and truncate the UTC dueDate similarly)
so the before check against dueDate.Truncate(24*time.Hour) uses consistent
timezones.
---
Outside diff comments:
In `@components/manager/internal/adapters/http/in/notification_test.go`:
- Around line 364-399: The subtest loop captures the loop variable `tt` while
launching parallel subtests with `t.Run(..., func(t *testing.T) { t.Parallel()
... })`, which can cause races; fix it by adding a per-iteration capture (e.g.
`tt := tt`) immediately inside the `for _, tt := range tests {` body before
calling `t.Run`, so each subtest closure gets its own copy of `tt` (affects the
loop over `tests`, the `tt` var used in `t.Run` and subsequent calls like
`tt.mockSetup`, `tt.expectedStatus`, and `tt.checkBody`).
In `@components/manager/internal/services/deadline-service_test.go`:
- Around line 33-40: Add two explicit table-driven tests inside the tests slice
in deadline-service_test.go to guard the day-granularity fix: one case where
input is a *deadline.CreateDeadlineInput with DueDate set to today’s date (use
time.Now().Truncate(24*time.Hour) or equivalent) which should have
expectErr=false and expectedResult=true, and one case with DueDate set to
yesterday (today minus 24h) which should have expectErr=true and errContains
matching the validation message; ensure both cases provide a mockSetup
(returning a *UseCase) consistent with other entries so the test exercises the
same validation path in the CreateDeadline use-case.
- Around line 215-227: The test expects monthsOfYear to be mandatory, so remove
the implicit fallback behavior in the domain: update
pkg/mongodb/deadline/deadline.go by editing the NextDueDate function to delete
the branch logic that defaults semiannual to +6 months and annual to +1 year
when MonthsOfYear is empty; ensure NextDueDate relies on validation (and
surfaces constant.ErrMonthsOfYearRequired when MonthsOfYear is missing) so
validation and domain behavior are consistent with the CreateDeadlineInput
requirement.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cc390ec1-84a3-48f6-a6af-32d54ccf551e
📒 Files selected for processing (18)
components/manager/api/docs.gocomponents/manager/api/openapi.yamlcomponents/manager/api/swagger.jsoncomponents/manager/api/swagger.yamlcomponents/manager/internal/adapters/http/in/deadline_test.gocomponents/manager/internal/adapters/http/in/notification.gocomponents/manager/internal/adapters/http/in/notification_test.gocomponents/manager/internal/adapters/http/in/template-builder.gocomponents/manager/internal/services/create-deadline.gocomponents/manager/internal/services/deadline-service_test.gocomponents/manager/internal/services/update-deadline-by-id.gopkg/constant/errors.gopkg/errors.gopkg/mongodb/deadline/deadline.gotests/chaos/chaos-deadline-mongodb_test.gotests/integration/deadlines-integration_test.gotests/integration/notification-integration_test.gotests/property/property-deadline_test.go
💤 Files with no reviewable changes (8)
- tests/integration/notification-integration_test.go
- tests/chaos/chaos-deadline-mongodb_test.go
- components/manager/api/swagger.yaml
- pkg/constant/errors.go
- components/manager/internal/adapters/http/in/notification.go
- tests/integration/deadlines-integration_test.go
- components/manager/api/docs.go
- components/manager/api/swagger.json
3 participants
Pull Request Checklist
Pull Request Type
Checklist
Please check each item after it's completed.
Additional Notes
Obs: Please, always remember to target your PR to develop branch instead of main.