feat: add routeId to transaction for FK-backed route identification#1930
feat: add routeId to transaction for FK-backed route identification#1930ClaraTersi merged 6 commits intodevelopfrom
Conversation
… transaction_route ✨
WalkthroughThis pull request introduces a new UUID-based 🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
Comment Tip CodeRabbit can use OpenGrep to find security vulnerabilities and bugs across 17+ programming languages.OpenGrep is compatible with Semgrep configurations. Add an |
📊 Unit Test Coverage Report:
|
| Metric | Value |
|---|---|
| Overall Coverage | 85.4% ✅ PASS |
| Threshold | 85% |
Coverage by Package
| Package | Coverage |
|---|---|
github.com/LerianStudio/midaz/v3/components/transaction/internal/adapters/grpc/in |
100.0% |
github.com/LerianStudio/midaz/v3/components/transaction/internal/adapters/http/in |
78.5% |
github.com/LerianStudio/midaz/v3/components/transaction/internal/adapters/mongodb |
66.7% |
github.com/LerianStudio/midaz/v3/components/transaction/internal/adapters/postgres/assetrate |
100.0% |
github.com/LerianStudio/midaz/v3/components/transaction/internal/adapters/postgres/balance |
100.0% |
github.com/LerianStudio/midaz/v3/components/transaction/internal/adapters/postgres/operation |
90.0% |
github.com/LerianStudio/midaz/v3/components/transaction/internal/adapters/postgres/operationroute |
100.0% |
github.com/LerianStudio/midaz/v3/components/transaction/internal/adapters/postgres/transaction |
97.4% |
github.com/LerianStudio/midaz/v3/components/transaction/internal/adapters/postgres/transactionroute |
100.0% |
github.com/LerianStudio/midaz/v3/components/transaction/internal/adapters/rabbitmq |
93.1% |
github.com/LerianStudio/midaz/v3/components/transaction/internal/adapters/redis/balance |
100.0% |
github.com/LerianStudio/midaz/v3/components/transaction/internal/services/command |
90.4% |
github.com/LerianStudio/midaz/v3/components/transaction/internal/services/query |
95.2% |
github.com/LerianStudio/midaz/v3/components/transaction/internal/services |
100.0% |
Generated by Go PR Analysis workflow
🔒 Security Scan Results —
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/transaction/internal/adapters/postgres/transaction/transaction.postgresql.go (1)
149-166:⚠️ Potential issue | 🟠 MajorStop inserting by table position before shipping this migration.
INSERT INTO transaction VALUES (...)is now rollout-sensitive: once000027_add_route_id_to_transactionruns, any still-running pod on the previous 15-value statement will start failing writes until it is replaced. An explicit column list keeps this nullable-column migration backward-compatible for rolling deploys and avoids the same breakage on the next schema addition.Suggested change
-result, err := db.ExecContext(ctx, `INSERT INTO transaction VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16) RETURNING *`, +result, err := db.ExecContext(ctx, `INSERT INTO transaction ( + id, + parent_transaction_id, + description, + status, + status_description, + amount, + asset_code, + chart_of_accounts_group_name, + ledger_id, + organization_id, + body, + created_at, + updated_at, + deleted_at, + route, + route_id + ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16) RETURNING *`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/transaction/internal/adapters/postgres/transaction/transaction.postgresql.go` around lines 149 - 166, The SQL uses positional INSERT INTO transaction VALUES (...) which breaks rolling upgrades when columns are added; update the ExecContext call that constructs the INSERT (the db.ExecContext(...) call with `INSERT INTO transaction VALUES ... RETURNING *`) to use an explicit column list matching the fields passed (include the new RouteID/Route columns and all other columns in the same order) so the statement remains backward-compatible during migrations; keep RETURNING * or change to explicitly listed returning columns if desired.components/transaction/internal/adapters/postgres/transaction/transaction.go (1)
171-293: 🧹 Nitpick | 🔵 TrivialSwagger documentation models are missing
RouteIDfield.The
CreateTransactionSwaggerModel(and similarlyCreateTransactionInflowSwaggerModelat lines 744-827 andCreateTransactionOutflowSwaggerModelat lines 956-1044) are used for Swagger documentation generation but don't include the newrouteIdfield. This will result in incomplete API documentation for consumers.Consider adding
RouteIDto these Swagger models for documentation consistency.📝 Proposed addition for CreateTransactionSwaggerModel
type CreateTransactionSwaggerModel struct { // ... existing fields ... + // Deprecated: legacy route identifier, use routeId instead + // example: 00000000-0000-0000-0000-000000000000 + Route string `json:"route,omitempty"` + + // UUID of the transaction route. Used instead of route for proper UUID validation. + // example: 00000000-0000-0000-0000-000000000000 + // format: uuid + RouteID *string `json:"routeId,omitempty"` + // TransactionDate Period from transaction creation date until nowApply similar changes to
CreateTransactionInflowSwaggerModelandCreateTransactionOutflowSwaggerModel.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/transaction/internal/adapters/postgres/transaction/transaction.go` around lines 171 - 293, Add the missing RouteID field to the Swagger models so API docs include routeId: update CreateTransactionSwaggerModel, CreateTransactionInflowSwaggerModel, and CreateTransactionOutflowSwaggerModel by adding a RouteID field (e.g., RouteID string `json:"routeId,omitempty"`) with an appropriate comment/example and JSON tag to match other fields, ensuring it appears in the generated Swagger docs; make the field optional (omitempty) to preserve existing behavior.
🤖 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/transaction/internal/bootstrap/redis.consumer.go`:
- Around line 364-365: processMessage currently only checks
m.Validate.TransactionRoute when resolving routeCache, causing rebuilt ops to
miss routeCode/routeDescription if a message carries only the new
transaction-level m.TransactionInput.RouteID; update the resolve logic in
processMessage to first attempt a lookup using m.TransactionInput.RouteID
(TransactionRouteID) and assign routeCache from that result, and only if that is
nil/empty fall back to the deprecated m.Validate.TransactionRoute string before
calling BuildOperations so rebuilt operations get route metadata populated
correctly.
In
`@components/transaction/internal/services/query/validate-accounting-routes.go`:
- Around line 50-85: Detect and reject conflicting route values instead of
silently preferring TransactionRouteID: when both validate.TransactionRouteID
and validate.TransactionRoute are present, parse both into UUIDs (using
uuid.Parse) and if they differ return a business validation error (use
pkg.ValidateBusinessError with an appropriate constant or reuse
ErrInvalidTransactionRouteID) and emit the same span/log events as other
validation failures; otherwise proceed using the parsed UUID. Update the logic
around transactionRouteID parsing in validate-accounting-routes.go (the branches
that read validate.TransactionRouteID and validate.TransactionRoute) to perform
this comparison and early rejection.
In `@components/transaction/migrations/000027_add_route_id_to_transaction.up.sql`:
- Around line 1-3: Add an index on the new referencing column to avoid
full-table scans and long locks: in the migration that adds route_id to the
"transaction" table (and which creates fk_transaction_route_id), create an index
on transaction.route_id (or create it in a follow-up migration using CREATE
INDEX CONCURRENTLY if you cannot block writes). Ensure the index name is unique
and descriptive (e.g., ix_transaction_route_id) and create it against the
route_id column to support foreign-key checks and route-based lookups.
---
Outside diff comments:
In
`@components/transaction/internal/adapters/postgres/transaction/transaction.go`:
- Around line 171-293: Add the missing RouteID field to the Swagger models so
API docs include routeId: update CreateTransactionSwaggerModel,
CreateTransactionInflowSwaggerModel, and CreateTransactionOutflowSwaggerModel by
adding a RouteID field (e.g., RouteID string `json:"routeId,omitempty"`) with an
appropriate comment/example and JSON tag to match other fields, ensuring it
appears in the generated Swagger docs; make the field optional (omitempty) to
preserve existing behavior.
In
`@components/transaction/internal/adapters/postgres/transaction/transaction.postgresql.go`:
- Around line 149-166: The SQL uses positional INSERT INTO transaction VALUES
(...) which breaks rolling upgrades when columns are added; update the
ExecContext call that constructs the INSERT (the db.ExecContext(...) call with
`INSERT INTO transaction VALUES ... RETURNING *`) to use an explicit column list
matching the fields passed (include the new RouteID/Route columns and all other
columns in the same order) so the statement remains backward-compatible during
migrations; keep RETURNING * or change to explicitly listed returning columns if
desired.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 08caa9e9-10ee-4011-9fa0-efdffcf0c344
📒 Files selected for processing (9)
components/transaction/internal/adapters/http/in/transaction-creation-helpers.gocomponents/transaction/internal/adapters/postgres/transaction/transaction.gocomponents/transaction/internal/adapters/postgres/transaction/transaction.postgresql.gocomponents/transaction/internal/bootstrap/redis.consumer.gocomponents/transaction/internal/services/query/validate-accounting-routes.gocomponents/transaction/migrations/000027_add_route_id_to_transaction.down.sqlcomponents/transaction/migrations/000027_add_route_id_to_transaction.up.sqlpkg/transaction/transaction.gopkg/transaction/validations.go
components/transaction/internal/services/query/validate-accounting-routes.go
Show resolved
Hide resolved
components/transaction/migrations/000027_add_route_id_to_transaction.up.sql
Outdated
Show resolved
Hide resolved
gandalf-at-lerian
left a comment
gandalf-at-lerian
left a comment
There was a problem hiding this comment.
Solid feature. The design is well thought-out and the backward compat story is clean.
What I checked:
Migration (000027): the up migration adds route_id as nullable with FK + partial index (WHERE route_id IS NOT NULL). The partial index is a nice touch — avoids indexing the NULLs from legacy records. CodeRabbit's initial concern about a missing index was addressed. One note: ALTER TABLE ... ADD CONSTRAINT on a large table takes an ACCESS EXCLUSIVE lock. If this runs against a production table with significant volume, consider whether this needs a maintenance window.
INSERT without explicit column list (transaction.postgresql.go): CodeRabbit flagged the positional INSERT INTO transaction VALUES ($1...$16) as a rolling-deploy hazard. The concern is valid in theory (pod on old code will fail writes after migration runs), but since this PR itself adds the column and updates the INSERT in the same deploy, in practice it's only a risk if you roll back the code without rolling back the migration. Acceptable, but worth keeping in mind.
Validation logic (validate-accounting-routes.go): prefer routeId → fallback to route → error if neither provided and accounting is on. Clean. The TransactionRoute → UUID parse path is preserved. CodeRabbit suggested rejecting conflicting routeId vs route values; I'd say it's defensible to silently prefer routeId (priority rule is documented in the PR description), but if you want to be extra strict you could add that check later without breaking anything.
Redis consumer: processMessage correctly prefers TransactionRouteID over deprecated TransactionRoute for route cache lookup. The fallback chain mirrors the sync path.
Swagger models: CodeRabbit flagged that CreateTransactionSwaggerModel / inflow / outflow variants don't include routeId. Minor doc gap. Worth a follow-up but not blocking.
Tests: coverage stays at 85.4% (above threshold). Security scan clean.
✅ Approve.
📊 Unit Test Coverage Report:
|
| Metric | Value |
|---|---|
| Overall Coverage | 87.1% ✅ PASS |
| Threshold | 85% |
Coverage by Package
| Package | Coverage |
|---|---|
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/http/in |
86.8% |
Generated by Go PR Analysis workflow
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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/ledger/api/docs.go`:
- Line 10157: Update the deprecated field description for the legacy `route` to
reference the correct route type: replace the phrase "operation route UUID" with
"transaction route UUID" in the `route` description string (the legacy
transaction-level `route` field) so the message correctly states it contains the
transaction route UUID for backwards compatibility.
In `@components/ledger/api/swagger.json`:
- Line 10133: Update the deprecated description for the Transaction.route field
to reference the correct entity: change the phrase "operation route UUID" to
"transaction route UUID" so the description correctly states it contains the
transaction route UUID for backward compatibility; target the Transaction.route
property description string in swagger.json and ensure the rest of the text
(example, maxLength, deprecated flag) remains unchanged.
In `@components/ledger/api/swagger.yaml`:
- Around line 7946-7947: The description for Transaction.route incorrectly
references the "operation route"—update the description text for the
Transaction.route field to state that it is deprecated and refers to the legacy
transaction route (string form) for backwards compatibility, and recommend using
routeId instead; ensure the wording explicitly mentions "transaction route
(legacy string form)" and "use routeId" to avoid ambiguity.
- Around line 7957-7966: The Transaction response schema includes routeId but
the transaction create request schemas are missing it (and the deprecated
route), so update the three transaction create models to match runtime behavior
by adding routeId (type: string, format: uuid, example:
00000000-0000-0000-0000-000000000000) and the deprecated route field (string,
marked deprecated) to each create model; locate the response Transaction schema
that defines routeId for the exact field spec and apply the same properties to
the create models (e.g., models named TransactionCreate,
CreateTransactionRequest, or similar) so the OpenAPI spec and
runtime/back-compat remain consistent.
In `@components/transaction/api/openapi.yaml`:
- Line 4258: Update the OpenAPI docs so the new routeId is documented as
accepted in request schemas CreateTransactionSwaggerModel,
CreateTransactionInflowSwaggerModel, and CreateTransactionOutflowSwaggerModel
(not only in the Transaction response), and change the deprecated "route"
description to state it contains a legacy transaction-route UUID (not an
operation-route UUID) for backward compatibility; make the same text corrections
in the Swagger2 companion spec and ensure the fallback mapping semantics (route
-> transaction route) are clearly described.
In `@components/transaction/api/transaction_swagger.json`:
- Around line 4738-4743: The request schemas are missing the new routeId field;
update transaction.CreateTransactionSwaggerModel,
CreateTransactionInflowSwaggerModel, and CreateTransactionOutflowSwaggerModel to
include the routeId property (type string, format uuid, with the same
description/example used in the response) so the OpenAPI docs and codegen match
the new input contract and validation for create requests.
- Line 4733: The "route" deprecated field's description incorrectly says it
contains an "operation" route UUID; update the description for the deprecated
"route" field in transaction_swagger.json to state it maps to the transaction
route identifier (transaction route UUID) for backward compatibility, keep the
deprecated flag and example/constraints (example:
00000000-0000-0000-0000-000000000000, maxLength: 250) intact, and ensure wording
clearly indicates it's the legacy transaction route identifier and that
consumers should use "routeId" instead; locate the description string for the
"route" property to apply this change.
In `@components/transaction/migrations/000027_add_route_id_to_transaction.up.sql`:
- Line 4: Change the index creation in migration 000027 to use online index
creation: replace the non-concurrent CREATE INDEX with CREATE INDEX CONCURRENTLY
(preserving IF NOT EXISTS, the index name idx_transaction_route_id, the target
table "transaction" and the WHERE route_id IS NOT NULL clause) so building the
index won’t block writes; also update the corresponding .down.sql to DROP INDEX
CONCURRENTLY IF EXISTS idx_transaction_route_id to mirror the safe, non-blocking
behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2c63f413-9e38-4298-b1d1-0e0734b7bf1d
📒 Files selected for processing (11)
components/ledger/api/docs.gocomponents/ledger/api/swagger.jsoncomponents/ledger/api/swagger.yamlcomponents/transaction/api/openapi.yamlcomponents/transaction/api/transaction_docs.gocomponents/transaction/api/transaction_swagger.jsoncomponents/transaction/api/transaction_swagger.yamlcomponents/transaction/migrations/000027_add_route_id_to_transaction.up.sqlcomponents/transaction/migrations/000028_validate_fk_transaction_route_id.down.sqlcomponents/transaction/migrations/000028_validate_fk_transaction_route_id.up.sqlpostman/MIDAZ.postman_collection.json
components/transaction/migrations/000027_add_route_id_to_transaction.up.sql
Outdated
Show resolved
Hide resolved
🔒 Security Scan Results —
|
gandalf-at-lerian
left a comment
gandalf-at-lerian
left a comment
There was a problem hiding this comment.
The latest commits addressed everything I raised in the first pass. Looking good.
What changed since my last review:
Migration split (3 files): The refactor into 000027 (add column + FK NOT VALID) → 000028 (CREATE INDEX CONCURRENTLY) → 000029 (VALIDATE CONSTRAINT) is the right approach. Each step is safe to run without a full table lock:
ADD COLUMN+ADD CONSTRAINT ... NOT VALID→ fast, no lockCREATE INDEX CONCURRENTLY→ non-blocking (requires its own transaction boundary, separate migration)VALIDATE CONSTRAINT→SHARE UPDATE EXCLUSIVElock, which doesn't block reads/writes
This is production-safe for a live table.
Documentation commits: the route vs routeId description in the Swagger/OpenAPI files is now corrected ("operation route" → "transaction route").
Remaining CodeRabbit nitpick (non-blocking): The CreateTransactionSwaggerModel / inflow / outflow variants still don't include routeId in the request schema docs. Minor doc gap — clients using the old route field will still work, and the response schema is complete. Fine to follow-up separately.
Security scan failures: unrelated to this PR. Both security_scan jobs fail with:
Unable to resolve action `aquasecurity/trivy-action@0.34.1`
The pr-security-scan.yml in the shared workflows repo is pinned to trivy-action@0.34.1, which no longer exists (version was likely yanked or renamed). This is an infra issue — needs a fix in LerianStudio/github-actions-shared-workflows to bump the Trivy action version. Nothing to fix here.
✅ Approved.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
components/transaction/migrations/000028_create_idx_transaction_route_id.down.sql (1)
1-1:⚠️ Potential issue | 🟠 MajorUse non-blocking index drop in rollback path.
DROP INDEX IF EXISTSmay block writes on rollback for large tables. PreferDROP INDEX CONCURRENTLY IF EXISTSto mirror the online strategy used in the up migration.Suggested change
-DROP INDEX IF EXISTS idx_transaction_route_id; +DROP INDEX CONCURRENTLY IF EXISTS idx_transaction_route_id;Run this read-only verification to confirm migration runner compatibility and existing patterns:
#!/bin/bash set -euo pipefail echo "== migration tool / transaction mode hints ==" rg -n "goose|NO TRANSACTION|golang-migrate|sql-migrate|migrate" Makefile .github/workflows components/transaction/migrations -S || true echo echo "== concurrent index patterns in migrations ==" rg -n "CREATE INDEX CONCURRENTLY|DROP INDEX CONCURRENTLY|idx_transaction_route_id" components/transaction/migrations -S || true echo echo "== target files ==" cat -n components/transaction/migrations/000028_create_idx_transaction_route_id.up.sql cat -n components/transaction/migrations/000028_create_idx_transaction_route_id.down.sql🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/transaction/migrations/000028_create_idx_transaction_route_id.down.sql` at line 1, The rollback currently uses a blocking DROP INDEX IF EXISTS for the index idx_transaction_route_id; change it to use a non-blocking DROP INDEX CONCURRENTLY IF EXISTS to mirror the online create and avoid write blocking during rollback, ensuring the migration runner executes the statement outside of a transaction if required by your migration tool (adjust runner settings or add a NO TRANSACTION marker if needed).components/ledger/api/swagger.yaml (1)
7957-7966:⚠️ Potential issue | 🟠 MajorRequest schemas still miss
routeId/deprecatedroutedespite runtime support.
Transactionresponse now documentsrouteId, but create payload models still omit these fields, causing spec/runtime drift for clients.Suggested spec alignment patch
CreateTransactionInflowSwaggerModel: type: object properties: + route: + description: 'Deprecated: legacy route identifier, use routeId instead.' + type: string + maxLength: 250 + routeId: + description: 'UUID of the transaction route. Preferred over route when both are provided.' + type: string + format: uuid + example: 00000000-0000-0000-0000-000000000000 CreateTransactionOutflowSwaggerModel: type: object properties: + route: + description: 'Deprecated: legacy route identifier, use routeId instead.' + type: string + maxLength: 250 + routeId: + description: 'UUID of the transaction route. Preferred over route when both are provided.' + type: string + format: uuid + example: 00000000-0000-0000-0000-000000000000 transaction.CreateTransactionSwaggerModel: type: object properties: + route: + description: 'Deprecated: legacy route identifier, use routeId instead.' + type: string + maxLength: 250 + routeId: + description: 'UUID of the transaction route. Preferred over route when both are provided.' + type: string + format: uuid + example: 00000000-0000-0000-0000-000000000000🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/ledger/api/swagger.yaml` around lines 7957 - 7966, The OpenAPI request/create payload schemas for transactions are missing the runtime-supported route identifiers; update any transaction request schemas (e.g., schemas related to Transaction, CreateTransaction, TransactionCreate or similar in components/schemas) to include a routeId property (type: string, format: uuid, example: 00000000-0000-0000-0000-000000000000, with the same description used in the Transaction response) and also add the deprecated route property (type: string, mark deprecated: true, with a short note pointing to routeId) so clients and spec match runtime behavior. Ensure descriptions, format and examples mirror the existing Transaction response definition and regenerate/validate the spec.components/transaction/api/openapi.yaml (1)
4265-4272:⚠️ Potential issue | 🟠 Major
routeIdis still documented only on response models, not create request schemas.
Transaction.routeIdis now documented, buttransaction.CreateTransactionSwaggerModel,CreateTransactionInflowSwaggerModel, andCreateTransactionOutflowSwaggerModelstill don’t declarerouteId. That keeps the request contract incomplete versus backend behavior.Suggested OpenAPI patch
components: schemas: CreateTransactionInflowSwaggerModel: properties: + routeId: + description: |- + UUID of the transaction route. Preferred over deprecated route. + example: 00000000-0000-0000-0000-000000000000 + format: uuid + example: 00000000-0000-0000-0000-000000000000 + format: uuid + type: string CreateTransactionOutflowSwaggerModel: properties: + routeId: + description: |- + UUID of the transaction route. Preferred over deprecated route. + example: 00000000-0000-0000-0000-000000000000 + format: uuid + example: 00000000-0000-0000-0000-000000000000 + format: uuid + type: string transaction.CreateTransactionSwaggerModel: properties: + routeId: + description: |- + UUID of the transaction route. Preferred over deprecated route. + example: 00000000-0000-0000-0000-000000000000 + format: uuid + example: 00000000-0000-0000-0000-000000000000 + format: uuid + type: string🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/transaction/api/openapi.yaml` around lines 4265 - 4272, The create request schemas are missing routeId while responses include it; add a routeId property to transaction.CreateTransactionSwaggerModel, CreateTransactionInflowSwaggerModel, and CreateTransactionOutflowSwaggerModel with type: string, format: uuid, a brief description like "UUID of the transaction route" and an example (00000000-0000-0000-0000-000000000000); ensure any required arrays for those models include routeId if backend requires it, and update the example objects for those models to include routeId so the request contract matches backend behavior.
🤖 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/transaction/api/openapi.yaml`:
- Around line 4258-4264: Update the deprecated "route" property in the OpenAPI
spec: change its description from "free-form string" to indicate it is a legacy
UUID (e.g., "Deprecated legacy UUID route identifier; use routeId instead"), and
add format: uuid to the property alongside deprecated: true and type: string so
validators/clients treat it as a UUID (target the "route" property definition in
components/transaction/api/openapi.yaml).
---
Duplicate comments:
In `@components/ledger/api/swagger.yaml`:
- Around line 7957-7966: The OpenAPI request/create payload schemas for
transactions are missing the runtime-supported route identifiers; update any
transaction request schemas (e.g., schemas related to Transaction,
CreateTransaction, TransactionCreate or similar in components/schemas) to
include a routeId property (type: string, format: uuid, example:
00000000-0000-0000-0000-000000000000, with the same description used in the
Transaction response) and also add the deprecated route property (type: string,
mark deprecated: true, with a short note pointing to routeId) so clients and
spec match runtime behavior. Ensure descriptions, format and examples mirror the
existing Transaction response definition and regenerate/validate the spec.
In `@components/transaction/api/openapi.yaml`:
- Around line 4265-4272: The create request schemas are missing routeId while
responses include it; add a routeId property to
transaction.CreateTransactionSwaggerModel, CreateTransactionInflowSwaggerModel,
and CreateTransactionOutflowSwaggerModel with type: string, format: uuid, a
brief description like "UUID of the transaction route" and an example
(00000000-0000-0000-0000-000000000000); ensure any required arrays for those
models include routeId if backend requires it, and update the example objects
for those models to include routeId so the request contract matches backend
behavior.
In
`@components/transaction/migrations/000028_create_idx_transaction_route_id.down.sql`:
- Line 1: The rollback currently uses a blocking DROP INDEX IF EXISTS for the
index idx_transaction_route_id; change it to use a non-blocking DROP INDEX
CONCURRENTLY IF EXISTS to mirror the online create and avoid write blocking
during rollback, ensuring the migration runner executes the statement outside of
a transaction if required by your migration tool (adjust runner settings or add
a NO TRANSACTION marker if needed).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2e920f40-2d45-406b-acdb-ebaf542de465
📒 Files selected for processing (15)
components/ledger/api/docs.gocomponents/ledger/api/swagger.jsoncomponents/ledger/api/swagger.yamlcomponents/transaction/api/openapi.yamlcomponents/transaction/api/transaction_docs.gocomponents/transaction/api/transaction_swagger.jsoncomponents/transaction/api/transaction_swagger.yamlcomponents/transaction/internal/adapters/postgres/transaction/transaction.gocomponents/transaction/migrations/000027_add_route_id_to_transaction.down.sqlcomponents/transaction/migrations/000027_add_route_id_to_transaction.up.sqlcomponents/transaction/migrations/000028_create_idx_transaction_route_id.down.sqlcomponents/transaction/migrations/000028_create_idx_transaction_route_id.up.sqlcomponents/transaction/migrations/000029_validate_fk_transaction_route_id.down.sqlcomponents/transaction/migrations/000029_validate_fk_transaction_route_id.up.sqlpkg/transaction/transaction.go
3 participants
Summary
routeId(UUID) field to the transaction entity as the primary route identifier, replacing the deprecated free-formroutestring fieldtransaction_route(id), giving referential integrity that the legacy string field never hadroutefield continue to workMotivation
The existing
routefield on transaction is a plain string that holds a UUID but has no database-level constraint tying it to an actual transaction route record. This means invalid or stale route references can be stored silently. By introducingrouteIdas a proper UUID column with a FK, we get:routeIdpattern already established on the operation entityChanges
Migration
000027_add_route_id_to_transaction— addsroute_id UUIDcolumn (nullable) with FKto
transaction_route(id)API layer
CreateTransactionInput,CreateTransactionInflowInput,CreateTransactionOutflowInput— new optionalrouteIdfield (validate:"omitempty,uuid")Transaction— returnsrouteIdalongside the deprecatedrouteBusiness logic
ValidateAccountingRules— prefersrouteIdwhen present, falls back toroutefor backward compat. When accounting validation is enabled, at least one must be providedValidateSendSourceAndDistribute— propagatesrouteIdinto the validation contextPersistence
ToEntity/FromEntityupdatedTransactionRevertpreservesrouteIdon the reversal transactionrouteIdthrough the queueBehavior
routesentrouteIdsentrouteIdused for validation, stored as FKrouteparsed as UUID (existing behavior)routeIdtakes priorityroute_idstored as NULL