Dansday

The Month Vouchers Took Twelve Pull Requests

Published on Oct 31, 2024

Thirty-one pull requests in October and 134 contributions — my biggest month at 3cat by a wide margin, against a previous high of eighty-five in March. Twelve of those pull requests were the same feature.

The feature was a discount code. It is the clearest illustration I have of how a simple-sounding request expands once it meets a real checkout.

A promo with a start date

The ticket arrived with a date attached, which changes the character of the work:

Between 10-20 Oct, we're launching a promo for 2ndhand iPhones.

3CAT50  --> RM 50 off
3CAT100 --> RM 100 off
3CAT200 --> RM 200 off

Each device had one specific voucher attached to it from a spreadsheet, and the promotion ran for ten days. Most of what I had built up to this point could be late by a day without anybody minding. A promotion cannot: marketing had bought the campaign, and the code either exists on the tenth or the campaign has nothing behind it.

The main voucher pull request merged on 10 October. 443 additions, 185 deletions, twenty-one files, landing on the day the promo opened. Two earlier attempts at the interface — opened minutes apart, both 64 additions across seven files, byte-identical to each other — were closed the same day. That pattern of two or three identical pull requests keeps appearing in my history and it always means the same thing: I was reconciling branches rather than writing code, and I could not see a way through except to try one and look at it.

Most of the diff is not the discount. It is a voucher input on the checkout page, a reset button with its own icon, a price breakdown that has to show a discount line that did not exist before, the product summary, the order summary, and ninety-one lines in the order controller. The arithmetic of taking fifty ringgit off a number is one line. Everything else is telling the customer it happened.

Twelve pull requests for one discount

Then a second ticket, and this is where it got interesting. Delivery vouchers — discounts you only get if you choose delivery over in-store pickup. The reasoning in the ticket is the most explicitly behavioural thing I was ever asked to build:

1) Delivery Vouchers are NOT auto-applied on checkout.
Why: This provides a sense of accomplishment/achievement for customers -
because they perceive they've been given something special.

A discount the system knows about, deliberately withheld until the customer types the code, because being given something feels different from being charged less. I have no argument with it. It is worth recording that the requirement was not a technical constraint at all — the harder implementation was chosen on purpose.

That ticket alone produced eight pull requests over four days: the feature, two closed attempts at the condition logic, a fix for the amounts, backend review feedback, back-button handling, a staging fix, and then a second staging fix. Two of the eight are one-line and two-line changes. The work was not large; it was fiddly in a way that only reveals itself on staging.

The framework could say no, but not why

The requirement that shaped the whole implementation was an error message. Two error messages, really, and the distinction between them:

If user enters 3CAT50 but picks reservation, error message should be
"Switch to delivery to use voucher" (special error message)

If user enters 3CAT100 but picks reservation, error message should be
"Voucher not valid for your order" (default error message)

Same customer action, same rejection, two different messages — because in the first case the voucher would work if they changed one thing, and in the second it was never going to. One message is a nudge and the other is a refusal, and getting them the wrong way round either misleads the customer or wastes the sale.

Bagisto has a cart rule engine that evaluates conditions and tells you whether a coupon applies. What it does not tell you is which condition failed. So I read the conditions myself:

foreach ($conditions as $condition) {
    if ($condition['attribute'] === 'cart|shipping_method') {
        $ruleShippingMethod = $condition['value'];

        if ($cartShippingCode !== $ruleShippingMethod) {
            $invalidShippingMethod = true;
        }
    }

    if ($condition['attribute'] === 'cart|base_sub_total') {
        // ... track min and max from the operator
    }
}

if ($invalidShippingMethod && $priceInRange) {
    return new JsonResource([
        'success' => false,
        'incorrect_shipping' => true,
        'message' => trans('web.checkout.errors.invalid_voucher_shipping_method'),
    ]);
}

That conjunction is the entire feature. The shipping method is wrong and everything else about the voucher fits — that is the only combination that earns the encouraging message. Any other failure falls through to the generic one.

I am aware of what this code is. It is a partial reimplementation of somebody else's rule evaluator, reading their stored condition format, and it will drift the moment they change it. The alternative was to run their evaluator and then guess at the reason, which is worse. When a product requirement needs to know why a validation failed and the library only returns whether it failed, you end up here.

Two things in it I would tidy. The price range check sits inside the loop rather than after it, so it evaluates against partially collected minimum and maximum values as the loop walks the conditions. And the shipping code is extracted with explode('_', $cartShippingMethod)[0], which is a string operation standing in for knowing the format.

Applying a voucher should not create a cart

The staging fix on 21 October is titled FIx UI and cart creation every load, typo included, and it is August's lesson arriving at a new address.

In August I wrote about the checkout page creating a cart in order to render itself, and how a GET request should not write. Six weeks later the voucher endpoint was doing this on every attempt:

- Cart::deActivateCart();
- $this->createCart($validatedData);
+ if (!empty(Cart::getCart())) {
+     Cart::setCart(Cart::getCart());
+ } else {
+     $this->createCart($validatedData);
+ }

Every time a customer typed a code and pressed apply, the existing cart was deactivated and a fresh one built. Try three codes and you have created three carts and abandoned two. It is the same mistake in a different verb: I had learned that rendering should not write, and then wrote a validation endpoint that did.

I do not think that is carelessness so much as the shape of the codebase. Cart::addProduct was the established way to get a cart you could ask questions of, so it kept getting called by code that only wanted to ask a question. The fix is to reuse what exists; the deeper fix would have been an API that lets you evaluate a cart without owning one.

The back button, a fourth time

September's shipping-option fix used the browsing session to remember a choice the DOM forgot. October needed the same treatment for vouchers, and the pull request is called simply handle back:

let voucherObject = {};
voucherObject[this.productUrlValue] = voucherCode;

sessionStorage.setItem('voucher', JSON.stringify(voucherObject));

Keyed by product URL, because a voucher belongs to a device and the customer might browse to another one. On page load it reads the stored code, restores September's remembered shipping method alongside it, and re-applies the voucher automatically so the customer returns to the state they left.

That is the fourth month running that a browser back button has generated a ticket — wrong cart in August, wrong shipping option in September, lost voucher in October. It is the single most reliable source of bugs in this application, because it is the one navigation the server never sees.

WebP needed a way out

In January I converted the site's images to WebP and wrote that a 475KB banner became 58KB. In October I built the way back out.

Two pull requests, the first closed and the second merged at 238 additions across six files. It works by asking the browser directly, with a two-pixel image encoded into the page:

function checkWebPSupport(callback) {
    var webP = new Image();
    webP.onload = webP.onerror = function () {
        callback(webP.height === 2);
    };

    webP.src = "data:image/webp;base64,UklGRjoAAABXRUJQVlA4IC4AAACy...";
}

If the image decodes to the height it should be, the browser understands WebP. If it does not, every img source and every stylesheet link on the page gets rewritten from .webp to .png. The server half is a custom S3 adapter and filesystem manager, seventy-four and forty-three lines, so the PNG a fallback browser asks for actually exists.

There is also a pull request from 17 October titled Do not merge - Akbar | WEBP to PNG Script, sixty lines, still open as I write this. Somebody had to bulk-convert nine months of accumulated WebP assets, and that somebody wrote a script and parked it on a branch.

The January decision was still right. Serving eight-times-lighter images to the browsers that can take them is correct, and so is not breaking the ones that cannot. But I had shipped the optimisation without the fallback, and it took nine months and somebody's device to notice.

Two lines I am not proud of

Late in the month I merged a one-line change to a configuration file:

- memory_limit = 128M
+ memory_limit = 256M

The branch is called memory-limit and the title is Increase memory limit for promotion products. The promotion system — the one I spent May reducing from a hundred and eighty queries to twenty, and June restructuring into its own namespace with tests — was exhausting 128 megabytes while rendering products. Doubling the ceiling made the symptom go away in one line and one minute, and it moved the same problem to 256 megabytes.

Sometimes that is the right trade. A promotion is live, memory is cheap, and the alternative is a day of profiling. But I merged it without opening a ticket to find out what was actually consuming the memory, and there is no such ticket in the history afterwards.

The other one is a hotfix I raised against myself and merged the same day, removing the Hotjar session-recording script from every page:

- <!-- Hotjar Tracking Code for https://3cat.my -->
- <script> (function (h, o, t, j, a, r) { ... })(window, document, ...); </script>

Two lines deleted, marked HOTFIX, straight to production. The issue body is empty. Whatever it was — page weight, a customer complaint, something about recording people's checkout sessions — I did not write it down, and I cannot reconstruct it now from the repository. That is the one genuinely irrecoverable thing in this whole series. Every other decision I have been able to reread from a diff.

The nine I did not count

Twenty-two pull requests merged in October and this article names about half of them. The half I left out is worth a paragraph, not because any one of them matters, but because of what they are.

The largest is 120 additions and 111 deletions to adjust where the page jumps to when somebody taps Our Stores — the third pull request in three months about that anchor. Then: shortening the Our Stores text on the homepage, two attempts at the homepage above-the-fold section titled Update homepage ATF and Update homepage ATF 2, a one-line fix for a missing desktop indicator, a one-line fix to the More Devices section on product pages, and an improvement to the favicon — which I do mention, because it took three attempts, but which I did not connect to the rest.

Two more were opened and closed without landing: iPad alignment issues, and a category removal.

Nine changes, almost all of them one line or a few, all of them somebody looking at a screen and finding it slightly wrong. In the same month I built a voucher system across twelve pull requests and wrote about the framework being unable to say why it had rejected a code. Both kinds of work happened; only one of them reads like engineering. The count I opened this article with — twenty-two — is only honest if the other nine are somewhere in it.

What October was

Thirty-one pull requests, twenty-two merged. A discount system that shipped on the morning its campaign opened, an error message that had to know why it was saying no, a fallback for a nine-month-old optimisation, and two more stores — Batu Pahat Mall and Bukit Mertajam, week forty.

Also from 17 October: a pull request called Do not merge - Akbar | MySQL Production Dump, opened with no changes in it. It is still open today, and its title now names a date in 2026, which means I have been quietly reusing that branch as scratch space for nearly two years. In March it was a branch to open a local database port. Every repository I have worked in long enough has one of these, and in this one it has my name on it.

Revised: this article gave a pull request count of twenty-two and described about half of them. The section above accounts for the rest.