Add pre-authorization (confirmation_mode) support to Merchant resources - #165
paulosouza-stark wants to merge 1 commit into
Conversation
e82170e to
2457377
Compare
edu-stark
left a comment
There was a problem hiding this comment.
Review against the Python reference and the backend services (read-only pass over every open PR, 2026-09-17). Findings below; happy to help with the rebase once the content points are addressed.
#165 — Add pre-authorization (confirmation_mode) support to Merchant resource
Author: Paulo · 1 commit (2457377 Added pre-authorization) · 10 files · base 9ef6834 (2026-06-29) · approved, CONFLICTING
Verdict: request-changes
The behaviour is right and matches the backend. The problem is documentation shape: the branch
forked one commit before master's docstring pass, and every code file auto-merges cleanly, so the
missing docstring lines will land silently unless they are added before merge.
Rebase status
- Divergence: 8 behind, 1 ahead (the PR facts said 3 behind — stale).
- Merge base:
9ef6834= "Merge pull request #167 from starkbank/bump". - No merge commits on the branch; single clean commit. Fork-point rule satisfied once rebased.
- Conflicting files:
CHANGELOG.mdonly.README.md,
starkbank/merchantpurchase/__merchantpurchase.py,starkbank/merchantsession/__merchantsession.py
andstarkbank/merchantsession/__purchase.pyall auto-merge (verified with
merge-tree --write-tree origin/pr-165 origin/master→ tree178060b). - Rebase is mechanical. The CHANGELOG conflict is two additive bullet lists under the same
## [Unreleased] / ### Addedheading; resolution is to concatenate the PR's two bullets with
master's VerifiedAccount/VerifiedTransfer bullets and keep master's### Fixed / - Docstrings. - Merged README section order is sane:
## Confirm a MerchantPurchaseand
## Cancel a MerchantPurchaseland between## Get a MerchantPurchaseand
## Query MerchantInstallments.
Backend verification (api-v2-ms-card-merchant)
Everything the PR asserts checks out:
- Enum is exactly
automatic/manual—utils/merchantSession.py:23-25
(class MerchantSessionConfirmationMode). - Default is
automatic, applied server-side —models/merchantPurchase.py:85
("confirmationMode": self.confirmationMode or MerchantSessionConfirmationMode.automatic) and
utils/merchantSession.py:41. - Which resources carry it:
- MerchantSession — writable input:
middlewares/merchantSession.py:196lists
confirmationModeinoptionalParameters; returned viautils/merchantSession.py:41. - MerchantPurchase — writable input:
middlewares/merchantPurchase.py:335
optionalParameters = ["challengeMode", "confirmationMode", ...]; returned via
models/merchantPurchase.py:85. - MerchantSession.Purchase — return-only.
middlewares/merchantSessionPurchase.py:36
accepts only["installmentCount", "tags"]plus the challenge fields; the value is inherited
from the session (tests/handlers/public/merchantSessionPurchaseTest.py:273,288).
- MerchantSession — writable input:
- "credit only" is real:
middlewares/merchantSession.py:523-527rejects debit + manual, and the
purchase middleware does the same (tests/handlers/public/merchantPurchaseTest.py:279,
LocalApiMessageKey.debitNotAllowedInManualConfirmationMode). - Confirm-via-PATCH is real:
models/merchantPurchase.py:29-30
patchStatus() == ["confirmed", "reversed", "canceled"];
handlers/public/merchantPurchaseInfo.py:45-66routesconfirmedto_handlePurchaseConfirmation;
middlewares/merchantPurchase.py:223-250requiresconfirmationMode == manualand
status == approved. PATCH requires bothstatusandamount
(middlewares/merchantPurchase.py:88) — the README example correctly passesamount. - DELETE is real:
routes/public.py:41→handlers/public/merchantPurchaseInfo.py:72, with
_checkPurchaseDelete(middlewares/merchantPurchase.py:152-168) inferring the operation from
current status:approved→canceled,confirmed→reversed,paid→reversed. rest.delete_idsends no payload (core-python/starkcore/utils/rest.py:178-191) and the handler's
getJsonBody()returns{}on an empty body, so the bodyless DELETE is accepted.
Blocking findings
-
starkbank/merchantpurchase/__merchantpurchase.py:16—confirmation_modeadded to the
signature but absent from the class docstring. Master landed a full docstring pass on this class
(### Fixed / - Docstringsin Unreleased) with explicit## Parameters (optional)and
## Attributes (return-only)lists. Verified on the merged tree178060b: the docstring ends at
- updated [...]and never mentionsconfirmation_mode. Must be added under
## Parameters (optional). -
starkbank/merchantsession/__merchantsession.py:17— same omission. Merged docstring
documentschallenge_modeat line 21 but notconfirmation_mode. Belongs under
## Parameters (optional), next tochallenge_mode. -
starkbank/merchantsession/__purchase.py:15— same omission, and here it must go under
## Attributes (return-only), not## Parameters (optional): the session-purchase endpoint does
not acceptconfirmationModeas input (middlewares/merchantSessionPurchase.py:36). -
starkbank/merchantpurchase/__merchantpurchase.py:101—delete()ships with no docstring.
Every other public function in the module (create,get,query,page,update) has the
standard## Parameters (required) / (optional) / ## Returnblock. Needs one, stating that the
operation is inferred from the purchase's status. -
starkbank/merchantpurchase/__merchantpurchase.py:93— theupdate()docstring inherited
from master saysstatus [string, default None]: "canceled" or "reversed", per the rules above
and its summary line covers only approved→cancel and confirmed→reverse. This PR's own README
(## Confirm a MerchantPurchase) documentsupdate(status="confirmed"), and the backend allows
it. After the rebase the repo contradicts itself: the README teaches a status the docstring says
does not exist. Theupdatedocstring must gain theconfirmedcase. -
tests/sdk/test_merchant_purchase.py— no test exercises the headline feature. The PR adds
TestMerchantPurchaseDelete(queriesstatus="approved", so it only ever hits the cancel path)
andTestMerchantSessionCreateManualConfirmation, but nothing confirms a manual-mode purchase
(update(status="confirmed", amount=...)) and nothing assertsconfirmation_moderound-trips on
a returned MerchantPurchase. ATestMerchantPurchaseConfirmis needed.
Non-blocking notes
- README
## Cancel a MerchantPurchasesays "Cancel an approved purchase or fully reverse a
confirmed one" but the backend also reverses apaidpurchase
(middlewares/merchantPurchase.py:155). Worth a word. - CHANGELOG lists the
confirmation_modeattribute and thedeletemethod but not the
update(status="confirmed")confirm path that the README now documents. confirmation_modeis appended afterupdatedin all three signatures and aftertagsin the
assignment blocks; master's convention keepscreated/updatedlast and the rest roughly
alphabetical. Cosmetic.- No collision between the new
TestMerchantPurchaseDelete(queriesapproved) and the existing
TestMerchantPurchaseUpdate(queriesconfirmed). - Test class/util naming and the "query-then-loop, vacuously pass if empty" pattern match the
existing file. Tree position of all 10 files is correct.
Two-line summary for the owner
Code and README are correct and line up with api-v2-ms-card-merchant (enum automatic/manual,
writable on MerchantSession and MerchantPurchase, return-only on MerchantSession.Purchase, confirm
via PATCH confirmed, bodyless DELETE inferring cancel/reverse); only CHANGELOG.md conflicts and
the rebase onto the 8 new master commits is a mechanical two-list concatenation.
Do not merge on the rebase alone: the branch forked before master's docstring pass and auto-merges
silently, so confirmation_mode would land undocumented on three classes, delete() with no
docstring, update()'s docstring denying the confirmed status its own README teaches, and the
confirm path untested — ask Paulo for those five doc lines plus a confirm test, then rebase and merge.
Description and impact
The
feat/pre-approvebranch ofapi-v2-ms-card-merchantintroduced manual confirmation mode (pre-authorization) for merchant purchases: a newconfirmationModefield (automatic/manual, defaultautomatic) onMerchantSessionandMerchantPurchase. Inmanualmode the purchase is approved but only captured after an explicit confirmation (PATCH /merchant-purchase/{id}withstatus=confirmed), and a newDELETE /merchant-purchase/{id}endpoint cancels (approved) or fully reverses (confirmed) a purchase.The Python SDK did not expose
confirmationModenor the delete endpoint, so integrators could not use pre-authorization through the SDK. This PR adds that support.Planning card: ACQ-413.
Change
confirmation_modeattribute to theMerchantSession,MerchantPurchaseandMerchantSession.Purchaseresources. It is serialized asconfirmationModeand omitted whenNone(so the server defaultautomaticapplies).starkcore.from_api_jsonsilently drops unknown fields, so the attribute must exist in the resource for the value to be exposed on responses.merchantpurchase.delete(id)→DELETE /merchant-purchase/{id}(cancels anapprovedpurchase or fully reverses aconfirmedone; the operation is inferred server-side from the current status).merchantpurchase.update(id, status="confirmed", amount=...).README.md(field in the session/purchase examples + new Confirm/Cancel a MerchantPurchase sections),CHANGELOG.md([Unreleased]), and the builders/tests undertests/.POST /merchant-purchase/{id}route listed in the servicepermissions.yamlwas intentionally not implemented (it has no handler). Themanual + debitrestriction is enforced server-side; the SDK just forwards the value.SDK usage examples
1. Create a pre-authorization session (
confirmation_mode="manual")2. Create a purchase directly in manual mode (optional — credit only)
3. Confirm (capture) the pre-authorized purchase
4. Cancel an approved purchase / fully reverse a confirmed one
Rollback Plan
e82170e(Added pre-authorization) and/or pin the dependency to the previously released SDK version.Acceptance Criteria
merchantsession.create(...)withconfirmation_mode="manual"returns a session andconfirmation_moderound-trips on the response.approved(not captured) until confirmed.merchantpurchase.update(id, status="confirmed", amount=...)confirms (captures) a manual purchase.merchantpurchase.delete(id)cancels anapprovedpurchase and fully reverses aconfirmedone.