fix(wallet): read the transaction direction set from one place - #197
Merged
Merged
Conversation
`dashboard` decided each type's sign with an inline
`match ty { "earn" | "topup" | "gift" => … }` — a second copy of the
income set that `signed_pts_expr` / `summary` / `trend` render from
`TX_INCOME_TYPES`. The SQL sites and the Rust site could therefore
drift apart, and editing the constant left `net` unchanged.
Both sets are now arrays and every consumer reads them: `sql_in_list`
renders the `IN (…)` value list (byte-identical to the previous
literals), and the `net` fold uses `contains`. A new test drives the
real handler over all six types with a hardcoded expectation.
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.
Summary
PR #196 introduced
TX_INCOME_TYPES/TX_EXPENSE_TYPESand replaced the inline SQL literals in the summary / trend / dashboard aggregates with them, describing the constants as the single place to change. ButGET /api/dashboardstill decided each type's sign with an inline Rustmatch:That is a second copy of the same rule — and one that no SQL-
grepfinds. Changing the constant (say, decidinggiftshould be an expense) would move every SQL aggregate while leavingnetunchanged, and nothing asserted the two agreed. Today the two agree by coincidence of spelling, not because anything enforces it.This PR makes the claim true: both sets become arrays and every consumer reads them — SQL through
sql_in_list(), Rust throughcontains().The generated SQL is byte-identical to the previous literals (asserted, see Tests), so no response changes.
Related Issue
None (no issue for this).
Changes
src/routes/wallet.rs—TX_INCOME_TYPES/TX_EXPENSE_TYPESchange from SQL literals (&str) to arrays ([&str; 3]), and a newsql_in_list()renders them as theIN (…)value list.src/routes/wallet.rs—signed_pts_expr(),summaryandtrendinterpolatesql_in_list()instead of the literal (generated SQL is byte-identical ⇒ no response change).src/routes/wallet.rs—dashboard'snetfolds withTX_INCOME_TYPES.contains(...); the inlinematcharm is gone.src/routes/wallet.rs— the doc comments now state exactly what the module enforces (both consumers read the arrays) and no longer re-spell the type list in prose, where it could rot a third time.Tests
cargo test— 187 passed, 0 failed (was 186; the new test is the only addition)cargo fmt --checkandcargo clippy --all-targets -- -D warnings— cleanNew unit test
dashboard_net_direction_holds_for_every_type— drives the real/api/dashboardhandler with one row of each of the six types (earn 3.0,topup 2.0,gift 1.0,consume 10.0,expire 0.5,withdraw 0.25) and assertsnet == −4.75plus the per-type month rows, with the expectation hardcoded in the test rather than derived from the constants.Byte-identity of the SQL — verified by assertion, not by reading:
sql_in_list(&TX_INCOME_TYPES) == "'earn','topup','gift'",sql_in_list(&TX_EXPENSE_TYPES) == "'consume','expire','withdraw'", andsigned_pts_expr("t")/signed_pts_expr("")equal the previous literals verbatim.Proof the new test has teeth (count-neutral injection, per the project's rule that a gate must be shown to fail): add
withdrawto the income array only —netbecomes−4.25, the assertion reads−4.75→-4.25atwallet.rs:917);netdoes not read the array at all).That asymmetry is the evidence that the two sites were genuinely independent copies before, and that this PR is what joins them.
No behaviour change: the existing summary / trend / dashboard assertions (including
dashboard_net_is_the_month_net_not_the_7_day_seriesandtransactions_summary_and_dashboard_net_with_topup) pass untouched.Checklist
Not in scope
src/routes/ops.rs(month_in/month_out),src/gift.rsandui/js/app.js(PTS_INCOME_TYPES) keep their own consistent copies — different modules/languages, swept by #196; the "one place" claim here is scoped toroutes/wallet.rs, as the doc comment now says.