Conversation
rlorenzo
force-pushed
the
bulk-update-merchant
branch
from
September 18, 2026 22:29
e9c10c6 to
1a22e0b
Compare
Owner
|
Held only by an unrelated gate failure: two advisories landed against anyio 4.10.0 after the baseline was seeded, one of them critical. Fixed in #152, so this should go green on a rebase or a rerun. |
Monarch makes a merchant out of whatever string it is handed, so a blank or whitespace-only name quietly creates a junk merchant record rather than failing. create_transaction, update_transaction and update_merchant all took one unchecked. Both rule tools were worse. They guarded with `if set_merchant_name:`, which is false for " ", so a blank name skipped the check and reached the API. The validator lives in helpers.py because the hole belongs to the field rather than to any one caller, and raises rather than returning, since every tool that writes a merchant name already funnels exceptions through json_error.
Applying the same edit to a set of transactions meant one update_transaction call per row. Sets run to hundreds, and each is a single user decision. Monarch has a real bulk mutation. The web client calls Common_BulkUpdateTransactionsMutation when you select transactions and edit them, so a whole selection costs one round trip. The client library does not wrap it, but it exposes gql_call, so no fork of the library is needed. This is the same endpoint f5aa023 uses for the review flag; if both land, that tool becomes a special case of this one. updates takes TransactionUpdateParams, the shape a single transaction update takes, so one request can set any field it supports. Which fields those are was confirmed against the live API by sending each name with an empty selection, where an unknown field fails validation before anything can be written: categoryId, merchantName, notes, goalId, reviewStatus and date are accepted, while merchant, hideFromReports, needsReview and amount are rejected. So the merchant field is merchantName, the review flag is the reviewStatus enum rather than a boolean, and hiding from reports has no bulk form at all. date is accepted but not offered, since overwriting dates across a set is rarely meant and cannot be undone without the original values. Reassigning merchantName is the case with no alternative. Monarch has no merge endpoint, and update_merchant cannot rename one merchant onto another: it fails with "A merchant with this name already exists". Pointing every transaction at the canonical name is the only way to fold a duplicate spelling in. allSelected is hardcoded False. Set True, Monarch updates everything matching filters rather than the listed ids, which is the difference between editing three transactions and editing an entire account; a test pins it. The endpoint is all or nothing: it reports affectedCount for the batch and cannot express a partial failure. bulk_categorize_transactions keeps its per item reporting and is left untouched, so this adds a tool rather than changing one. It also reports a refusal as success: false with a null errors object, so that flag is checked alongside the payload errors rather than inferring the write took from their absence, and an update carrying no fields is refused because Monarch would accept it, change nothing, and not complain.
rlorenzo
force-pushed
the
bulk-update-merchant
branch
from
September 20, 2026 20:05
1a22e0b to
03ab306
Compare
Author
|
@robcerda Thanks for considering this PR. I have rebased the branch off the latest main. |
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.
Applying the same edit to a set of transactions meant one
update_transactioncall per row. Sets run to hundreds. Each row is a single user decision.
bulk_update_transactions(transaction_ids, ..., dry_run)does it in onerequest.
Monarch has a bulk mutation
The web client calls
Common_BulkUpdateTransactionsMutationwhen you selecttransactions and edit them, so a whole selection costs one round trip. The
client library does not wrap it.
gql_callreaches it anyway, so nothing needsforking.
gql_callships inmonarchmoneycommunity1.5.2, the pinned floor, with thesignature this uses, and the mutation rides the existing authenticated session,
so it costs no extra API call and adds no dependency.
Which fields it accepts
updatestakesTransactionUpdateParams, the shape a single update takes. Iconfirmed which names it accepts by sending each one with an empty selection,
where an unknown field fails validation before anything can be written:
categoryId,merchantName,notes,goalId,reviewStatus,datemerchant,hideFromReports,needsReview,amountSo the merchant field is
merchantName, notmerchant. The review flag is thereviewStatusenum rather than a boolean, soneeds_reviewmaps onto it.Hiding from reports has no bulk form at all.
dateis accepted but left out anyway. Overwriting dates across a set israrely meant and cannot be undone without the original values.
What it is for
Clearing
needs_reviewis the everyday case. A review queue is where sets runlongest, and
bulk_update_transactions(ids, needs_review=False)empties astretch of it in one request.
Reassigning
merchant_nameis the case with no alternative. There is no mergeendpoint, and
update_merchantrefuses to rename one merchant onto another,failing with
A merchant with this name already exists. Pointing everytransaction at the canonical name is the only way to fold a duplicate spelling
in. Spelling has to be exact: an existing name attaches to that record, a new
one creates it.
Why the code looks like this
allSelectedis hardcodedFalse. Set itTrueand Monarch updateseverything matching filters rather than the listed ids, which is the difference
between editing three transactions and editing an entire account. A test pins
it.
The endpoint is all or nothing. It reports
affectedCountfor the batch andcannot express a partial failure, so
bulk_categorize_transactionskeeps itsper-item reporting and I left it alone. This adds a tool rather than changing
one, and the diff carries no deletions outside the README.
A refusal comes back as
success: falsewith a nullerrorsobject, so thecode checks that flag rather than inferring success from absent errors. An
update carrying no fields fails too. Monarch would accept it, change nothing,
and not complain.
One thing carried over
_require_nonblankrejects whitespace-only merchant names on every path thatwrites one. Monarch makes a merchant out of whatever string it is handed, so a
blank name creates a junk merchant instead of failing, and nothing surfaces the
mistake. It rides along from an earlier draft of this work.
Written with Claude Code; I verified the accepted and rejected field names
against a live Monarch account myself, and reviewed every line.