Repository navigation
refactor(filler): share one guarded update helper in the SimpleAssets asset processor - #240
Merged
robrigo merged 1 commit intoOct 8, 2026
Conversation
… asset processor Five actions of the SimpleAssets asset processor each built the same guarded UPDATE: set fields, match the asset, and require that the stored update block is not newer than the action. The transfer and the claim action also each wrote the transfer record with the same two inserts. One helper now builds the guarded update and one helper writes the transfer record. Each caller passes its own match text and values, so the generated SQL, the parameter order and the order of the database calls do not change. A new test covers the transfer record of a transfer action, which no test executed before. Signed-off-by: Rob Konsdorf <rob@facings.io>
There was a problem hiding this comment.
🟢 Approval recommended
The refactor preserves SQL parameters, conflict behavior, and operation ordering while existing and added tests cover the affected paths.
0 open findings
What changed in this PR
Refactors SimpleAssets asset updates and transfer recording into shared helpers without changing database behavior.
Changes:
- Centralizes guarded asset updates.
- Reuses transfer-record insertion for transfer and claim actions.
- Adds integration coverage for transfer records.
| File | Description |
|---|---|
src/filler/handlers/simpleassets/processors/assets.ts |
Introduces shared update and transfer helpers. |
src/filler/handlers/simpleassets/processors/assets.integration.test.ts |
Tests stored transfer details and associated assets. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
robrigo
deleted the
refactor/eng-1278-simpleassets-guarded-update-helper
branch
October 8, 2026 19:41
Merged
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
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.
Why
Five actions of the SimpleAssets asset processor (
transfer,burnlog,update,changeauthor,claim) each built the same guarded UPDATE: set fields, match the asset, and require that the stored update block is not newer than the action. Thetransferand theclaimaction also each wrote the transfer record with the same two inserts.One helper now builds the guarded update, and one helper writes the transfer record. Each caller passes its own match text and values, so the generated SQL, the parameter order, and the order of the database calls do not change. The match text has a type that lists each allowed text, so a later caller cannot build one from action data.
Validation
assets.integration.test.ts: 23 passing on the base and 24 passing with this change. The new test covers the transfer record of atransferaction (sender, recipient, memo cut to 256 characters, one row per asset), which no test executed before. It passes on the old processor code too.pnpm check-typesandpnpm lintpass.pnpm testreads 607 passing and 37 pending with no database.The release has no changelog entry for this change: it has no effect that an operator or an API client can see.