В магазине на Medusa есть накопленные баллы — «складской кредит». Их можно частично потратить на заказ.
Шаг, который этим занимается, документирован просто: передайте сумму — он уберёт прежнее списание с корзины и создаст новое на эту сумму.
А чтобы вообще убрать списание — передайте ноль.
Читается как «если сумму передали — берём её, иначе весь остаток». И это верно ровно до того дня, когда кто-то передаст ноль.
Ноль в JavaScript ложный. Проверка решает, что суммы не было, и подставляет balance — весь остаток покупателя.
Из описания исправления #16378:
Clearing the store credit from a cart left it with a credit line for everything the customer had.
Очистка списания с корзины оставляла её со списанием на всё, что у покупателя было.
Кнопка «убрать скидку» выдавала максимальную скидку. Механика, за которую в другом месте платят маркетологам.
Самое поучительное в этой истории — не сама ошибка, а где нашлось лекарство.
Из описания того же исправления:
Changed the guard to
isDefined(input.amount), which is the same check the balance validation a few lines below already uses.…ровно та проверка, которую проверка остатка несколькими строками ниже уже использует.
Верный образец лежал в том же файле, в пределах экрана. Просто в одном месте написали ?, а в другом — isDefined.
Это типично для больших кодовых баз: две разные проверки одного и того же живут рядом годами, пока одна из них не встретит граничное значение.
Ошибку чинили трижды:
Разные авторы, одна строка.
Это не про несогласованность, а про диагностичность. Когда баг звучит как «я убираю списание, а он списывает всё», он воспроизводится с первой попытки и находится за минуты. Такие ошибки собирают очередь из желающих починить.
А вот баг, который звучит как «иногда сумма не та», живёт годами.
Через два дня в том же шаге нашли второе — #16405. Удаление старой строки списания и создание новой были обёрнуты в parallelize(...) — выполнялись одновременно.
Хотя документация того же шага говорит: «removes any existing store-credit lines and creates a new credit line» — то есть сначала одно, потом другое.
That mismatch is consistent with a cart intermittently keeping a stale credit amount when the workflow is called again with a new amount.
Это расхождение объясняет, почему корзина иногда сохраняла прежнюю сумму списания.
Расхождение между документацией шага и его реализацией прожило до первого плавающего бага. Плавающие баги на то и плавающие, что живут долго.
Опасны не все, а те, где ноль — значащее значение: сумма скидки, количество, порог, цена по акции.
Каждое найденное место проверьте одним вопросом: что случится, если сюда придёт ноль? Если ответ «то же, что и при отсутствии» — нужна явная проверка на undefined/null.
grep -rnE '(amount|price|quantity|total|balance)[A-Za-z]*\s*(\?|\|\|)' src/Вторая ошибка из этой истории нашлась именно так: в описании написано «убирает, потом создаёт», в коде стояло parallelize.
Пройдите по местам, где порядок важен, и сравните три вещи: что обещает комментарий, что обещает название функции и что делает код.
Ноль — настоящее значение. Где он означает «ничего не делать», там нужна проверка на существование, а не на истинность. То же касается цены 0 (акция или подарок), порога 0 (бесплатная доставка всегда) и количества 0.
Если рядом есть верная проверка — берётся она. Две разные проверки одного и того же в одном файле — это не стиль, а будущая ошибка.
Документация шага — часть кода. Если написано «сначала убрать, потом создать», а в коде они идут одновременно — неправы оба, но виноват код.