mirror of
https://github.com/priyanshujain/sanderling.git
synced 2026-10-02 19:17:10 +00:00
fix(folio): stop spending the submit budget on taps the app refused
The window is an upper bound on the transactions an interval could hold, and a bound inflated by taps that commit nothing is a bound the app can never exceed: #78 read a rise of 15 transactions against 37 submits. TxnSubmit is clickable(enabled = amount.isNotBlank()) and parseCents refuses anything its regex misses, so a tap whose landing frame shows a refused amount cannot have committed. Over four recorded android runs that is 19, 11, 25 and 25 of 35, 26, 42 and 42 submit taps. A relaunch is excepted: a fresh process draws an empty field whatever was submitted.
This commit is contained in:
1 parent
ff7dde4be8
commit
6b276253e8
2 files changed
+129
-1
No files matched your search
@@ -249,16 +249,54 @@ export function acrossRelaunch(lastAction: ObservedAction | null): boolean {
|
|||||||
// well have landed. Leaving it out is what convicted a healthy app:
|
// well have landed. Leaving it out is what convicted a healthy app:
|
||||||
// committedTransactionsExceedSubmits saw a transaction rise of one against a
|
// committedTransactionsExceedSubmits saw a transaction rise of one against a
|
||||||
// window of zero and called it a double submit.
|
// window of zero and called it a double submit.
|
||||||
|
//
|
||||||
|
// A submit the app must have refused does not count, for the mirror reason: it
|
||||||
|
// cannot have committed anything, so the bound it would raise is slack the app
|
||||||
|
// can hide a real double submit behind. See submitCouldCommit for what "must
|
||||||
|
// have refused" is allowed to mean.
|
||||||
export function countSubmitsInWindow(args: {
|
export function countSubmitsInWindow(args: {
|
||||||
previousCount: number;
|
previousCount: number;
|
||||||
lastAction: ObservedAction | null;
|
lastAction: ObservedAction | null;
|
||||||
|
amountText?: string;
|
||||||
fresh: boolean;
|
fresh: boolean;
|
||||||
}): { reported: number; next: number } {
|
}): { reported: number; next: number } {
|
||||||
const { previousCount, lastAction, fresh } = args;
|
const { previousCount, lastAction, fresh } = args;
|
||||||
const reported = previousCount + (isTxnSubmitTap(lastAction) ? 1 : 0);
|
// A relaunch is the one thing that can put a form state on screen other than
|
||||||
|
// the one the tap read, so the field it draws proves nothing about it.
|
||||||
|
const refused = !acrossRelaunch(lastAction) && !submitCouldCommit(args.amountText);
|
||||||
|
const reported = previousCount + (isTxnSubmitTap(lastAction) && !refused ? 1 : 0);
|
||||||
return { reported, next: fresh ? 0 : reported };
|
return { reported, next: fresh ? 0 : reported };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Could the app have committed anything for that submit? The amount field as
|
||||||
|
// the LANDING frame shows it is the form state the tap read: the tap changes
|
||||||
|
// nothing about it, and one action runs per step, so nothing else could have.
|
||||||
|
// Off the transaction screen there is no field to read, and undefined is
|
||||||
|
// unknown, which counts.
|
||||||
|
//
|
||||||
|
// False only where Folio's own code must have refused. parseCents takes
|
||||||
|
// `^\d+(\.\d{1,2})?$` with commas stripped and refuses everything else, and
|
||||||
|
// AddTransactionViewModel refuses a parsed zero on top of that. An empty field
|
||||||
|
// never even reaches the parser: TxnSubmit is
|
||||||
|
// clickable(enabled = amount.isNotBlank()), so the click does not fire.
|
||||||
|
//
|
||||||
|
// This is the difference between a bound and a useless one. The window is an
|
||||||
|
// upper bound on the transactions the interval could hold, and a bound inflated
|
||||||
|
// by taps that commit nothing is a bound the app can never exceed: the iOS run
|
||||||
|
// in #78 read a rise of 15 transactions against a window of 37 submits and had
|
||||||
|
// nothing to say. Measured over four recorded android runs, 19, 11, 25 and 25
|
||||||
|
// of 35, 26, 42 and 42 submit taps landed with the amount field empty.
|
||||||
|
//
|
||||||
|
// An amount too large for a Kotlin Long is refused by the app too, and still
|
||||||
|
// counts here: over-counting can only cost a detection, and the reading that
|
||||||
|
// would have to prove the overflow is a float that cannot hold the number.
|
||||||
|
export function submitCouldCommit(amountText: string | undefined): boolean {
|
||||||
|
if (amountText === undefined) return true;
|
||||||
|
const trimmed = amountText.trim().replace(/,/g, "");
|
||||||
|
if (!/^\d+(\.\d{1,2})?$/.test(trimmed)) return false;
|
||||||
|
return /[1-9]/.test(trimmed);
|
||||||
|
}
|
||||||
|
|
||||||
// Parses formatCents output like "$5.00", "-$1,234.56", "+$0.50" back to
|
// Parses formatCents output like "$5.00", "-$1,234.56", "+$0.50" back to
|
||||||
// integer cents. Anything that is not a complete amount is null, not 0: a
|
// integer cents. Anything that is not a complete amount is null, not 0: a
|
||||||
// balance we could not read is unknown, and reading it as zero silently moves
|
// balance we could not read is unknown, and reading it as zero silently moves
|
||||||
|
|||||||
@@ -67,6 +67,96 @@ test("a second submit with no Home reading between them counts two", () => {
|
|||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// The window is a budget: an upper bound on the transactions the interval could
|
||||||
|
// hold. A tap the app's own parser must have refused spends none of it, and on
|
||||||
|
// android it does not even reach the parser, because TxnSubmit is
|
||||||
|
// clickable(enabled = amount.isNotBlank()). Measured over four recorded android
|
||||||
|
// runs, 19, 11, 25 and 25 of 35, 26, 42 and 42 submit taps landed on the
|
||||||
|
// transaction screen with the amount field empty, so more than half the budget
|
||||||
|
// was being spent on taps that cannot commit anything.
|
||||||
|
test("a submit the app must have refused does not spend the window's budget", () => {
|
||||||
|
for (const amountText of ["", " ", "0", "0.00", "00", "5.", "abc"]) {
|
||||||
|
assert.deepEqual(
|
||||||
|
countSubmitsInWindow({
|
||||||
|
previousCount: 0,
|
||||||
|
lastAction: { kind: "Tap", on: submitOn },
|
||||||
|
amountText,
|
||||||
|
fresh: false,
|
||||||
|
}),
|
||||||
|
{ reported: 0, next: 0 },
|
||||||
|
`amount ${JSON.stringify(amountText)} was counted as a possible commit`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
// The field as the landing frame shows it, which is the form state the tap read:
|
||||||
|
// nothing between the two changes it. Anywhere but the transaction screen there
|
||||||
|
// is no field to read, and unknown has to count.
|
||||||
|
test("an amount that could commit, or that nobody could read, spends the budget", () => {
|
||||||
|
for (const amountText of ["5", "0.01", "1,000", "999999999999999999999", undefined]) {
|
||||||
|
assert.deepEqual(
|
||||||
|
countSubmitsInWindow({
|
||||||
|
previousCount: 0,
|
||||||
|
lastAction: { kind: "Tap", on: submitOn },
|
||||||
|
amountText,
|
||||||
|
fresh: false,
|
||||||
|
}),
|
||||||
|
{ reported: 1, next: 1 },
|
||||||
|
`amount ${JSON.stringify(amountText)} was dropped from the budget`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
// The one thing that can put a different form state on screen than the one the
|
||||||
|
// tap read: the runner restarting the app, which the tap survives and the typed
|
||||||
|
// amount does not. The field a fresh process draws is empty whatever was
|
||||||
|
// submitted, so it proves nothing and the submit keeps its place in the budget.
|
||||||
|
test("a submit across a relaunch spends the budget whatever the field shows", () => {
|
||||||
|
assert.deepEqual(
|
||||||
|
countSubmitsInWindow({
|
||||||
|
previousCount: 0,
|
||||||
|
lastAction: { kind: "Tap", on: submitOn, applied: true, relaunched: true },
|
||||||
|
amountText: "",
|
||||||
|
fresh: false,
|
||||||
|
}),
|
||||||
|
{ reported: 1, next: 1 },
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
// What the budget costs the counting invariant, in the shape of the iOS run in
|
||||||
|
// #78: a stretch of the walk that never went Home, most of it taps on a submit
|
||||||
|
// button with nothing typed into the form, and one double tap that committed
|
||||||
|
// twice. Counting the refused taps hands the app five transactions of slack it
|
||||||
|
// never used, and two rows against six actions is no violation.
|
||||||
|
test("refused submits used to hide a double submit behind their own budget", () => {
|
||||||
|
const frames = [
|
||||||
|
{ amountText: "", lastAction: { kind: "Tap", on: submitOn } },
|
||||||
|
{ amountText: "", lastAction: { kind: "Tap", on: submitOn } },
|
||||||
|
{ amountText: "", lastAction: { kind: "Tap", on: submitOn } },
|
||||||
|
{ amountText: "", lastAction: { kind: "Tap", on: submitOn } },
|
||||||
|
{ amountText: "", lastAction: { kind: "Tap", on: submitOn } },
|
||||||
|
{ amountText: undefined, lastAction: { kind: "DoubleTap", on: submitOn } },
|
||||||
|
];
|
||||||
|
let budget = 0;
|
||||||
|
for (const frame of frames) {
|
||||||
|
budget = countSubmitsInWindow({
|
||||||
|
previousCount: budget,
|
||||||
|
lastAction: frame.lastAction,
|
||||||
|
amountText: frame.amountText,
|
||||||
|
fresh: false,
|
||||||
|
}).next;
|
||||||
|
}
|
||||||
|
assert.equal(budget, 1);
|
||||||
|
assert.equal(
|
||||||
|
committedTransactionsExceedSubmits({
|
||||||
|
countsBefore: { Checking: 3 },
|
||||||
|
countsAfter: { Checking: 5 },
|
||||||
|
submitsInWindow: budget,
|
||||||
|
}),
|
||||||
|
true,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
// The two traces the freshness rule exists to tell apart, driven step by step
|
// The two traces the freshness rule exists to tell apart, driven step by step
|
||||||
// through the same pair of carriers the spec holds.
|
// through the same pair of carriers the spec holds.
|
||||||
function run(steps: { route: string | null; totalText?: string; lastAction: unknown }[]) {
|
function run(steps: { route: string | null; totalText?: string; lastAction: unknown }[]) {
|
||||||
|
|||||||
Reference in new issue
Block a user