feat(crm): add tenant-wide alias resolver#2109
Open
fredcamaral wants to merge 5 commits into
Open
Conversation
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (7)
💤 Files with no reviewable changes (3)
WalkthroughAdds tenant-wide alias resolution endpoints and a bank-account identity index with backfill; updates OpenAPI/Swagger and domain models; adds HTTP handlers, validation, service use-cases, transactional MongoDB persistence, index maintenance, mocks, and extensive tests plus replica-set test infra. ChangesAlias Resolution and Bank Account Index Backfill
Comment |
Contributor
📊 Unit Test Coverage Report:
|
| Metric | Value |
|---|---|
| Overall Coverage | 86.1% ✅ PASS |
| Threshold | 85% |
Coverage by Package
| Package | Coverage |
|---|---|
github.com/LerianStudio/midaz/v3/components/crm/internal/adapters/http/in |
81.0% |
github.com/LerianStudio/midaz/v3/components/crm/internal/adapters/mongodb/alias |
93.3% |
github.com/LerianStudio/midaz/v3/components/crm/internal/adapters/mongodb/holder |
87.1% |
github.com/LerianStudio/midaz/v3/components/crm/internal/services |
88.8% |
Generated by Go PR Analysis workflow
Contributor
🔒 Security Scan Results —
|
| Policy | Status |
|---|---|
| Default non-root user | ✅ Passed |
| No fixable critical/high CVEs | ✅ Passed |
| No high-profile vulnerabilities | ✅ Passed |
| No AGPL v3 licenses | ✅ Passed |
Pre-release Version Check
✅ No unstable version pins found.
Contributor
🔒 Security Scan Results —
|
| Policy | Status |
|---|---|
| Default non-root user | ✅ Passed |
| No fixable critical/high CVEs | ✅ Passed |
| No high-profile vulnerabilities | ✅ Passed |
| No AGPL v3 licenses | ✅ Passed |
Pre-release Version Check
✅ No unstable version pins found.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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 `@components/crm/internal/adapters/http/in/alias.go`:
- Around line 375-489: The file contains multiple handler operations
(ResolveBankAccount, ResolveAccount, BackfillBankAccountIndex) in the same
source which violates the guideline "one handler/operation per file"; split each
handler into its own file (e.g., create separate files for ResolveBankAccount,
ResolveAccount, and BackfillBankAccountIndex) preserving the AliasHandler
methods, imports, tracer/log setup and associated comments/Swagger annotations,
and update package-level imports and build (if any) so each new file compiles
standalone within the adapters/http/in package.
In
`@components/crm/internal/adapters/mongodb/alias/alias.mongodb_integration_test.go`:
- Line 371: The test currently creates closingDate using time.Now() (closingDate
:= mmodel.Date{Time: time.Now().UTC().Add(24 * time.Hour)}), which makes the
test time-dependent; replace this with a fixed instant (e.g., construct a
deterministic time via time.Date(...) or the project's fixed-time test utility)
and seed mmodel.Date from that fixed instant so closingDate is deterministic
across runs.
In `@components/crm/internal/adapters/mongodb/alias/alias.mongodb.go`:
- Around line 89-100: The withTransaction method on MongoDBRepository should
short-circuit when the provided context is already canceled: before calling
db.Client().StartSession() check ctx.Err() and immediately return that error if
non-nil so you don't open a session for canceled requests; update the function
to perform this ctx.Err() guard at the top of withTransaction and keep the
existing session.StartSession/WithTransaction flow unchanged otherwise.
In `@components/crm/internal/adapters/mongodb/alias/bank_account_index_test.go`:
- Line 86: Replace the nondeterministic now := time.Now() in
bank_account_index_test.go with a fixed timestamp (e.g., time.Date(...)) and use
that fixed value for any CreatedAt/UpdatedAt fields in the test setup (the
variable currently named "now" and any structs that set CreatedAt/UpdatedAt) so
the test is deterministic; update any assertions expecting those timestamps to
use the same fixed value.
In
`@components/crm/internal/adapters/mongodb/alias/bank_account_index.mongodb.go`:
- Around line 315-357: Add explicit context cancellation checks (ctx.Err()) in
BackfillBankAccountIndex and backfillAliasCollection: before starting any
DB‑intensive operation (e.g., before db.ListCollectionNames, before calling
am.backfillAliasCollection, and before any heavy query/aggregation) and inside
long-running loops such as the cursor iteration in backfillAliasCollection; if
ctx.Err() != nil return early with the context error (or wrap it) so
cancellation/timeout stops work promptly and resources (cursors/span) are
cleaned up. Ensure the same pattern is applied to the later related flow (the
functions covering lines 360-448) and keep context as the first parameter in
those helper calls.
In `@components/crm/internal/services/resolve-alias.go`:
- Around line 21-49: Add an early context-cancellation short-circuit in
ResolveBankAccount by checking ctx.Err() immediately after starting the span and
before any expensive repo calls; if ctx.Err() != nil return nil, ctx.Err().
Specifically, insert the check before calling uc.AliasRepo.ResolveBankAccount
and before calling resolveOneAlias to avoid performing DB work for canceled
contexts; apply the same pattern to the other resolver methods referenced (the
methods around lines 51-83 and 166-186) so each method checks ctx.Err() right
after tracer.Start(...) and before any repository or heavy work.
In `@components/crm/internal/services/validate-banking-details.go`:
- Around line 65-73: The validation function is creating a child span for pure
in-memory validation: remove the tracer.Start(...) / defer span.End() lines and
stop calling libOpenTelemetry.HandleSpanBusinessErrorEvent from this validator;
instead keep NewTrackingFromContext(...) and only emit the logger.Log(...)
warning with the constructed error (err) so span emission remains at the
caller’s I/O boundary (refer to tracer.Start, span.End, and
libOpenTelemetry.HandleSpanBusinessErrorEvent in this file).
In `@pkg/mmodel/alias.go`:
- Line 81: Change the ID fields from plain strings to UUID types: replace
OrganizationID *string with *uuid.UUID, change any resolver ID fields and
backfill alias ID lists from string/[]string to uuid.UUID/*uuid.UUID and
[]uuid.UUID respectively, and update their struct definitions (e.g., the alias
DTO where OrganizationID is defined plus the other affected DTOs referenced in
the review) so JSON marshalling still uses the same tags; also add/import the
uuid package (github.com/google/uuid or your project's preferred uuid package)
where these structs are declared.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 439856a4-036c-4749-8434-315977ca6771
📒 Files selected for processing (28)
components/crm/api/docs.gocomponents/crm/api/openapi.yamlcomponents/crm/api/swagger.jsoncomponents/crm/api/swagger.yamlcomponents/crm/internal/adapters/http/in/alias.gocomponents/crm/internal/adapters/http/in/alias_test.gocomponents/crm/internal/adapters/http/in/routes.gocomponents/crm/internal/adapters/mongodb/alias/alias.mongodb.gocomponents/crm/internal/adapters/mongodb/alias/alias.mongodb_integration_test.gocomponents/crm/internal/adapters/mongodb/alias/alias.mongodb_mock.gocomponents/crm/internal/adapters/mongodb/alias/alias.mongodb_test.gocomponents/crm/internal/adapters/mongodb/alias/alias_maintenance.mongodb.gocomponents/crm/internal/adapters/mongodb/alias/alias_query.mongodb.gocomponents/crm/internal/adapters/mongodb/alias/bank_account_index.gocomponents/crm/internal/adapters/mongodb/alias/bank_account_index.mongodb.gocomponents/crm/internal/adapters/mongodb/alias/bank_account_index_test.gocomponents/crm/internal/services/create-alias.gocomponents/crm/internal/services/create-alias_test.gocomponents/crm/internal/services/resolve-alias.gocomponents/crm/internal/services/resolve-alias_test.gocomponents/crm/internal/services/update-alias.gocomponents/crm/internal/services/update-alias_test.gocomponents/crm/internal/services/validate-banking-details.gopkg/constant/entity.gopkg/mmodel/alias.gopkg/net/http/httputils.gopkg/net/http/httputils_alias_bank_filters_test.gotests/utils/mongodb/container.go
X-Lerian-Ref: 0x1
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@components/crm/internal/adapters/mongodb/alias/alias.mongodb.go`:
- Around line 550-589: mergeAliasForBankAccountIndex currently only clears the
whole banking_details object but must also clear identity fields removed by
PATCH; update mergeAliasForBankAccountIndex to inspect fieldsToRemove for
"document"/"document", "account_id"/"accountId", "holder_id"/"holderId" and
nested banking fields like "banking_details.account", "bankingDetails.account",
"banking_details.bank", "bankingDetails.bank", "banking_details.iban", etc., and
when present set merged.Document = nil, merged.AccountID = nil, merged.HolderID
= nil, and clear the corresponding fields on merged.BankingDetails (or set
BankingDetails = nil if all its identity subfields are removed); handle both
snake_case and camelCase variants and perform these clears after merging (near
the existing loop over fieldsToRemove) so the candidate used by
preflightBankAccountIdentityConflict/replaceBankAccountIndex no longer contains
stale identity values.
In `@components/crm/internal/adapters/mongodb/alias/bank_account_index.go`:
- Around line 130-137: The code only validates organization_id with
parseRequiredUUIDString but leaves ledger_id and account_id unparsed; add
explicit parseRequiredUUIDString calls for model.LedgerID and model.AccountID
(after organizationID parsing) and return errors.New("malformed bank account
index row: invalid ledger_id") and errors.New("malformed bank account index row:
invalid account_id") on parse errors, keeping the existing zero-UUID check
(isZeroUUIDString) intact; reference the functions isZeroUUIDString and
parseRequiredUUIDString and the model fields LedgerID and AccountID to locate
where to insert the checks.
In `@components/crm/internal/services/resolve-alias.go`:
- Around line 117-121: The child span created for duplicate matches should be
removed and the business-error event should be emitted on the existing parent
span parameter instead; replace the tracer.Start/duplicateSpan creation and
duplicateSpan.End() with a direct call to
libOpenTelemetry.HandleSpanBusinessErrorEvent(span, "Duplicate active alias
resolver matches", err) after creating err via
pkg.ValidateBusinessError(cn.ErrAccountAlreadyAssociated, cn.EntityAlias),
mirroring the approach used in the case 0 branch and avoiding
libCommons.NewTrackingFromContext/tracer usage here.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ac5d63cb-f43f-44a3-b919-c91d36a1e078
📒 Files selected for processing (14)
components/crm/internal/adapters/http/in/alias.gocomponents/crm/internal/adapters/http/in/alias_test.gocomponents/crm/internal/adapters/http/in/backfill_bank_account_index.gocomponents/crm/internal/adapters/http/in/resolve_account.gocomponents/crm/internal/adapters/http/in/resolve_bank_account.gocomponents/crm/internal/adapters/mongodb/alias/alias.mongodb.gocomponents/crm/internal/adapters/mongodb/alias/alias.mongodb_integration_test.gocomponents/crm/internal/adapters/mongodb/alias/bank_account_index.gocomponents/crm/internal/adapters/mongodb/alias/bank_account_index.mongodb.gocomponents/crm/internal/adapters/mongodb/alias/bank_account_index_test.gocomponents/crm/internal/services/resolve-alias.gocomponents/crm/internal/services/resolve-alias_test.gocomponents/crm/internal/services/validate-banking-details.gopkg/mmodel/alias.go
X-Lerian-Ref: 0x1
Contributor
📊 Unit Test Coverage Report:
|
| Metric | Value |
|---|---|
| Overall Coverage | 85.4% ✅ PASS |
| Threshold | 85% |
Coverage by Package
| Package | Coverage |
|---|---|
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/http/in |
85.8% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/mongodb/onboarding |
66.7% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/mongodb/transaction |
66.7% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/postgres/account |
100.0% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/postgres/accounttype |
66.7% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/postgres/asset |
100.0% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/postgres/assetrate |
100.0% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/postgres/balance |
97.5% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/postgres/ledger |
100.0% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/postgres/operation |
89.9% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/postgres/operationroute |
100.0% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/postgres/organization |
100.0% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/postgres/portfolio |
100.0% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/postgres/segment |
100.0% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/postgres/transaction |
94.9% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/postgres/transactionroute |
100.0% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/rabbitmq |
91.5% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/redis/transaction/balance |
100.0% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/services/command |
86.9% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/services/query |
93.1% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/services |
0.0% |
Generated by Go PR Analysis workflow
X-Lerian-Ref: 0x1
Member
Author
Fixes Applied SuccessfullyFixed 5 file(s) based on 3 CodeRabbit feedback item(s). Files modified:
Commit: The latest autofix changes are on the |
Format struct initializations, remove extraneous blank lines, and realign map keys and comments across several test and configuration files in the ledger component. This ensures compliance with standard Go formatting conventions and improves code structure and readability.
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
2 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.
Summary
Validation