The Month Five Pull Requests Took Three Weeks

By pull request count May was my quietest month: five, against sixteen in March. By commit count it was one of the busiest. Almost all of it went into a single feature that took three weeks and taught me more than the previous three months combined — and that I did not, in the end, merge.

A new ticket number

The first thing that changed in May was not code. My branches had been named ip-28437, ip-28959, ip-29311 since December, because tickets lived in Jira and carried a five-digit number. Halfway through May my branches start reading i-441, and my pull request titles start with plain numbers — 421:, 566:, 753:.

The team had moved its tracker into the repository. There are test tickets from 23 April with names like Test ticket 1, and then from 2 May the real ones begin. It sounds like housekeeping. What it actually meant was that the ticket, the discussion, the branch, the diff and the review finally lived in one place, and I stopped keeping two tabs open to understand one piece of work.

Nine properties, nine queries

The feature sounds simple. Products on promotion should look different — a badge, a coloured border, decorative ears on the card, custom title text, a framed banner. Marketing wanted to control all of it per product, without a developer touching anything.

It had actually started on 23 April, six weeks before it shipped. The first version worked. There is a commit from 14 May called 70% work done, and by that evening the promotion styling rendered correctly on every card. If the ticket had been closed there, nobody would have complained for about a week.

The problem was how it fetched things. Each visual property had its own method, and each method ran its own database query:

{!! $product->getPromotionIcon() !!}
{!! $product->getPromotionTitle1Text() !!}

Nine properties, nine queries — per product. A homepage showing twenty products fired one hundred and eighty queries to draw some borders and badges. This is the N+1 problem, and it is invisible in development where you have four test products and a local database, then merciless in production where you have hundreds and a network hop.

The fix was to ask once and return everything:

@define $promotion = $product->getPromotionAttributes();

{!! $promotion->icon !!}
{!! $promotion->borderColor !!}
{!! $promotion->title1Text !!}

One query per product, returning an object with every property on it. The template reads fields instead of calling methods, and nothing in the markup betrays how much work disappeared.

The commits from that week are the whole lesson in order: Optimizing, Optimizing, Slice the data, Query check, Return empty, Logic change, Use query, Eager load, Improvement, Improvement. Ten commits in three days, none of which added a feature.

Teaching the query to fetch once

Then I had to make that single query stop being lazy about relationships. The original filtered products by a category the database had to look up separately, then loaded price data in a second pass. Eager loading pulls it all in one go:

$this->with([
    'price_indices' => function ($query) {
        $query->orderBy('min_price', 'asc');
    },
    'categories'
])

Note the closure. You can constrain an eager load, which means the sorting happens inside the same query rather than in PHP after the fact. Getting the database to do the ordering is almost always cheaper than fetching everything and sorting it yourself.

Four copies of a nine-argument constructor

Halfway through, the code had become unreadable in a specific way. A promotion object takes nine constructor arguments, and I was building it in several branches — once for a parent product, once for a promotional variant, once for a price-drop variant, once for the empty fallback. Four copies of a nine-argument constructor call, each subtly different, each a place to make a typo. There is a genuine one in that code: promotion::ATTR_PROMOTION_BORDER_COLOR with a lowercase class name, which PHP tolerated and I did not notice for days.

The commit is called DRY the code, and it replaced all four with one closure taking a product and a set of defaults:

$createPromotion = function ($product, $defaultSet) use ($getAttributeValue) {
    return new Promotion(
        $getAttributeValue($product, Promotion::ATTR_PROMOTION_ICON, $defaultSet['icon']),
        $getAttributeValue($product, Promotion::ATTR_PROMOTION_BORDER_COLOR, $defaultSet['borderColor']),
        // ...
    );
};

Four call sites became one definition and four short calls. The empty case got an explicit early return rather than falling through the branches by accident.

Two days later a commit called Use bagisto default configuration panel deleted something else I had built: my own settings screen for the promotion defaults. The framework already had a configuration system with validation, permissions and a UI the team knew how to use. Building a second one was three days of work I got to throw away, which is cheaper than maintaining it forever.

Which promotion wins

The hardest part was not performance or tidiness. It was deciding which promotion wins. A configurable product — an iPhone with several storage sizes and colours — can have promotions on the parent, on individual variants, on some variants and not others, with date ranges that may have expired, and sometimes with the sentinel value 0000-00-00 00:00:00 meaning no date was ever set. Which badge does the card show?

My first attempt took whichever variant the query returned first, which is to say an arbitrary one. The answer I settled on was to sort by intent:

$promotionalVariants = $variants->filter(/* has promotion, no active date range */)
    ->sortBy('special_price');

$priceDropVariants = $variants->filter($isWithinPromotionPeriod)
    ->sortBy('special_price');

$allVariants = $promotionalVariants->merge($priceDropVariants)->first();

Standing promotions outrank temporary price drops, and within each group the cheapest variant wins — because a card advertising a saving should advertise the best one available. Merging the two ordered collections and taking the first gives a deterministic answer instead of whatever the database felt like returning. The commit is called Prioritize by price and promotion variant, and it is dated 20 May, seventeen days after the version that already worked.

The frame, and somebody else's branch

On 21 May somebody else started committing to my branch. A colleague picked up the product page half — the framed banner around the gallery, which had to reposition itself when a customer switched variants: add frame to pdp, frame positioning, pdp frame cleanup 1, variant level selectors. My last two commits on it are Fix cases and update the sort, on 27 and 28 May.

Then the honest part, which is the bit I would have skipped a year ago. Neither of my promotion pull requests was ever merged. Both were closed on 1 August, unmerged, months after the fact.

The code shipped on 6 June, in a pull request opened by my colleague and titled #411 & #412 Promotion and price drop product indicator in PDP and Product Cards - 2. 563 additions across eighteen files. Its description is not a description at all — it is my commit log, pasted in and preserved in order:

Get promotion
Backend
Indicator finalize
70% work done
sorting by price
Revert
Config added
Optimizing
...
DRY the code

That is what handover looks like when it goes well. Three weeks of my work rebased into someone else's branch, merged under their name, with the history intact. I shipped two follow-ups in the days after — a staging fix on 6 June and a slug and naming change on 7 June — and then promotion banners on category pages later that month.

A pull request being closed unmerged is not the same as work being wasted. It took me a while to be relaxed about the distinction. What went to production was the query that runs once instead of nine times, the ordering that is deterministic, and the configuration panel I did not build.

The customer's voice

Alongside that, the smaller work. April's WhatsApp button had been bouncing every half second, forever; May's first pull request replaced it with an animation that fires once when you scroll to it, which is 128 additions to make something happen less.

And a fix I liked. The widget on checkout prefilled a message written in the shop's voice:

'checkout_widget' => 'Hi, need any help to checkout?'

Which is what a shop assistant says, not what a customer types. The customer is the one sending it, and they know what they are looking at:

'checkout_widget' => 'Hi, i need help to checkout :product'

Now the message arrives in the customer's voice with the product name already in it, so whoever answers knows what is being discussed before asking. The lowercase i shipped exactly as written, and I have made peace with it.

The month closed with a FAQ addition for a new instalment provider — fifteen lines across ten files, the first ticket I did under the new numbering.

What May was

Five pull requests opened, thirty-five commits on one branch, and a hundred and eighty queries that became twenty.

Six months earlier I had joined a repository that was two weeks old and could not show a product. The thing I would point at from May is not the feature. It is that the first working version arrived on 14 May and I spent the next two weeks not shipping it — because working and correct are different, and the gap between them is where the actual job is.