DansdayMay ended with a feature that worked and had never been written down. June was the month I wrote it down — first as six numbered sentences in a pull request comment, then as six test cases, then as a class that did not live in the wrong place.
Eight pull requests. Two of them merged in June. The rest landed in July or never landed at all, which is its own kind of month.
Two days of aftermath
The promotion indicator went live on 6 June. I spent 6 and 7 June fixing it, which is the honest shape of every launch I have been part of.
The first bug was a rule nobody had stated. A promotion card shows two lines of text, and each product can override the site-wide default. Marketing filled in line one for a product and left line two empty, and the page rendered their line one above the default's line two — two halves of two different sentences, stitched together into something neither of them wrote.
The fix is a rule: if you override one line, you own both.
$hasTitle1Value = !empty($product->{Promotion::ATTR_PROMOTION_TITLE_1_TEXT});
foreach ($attributes as $attribute) {
if ($attribute === Promotion::ATTR_PROMOTION_TITLE_1_TEXT
|| $attribute === Promotion::ATTR_PROMOTION_TITLE_2_TEXT) {
if ($hasTitle1Value) {
$attributeValues[] = $product->$attribute;
} else {
$defaultConfig = $isPriceDrop ? null : Promotion::getDefaultConfig($attribute);
$attributeValues[] = $this->getAttributeValuePromotion($product, $attribute, $defaultConfig);
}
} else {
$defaultConfig = $isPriceDrop ? null : Promotion::getDefaultConfig($attribute);
$attributeValues[] = $this->getAttributeValuePromotion($product, $attribute, $defaultConfig);
}
}An empty line two now means an intentionally blank line two. That is thirteen lines to express one sentence of policy, and look at the shape of it — two branches computing the identical $defaultConfig, a special case for two of eight attributes wedged into a loop over all of them. I knew it was bad while writing it. The site was live and marketing was waiting, so it shipped.
The next day I hoisted the duplicated line out and changed a constant:
- const PROMOTION_CATEGORY_SLUG = 'promotion';
+ const PROMOTION_CATEGORY_SLUG = 'cat/promotion';The promotion category had been living at the site root, competing for URL space with every real page. Moving it under a prefix is five characters and the sort of thing that becomes impossible to change once search engines have indexed it. Two days after launch is roughly the last cheap moment.
Writing the rules down
Six days later I opened a pull request called Refactor promo indicators, and the most valuable thing in it was a comment.
A configurable product — an iPhone with storage sizes and colours — can carry a promotion on the parent, or on individual variants, with date ranges on either, some expired, some set to the sentinel 0000-00-00 00:00:00 that means no date was ever entered. In May I had made all of that behave correctly by argument and intuition. I could not have told you the rule.
So I wrote out all six cases longhand:
2. Product parent(configurable) in promotion category and have period, but the
variant in promotion category and have own value but the period is
0000-00-00 00:00:00. then promotion will used from variant but the period
will use from Product parent. if variant value empty will use product parent
value and if empty also will use from default config.
3. Product parent(configurable) in promotion category and have period, but the
variant in promotion category and have own value but the period is not
0000-00-00 00:00:00 and expired. then no promotion.That is not documentation. The grammar is rough and it reads like someone thinking out loud, because that is what it was. But it is the first time the behaviour existed anywhere other than inside my head and inside a method too tangled to read back. Every branch in the code now had a sentence it was answerable to, and two of those sentences turned out to describe things the code did not actually do.
Nine arguments become one
Then the refactor those rules made possible. 454 additions, 243 deletions, twelve files.
In May the promotion object took nine positional constructor arguments, and I had written about how unpleasant that was. In June I deleted it:
public function __construct(private array $fallbackAttrValue = []) {}One argument. The nine properties became named constants held in an array, so adding a tenth is a line in a list rather than a tenth positional slot to miscount at four call sites. The class also moved out of src/Models/ and into src/Promotion/, growing from 51 lines to 174 as it absorbed the logic that had been living in the product model — where Product.php lost 140 lines and gained 46.
That is the part I would point at. A promotion was never a database row, and it had been sitting in the models directory because that was where the file got created. Moving it is not cosmetic: it is the difference between a product that knows how to be a product and a product that also knows the marketing team's discount precedence rules.
The variant selection got rewritten too, and inverted from what I had written in May:
usort($variantsWithEmptyAttributes, fn ($a, $b) => $a->min_price <=> $b->min_price);
usort($variantsWithNonEmptyAttributes, fn ($a, $b) => $a->min_price <=> $b->min_price);
$sortedVariants = array_merge($variantsWithEmptyAttributes, $variantsWithNonEmptyAttributes);Variants that inherit everything from the parent are considered before variants carrying their own overrides, each group cheapest first. Then it walks the list and takes the first that yields a promotion at all — so a variant whose dates have expired falls through to one whose dates have not, instead of the card going blank because the first candidate happened to be dead.
The whole thing also learned to fail quietly:
try {
if ($product->type === Product::CONFIGURABLE_TYPE) {
return $this->getPromotionForConfigurableProduct($product);
}
return $this->determinePromotion($product);
} catch (\Exception $e) {
Log::error('Error in getPromotion: ' . $e->getMessage(), ['product_id' => $product->id]);
return null;
}A promotion is decoration. If the precedence logic hits something it cannot resolve, the correct outcome is a product card with no badge and a line in the log — not a 500 on a page where somebody was about to spend RM 999. Returning null with the product id attached means I find out without the customer finding out first.
One thing that refactor made worse, which I should say plainly. The promotion became an array instead of an object, so templates went from reading typed properties to indexing string keys:
- @define $promoFrameEarsColor = $promotion?->earsColor;
+ promoFrameEarsColor="{{ $promotion['promotion_ears_color'] ?? '' }}"Six @define lines collapsed into one, which is the win. But ?->earsColor was checkable and ['promotion_ears_color'] is a string that fails silently when misspelled. I traded a small amount of safety for a large amount of noise and I still think it was the right call at the time, but it is a trade, not an improvement.
The first tests
The same pull request added the first test file I had written in this codebase: 176 lines covering exactly the six scenarios.
'scenario 1' => [
[
'type' => 'configurable',
'promotion_period_start' => static::generateDate(0),
'promotion_period_end' => static::generateDate(5),
'variantPromotionPeriodStart' => Promotion::DEFAULT_TIME_PERIOD,
'variantPromotionPeriodEnd' => Promotion::DEFAULT_TIME_PERIOD,
'hasPromotionCategory' => true,
'variantHasPromotionCategory' => true,
'variantHasOwnValue' => false,
'hasPromotion' => true,
]
],A data provider, one row per rule, dates generated relative to now rather than hardcoded so the suite does not start failing in a fortnight. The filesystem got mocked because the promotion icon resolves through a storage disk and a unit test has no business touching one.
Seven months into the job and this is my first test, which is a fact about the pace rather than a defence of it. What convinced me was not discipline. It was that I had just spent two days fixing a launch, and I had six sentences in front of me that were trivially checkable, and I could not think of a reason not to.
Handing over the banners
The last real feature of June gave marketing the promotional banners on category pages. 384 additions, 219 deletions, twenty-four files.
Most of it is admin surface — 151 new lines for a modal in the theme customisation screen, so banners get chosen per category from a UI instead of a config file. A PromotionBanner class joined the new promotion namespace. Fifty-one lines of banner styling moved out of home.css, because banners had stopped being a homepage thing.
My favourite part of that diff is the deletions. simple.blade.php, thirty-six lines, gone. Five lines from a customer order page, nine from a static page, a stale line from the homepage index. Layouts nobody had rendered in months, still being maintained by anyone who touched the files around them. A pull request that adds a feature is a good moment to delete the things that feature reveals as dead.
What the checkout was not telling us
The month closed with analytics. The checkout already tracked that somebody clicked pay; it did not track what they had chosen when they clicked it. So the button learned to report the shipping and payment method alongside the product:
onclick="
app.util.trackClick('{{ EventActions::ACTION_CHECKOUT_PAY }}', '{!! $productName !!}');
app.util.trackClick('{{ EventActions::ACTION_SHIPPING_METHOD }}', document.querySelector('input[name=delivery-option]:checked').id);
app.util.trackClick('{{ EventActions::ACTION_PAYMENT_METHOD }}', document.querySelector('input[name=payment-method]:checked').id);"Now the business can see whether people who choose self-pickup convert differently to people who choose delivery, which is a question you cannot answer retroactively — the data either was collected or it was not.
Three inline handler calls reading the DOM at click time is not elegant, and there is a real fragility in it: querySelector(':checked') returns null when nothing is selected, and reading .id from null throws. It is safe only because the form guarantees a default selection, which means it is safe by circumstance rather than by construction. The same pull request deleted a dead cart endpoint and an unused path variable, so it was not all debt.
A branch for QA to look at
Two of June's pull requests were never meant to merge. One is called Akbar qa and its branch is akbar-qa-3pr — three pull requests, combined onto master so that QA had one environment showing all of them at once instead of three branches that each work alone.
It carries sixty-eight commits from six different people. I opened it on 27 June and closed it the same day. Its sibling lived from 28 June to 11 July and collected my own follow-up work in the meantime: Feedback, QA feedback, Fix period, Fix 500.
Nobody teaches you this and every team invents it. When work is reviewed in isolation but experienced in combination, somebody has to build the combination, and the artefact is a branch you throw away.
Three hundred more URLs
Outside the main repository, one commit: 334 lines added to the URL checker's list of 3cat pages, nine removed.
In February that file was 288 lines. By June it was over six hundred. It is a hand-maintained list of every product page the script should confirm is still alive, and it grows because somebody adds to it when the catalogue changes — which means it is always slightly behind, and nobody would know which parts.
The script has a sitemap crawler in it. It also has this list. I never reconciled the two, and a monitoring tool with a manually curated target list has the same problem as the New Store labels I would spend three months removing a year later: a field that means current with nothing keeping it current.
What June was
Eight pull requests, two merged inside the month, and three hundred more URLs under watch. A promotion system that moved out of the models directory, lost 140 lines of the product model, gained a specification in plain English and six tests that hold it to it, plus banners that marketing can change without me.
The thing I would point at is the comment. Not the refactor it justified or the tests that came from it — the six rough sentences. In May I shipped behaviour I could not describe. In June I described it, and describing it was what showed me two of the branches were wrong.