Skip to content

Name the device that sent each payment, and drive the flows through the form - #47

Merged
Alex Arguello (alex-arguello) merged 17 commits into
mainfrom
alexarguello/pla-2488-android-p736-the-sample-app-cannot-say-which-device-sent-a
Aug 14, 2026
Merged

Name the device that sent each payment, and drive the flows through the form#47
Alex Arguello (alex-arguello) merged 17 commits into
mainfrom
alexarguello/pla-2488-android-p736-the-sample-app-cannot-say-which-device-sent-a

Conversation

@alex-arguello

@alex-arguello Alex Arguello (alex-arguello) commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Ticket: PLA-2488 (the sample app cannot say which device sent a payment)

Running the card-not-present flows on several devices at once produces transactions nothing can tell apart: every device submits the same customer, the same instrument and the same $1.10, so a dashboard shows four identical rows minutes apart. This derives the customer from the device model instead, so one build installs everywhere and each model names itself, and draws the charged amount per attempt so a row carries a second signal beyond the customer and the order identifier.

It also adds a test that drives the four flows through the form the app actually shows, which is what lets several devices run them together with each screen showing what it is doing.

Structure

The identity is derived, not configured. QaIdentity takes the device model and produces the holder name, the first and last name, the customer number, the billing email, the order description and an order identifier. A configured value would mean a build per device; a derived one means one APK for the whole run. The model arrives as a parameter rather than as a Build read, so the derivation is exercised on the JVM against models no machine here has.

A capture supplies the customer number the form does not collect. The capture form asks for no customer number, and a real integration identifies the customer from its own records rather than asking the payer, so the value comes from the request. Without it, and with forceCustomerCreation set, the paypoint has nothing to match on: three captures from one device produced three customer records, each with no number at all. Whether the number is sent is a switch on the Configuration screen. Off, the request still names a payer from the fields the form collects and carries no number, so a paypoint with no custom identifiers files a new customer for every payment, and which kind a paypoint is cannot be seen from here.

Asking for the walkthrough narrows the run to it. NavigationSmokeTest and PayInSessionSourceInstrumentedTest point the app at a fake token server on a random port and pin a fake entry point, and both writes are process-wide with no way back. A walkthrough sharing that process talks to a closed port and a paypoint that does not exist, and every flow times out waiting for a form that stayed locked. Three of three failed that way in a whole-suite run and all three passed alone.

Notable changes

  • QaIdentity replaces punctuation in the account holder name with a space. The store route answers Bad Request: Account holder name cannot contain special characters for QA Samsung SM-S908U1 and stores the same name without the hyphen. A card holder name takes those characters without complaint, so only the bank account showed it.
  • The form's amount row and the request's total came from two literals in two files and disagreed: the row read $ 1.00 while the request charged 1.10. Both now come from one value, and a test sweeps 500 draws asserting the rows add up to what is charged.
  • The capture screen shows a Total under the form, inline and in the sheet. The form's own rows read back the amount and the fee and never their sum, so the figure that leaves the payer's account was visible only after submitting.
  • The setting row is one labelled toggle. A Switch beside a Text is a separate semantics node, so a screen reader announced "on, switch" without saying what it switched, and the label was not a target. Covered on a device: reverting to the sibling layout fails three assertions.
  • QaWalkthroughTest waits for the form to report the bank tab before prefilling. The prefill fills the method the form last reported, and that report arrives a frame after the tap: without the wait, card values are seeded into a bank form and it refuses itself. Three of four devices lost that race and the fastest won it.
  • The walkthrough reports what the screen said when a submission is refused. A bare timeout is the same message for a declined payment, an unreachable service and a button that moved.
  • The bank capture is excluded by name unless payabli.qaWalkthrough.achDebits=true, as :payin excludes its live counterpart: whether a paypoint's connector takes an ACH debit is its configuration rather than anything this app sends.

Verification

Unit tests, ktlintCheck and lintDebug pass. Instrumented on a Pixel 7a: ten tests in an ordinary run with the walkthrough excluded, three in an invocation of its own, both green with no skips. The three covering the setting row also ran on an SM-A136U1 and an API 37 emulator; the SM-S908U1 was locked and could not be reached, so it did not answer for them.

Every guarantee was broken in turn and required to turn a test red, ten of ten caught; the table is on the ticket.

Live against a real paypoint on a Pixel 7a, an SM-S908U1, an SM-A136U1, an API 37 emulator and an iPhone 17 simulator running the iOS counterpart, all five firing together: fifteen submissions, every one attributable to the device that sent it. Transaction identifiers are on the ticket.

Not included

The same work for iOS, which needs its own ticket because one ticket carries one surface. It exists on a branch alongside unrelated work and is not ready to open.

A discriminator that separates two handsets of the same model. The identity is derived from manufacturer and model, so a second SM-S908U1 would repeat the first one's customer number and order prefix. Adding one changes the customer number that makes a device's captures land on a single record, and the counterpart derives its identity the same way, so it is queued for a change covering both platforms rather than taken here. The four targets in the run are four distinct models.

Three phones and a simulator running the card-not-present flows produce rows
nothing can tell apart: the same customer, the same instrument, the same amount,
minutes apart. A dashboard cannot say which device sent which payment, which is
the whole point of running several at once.

`QaIdentity` derives the customer from the device model, so one build installs
everywhere and each install still names itself: the holder name, the first and
last name, the customer number, the billing email, the order description, and an
order identifier carrying the model and the second the attempt was made.

Punctuation becomes a space in the holder name. The store route refuses
`QA Samsung SM-S908U1` with "Account holder name cannot contain special
characters" and stores the same name without the hyphen. A card holder name takes
the same characters without complaint, which is why only the bank account showed
it, and why it looked device-specific: the one model code with no punctuation
passed.

`QaAmount` draws the charge per attempt, in whole cents between two and fifteen
dollars, so two rows from one device differ by the figure as well.

The form's amount row and the request's total were two literals in two files and
they disagreed: the row read `$ 1.00` while the request charged `1.10`. Both now
come from one value, and `CaptureRequestTest` sweeps five hundred draws asserting
the rows add up to what is charged.

A capture now names the customer it is for. With no customer number and
`forceCustomerCreation` set, the paypoint has nothing to match on: three captures
from one device produced three customers, each with no number at all. Whether the
number is sent is a switch on the Configuration screen, defaulting to on, because
a paypoint with no custom identifiers accepts an empty customer and records none.

Unit tests, ktlintCheck and lint pass.
…l environment

`PayInLiveFlowsInstrumentedTest` in `:payin` submits the same flows by calling the
SDK directly, so nothing covers the path a person takes: the form, the tabs, the
prefill and the submit button. `NavigationSmokeTest` walks those screens and stops
before submitting, against a fake token endpoint.

`QaWalkthroughTest` submits. Save and capture, card and bank account, driven
through the form the sample actually shows, so several devices can run it at once
and each screen shows what it is doing.

Excluded by name unless `payabli.qaWalkthrough=true`, as `:payin` excludes its live
tier and for the same reason: it sends real requests and needs a reachable token
server and a configured paypoint. Excluded rather than skipped, so a run without
the property reports no standing skip. The bank capture is excluded on its own
unless `payabli.qaWalkthrough.achDebits=true`, because whether a paypoint's
connector takes an ACH debit is its configuration rather than anything the app
sends.

Two waits in it are load-bearing, and both were measured rather than added for
safety. The prefill fills the method the form last reported it was on, and that
report arrives a frame after the tab is tapped: without the wait the card values
are seeded into a bank form, every bank field stays empty and the form refuses
itself. Three of four devices lost that race and the fastest won it. The outcome
wait reports what the screen said, because a bare timeout is the same message for
a declined payment, an unreachable service and a button that moved.

Run on a Pixel 7a, an SM-S908U1, an SM-A136U1 and an API 37 emulator: three flows
each, all passing, with the bank capture excluded against the qa paypoint.
Copilot AI balanced review requested due to automatic review settings August 14, 2026 17:50

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the sample app’s QA payment flows so transactions identify their originating device and display the charged amount consistently.

Changes:

  • Derives payer and order identity from the device model.
  • Adds randomized capture amounts and optional customer-number submission.
  • Adds unit coverage and a gated live UI walkthrough for card-not-present flows.

Reviewed changes

Copilot reviewed 29 out of 29 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
example/build.gradle.kts Gates and filters the live walkthrough.
example/src/androidTest/java/com/payabli/example/app/QaWalkthroughTest.kt Exercises four payment-form flows.
example/src/main/java/com/payabli/example/app/AppContainer.kt Provides shared QA identity and customer setting.
example/src/main/java/com/payabli/example/app/demo/qa/DemoCustomerSetting.kt Stores the customer-number toggle.
example/src/main/java/com/payabli/example/app/demo/qa/QaAmount.kt Generates QA capture amounts.
example/src/main/java/com/payabli/example/app/demo/qa/QaIdentity.kt Derives transaction identity from device models.
example/src/main/java/com/payabli/example/app/demo/ui/capture/CaptureScreen.kt Updates capture preview data.
example/src/main/java/com/payabli/example/app/demo/ui/capture/CaptureViewModel.kt Coordinates amount, identity, and capture requests.
example/src/main/java/com/payabli/example/app/demo/ui/components/SwitchRow.kt Adds a reusable setting switch.
example/src/main/java/com/payabli/example/app/demo/ui/method/PaymentMethodScreen.kt Adds identity to the preview.
example/src/main/java/com/payabli/example/app/demo/ui/method/PaymentMethodViewModel.kt Exposes device identity for prefilling.
example/src/main/java/com/payabli/example/app/demo/ui/nav/PayabliDemoNavHost.kt Wires the customer toggle callback.
example/src/main/java/com/payabli/example/app/demo/ui/payment/PaymentFlowScreen.kt Prefills device-specific payer data.
example/src/main/java/com/payabli/example/app/demo/ui/setup/SetupScreen.kt Displays the customer setting.
example/src/main/java/com/payabli/example/app/demo/ui/setup/SetupViewModel.kt Synchronizes customer-setting state.
example/src/main/java/com/payabli/example/app/sdk/PayInForms.kt Derives displayed amounts from capture totals.
example/src/main/java/com/payabli/example/app/sdk/PayInOperation.kt Adds identity, amount, and customer data to captures.
example/src/main/java/com/payabli/example/app/sdk/PayInPrefill.kt Replaces fixed payer data with device identity.
example/src/test/java/com/payabli/example/app/demo/qa/DemoCustomerSettingTest.kt Tests customer-setting behavior.
example/src/test/java/com/payabli/example/app/demo/qa/QaAmountTest.kt Tests amount range and precision.
example/src/test/java/com/payabli/example/app/demo/qa/QaIdentityTest.kt Tests identity derivation and formatting.
example/src/test/java/com/payabli/example/app/demo/ui/CaptureIdempotencyTest.kt Adapts idempotency tests to new capture inputs.
example/src/test/java/com/payabli/example/app/demo/ui/DiagnosticsGateTest.kt Updates capture and method fixtures.
example/src/test/java/com/payabli/example/app/demo/ui/OutcomeNavigationTest.kt Updates navigation fixtures.
example/src/test/java/com/payabli/example/app/demo/ui/RecheckReadsTheDeviceAgainTest.kt Supplies the shared customer setting.
example/src/test/java/com/payabli/example/app/demo/ui/TokenGateTest.kt Updates token-gate fixtures.
example/src/test/java/com/payabli/example/app/sdk/CaptureRequestTest.kt Tests capture totals and customer options.
example/src/test/java/com/payabli/example/app/sdk/PayInFormSummaryTest.kt Adapts form-summary tests to explicit totals.
example/src/test/java/com/payabli/example/app/sdk/PayInPrefillTest.kt Verifies device-specific prefill values.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread example/src/main/java/com/payabli/example/app/demo/qa/QaAmount.kt
Comment thread example/src/test/java/com/payabli/example/app/sdk/CaptureRequestTest.kt Outdated
Comment thread example/src/main/java/com/payabli/example/app/demo/ui/setup/SetupScreen.kt Outdated
Comment thread example/src/main/java/com/payabli/example/app/demo/ui/components/SwitchRow.kt Outdated
…every time

`QaAmount.random` samples independently from thirteen hundred values, so two
attempts can land on the same figure. The documentation said "a different amount
for every attempt", and the test that read as covering it asserts variety across
twenty draws rather than that consecutive draws differ.

What makes a row attributable is the customer and the order identifier, both of
which are per device and per second. The figure narrows a list by eye and is a
second signal, so the claim goes rather than the sampling: excluding the previous
draw would add state to carry a one-in-thirteen-hundred repeat that costs a reader
nothing.
…the customer

The test read as covering a request that names nobody. It inspects the configured
customer data alone, and the SDK writes the form's entered values over that before
sending: the capture form collects a first name, a last name and a billing email,
so a request configuring no customer still carries a payer.

What the switch decides is whether the paypoint has a number to match on, which is
what stops it writing a fresh customer for every payment, so that is what the test
now asserts.
The off position read as naming nobody. The capture form's first name, last name
and billing email are still sent: the SDK writes entered values over the configured
customer, so what the switch withholds is the number alone. A tester validating the
switch against that description would look for a request that carries no payer and
find one that does.

The number is what the paypoint matches on, so its absence is why a new customer is
filed per payment, and the note now says that.
A `Switch` beside a `Text` is a separate semantics node from its label, so a
screen reader lands on "on, switch" and never reads what it turns on, and the
label is not a target for anyone who taps at it.

`toggleable` on the row merges the two and states the role once; the switch
inside takes no click of its own, so there is one target and one announcement
rather than two nodes and a silent one.

Covered on a device, because both are properties of the composition: the label
carries the state, a tap on it toggles, and the row reports the state it was
given. Reverting to the sibling layout fails all three.
…s into

The capture form reads back an amount and a service fee and never their sum, so
the figure that leaves the payer's account appears nowhere before submitting. The
result screen shows a total afterwards, which is the wrong side of the decision.

The demo supplies it rather than the form: the SDK's summary section renders the
fields it knows, and a total is not one of them, so showing it there would mean
widening a public enum for the sample app's benefit. The screen with a figure to
add passes a footer under the form, in the sheet as well as inline.

No arithmetic at submission: `totalAmount` is what the request already carries and
the fee is part of it, so this shows that one value rather than adding the rows up
on screen. Asserted in the live walkthrough, which goes red without it.
Copilot AI review requested due to automatic review settings August 14, 2026 18:37

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

example/src/main/java/com/payabli/example/app/demo/ui/setup/SetupScreen.kt:158

  • This off-state explanation omits the paypoint behavior described in the PR: a paypoint with custom identifiers may still match the typed name/email, so it is not guaranteed to create a customer for every payment. Qualify the statement so the Configuration screen does not misreport what the switch does.
                    "Charging sends the name and email typed into the form and no customer number, so the " +
                        "paypoint has nothing to match on and files a new customer for every payment."

example/src/main/java/com/payabli/example/app/demo/qa/QaIdentity.kt:65

  • The model is not a device-unique value. DeviceFactsReader.kt:27 supplies only manufacturer plus model, so two handsets of the same model derive identical labels, customer numbers, emails, and descriptions; QaIdentityTest misses this by testing only distinct model strings. Such devices can still produce the indistinguishable rows this change is intended to prevent. Include a stable per-device or per-install discriminator in the identity.
        fun from(model: String): QaIdentity {
            val label = labelOf(model).ifBlank { UNKNOWN_LABEL }
            return QaIdentity(label = label, slug = slugOf(label))

The footer parameter landed with no caller. `CaptureScreen` invoked
`PaymentFlowScreen` without it, so the default empty lambda stood and the capture
screen drew no total before submitting: the form's own rows read back the amount
and the fee and never their sum, so the figure that leaves the payer's account
appeared nowhere before the button.

The capture screen now passes it, rendering `state.amount` through
`TransactionSummary.formatAmount`, which the form rows and the result screen
already use, so the readout before a submission and the one after are the same
string. The sheet is covered by the same change, since `PaymentFlowScreen`
forwards the footer to it.

Live against the qa paypoint on a Pixel 7a: the three walkthrough tests pass.
Without the footer, `capturingACardThePayerEntered` fails at `performScrollTo()`
on the `Total` node.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 14, 2026 18:56
@alex-arguello

Copy link
Copy Markdown
Collaborator Author

Suppressed comment · demo/ui/setup/SetupScreen.kt:158 · off-state customer wording

This off-state explanation omits the paypoint behavior described in the PR: a paypoint with custom identifiers may still match the typed name/email, so it is not guaranteed to create a customer for every payment. Qualify the statement so the Configuration screen does not misreport what the switch does.

Declined

Measured on the paypoint this app targets, with this request shape: three captures from one device, carrying the same first name, last name and billing email and no customer number, produced three separate customer records, each with no customer number at all. That is the scenario the qualification would describe as uncertain, observed happening. Every capture the app sends also sets forceCustomerCreation (PayInOperation.kt:70), so creation is asked for rather than left to whatever the paypoint would otherwise match on.

Qualifying the sentence would make the screen hedge about an outcome that was measured, which is the failure the original wording already had in the other direction.

Change None.
Test None. The measurement is on the ticket, with the customer records it produced.
Commit none. Nothing in this pull request changes.

Suppressed comment · demo/qa/QaIdentity.kt:65 · same-model devices share an identity

The model is not a device-unique value. DeviceFactsReader.kt:27 supplies only manufacturer plus model, so two handsets of the same model derive identical labels, customer numbers, emails, and descriptions; QaIdentityTest misses this by testing only distinct model strings. Such devices can still produce the indistinguishable rows this change is intended to prevent. Include a stable per-device or per-install discriminator in the identity.

Deferred

Correct, and confirmed at the source: DeviceFactsReader.kt:27 composes "${Build.MANUFACTURER} ${Build.MODEL}", nothing downstream adds a discriminator, and the order identifier separates two such devices only by the second of the attempt. The live run in the description shows two devices already sharing one second, so that is not a reliable separator.

It is deferred rather than fixed here for two reasons. A discriminator changes the customer number, and the customer number is what makes every capture from one device land on one record; whether reinstalling should file a new customer is a real choice rather than an implementation detail. And the counterpart has the same hole, so fixing one platform alone is an asymmetry rather than a fix: QAIdentity.swift states that "the machine identifier is what separates them", while the same file documents that value as "the only per-model answer an app gets", and the fallback only engages when the device name is the generic iPhone.

The current test fleet is four distinct models, so the behaviour this pull request adds holds today and the collision is latent.

Change None.
Test None added. QaIdentityTest covers distinct models, which is what the shipped derivation guarantees.
Commit none. Queued for a ticket covering both platforms.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated no new comments.

Suppressed comments (2)

example/src/main/java/com/payabli/example/app/demo/qa/QaIdentity.kt:66

  • The fallback only handles a blank pre-sanitized label. A model such as "---" is nonblank here but slugOf reduces it to an empty string, producing non-identifying values such as qa-android- and an order ID with no device prefix. Apply the unknown-device fallback when sanitization leaves no slug as well.
        fun from(model: String): QaIdentity {
            val label = labelOf(model).ifBlank { UNKNOWN_LABEL }
            return QaIdentity(label = label, slug = slugOf(label))
        }

example/src/main/java/com/payabli/example/app/AppContainer.kt:74

  • The manufacturer/model string identifies a device type, not an individual device. Two phones of the same model therefore receive the same customer number, email, and order prefix, so their payments are still indistinguishable and may be attached to one customer. Include a stable app-scoped device/install discriminator in the derived identity while retaining the model as the readable label.
    val qaIdentity: QaIdentity = QaIdentity.from(factsAtLaunch.model)

…he model is blank

The blank check runs before sanitising, so a model carrying no letter or digit
passes it: `---` is not blank, and reducing it to a slug empties it. The slug is
what the customer number, the billing email and the order identifier are built
from, so such a device charged as `qa-android-` under `qa+@example.com` and
ordered under a name beginning with the timestamp.

The existing check was written because an empty customer number is a 400 that
names no field. This puts the same check after sanitising, where the value the
request actually carries is decided.

The counterpart has the same gap, guarding the label before slugging it and
reducing punctuation the same way, and is not fixed here because it has no ticket
yet.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 14, 2026 19:13
@alex-arguello

Copy link
Copy Markdown
Collaborator Author

Suppressed comment · demo/qa/QaIdentity.kt:66 · unknown-device fallback misses an emptied slug

The fallback only handles a blank pre-sanitized label. A model such as "---" is nonblank here but slugOf reduces it to an empty string, producing non-identifying values such as qa-android- and an order ID with no device prefix. Apply the unknown-device fallback when sanitization leaves no slug as well.

Fixed · 2fbc5e9

Correct, including the mechanism. labelOf("---") returns ---, which is not blank, so ifBlank never fires; slugOf then maps every non-letter, non-digit to a space and keeps what survives, which is nothing. The slug is what customerNumber, billingEmail and orderId are built from, so the result was qa-android-, qa+@example.com, and an order identifier beginning with the timestamp.

The check this extends was already there for this reason. The test covering the blank case says so in its own comment: an empty customer number is a 400 that names no field. The gap was that the check ran before sanitising rather than after, so it guarded the label while the request carried the slug.

Change from slugs first and falls back to the unknown label when the slug comes out empty. holderName sanitises the label the same way and is covered by the same change, since the label is no longer punctuation-only past this point.
Test a model of punctuation says nothing either, and is named the same way, asserting the customer number, the billing email and the order identifier. It fails on the previous commit and passes on this one.

Parity The counterpart has the identical gap: QAIdentity.swift guards with trimmed.isEmpty ? "Unknown device" : trimmed and then slugs, and its slug(of:) reduces punctuation the same way. Not fixed here, because that work has no ticket; queued as a parity bug rather than left as an asymmetry.

Suppressed comment · AppContainer.kt:74 · model is not a per-device value

The manufacturer/model string identifies a device type, not an individual device. Two phones of the same model therefore receive the same customer number, email, and order prefix, so their payments are still indistinguishable and may be attached to one customer. Include a stable app-scoped device/install discriminator in the derived identity while retaining the model as the readable label.

Deferred

Same finding as the previous round's item on QaIdentity.kt:65, restated against the call site. The disposition is unchanged and is recorded in full here: confirmed at DeviceFactsReader.kt:27, deferred because a discriminator changes the customer number that makes one device's captures land on one record, and because the counterpart has the same hole, so fixing one platform alone would be an asymmetry rather than a fix.

Change None.
Test None added.
Commit none. Queued for a ticket covering both platforms.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated no new comments.

Suppressed comments (1)

example/src/main/java/com/payabli/example/app/sdk/PayInOperation.kt:45

  • suppliesDemoCustomer = false does not stop the submitted request from naming the payer: the form still merges its first name, last name, and billing email into customerData. As the updated test and UI wording now reflect, this flag controls only the customer number, so the parameter contract should say that explicitly.
 * @param suppliesDemoCustomer whether the request names the customer, which [DemoCustomerSetting] decides.

… it a customer

The parameter contract and the setting's own summary read as deciding whether the
request names a payer. It always does: the capture form collects a first name, a
last name and a billing email, and the SDK writes those over whatever the request
configures. What the switch carries is the number the paypoint files against.

The test and the Configuration screen were corrected in an earlier commit and these
two were not, which is the same claim surviving in the places a reader reaches
through the API rather than through the screen.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated no new comments.

Suppressed comments (3)

example/src/main/java/com/payabli/example/app/sdk/PayInOperation.kt:66

  • The new device attribution is not covered at the request boundary. QaIdentityTest tests the formatter, but no test asserts that capturePayment puts its order ID and description into PayInTransactionOptions; deleting either assignment leaves the suite green while dashboard attribution is lost. Add assertions for both fields to CaptureRequestTest.
                orderId = identity.orderId(atMillis),
                orderDescription = identity.note("capture"),

example/src/main/java/com/payabli/example/app/demo/ui/capture/CaptureViewModel.kt:136

  • No test verifies that changing the shared setting rebuilds the operation submitted by an existing CaptureViewModel. The setting test and request-builder test only cover each object independently, so removing this collector makes the Configuration switch a no-op while every test still passes. Add a ViewModel test that toggles the shared setting and inspects the operation's customer number.
            demoCustomer.suppliesDemoCustomer.collect {
                _uiState.update { state ->
                    if (state.payments.isBusy()) state else state.copy(operation = attempt(state.amount).operation)

example/src/main/java/com/payabli/example/app/demo/ui/capture/CaptureViewModel.kt:276

  • The existing startOver test checks only that the idempotency key changes. It does not verify this new financial invariant: the refreshed setup, amount, and operation total must still agree. Omitting either state update would make the form show a different amount from the request while all tests pass; add a test that reads the new summary rows and operation total from the resulting state.
                setup = attempt.setup,
                amount = attempt.amount,
                operation = attempt.operation,

Twelve comments this branch adds took the shape the guidelines rule out: a
contrast with something nobody proposed, or a defence of where a value lives
instead of what it is. `--loose` reports them and leaves the verdict to a reader,
which is why the gate passed while the prose did not.

Eight are kept, each because the contrast is the fact: "excluded rather than
skipped" is this repository's own convention and the rejected half is what a
standing skip would do; "the connector's configuration rather than anything this
app sends" is where responsibility lies; "read off the form rather than off the
identity" names what the assertion catches; and "the number and not the customer"
is the reading a reviewer actually had.

Comments inherited from main are left alone: rewording them widens the diff and
costs a reviewer more than the prose saves.
Copilot AI review requested due to automatic review settings August 14, 2026 19:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated no new comments.

Suppressed comments (4)

example/src/main/java/com/payabli/example/app/demo/ui/capture/CaptureViewModel.kt:264

  • This guarantee is still false: QaAmount.random() samples independently, so startOver() can draw the previous amount again. Describe this as a fresh draw that may repeat rather than promising every second payment has a different amount.
     * A second payment is a second amount, which is what makes two rows from one device tell themselves apart.

example/src/main/java/com/payabli/example/app/demo/ui/capture/CaptureViewModel.kt:299

  • The amount-preservation rule across an answered refusal is payment-critical but remains untested: CaptureIdempotencyTest exercises this branch only to compare keys. A regression that redraws the amount here would still pass while a payer correcting a rejected field is charged a different total than the form showed. Add an assertion that amount, the setup summary, and the replacement operation's total remain equal across this transition.
    private fun rotateIdempotencyKey(outcome: PayInOutcome.Refused) {
        if (outcome.keepsItsIdempotencyKey) return
        _uiState.update { it.copy(operation = attempt(it.amount).operation) }

example/src/main/java/com/payabli/example/app/demo/ui/capture/CaptureViewModel.kt:136

  • No test connects this collector to the request that the existing CaptureViewModel submits. DemoCustomerSettingTest checks only the flow value, while CaptureRequestTest passes the Boolean directly, so deleting this collector leaves both green and makes a switch change ineffective until another attempt is created. Add a view-model test that toggles the shared setting and verifies the current operation adds/removes customerNumber (including the intended busy-attempt behavior).
            demoCustomer.suppliesDemoCustomer.collect {
                _uiState.update { state ->
                    if (state.payments.isBusy()) state else state.copy(operation = attempt(state.amount).operation)

example/src/main/java/com/payabli/example/app/demo/qa/QaIdentity.kt:12

  • “Every value here comes from the device model” is inaccurate because firstName is always "QA". Narrow this statement to the distinguishing values so the identity contract matches the implementation.
 * Three phones and a simulator sending the sample's own test values produce rows nothing can tell apart: the
 * same customer, the same instrument, the same amount, minutes apart. Every value here comes from the device
 * model, so one build installs on all of them and each install still names itself.

…t from

Deleting both assignments from `capturePayment` left the whole unit suite green.
`QaIdentityTest` covers what an order identifier and a note look like; nothing
asserted that either reaches `PayInTransactionOptions`, and those two fields are
what a transaction list shows. Attribution is what this branch exists for, so the
one thing no test held was the thing being built.

The clock is a constant now, because the assertion reads the identifier back and
has to build it from the moment the request did.
…sagree

Both were covered on each side and not across. Removing the collector that follows
the shared setting left every test green while the Configuration switch became a
no-op, and dropping either state update in startOver left every test green while
the form showed a figure the request does not charge.

That second one is the defect this branch was opened to fix, in a second place: it
arrived first as two literals in two files, and the refresh is where they can part
again.

Read off the operation rather than off the setting or the amount, because what a
payer is charged is the request and the rest is what a screen says about it.
Copilot AI review requested due to automatic review settings August 14, 2026 19:55
@alex-arguello

Copy link
Copy Markdown
Collaborator Author

Suppressed comment · sdk/PayInOperation.kt:66 · device attribution untested at the request boundary

The new device attribution is not covered at the request boundary. QaIdentityTest tests the formatter, but no test asserts that capturePayment puts its order ID and description into PayInTransactionOptions; deleting either assignment leaves the suite green while dashboard attribution is lost. Add assertions for both fields to CaptureRequestTest.

Fixed · c96fd27

Confirmed the way it is stated: removing both assignments and running the whole unit suite left it green. Those two fields are the entire mechanism by which a transaction list says which device sent a payment, so the one behaviour with no test was the one the branch exists to add.

Change CaptureRequestTest asserts orderId and orderDescription on the built options. The clock became a constant in the same commit, because the assertion rebuilds the identifier and has to use the moment the request used.

Test a capture carries the device in the fields a transaction list shows. Deleting either assignment now fails it.

@alex-arguello

Copy link
Copy Markdown
Collaborator Author

Suppressed comment · demo/ui/capture/CaptureViewModel.kt:136 · the shared setting reaching an open screen

No test verifies that changing the shared setting rebuilds the operation submitted by an existing CaptureViewModel. The setting test and request-builder test only cover each object independently, so removing this collector makes the Configuration switch a no-op while every test still passes. Add a ViewModel test that toggles the shared setting and inspects the operation's customer number.

Fixed · 202568f

Confirmed: replacing the collector's body with an empty one left the whole unit suite green, and the switch on the Configuration screen stops reaching a capture screen that is already open. Each side was covered and the wiring between them was not.

Change None to the code. The gap was the test.

Test flipping the shared switch changes what an open screen would submit, which reads the customer number off the operation the state holds rather than off the setting: the request is what a paypoint sees, and the setting is only what it is built from.

@alex-arguello

Copy link
Copy Markdown
Collaborator Author

Suppressed comment · demo/ui/capture/CaptureViewModel.kt:276 · the refreshed state and the refreshed request

The existing startOver test checks only that the idempotency key changes. It does not verify this new financial invariant: the refreshed setup, amount, and operation total must still agree. Omitting either state update would make the form show a different amount from the request while all tests pass; add a test that reads the new summary rows and operation total from the resulting state.

Fixed · 202568f

Confirmed: dropping the setup and amount updates and keeping the operation left the whole unit suite green, with the form reading back a figure the request no longer charges.

This is the defect the branch was opened to fix, in a second place. It arrived first as two literals in two files, one saying $ 1.00 while the request charged 1.10; the refresh is the other point where the two can part, and it had no test.

Change None to the code. The gap was the test.

Test a second payment shows the figure it charges, with the first payment shows the figure it charges beside it so the invariant is pinned on the state the screen opens with as well as on the one it refreshes to. Both add the rows the form reads back, compare them to the amount the state holds, and compare that to the total on the operation.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 31 out of 31 changed files in this pull request and generated no new comments.

Suppressed comments (3)

example/src/main/java/com/payabli/example/app/demo/ui/payment/PaymentFlowScreen.kt:281

  • The sheet has the same ordering problem as the inline form: formFooter() runs after PaymentFormHost, whose submit button is already rendered. This lets a payer submit from the sheet before encountering Total; move the footer ahead of the form.
                formFooter()

example/src/main/java/com/payabli/example/app/demo/ui/payment/PaymentFlowScreen.kt:184

  • PaymentFormHost renders the SDK form’s submit button as its last child, so invoking the footer afterward places Total below the actionable button. A payer can therefore submit without ever seeing the full amount. Render the total before the form (or inside its summary before the button) so the charge is disclosed before submission.

This issue also appears on line 281 of the same file.

                    formFooter()

example/src/androidTest/java/com/payabli/example/app/QaWalkthroughTest.kt:47

  • This points readers to scripts/qa-fleet.sh, but that script does not exist in the repository. Replace the reference with an instruction that can actually be followed, or include the script in this PR.
 * which names Compose and not the lock. `scripts/qa-fleet.sh` clears it before it starts.

The form's last child is its own submit button, so a total rendered after the
form sits below the control it qualifies: measured on a Pixel 7a, the row was at
3376 against a button at 3203. A payer reaches the button first and can submit a
figure they have not been shown, which is the disclosure the row exists to make.

It renders before the form now, inline and in the sheet, and the parameter is
named for where it goes.

The walkthrough compared nothing but presence, so it passed either way. It now
reads both positions and fails when the total is the lower of the two, naming
both.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The walkthrough's own documentation pointed a reader at `scripts/qa-fleet.sh`
for clearing the keyguard, and no such file exists here. A sample app has to
stand on its own for whoever reads it, so the instruction says what to do
instead of naming a tool the reader cannot find.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nt figure

The third place carrying the claim the draw does not make: `QaAmount.random`
samples independently, so a second attempt can land on the previous figure. The
order identifier carries the second the attempt was made and is what separates
two rows from one device.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`firstName` is always `QA`, so "every value here comes from the device model"
was not true of the type it documents. The claim that matters is about the values
that separate two devices, which is what the sentence now makes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 14, 2026 20:11
@alex-arguello

Copy link
Copy Markdown
Collaborator Author

Two rounds are answered here, because commits landed between them: the round against a69bbae left two items unfixed, and the round against 202568f raised three more.

Suppressed comment · demo/ui/payment/PaymentFlowScreen.kt:184 and :281 · total renders after the submit button

PaymentFormHost renders the SDK form's submit button as its last child, so invoking the footer afterward places Total below the actionable button. A payer can therefore submit without ever seeing the full amount. Render the total before the form (or inside its summary before the button) so the charge is disclosed before submission.

Fixed · 31b065c

Correct, and measured: on a Pixel 7a the Total row sat at y 3376 against a submit button at y 3203. The row exists to disclose the charge before it is authorised, and below the button it does not do that.

The alternative offered, putting the total inside the form's own summary, would mean widening a public PayInField enum for the sample's benefit, so it renders before the form instead. The sheet took the same change, since the screen passes one slot to both.

Change The slot renders ahead of the form inline and in the sheet, and is named for where it goes.
Test The walkthrough compared nothing but presence and passed in both arrangements. It now reads both positions and asserts the total is the higher of the two, reporting them: reverting this commit alone gives Total sits at 3376.0, below the submit button at 3203.0. Live on a Pixel 7a against the qa paypoint, three tests, none skipped.

Suppressed comment · androidTest/QaWalkthroughTest.kt:47 · names a script that is not in this repository

This points readers to scripts/qa-fleet.sh, but that script does not exist in the repository. Replace the reference with an instruction that can actually be followed, or include the script in this PR.

Fixed · 9942bd1

The file is not here and is not going to be: it drives several devices across two repositories, so it lives with the tooling rather than inside one sample app. A reader of this repository cannot follow it either way.

Change The sentence says to dismiss the keyguard on every target before a run, which is the step the script performed. A sweep for other references to files outside this repository found none.
Test None. Documentation.

Suppressed comment · demo/ui/capture/CaptureViewModel.kt:264 · second payment is not a second amount

This guarantee is still false: QaAmount.random() samples independently, so startOver() can draw the previous amount again. Describe this as a fresh draw that may repeat rather than promising every second payment has a different amount.

Fixed · 68bd33d

The third site carrying a promise the draw does not make; the first two were corrected earlier in this branch. This one survived because it states the claim about a second payment rather than about the draw, so a sweep for the draw's own wording did not reach it.

Change The sentence says the draw has no memory and can repeat, and names the order identifier as what separates two rows from one device.
Test None on the prose. the draw varies rather than repeating one figure covers the behaviour it now describes.

Suppressed comment · demo/qa/QaIdentity.kt:12 · not every value comes from the model

"Every value here comes from the device model" is inaccurate because firstName is always "QA". Narrow this statement to the distinguishing values so the identity contract matches the implementation.

Fixed · 274c061

firstName is the constant QA, so the sentence was not true of the type it documents.

Change The claim is now about the values that separate one device from another, and names firstName as the constant and what it is for.
Test None on the prose.

Suppressed comment · sdk/PayInOperation.kt:45 · the flag controls only the customer number

suppliesDemoCustomer = false does not stop the submitted request from naming the payer: the form still merges its first name, last name, and billing email into customerData. As the updated test and UI wording now reflect, this flag controls only the customer number, so the parameter contract should say that explicitly.

Fixed by an earlier commit · 3fe6e9c

Already corrected before this round was raised, in both places that carried the claim: the parameter contract here, and DemoCustomerSetting's own summary. That commit landed after the review it answers had started, which is why it reads as outstanding.

Change None needed now.
Test the switch off sends no customer number, which reads the field the flag decides.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 31 out of 31 changed files in this pull request and generated no new comments.

@sonarqubecloud

Copy link
Copy Markdown

@alex-arguello
Alex Arguello (alex-arguello) merged commit a1e81ac into main Aug 14, 2026
5 checks passed
@alex-arguello
Alex Arguello (alex-arguello) deleted the alexarguello/pla-2488-android-p736-the-sample-app-cannot-say-which-device-sent-a branch August 14, 2026 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants