feat: notification and metrics endpoints#583
Merged
Conversation
…260224221350 chore(release): Update CHANGELOGs [skip ci]
Add CountAll to template and report repositories, CountByStatus to report repository, and FindActiveNotifiable to deadline repository to support dashboard metrics and notification endpoints. X-Lerian-Ref: 0x1
…s endpoints MetricsHandler returns consolidated KPI counters (templates, reports, dataSources, errors with trend) via parallel goroutines. NotificationHandler returns active deadlines needing alerts with computed severity (overdue/warning/info) and urgency ordering. X-Lerian-Ref: 0x1
…cs and notification endpoints X-Lerian-Ref: 0x1
|
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:
WalkthroughAdds two HTTP handlers and endpoints: GET /v1/metrics and GET /v1/deadlines/notifications. MetricsHandler concurrently aggregates counts for templates, reports, data sources and computes error counts for current and previous periods (configurable errorPeriodDays). NotificationHandler fetches active, not-delivered, non-deleted deadlines (with limit), computes days-until-due and severity, filters, sorts by urgency, and returns a paginated list. Bootstrap and routing were updated to initialize and wire the handlers; repository interfaces and MongoDB implementations gained methods to support the new handlers. Extensive unit, property, fuzz, integration, and chaos tests plus OpenAPI/Swagger docs were added. Sequence Diagram(s)sequenceDiagram
participant Client
participant MetricsHandler
participant UseCase
participant TemplateRepo
participant ReportRepo
participant DataSourceRepo
participant MongoDB
Client->>MetricsHandler: GET /v1/metrics?errorPeriodDays=7
MetricsHandler->>MetricsHandler: parseErrorPeriodDays()
par Concurrent fetches
MetricsHandler->>UseCase: Get template count
UseCase->>TemplateRepo: CountAll(ctx)
TemplateRepo->>MongoDB: count templates (active, non-deleted)
MongoDB-->>TemplateRepo: templateCount
TemplateRepo-->>UseCase: templateCount
MetricsHandler->>UseCase: Get report count & errors
UseCase->>ReportRepo: CountAll(ctx)
ReportRepo->>MongoDB: count reports (non-deleted)
MongoDB-->>ReportRepo: reportCount
ReportRepo-->>UseCase: reportCount
UseCase->>ReportRepo: CountByStatus(ctx, "error", from,to)
ReportRepo->>MongoDB: count error reports in window
MongoDB-->>ReportRepo: errorCounts
ReportRepo-->>UseCase: errorCounts
MetricsHandler->>UseCase: Get data source count
UseCase->>DataSourceRepo: CountAll(ctx)
DataSourceRepo->>MongoDB: count data sources
MongoDB-->>DataSourceRepo: dsCount
DataSourceRepo-->>UseCase: dsCount
end
MetricsHandler->>MetricsHandler: aggregate results
MetricsHandler-->>Client: 200 JSON metricsResponse
sequenceDiagram
participant Client
participant NotificationHandler
participant UseCase
participant DeadlineRepo
participant MongoDB
Client->>NotificationHandler: GET /v1/deadlines/notifications?limit=10
NotificationHandler->>NotificationHandler: parseNotificationLimit()
NotificationHandler->>UseCase: Fetch active notifiable deadlines
UseCase->>DeadlineRepo: FindActiveNotifiable(ctx, limit)
DeadlineRepo->>MongoDB: query active, undelivered, non-deleted deadlines sorted by due_date
MongoDB-->>DeadlineRepo: deadlines
DeadlineRepo-->>UseCase: deadlines
NotificationHandler->>NotificationHandler: filterAndBuildNotifications(deadlines)
NotificationHandler->>NotificationHandler: computeDaysUntilDue() / ComputeNotificationSeverity()
NotificationHandler->>NotificationHandler: sortNotificationsByUrgency()
NotificationHandler-->>Client: 200 JSON notificationResponse
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
Comment |
github-actions
bot
added
area: manager
area: documentation
area: worker
area: tests
labels
Contributor
|
This PR is very large (25 files, 4668 lines changed). Consider breaking it into smaller PRs for easier review. |
Contributor
📊 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
🔒 Security Scan Results —
|
Contributor
📊 Unit Test Coverage Report:
|
| Metric | Value |
|---|---|
| Overall Coverage | 88.3% ✅ 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.3% |
Generated by Go PR Analysis workflow
🔒 Security Scan Results —
|
…late-builder X-Lerian-Ref: 0x1
Contributor
|
This PR is very large (28 files, 5331 lines changed). Consider breaking it into smaller PRs for easier review. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🤖 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/metrics_fuzz_test.go`:
- Around line 48-53: The fuzz test builds the request URL by raw-concatenating
queryValue which allows characters like %, &, #, + to mutate the query before
parseErrorPeriodDays runs; URL-escape the fuzzed value (e.g., use
url.QueryEscape or url.Values to encode the parameter) when constructing the URL
in metrics_fuzz_test.go so the request reaches parseErrorPeriodDays reliably and
http.NewRequest doesn't fail due to unescaped characters.
In `@components/manager/internal/adapters/http/in/metrics_property_test.go`:
- Around line 205-288: The tests (TestProperty_PeriodCalculation_NoOverlapNoGap
and TestProperty_PeriodCalculation_TotalSpanIsTwiceN) reimplement GetMetrics'
window math and the handler test uses gomock.Any() for CountByStatus, so
regressions in GetMetrics can pass; fix by extracting the period/window
calculation into a shared helper used by the production GetMetrics and the tests
(e.g., move the now/currentPeriodStart/previousPeriodStart logic into a
GetWindow or computePeriods function) or alternatively replace gomock.Any() in
the CountByStatus mock expectations with strict matchers/capture logic that
asserts the actual from/to arguments passed by GetMetrics (use gomock Do or a
custom matcher) so the tests validate the real window values rather than a
reimplementation.
In `@components/manager/internal/adapters/http/in/metrics_test.go`:
- Around line 31-41: The setupMetricsTestApp function currently accepts an
unused parameter (handler *MetricsHandler); remove this parameter from
setupMetricsTestApp's signature and all call sites so it becomes
setupMetricsTestApp() and is invoked without arguments, or alternatively if you
prefer to use it, register the handler's routes inside setupMetricsTestApp
(e.g., call handler.RegisterRoutes(app) or similar) and keep the
parameter—ensure consistency between the function signature and every invocation
of setupMetricsTestApp.
- Around line 43-56: The test middleware setupMetricsContextMiddleware uses raw
string keys ("logger","tracer","requestId") so NewLoggerFromContext() and
NewTracerFromContext() cannot find them; replace the manual context.WithValue
usage with the pkg/ctxutil helpers: call ctxutil.ContextWithLogger(ctx, logger)
and ctxutil.ContextWithTracer(ctx, tracer) (and similarly set request ID via the
appropriate ctxutil helper if available or store it as the expected
CustomContextKeyValue form) before c.SetUserContext(ctx) so values are stored
under libCommons.CustomContextKey as a CustomContextKeyValue and will be
retrievable by NewLoggerFromContext() and NewTracerFromContext().
In `@components/manager/internal/adapters/http/in/notification_test.go`:
- Around line 135-149: The test "Success - Returns notifications with mixed
severities ordered by urgency" uses already-sorted data from the mock
FindActiveNotifiable call so it doesn't verify sorting; modify the mockSetup for
the test (and the other case at lines 243-249) to return the deadlines in a
deliberately shuffled order (e.g., reorder returns of makeDeadline for
id1,id2,id3) while keeping the same expected response assertions so
GetNotifications is actually validated for correct ordering; locate the
mockSetup closures tied to FindActiveNotifiable in notification_test.go and
reorder the slice entries accordingly.
In `@components/manager/internal/adapters/http/in/notification.go`:
- Around line 92-116: The repo is currently given the client `limit` (via
parseNotificationLimit) which causes the DB to trim results before the handler
applies filterAndBuildNotifications and sortNotificationsByUrgency; instead,
call nh.service.DeadlineRepo.FindActiveNotifiable without applying the client
limit (or pass a sufficiently large cap), then run
filterAndBuildNotifications(deadlines, now) and
sortNotificationsByUrgency(items) and only after that trim the slice to the
requested `limit`; update the call site that currently passes `limit` into
FindActiveNotifiable and ensure parseNotificationLimit is still used only to
slice the final items.
In `@components/manager/internal/adapters/http/in/routes.go`:
- Around line 115-116: The route registration for metrics (the f.Get call that
currently uses auth.Authorize(constant.ApplicationName, reportResource, "get")
with metricsHandler.GetMetrics and WhenEnabled(ttMiddleware)) is using the
narrower report:get permission and thus exposes template/data-source totals to
report readers; change the authorization to a dedicated metrics/admin permission
that reflects the data returned (e.g., introduce or use a metricsResource or
adminResource constant and require a "read" or "admin" scope) so the call
becomes auth.Authorize(constant.ApplicationName, metricsResource, "get") (or the
appropriate admin scope), and update any related constants/permission
definitions and tests to require that new permission for
metricsHandler.GetMetrics.
In `@components/manager/internal/adapters/http/in/template-builder.go`:
- Line 164: The Swagger `@Param` annotation in template-builder.go is missing the
location ("in") specifier; update the annotation for the ValidateBlocks input so
it supplies the "in" position (body) as required by swaggo (`@Param` <name> <in>
<type> <required> <description>), i.e., change the existing annotation that
references template_builder.ValidateBlocksInput to include the body location
specifier so the generated docs correctly mark the parameter as a request body.
- Line 109: Update the Swagger `@Param` annotation in template-builder.go for the
GenerateCode endpoint: modify the existing line that references
template_builder.GenerateCodeInput so it includes the missing location specifier
by inserting "body" between the parameter name and the type (i.e., change the
comment that begins with "@Param body" to specify the parameter location as body
for template_builder.GenerateCodeInput).
In `@components/manager/internal/bootstrap/init_helpers.go`:
- Line 419: The function currently returns eight separate handlers
(TemplateHandler, ReportHandler, DataSourceHandler, DeadlineHandler,
TemplateBuilderHandler, MetricsHandler, NotificationHandler) which is unwieldy;
create a Handlers struct that aggregates these fields (e.g., type Handlers
struct { Template *httpIn.TemplateHandler; Report *httpIn.ReportHandler;
DataSource *httpIn.DataSourceHandler; Deadline *httpIn.DeadlineHandler;
TemplateBuilder *httpIn.TemplateBuilderHandler; Metrics *httpIn.MetricsHandler;
Notification *httpIn.NotificationHandler }) and change the function signature to
return (*Handlers, error); update the function body to populate and return
&Handlers{Template: templateHandler, Report: reportHandler, DataSource:
dataSourceHandler, Deadline: deadlineHandler, TemplateBuilder:
templateBuilderHandler, Metrics: metricsHandler, Notification:
notificationHandler}, nil and replace all error return sites to return nil, err
(or nil, <wrapped err>) instead of multiple nil placeholders.
In `@pkg/mongodb/deadline/deadline.mongodb.go`:
- Around line 479-505: In FindActiveNotifiable, validate the limit parameter and
normalize null checks: ensure limit > 0 (either return an error or clamp to a
sensible default such as 1 or a configured max) before creating opts so SetLimit
is never called with 0; also make the filter null-checks consistent with the
rest of the file by replacing `"delivered_at": nil` and `"deleted_at": nil` with
the same bson expression used elsewhere (e.g., bson.D{{Key: "$eq", Value: nil}})
to keep behavior and style uniform.
In `@pkg/mongodb/report/report.mongodb.go`:
- Around line 394-410: CountByStatus currently filters reports by "created_at",
which misses reports that transitioned into the requested status later; change
the time-window field in the filter from "created_at" to the status-transition
timestamp (e.g., "completed_at") so the query slices on when the status was set
(the code that sets completed_at during status updates is already present), i.e.
update the filter in CountByStatus to use "completed_at": bson.M{"completed_at":
bson.M{"$gte": from, "$lt": to}} (or an appropriate transition-time field)
instead of "created_at".
In `@pkg/mongodb/template/template.mongodb.go`:
- Around line 249-271: CountAll is missing the request-id span attribute used
elsewhere; in the TemplateMongoDBRepository.CountAll function, after creating
the span (ctx, span := tracer.Start(...)), set the same request id attribute as
in Count by retrieving the request ID from the context (via
ctxutil.RequestIDFromContext(ctx)) and adding it to the span
(app.request.request_id) so tracing is consistent across methods.
In `@tests/chaos/chaos-metrics-mongodb_test.go`:
- Around line 275-279: The latencyObserved predicate in the test (where
latencyObserved := err != nil || latencyDuration > 2*time.Second) incorrectly
treats a fast non-200 (e.g., 5xx) response as a failure; update the predicate to
also consider a non-200 HTTP status as degraded (e.g., err != nil ||
latencyDuration > 2*time.Second || code != 200) in this test and make the
identical change in the duplicate check in chaos-notification-mongodb_test.go so
quick 5xx responses count as successful chaos behavior.
In `@tests/integration/metrics-integration_test.go`:
- Around line 33-39: The test helper newTestMongoConnection currently hard-codes
Database: "reporter"; change it to accept a configurable database name (e.g.,
add a dbName parameter or read from a TEST_DB env var) and update its callers
(including the repo-level tests like
TestIntegration_TemplateRepo_CountAll_FiltersDeletedDocuments and
TestIntegration_ReportRepo_CountAll_IncludesAllStatuses) to pass a unique
per-test DB (for example using the test name or a random suffix, e.g.,
fmt.Sprintf("reporter_%s", sanitize(t.Name()))). Leave the shared "reporter" DB
usage only for the HTTP endpoint tests; ensure all other references (the other
helper usage block) are updated to use the new signature or config.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 06a75c1d-1622-4514-8a9a-598166ca8dce
📒 Files selected for processing (25)
CHANGELOG.mdcomponents/manager/internal/adapters/http/in/metrics.gocomponents/manager/internal/adapters/http/in/metrics_fuzz_test.gocomponents/manager/internal/adapters/http/in/metrics_property_test.gocomponents/manager/internal/adapters/http/in/metrics_test.gocomponents/manager/internal/adapters/http/in/notification.gocomponents/manager/internal/adapters/http/in/notification_fuzz_test.gocomponents/manager/internal/adapters/http/in/notification_property_test.gocomponents/manager/internal/adapters/http/in/notification_test.gocomponents/manager/internal/adapters/http/in/routes.gocomponents/manager/internal/adapters/http/in/template-builder.gocomponents/manager/internal/bootstrap/config.gocomponents/manager/internal/bootstrap/init_helpers.gocomponents/worker/internal/bootstrap/config_runtime_test.gopkg/mongodb/deadline/deadline.gopkg/mongodb/deadline/deadline.mock.gopkg/mongodb/deadline/deadline.mongodb.gopkg/mongodb/report/report.mongodb.gopkg/mongodb/report/report.mongodb.mock.gopkg/mongodb/template/template.mongodb.gopkg/mongodb/template/template.mongodb.mock.gotests/chaos/chaos-metrics-mongodb_test.gotests/chaos/chaos-notification-mongodb_test.gotests/integration/metrics-integration_test.gotests/integration/notification-integration_test.go
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/api/openapi.yaml`:
- Around line 202-236: The secured endpoint definition for
/v1/deadlines/notifications declares security: - BearerAuth: [] but is missing
401 and 403 response entries; add "401" and "403" response objects (mirroring
other endpoints) under responses with application/json schema $ref:
'#/components/schemas/fiber.Map' and appropriate descriptions "Unauthorized" and
"Forbidden" so the contract is complete; apply the same addition to any other
endpoints in this OpenAPI file that declare BearerAuth but lack 401/403.
- Around line 207-213: Update the OpenAPI parameter schemas for the query params
named "limit" and "errorPeriodDays" to include explicit numeric bounds: add
minimum: 1 and maximum: 100 to the "limit" integer schema (keeping default: 10)
and add minimum: 1 and maximum: 365 to the "errorPeriodDays" integer schema;
ensure both remain type: integer and that these changes are applied at both
occurrences referenced in the spec so the documented (1-100) and (1-365) bounds
are encoded in the schema.
- Around line 220-231: The OpenAPI error responses for the /v1/metrics and
/v1/deadlines/notifications endpoints currently reference fiber.Map; change
their 400 and 500 response schemas to reference pkg.HTTPError (the same schema
used by other endpoints) and update the corresponding handler
annotations/comments so generated docs match; then modify the handlers that
serve these endpoints to return structured errors conforming to pkg.HTTPError
(error fields and types) instead of raw fiber.Map so runtime responses match the
new contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 48c22581-9f46-4ff6-a51b-e50b6d98f189
📒 Files selected for processing (4)
components/manager/api/docs.gocomponents/manager/api/openapi.yamlcomponents/manager/api/swagger.jsoncomponents/manager/api/swagger.yaml
- template.CountAll: add active=true filter (was counting inactive templates) - report.CountAll: add deleted_at filter (was counting soft-deleted reports) - report.CountByStatus: add deleted_at filter (was counting soft-deleted error reports) - FindActiveNotifiable: fetch limit*3 from DB before window filter (was silently under-returning) - NewMetricsHandler/NewNotificationHandler: add field-level nil checks for repos - metricsParallelGroups: add protective comment documenting coupling contract X-Lerian-Ref: 0x1
Contributor
|
This PR is very large (28 files, 5364 lines changed). Consider breaking it into smaller PRs for easier review. |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (4)
pkg/mongodb/template/template.mongodb.go (1)
251-255:⚠️ Potential issue | 🟡 MinorAdd request-id span attribute in
CountAllfor tracing consistency.
Countsetsapp.request.request_id, butCountAlldoes not. This breaks trace parity for repository metrics calls.🔍 Suggested consistency patch
func (tm *TemplateMongoDBRepository) CountAll(ctx context.Context) (int64, error) { tracer := ctxutil.NewTracerFromContext(ctx) + reqID := ctxutil.HeaderIDFromContext(ctx) ctx, span := tracer.Start(ctx, "repository.template.count_all") defer span.End() + span.SetAttributes(attribute.String("app.request.request_id", reqID))Also applies to: 267-273
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/mongodb/template/template.mongodb.go` around lines 251 - 255, CountAll is starting a tracer span ("repository.template.count_all") but doesn't set the app.request.request_id attribute like Count does, breaking trace parity; fix by extracting the same request ID from the context (use the same helper used in Count) immediately after starting the span and add it to the span via span.SetAttributes (use the same attribute key "app.request.request_id"), and apply the same change for the other CountAll span occurrence around lines 267-273 so both repository.template.count_all spans include the request id.pkg/mongodb/report/report.mongodb.go (1)
409-413:⚠️ Potential issue | 🟠 MajorCount status trends by transition time, not creation time.
Line 411 slices on
created_at, so reports created earlier but moved to this status inside[from,to)are excluded. Since status updates already persistcompleted_at(Line 131-Line 133), this query should use that transition timestamp.🛠️ Suggested fix
filter := bson.M{ "status": status, - "created_at": bson.M{"$gte": from, "$lt": to}, + "completed_at": bson.M{"$gte": from, "$lt": to}, "deleted_at": bson.D{{Key: "$eq", Value: nil}}, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/mongodb/report/report.mongodb.go` around lines 409 - 413, The query filter is using the creation timestamp ("created_at") to slice the time window but needs to count reports by their status transition time; replace the "created_at" predicate in the filter variable with the transition timestamp field ("completed_at") so the range uses completed_at: bson.M{"$gte": from, "$lt": to}; ensure the rest of the filter (status and deleted_at) remains unchanged and that any callers expecting the old behavior still receive the same filter variable name.pkg/mongodb/deadline/deadline.mongodb.go (2)
479-510:⚠️ Potential issue | 🟠 MajorReject non-positive limits at the repository boundary.
This method is public on the repository, but it still relies on the HTTP handler to sanitize
limit. For direct callers,limit <= 0makes the query sizing undefined and breaks the documented “up to the given limit” contract. Guard the input before derivingdbLimit.Possible guard
func (dl *DeadlineMongoDBRepository) FindActiveNotifiable(ctx context.Context, limit int) ([]*Deadline, error) { tracer := ctxutil.NewTracerFromContext(ctx) ctx, span := tracer.Start(ctx, "repository.deadline.find_active_notifiable") defer span.End() + if limit < 1 { + return nil, fmt.Errorf("limit must be greater than 0") + } + reqID := ctxutil.HeaderIDFromContext(ctx) span.SetAttributes( attribute.String("app.request.request_id", reqID), attribute.Int("limit", limit), ) @@ - dbLimit := int64(limit * 3) + dbLimit := int64(limit) * 3 if dbLimit > 300 { dbLimit = 300 }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/mongodb/deadline/deadline.mongodb.go` around lines 479 - 510, In FindActiveNotifiable ensure the repository validates the incoming limit and returns an error for non-positive values instead of relying on callers; add a guard at the start of the method (before computing dbLimit) that checks if limit <= 0 and returns a clear error (e.g., a wrapped or sentinel error) so callers always get a defined "up to limit" behavior; update any logs/span attributes as needed (the function name is FindActiveNotifiable, span is started via tracer.Start, and dbLimit is computed later) to reflect the invalid-argument rejection.
503-510:⚠️ Potential issue | 🔴 Critical
limit*3is still only a heuristic, so the endpoint can underfill.
components/manager/internal/adapters/http/in/notification.goLines 114-120 still apply the notification-window filter and final slice after this query. If enough earlier deadlines fall outside theirNotifyDaysBeforewindow, they can exhaustdbLimitand hide later qualifying rows, so the endpoint returns fewer than requested even when more notifications exist. Push the window predicate into MongoDB or keep paging until you collectlimitqualifying deadlines.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/mongodb/deadline/deadline.mongodb.go` around lines 503 - 510, The current DB query uses a heuristic dbLimit := int64(limit * 3) with SetLimit(dbLimit) which can underfill because the application-side notification-window filter (in components/manager/internal/adapters/http/in/notification.go) may drop many returned rows; change the query to apply the notification-window predicate server-side (push the NotifyDaysBefore/NotifyWindow filter into the Mongo Find filter) so Mongo filters out non-qualifying deadlines before SetLimit, or alternatively implement paging: repeatedly run the Find (using SetSort(bson.D{{Key:"due_date",Value:1}}) and a moving cursor/skip or last-seen due_date) and accumulate qualifying rows until you have 'limit' notifications before returning; update code around dbLimit, opts, and the callsite that slices results so it either relies on DB-filtered results or paginates until the required count is met.
🤖 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/metrics.go`:
- Around line 44-61: NewMetricsHandler currently validates several UseCase
fields but misses Logger and Tracer, which leads to panics when GetMetrics
accesses mh.service.Tracer and mh.service.Logger; update NewMetricsHandler to
return an error if service.Logger or service.Tracer are nil, keeping the same
error style (e.g., "service.Logger must not be nil for MetricsHandler" and
"service.Tracer must not be nil for MetricsHandler"), so MetricsHandler{service:
service} is only returned when UseCase.Logger and UseCase.Tracer are present.
In `@components/manager/internal/adapters/http/in/notification.go`:
- Around line 165-175: The sort in sortNotificationsByUrgency uses sort.Slice
which does not preserve original order for equal-urgency items; change it to use
sort.SliceStable so items with equal severity and DaysUntilDue retain the
repository (original) ordering coming into the function (keep the existing
comparison logic using severityOrder and DaysUntilDue).
In `@pkg/mongodb/report/report.mongodb.go`:
- Around line 373-377: CountAll and CountByStatus start OpenTelemetry spans
without attaching the request id, breaking trace correlation; update
ReportMongoDBRepository.CountAll and ReportMongoDBRepository.CountByStatus to
read the request id from context (the same helper used elsewhere in the repo)
immediately after starting the span and set it on the span (using
span.SetAttribute / span.SetAttributes with the key "app.request.request_id")
before doing work so their spans match the other repository methods.
In `@pkg/mongodb/template/template.mongodb.go`:
- Around line 249-265: The CountAll method's implementation
(TemplateMongoDBRepository.CountAll) currently hard-filters "active: true" in
the filter variable which contradicts the comment "non-deleted templates
regardless of filters"; either remove the active filter from the filter bson (so
CountAll truly returns all non-deleted templates) or rename/re-document the
method to something like CountActive (and update any callers/tests) to reflect
"active non-deleted" semantics; locate the filter variable in CountAll and make
the corresponding change and update the method comment/signature/docs to match.
---
Duplicate comments:
In `@pkg/mongodb/deadline/deadline.mongodb.go`:
- Around line 479-510: In FindActiveNotifiable ensure the repository validates
the incoming limit and returns an error for non-positive values instead of
relying on callers; add a guard at the start of the method (before computing
dbLimit) that checks if limit <= 0 and returns a clear error (e.g., a wrapped or
sentinel error) so callers always get a defined "up to limit" behavior; update
any logs/span attributes as needed (the function name is FindActiveNotifiable,
span is started via tracer.Start, and dbLimit is computed later) to reflect the
invalid-argument rejection.
- Around line 503-510: The current DB query uses a heuristic dbLimit :=
int64(limit * 3) with SetLimit(dbLimit) which can underfill because the
application-side notification-window filter (in
components/manager/internal/adapters/http/in/notification.go) may drop many
returned rows; change the query to apply the notification-window predicate
server-side (push the NotifyDaysBefore/NotifyWindow filter into the Mongo Find
filter) so Mongo filters out non-qualifying deadlines before SetLimit, or
alternatively implement paging: repeatedly run the Find (using
SetSort(bson.D{{Key:"due_date",Value:1}}) and a moving cursor/skip or last-seen
due_date) and accumulate qualifying rows until you have 'limit' notifications
before returning; update code around dbLimit, opts, and the callsite that slices
results so it either relies on DB-filtered results or paginates until the
required count is met.
In `@pkg/mongodb/report/report.mongodb.go`:
- Around line 409-413: The query filter is using the creation timestamp
("created_at") to slice the time window but needs to count reports by their
status transition time; replace the "created_at" predicate in the filter
variable with the transition timestamp field ("completed_at") so the range uses
completed_at: bson.M{"$gte": from, "$lt": to}; ensure the rest of the filter
(status and deleted_at) remains unchanged and that any callers expecting the old
behavior still receive the same filter variable name.
In `@pkg/mongodb/template/template.mongodb.go`:
- Around line 251-255: CountAll is starting a tracer span
("repository.template.count_all") but doesn't set the app.request.request_id
attribute like Count does, breaking trace parity; fix by extracting the same
request ID from the context (use the same helper used in Count) immediately
after starting the span and add it to the span via span.SetAttributes (use the
same attribute key "app.request.request_id"), and apply the same change for the
other CountAll span occurrence around lines 267-273 so both
repository.template.count_all spans include the request id.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9573f126-77f3-4928-93bd-ab257809d4f5
📒 Files selected for processing (5)
components/manager/internal/adapters/http/in/metrics.gocomponents/manager/internal/adapters/http/in/notification.gopkg/mongodb/deadline/deadline.mongodb.gopkg/mongodb/report/report.mongodb.gopkg/mongodb/template/template.mongodb.go
…ments - CountAll/CountByStatus: add app.request.request_id span attribute for trace correlation - CountAll doc comments: align with actual behavior (active+non-deleted templates, non-deleted reports) - sortNotificationsByUrgency: use SliceStable to preserve due_date order as tie-breaker X-Lerian-Ref: 0x1
…dings - tests: fix context keys to use ctxutil helpers instead of raw string keys (critical) - FindActiveNotifiable: remove DB-level limit, apply after filter+sort in handler (critical) - notification_test: shuffle mock data in ordering tests to truly test sort logic (major) - routes: add metricsResource constant, replace reportResource on /v1/metrics (major) - handlers: add @failure 401/403 and use pkg.HTTPError in swagger annotations (major) - swagger: regenerated docs X-Lerian-Ref: 0x1
Contributor
|
This PR is very large (29 files, 5459 lines changed). Consider breaking it into smaller PRs for easier review. |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (6)
components/manager/internal/adapters/http/in/metrics.go (2)
44-61:⚠️ Potential issue | 🟠 MajorGuard
LoggerandTracerinNewMetricsHandler.
GetMetricsdereferences both dependencies. Missing guards here allow runtime panic on partially initializedUseCase.🛠️ Proposed fix
func NewMetricsHandler(service *services.UseCase) (*MetricsHandler, error) { if service == nil { return nil, errors.New("service must not be nil for MetricsHandler") } + + if service.Logger == nil { + return nil, errors.New("service.Logger must not be nil for MetricsHandler") + } + + if service.Tracer == nil { + return nil, errors.New("service.Tracer must not be nil for MetricsHandler") + } if service.TemplateRepo == nil { return nil, errors.New("service.TemplateRepo must not be nil for MetricsHandler") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/manager/internal/adapters/http/in/metrics.go` around lines 44 - 61, NewMetricsHandler currently only validates TemplateRepo, ReportRepo, and ExternalDataSources on the provided UseCase, but GetMetrics dereferences Logger and Tracer causing a potential panic; update NewMetricsHandler to also check that service.Logger and service.Tracer are non-nil and return an error like "service.Logger must not be nil for MetricsHandler" / "service.Tracer must not be nil for MetricsHandler" before constructing &MetricsHandler{service: service}, so MetricsHandler and GetMetrics can safely use those dependencies.
153-155:⚠️ Potential issue | 🟠 MajorReturn a structured 500 error payload matching
pkg.HTTPError.The current branch returns a raw
fiber.Map, while the endpoint contract documentspkg.HTTPError. Keep runtime response shape aligned with the API spec.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/manager/internal/adapters/http/in/metrics.go` around lines 153 - 155, Replace the ad-hoc fiber.Map error response with the documented pkg.HTTPError shape: where the handler currently does c.Status(fiber.StatusInternalServerError).JSON(fiber.Map{ "error": "failed to fetch metrics" }), construct and return an instance of pkg.HTTPError (using its exported fields, e.g., message/code or error/message fields) and pass that to JSON so the runtime response matches the API contract; update imports if needed and keep the same status code and descriptive message ("failed to fetch metrics") when returning pkg.HTTPError.components/manager/internal/adapters/http/in/template-builder.go (1)
109-109:⚠️ Potential issue | 🟡 MinorFix Swagger
@Paramformat for request bodies.Both annotations are missing the
intoken (body) and currently use an invalid order for swaggo.🛠️ Proposed fix
-// `@Param` body template_builder.GenerateCodeInput true "Block structure" +// `@Param` body body template_builder.GenerateCodeInput true "Block structure" ... -// `@Param` body template_builder.ValidateBlocksInput true "Blocks to validate" +// `@Param` body body template_builder.ValidateBlocksInput true "Blocks to validate"#!/bin/bash # Verify invalid `@Param` body annotations are gone after the fix. set -euo pipefail rg -nP '@Param\s+body\s+template_builder\.(GenerateCodeInput|ValidateBlocksInput)\s+true' \ components/manager/internal/adapters/http/in/template-builder.go # Expected result: # - No matches. # - Valid lines should include: `@Param` body body template_builder.<Type> true "..."Also applies to: 164-164
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/manager/internal/adapters/http/in/template-builder.go` at line 109, Swagger `@Param` annotations for the request bodies in template-builder.go use an invalid order/missing 'in' token; update the two annotations referencing template_builder.GenerateCodeInput and template_builder.ValidateBlocksInput (currently written as "@Param body template_builder.GenerateCodeInput true ...") to the correct swaggo format by inserting the 'in' value and proper ordering so they read like "@Param body body template_builder.GenerateCodeInput true \"...\"" and similarly for ValidateBlocksInput, ensuring the quoted description remains unchanged.pkg/mongodb/template/template.mongodb.go (1)
251-254:⚠️ Potential issue | 🟡 MinorAdd request-id span attribute in
CountAllfor trace correlation.
CountAllstarts a span but does not attachapp.request.request_id, unlike the other repository methods.🛠️ Proposed fix
func (tm *TemplateMongoDBRepository) CountAll(ctx context.Context) (int64, error) { tracer := ctxutil.NewTracerFromContext(ctx) + reqID := ctxutil.HeaderIDFromContext(ctx) ctx, span := tracer.Start(ctx, "repository.template.count_all") defer span.End() + span.SetAttributes(attribute.String("app.request.request_id", reqID))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/mongodb/template/template.mongodb.go` around lines 251 - 254, The CountAll method starts a tracer span but omits attaching the request id attribute for trace correlation; in CountAll (where tracer := ctxutil.NewTracerFromContext(ctx) and ctx, span := tracer.Start(...)) extract the request id from the context (using the same helper used in other repository methods) and call span.SetAttributes or the tracer's equivalent to add "app.request.request_id" with that value before returning/ending the span so the span includes the request id for correlation.components/manager/api/openapi.yaml (1)
207-213:⚠️ Potential issue | 🟡 MinorEncode documented query bounds in schema (
minimum/maximum).The descriptions declare numeric ranges, but the schema does not enforce them.
🛠️ Proposed fix
- description: Maximum number of notifications (1-100) in: query name: limit schema: default: 10 + minimum: 1 + maximum: 100 type: integer ... - description: Number of days for error period (1-365) in: query name: errorPeriodDays schema: default: 7 + minimum: 1 + maximum: 365 type: integerAlso applies to: 422-427
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/manager/api/openapi.yaml` around lines 207 - 213, The OpenAPI query parameter "limit" declares a numeric range in its description but does not enforce it in the schema; update the parameter schema for "limit" (and any other similar numeric query params referenced later) to include JSON Schema bounds by adding "minimum: 1" and "maximum: 100" (or the documented bounds) so the spec enforces the documented constraints; apply the same change to the other affected numeric query parameter(s) referenced in the later block (lines 422-427) so their schemas include corresponding "minimum"/"maximum" annotations.pkg/mongodb/report/report.mongodb.go (1)
415-418:⚠️ Potential issue | 🟠 MajorUse status-transition timestamp for
CountByStatuswindowing.Filtering by
created_atundercounts status transitions. A report created earlier but moved toErrorduring the period is excluded.🛠️ Proposed fix
filter := bson.M{ "status": status, - "created_at": bson.M{"$gte": from, "$lt": to}, + "completed_at": bson.M{"$gte": from, "$lt": to}, "deleted_at": bson.D{{Key: "$eq", Value: nil}}, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/mongodb/report/report.mongodb.go` around lines 415 - 418, In CountByStatus replace the current time-window filter on "created_at" with a filter that matches the status transition timestamp for the given status (e.g. use an $elemMatch on "status_transitions" or the equivalent field) so reports that changed to the target status during the window are counted; update the filter variable (where "created_at" is currently used) to query something like status_transitions: bson.M{"$elemMatch": bson.M{"status": status, "timestamp": bson.M{"$gte": from, "$lt": to}}} while keeping the "status" match and "deleted_at" nil check.
🤖 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/notification_test.go`:
- Around line 368-369: Remove the redundant loop-variable capture statements
(the standalone "tt := tt") in the table-driven test loops that call
t.Parallel(); Go 1.26 captures loop variables per-iteration so delete those
extra assignments in the test loops iterating over tests (the for _, tt := range
tests { ... } blocks) in notification_test.go where t.Parallel() is used (ensure
each subtest still calls t.Run with tt.name and t.Parallel()).
In `@pkg/mongodb/deadline/deadline.mongodb.go`:
- Around line 477-489: The FindActiveNotifiable repository method documents "up
to the given limit" but never applies it; update
DeadlineMongoDBRepository.FindActiveNotifiable to enforce the limit by passing
options.Find(...).SetLimit(int64(limit)) (or only set when limit > 0) to the
MongoDB find call so the query honors the requested max results, and keep the
existing tracing attribute for "limit" intact; alternatively, if limit is
intentionally handled upstream, change the function comment to remove the "up to
the given limit" wording and state that limiting is the caller's responsibility.
- Around line 503-505: The current repository fetch builds opts via
options.Find().SetSort(...) and then eagerly loads the entire cursor (e.g.,
Find(..., opts) and cursor.All(...)), which can OOM on large datasets; add an
internal safety cap (e.g., const repoMaxLimit) and apply it to opts using
opts.SetLimit(min(userProvidedLimitOrZero, repoMaxLimit)) or set
opts.SetLimit(repoMaxLimit) when no user limit is provided, then ensure cursor
iteration respects that cap instead of calling cursor.All on an unbounded result
set; update any usage of opts, Find, and cursor.All in the deadline fetch path
to enforce this repo-side limit.
---
Duplicate comments:
In `@components/manager/api/openapi.yaml`:
- Around line 207-213: The OpenAPI query parameter "limit" declares a numeric
range in its description but does not enforce it in the schema; update the
parameter schema for "limit" (and any other similar numeric query params
referenced later) to include JSON Schema bounds by adding "minimum: 1" and
"maximum: 100" (or the documented bounds) so the spec enforces the documented
constraints; apply the same change to the other affected numeric query
parameter(s) referenced in the later block (lines 422-427) so their schemas
include corresponding "minimum"/"maximum" annotations.
In `@components/manager/internal/adapters/http/in/metrics.go`:
- Around line 44-61: NewMetricsHandler currently only validates TemplateRepo,
ReportRepo, and ExternalDataSources on the provided UseCase, but GetMetrics
dereferences Logger and Tracer causing a potential panic; update
NewMetricsHandler to also check that service.Logger and service.Tracer are
non-nil and return an error like "service.Logger must not be nil for
MetricsHandler" / "service.Tracer must not be nil for MetricsHandler" before
constructing &MetricsHandler{service: service}, so MetricsHandler and GetMetrics
can safely use those dependencies.
- Around line 153-155: Replace the ad-hoc fiber.Map error response with the
documented pkg.HTTPError shape: where the handler currently does
c.Status(fiber.StatusInternalServerError).JSON(fiber.Map{ "error": "failed to
fetch metrics" }), construct and return an instance of pkg.HTTPError (using its
exported fields, e.g., message/code or error/message fields) and pass that to
JSON so the runtime response matches the API contract; update imports if needed
and keep the same status code and descriptive message ("failed to fetch
metrics") when returning pkg.HTTPError.
In `@components/manager/internal/adapters/http/in/template-builder.go`:
- Line 109: Swagger `@Param` annotations for the request bodies in
template-builder.go use an invalid order/missing 'in' token; update the two
annotations referencing template_builder.GenerateCodeInput and
template_builder.ValidateBlocksInput (currently written as "@Param
body template_builder.GenerateCodeInput true ...") to the correct swaggo format
by inserting the 'in' value and proper ordering so they read like "@Param body
body template_builder.GenerateCodeInput true \"...\"" and similarly for
ValidateBlocksInput, ensuring the quoted description remains unchanged.
In `@pkg/mongodb/report/report.mongodb.go`:
- Around line 415-418: In CountByStatus replace the current time-window filter
on "created_at" with a filter that matches the status transition timestamp for
the given status (e.g. use an $elemMatch on "status_transitions" or the
equivalent field) so reports that changed to the target status during the window
are counted; update the filter variable (where "created_at" is currently used)
to query something like status_transitions: bson.M{"$elemMatch":
bson.M{"status": status, "timestamp": bson.M{"$gte": from, "$lt": to}}} while
keeping the "status" match and "deleted_at" nil check.
In `@pkg/mongodb/template/template.mongodb.go`:
- Around line 251-254: The CountAll method starts a tracer span but omits
attaching the request id attribute for trace correlation; in CountAll (where
tracer := ctxutil.NewTracerFromContext(ctx) and ctx, span := tracer.Start(...))
extract the request id from the context (using the same helper used in other
repository methods) and call span.SetAttributes or the tracer's equivalent to
add "app.request.request_id" with that value before returning/ending the span so
the span includes the request id for correlation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a276f71c-7c54-4c43-8353-29db3ec02da5
📒 Files selected for processing (13)
components/manager/api/docs.gocomponents/manager/api/openapi.yamlcomponents/manager/api/swagger.jsoncomponents/manager/api/swagger.yamlcomponents/manager/internal/adapters/http/in/metrics.gocomponents/manager/internal/adapters/http/in/metrics_test.gocomponents/manager/internal/adapters/http/in/notification.gocomponents/manager/internal/adapters/http/in/notification_test.gocomponents/manager/internal/adapters/http/in/routes.gocomponents/manager/internal/adapters/http/in/template-builder.gopkg/mongodb/deadline/deadline.mongodb.gopkg/mongodb/report/report.mongodb.gopkg/mongodb/template/template.mongodb.go
components/manager/internal/adapters/http/in/notification_test.go
Outdated
Show resolved
Hide resolved
- Remove redundant tt := tt loop variable capture (unnecessary in Go 1.26+) - FindActiveNotifiable: add 1000-document safety cap to prevent unbounded memory spikes - FindActiveNotifiable: update doc comment to clarify limit is applied by caller post-filter X-Lerian-Ref: 0x1
Contributor
|
This PR is very large (29 files, 5464 lines changed). Consider breaking it into smaller PRs for easier review. |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/metrics_test.go`:
- Around line 147-178: The tests in metrics_test.go (e.g., the "Success - Get
metrics with custom errorPeriodDays" case) only assert mocked counts while
passing gomock.Any() for the CountByStatus from/to params, so a handler that
uses wrong time windows would still pass; update these tests to capture and
assert the actual from/to timestamps passed into template.MockRepository/
report.MockRepository.CountByStatus (use gomock.AssignableToTypeOf/time parsing
or gomock.Do to capture args) or refactor the handler to accept an injected
clock and in tests set a fixed clock and assert the expected current and
previous period ranges are used for the CountByStatus calls (verify the captured
timestamps match the bounds for errorPeriodDays values such as 30, 1, 365 and
the default case).
In `@components/manager/internal/adapters/http/in/notification_test.go`:
- Around line 64-80: Add a test that exercises the constructor guard for a
missing DeadlineRepo: create a services.UseCase with Logger and Tracer set (like
in TestNewNotificationHandler_ValidService) but set DeadlineRepo to nil, call
NewNotificationHandler(svc) and assert that the returned handler is nil and an
error is returned (e.g., require.Error and assert.Nil). Name the test something
like TestNewNotificationHandler_NilDeadlineRepo and include t.Parallel(); no
gomock controller is required since DeadlineRepo is nil.
- Around line 392-401: The test leaks the HTTP response body from app.Test;
ensure resp.Body is closed in every subtest by calling resp.Body.Close() after
app.Test returns (either via defer resp.Body.Close() immediately after
require.NoError(t, err) inside the subtest or by closing it explicitly after
reading and checking the body). Update the block around app.Test/resp to
guarantee resp.Body.Close() is always executed (even when tt.checkBody is nil)
so no response bodies remain open.
In `@pkg/mongodb/deadline/deadline.mongodb.go`:
- Around line 481-491: The FindActiveNotifiable method currently accepts a limit
parameter that is only recorded in tracing and not used in the DB query; remove
the limit parameter to avoid confusion: change the function signature of
FindActiveNotifiable(ctx context.Context, limit int) to FindActiveNotifiable(ctx
context.Context), remove the span.SetAttributes attribute referencing "limit"
and any use of the limit variable, and update all callers to stop passing a
limit (or add a lightweight wrapper if you need backward compatibility); ensure
tests and documentation are updated accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 727760f6-aec5-4eb8-868f-94d4a5232719
📒 Files selected for processing (3)
components/manager/internal/adapters/http/in/metrics_test.gocomponents/manager/internal/adapters/http/in/notification_test.gopkg/mongodb/deadline/deadline.mongodb.go
The limit parameter was accepted but never applied to the query — only recorded in span attributes. The repository-side safety cap (maxNotifiableFetch=1000) handles unbounded fetches. Callers apply the user-facing limit after notification-window filtering and urgency sort. X-Lerian-Ref: 0x1
Contributor
|
This PR is very large (29 files, 5463 lines changed). Consider breaking it into smaller PRs for easier review. |
Contributor
gandalf-at-lerian
left a comment
gandalf-at-lerian
left a comment
There was a problem hiding this comment.
Good PR, Arthur. Two new endpoints (metrics + notifications) with clean handler patterns, solid test coverage (unit + property + fuzz), and proper OTel instrumentation throughout.
A few things I'd flag:
1. Swagger annotation regression in template-builder.go
The @Param annotation change drops the body location keyword:
// Before:
// @Param body body template_builder.GenerateCodeInput true "Block structure"
// After:
// @Param body template_builder.GenerateCodeInput true "Block structure"
swag expects the format @Param name paramType dataType required description. Removing body (the paramType) will likely break swagger generation for GenerateCode and ValidateBlocks. This looks unintentional — might want to revert those two lines.
2. Duplicate constant — same package, different names
metrics.go defines hoursPerDay = 24, notification.go defines hoursInDay = 24. Same value, same package. Pick one and share it.
3. Minor: third goroutine in GetMetrics does two sequential DB calls on one error slot
errs[2] covers both CountByStatus calls (current + previous period). It works correctly — if the first fails, it short-circuits; if the second fails, it overwrites. But since you already have a well-documented metricsParallelGroups constant, consider splitting into 4 goroutines (one per DB call) for consistency. Not blocking, just cleaner.
Everything else looks solid — constructor validation, repository safety cap (maxNotifiableFetch=1000), severity ordering, input validation with fuzz coverage. 👍
…nstant - template-builder: restore missing 'body' paramType in @param annotations for GenerateCode and ValidateBlocks (regression introduced by linter) - notification: replace hoursInDay with hoursPerDay to share single constant across the in package - swagger: regenerated docs X-Lerian-Ref: 0x1
Contributor
|
This PR is very large (28 files, 5458 lines changed). Consider breaking it into smaller PRs for easier review. |
brunobls
approved these changes
Mar 18, 2026
Template entity has no active field. The filter active=true was incorrectly added based on a false positive review finding, causing CountAll to return 0 for all existing templates. Reverts to deleted_at filter only. X-Lerian-Ref: 0x1
Contributor
|
This PR is very large (28 files, 5457 lines changed). Consider breaking it into smaller PRs for easier review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.