Small bugs, big impact: three real cases in Symfony and PrestaShop

Small bugs, big impact: three real cases in Symfony and PrestaShop

Most of the bugs I have fixed at work were not hard because of the amount of code involved. The fix was usually one or two lines. They were hard because the code looked correct, the framework did exactly what it was told, and the symptom appeared somewhere else: in a payment provider, in a database that kept old data, or in a form that saved "successfully" without changing anything.

These three cases come from my work on MasMusculo's gym management platform (Symfony) and its PrestaShop store at Osunasport. Names and code are simplified, but the root causes are the real ones.

Case 1: Doctrine's filter() returns a copy

The symptom

Gym staff build training routines in an EasyAdmin back office. A routine has several training days, and each day has a list of exercise rows (sets, reps, rest). After editing the exercises of one day and saving, the routine ended up inconsistent: rows that should have been replaced were still there next to the new ones.

How saving worked

Each training day is rendered as its own collection field in EasyAdmin, and when the form is submitted the entity's setter is called once per day. The setter's logic was simple: empty the rows of that day, then add the rows received from the form. Days without changes were left alone.

The problematic version looked roughly like this:

public function setRowsForDay(int $day, iterable $newRows): void
{
    // filter() returns a NEW ArrayCollection
    $rowsOfDay = $this->rows->filter(
        fn (RoutineRow $row) => $row->getDay() === $day
    );
    $rowsOfDay->clear(); // clears the copy, not $this->rows

    foreach ($newRows as $row) {
        $row->setDay($day);
        $this->addRow($row);
    }
}

Root cause

Collection::filter() does not filter in place. It builds a new collection with the matching elements. Clearing that new collection has no effect on $this->rows, which is the collection Doctrine is tracking. So the old rows were never removed and the new ones were appended on top.

The fix

Remove the elements from the collection Doctrine actually manages:

foreach ($this->rows->toArray() as $row) {
    if ($row->getDay() === $day) {
        $this->removeRow($row); // removeElement() + orphanRemoval on the relation
    }
}

Iterating over toArray() avoids modifying the collection while looping over it, and orphanRemoval on the relation makes Doctrine delete the detached rows on flush.

Lesson: with Doctrine collections, always ask "am I modifying the tracked collection or a copy?". filter(), map() and slice() all give you something new.

Case 2: a discount of 0 € that Stripe will not accept

The symptom

Customers subscribing through a promotion occasionally hit an error coming from Stripe, and the subscription could not be completed.

Root cause

Promotions were stored as cookies once the promo code was validated, and the product price was calculated according to the promotion type. Some promotions set a fixed final price instead of a percentage. When that fixed price was equal to or higher than the product's normal price, the calculation still produced a "discount", but its amount was zero (or would have been negative). That value was passed on to Stripe, which does not allow a discount of 0 €.

So the platform was trying to apply a promotion that gave the customer no benefit at all, and the only visible effect was a failed checkout.

The fix

The decision belongs in the price calculation, before anything reaches the payment provider: if the final price is not lower than the base price, there is no discount to apply.

public function calculatePrice(int $baseCents, ?Promotion $promo): int
{
    if ($promo === null) {
        return $baseCents;
    }

    $finalCents = $promo->apply($baseCents);

    // No real discount: behave as if there were no promotion
    return $finalCents < $baseCents ? $finalCents : $baseCents;
}

And when building the checkout, a discount is only attached when it is greater than zero.

Lesson: "no-op" values are still values. A zero discount, an empty array or a null date can be perfectly valid inside your domain and still be rejected by an external API. Normalise them at the boundary, not in the error handler.

Case 3: the empty list that never reaches the server

The symptom

A custom PrestaShop module lets the shop exclude suppliers, products or categories from a feature. Adding exclusions worked. Removing all exclusions of one type did not: after saving, the old ones were still there.

(As a side note, the module's configuration page did not even open on my local environment until I raised PHP's memory_limit to 512M. Worth checking before assuming the bug is in your code.)

Root cause

In the module's postProcess(), the exclusions were only saved if the submitted value was present:

if (Tools::getValue('excluded_products')) {
    $this->saveExclusions('product', Tools::getValue('excluded_products'));
}

An HTML multi-select with nothing selected is not sent at all by the browser. So "the user emptied the list" and "this field was not part of the request" look exactly the same on the server, and the branch that would have deleted the old exclusions was simply skipped.

The fix

Always process the field when the form is submitted, and treat a missing value as an empty list:

if (Tools::isSubmit('submitExclusions')) {
    $ids = Tools::getValue('excluded_products', []);
    $ids = is_array($ids) ? array_map('intval', $ids) : [];

    $this->saveExclusions('product', $ids); // [] means "remove all"
}

Lesson: the absence of data is data. When a form can legitimately send "nothing", decide explicitly what "nothing" means instead of letting a truthiness check decide for you.

What the three have in common

None of these bugs threw an exception in our own code. Each one was a mismatch between what the code seemed to say and what actually happened: a copy instead of a reference, a zero that only matters to someone else's API, an empty value that is indistinguishable from a missing one.

The habit I took from them is to look first at the boundaries: between my code and the ORM, between my code and a payment provider, between the browser and the server. That is usually where a correct-looking line stops being correct.