DEV Community

Cover image for My Payment Contract Was Typed on a Subscription. Ten Drivers Paid for It.
Nasrul Hazim
Nasrul Hazim

Posted on

My Payment Contract Was Typed on a Subscription. Ten Drivers Paid for It.

TL;DRlaravel-billing 2.0.1 makes the one-off charge the primitive. A plan checkout is now just a one-off charge described by a plan. Ten gateway drivers stopped depending on the package's own Plan and Billable models, and the package can finally charge for an invoice. Also: 2.0.0 shipped empty and I had to burn a version number to fix it. That part's at the bottom.


The tell

Here's the thing I'd been looking straight past for months. The contract read like this:

public function createCheckout(
    Billable $billable,
    Plan $plan,
    PlanInterval $interval,
    string $returnUrl,
): CheckoutIntent;
Enter fullscreen mode Exit fullscreen mode

Ten drivers implement that. Nine of them are Malaysian gateways — Billplz, toyyibPay, BayarCash, senangPay, eGHL, iPay88, SecurePay — plus Stripe and PayPal.

And not one of the nine actually has a subscription concept in the call they make. Billplz posts to /bills. toyyibPay calls createBill. BayarCash posts to /payment-intents. They create a bill. One bill. Once.

So what were Plan and PlanInterval doing in there? Deriving four values:

  • an amount
  • a description
  • who is paying
  • where to send them back afterwards

That's it. Four scalars, and to get them I made every driver import two Eloquent models and an enum from my package. A contract is like a job description — it should say what must be done, not drag in the org chart. Mine dragged in the org chart.

The cost of that wasn't abstract:

  1. Every driver was coupled to my subscription tables for no reason at all.
  2. The package couldn't do the other thing every host app eventually needs — charge for an invoice, a top-up, a one-time fee. You had to invent a fake Plan row to bill someone RM125.

Both symptoms, one cause: the contract was typed on the wrong thing.

Inverting it

The fix is to state what the drivers were already doing, directly:

public function checkout(CheckoutRequest $request): CheckoutIntent;
Enter fullscreen mode Exit fullscreen mode

And CheckoutRequest is a plain DTO with no model from this package anywhere in it:

final class CheckoutRequest
{
    public function __construct(
        public int $amountCents,
        public string $description,
        public string $customerName,
        public string $customerEmail,
        public string $returnUrl,
        public ?string $reference = null,
        public string $currency = 'MYR',
        public ?string $customerPhone = null,
        public ?string $callbackUrl = null,
        public array $metadata = [],
    ) {}
}
Enter fullscreen mode Exit fullscreen mode

Three details in there earned their place the hard way.

amountCents, not a float. Money in a float is a rounding error waiting for a reconciliation meeting. Minor units, integer, always. The DTO exposes amountDecimal() because most Malaysian gateways post a decimal string on the wire — but the conversion happens in one place, not ten.

reference belongs to the caller. Every driver used to mint its own order id (SUB + random) unconditionally. That's correct when the only caller is the package's own subscription flow, and wrong for absolutely everyone else. An application charging invoice #1042 needs the gateway to echo back something it can find #1042 from. Pass one; leave it null and the driver mints one exactly like before.

callbackUrl is per-request. A multi-tenant app routing webhooks per tenant cannot express that in a single config value. Previously it just… couldn't.

Keeping the plan checkout, once

createCheckout() didn't go away — it moved. The plan → charge mapping now lives in exactly one place:

trait MapsPlanToCheckout
{
    public function createCheckout(
        Billable $billable,
        Plan $plan,
        PlanInterval $interval,
        string $returnUrl,
    ): CheckoutIntent {
        return $this->checkout(new CheckoutRequest(
            amountCents: $plan->priceCents($interval),
            description: "$plan->name.' ('.$interval->value.')',"
            customerName: $billable->billingName(),
            customerEmail: $billable->billingEmail(),
            returnUrl: $returnUrl,
            currency: (string) config('billing.currency', 'MYR'),
            metadata: [
                'billable_type' => $billable->getMorphClass(),
                'billable_id' => (string) $billable->getKey(),
                'plan' => $plan->tier,
                'interval' => $interval->value,
            ],
        ));
    }
}
Enter fullscreen mode Exit fullscreen mode

Why a trait and not just the abstract base class? Because LocalGateway implements the contract directly instead of extending Gateway, and a second copy of a mapping is a future disagreement. The base class uses the trait; LocalGateway uses the trait. One copy.

The single most important test in the whole PR is the boring one: a plan checkout sends the same payload it sent in 1.0. If a refactor this size can't prove that, it isn't a refactor, it's a rewrite with optimism attached.

The capability question: null vs. throw

The other half of 2.0 is fetch():

public function fetch(string $externalId): ?PaymentStatus;
Enter fullscreen mode Exit fullscreen mode

A webhook is the only signal a driver gets by default. And a webhook that was never delivered is indistinguishable from a payment that never happened — which is precisely the situation your customer is in when they say "I paid" and your app says otherwise. fetch() is how that argument gets settled: ask the gateway instead of waiting to be told.

Not every gateway can be asked, though. Of the ten, Billplz, Stripe and toyyibPay have a usable lookup; the others don't. So what does fetch() do on a gateway with no lookup?

The tempting answer is return null. That's wrong, and it's wrong in a way worth internalising: null is already taken. On this method it means "I asked, and there is nothing there" — which is an answer. "This gateway has no way to be asked" is not an answer, it's the absence of the question being available. Collapse the two and a caller can't tell a missing payment from a missing capability.

So:

final class UnsupportedByGateway extends RuntimeException
{
    public static function cannot(string $gateway, string $capability): self
    {
        return new self("The {$gateway} gateway cannot {$capability}.");
    }
}
Enter fullscreen mode Exit fullscreen mode
public function fetch(string $externalId): ?PaymentStatus
{
    throw UnsupportedByGateway::cannot('senangpay', 'be asked about a payment');
}
Enter fullscreen mode Exit fullscreen mode

This is the general rule: when a driver-based abstraction has an optional capability, make the absence of the capability a distinct, loud outcome. A silent null makes the caller write if ($status === null) and quietly get it wrong for one of the two reasons it can be null.

PaymentStatus follows the same instinct:

final class PaymentStatus
{
    public function __construct(
        public string $externalId,
        public bool $paid,
        public string $status,
        public ?int $amountCents = null,
        public ?string $currency = null,
        public array $raw = [],
    ) {}
}
Enter fullscreen mode Exit fullscreen mode

$paid is deliberately separate from $status. Raw statuses are gateway-specific — paid, succeeded, complete, 1 — and a caller shouldn't have to learn ten vocabularies to answer one boolean question. $raw is there for the cases a normalised shape genuinely can't cover.

Where I did not collapse things

Stripe and PayPal keep their own createCheckout(). This one's a judgement call and it could have gone the other way, so here's the reasoning.

A native subscription and an ad-hoc charge are different objects at the vendor, not two spellings of one. Forcing a Stripe plan checkout through the one-off primitive would mean either inventing a Stripe price per invoice, or quietly turning a one-time fee into a recurring charge. Neither is a refactor; both are bugs with good intentions.

So both gateways gained a one-off path in addition: Stripe via mode: payment with price_data, PayPal via Orders API v2. And PayPal's one-off uses intent: CAPTURE, not AUTHORIZE — an authorisation nobody captures expires silently, which looks exactly like a payment that worked right up until the money never arrives.

Testing it

it('charges an arbitrary amount with no plan and no subscription', function () {
    Http::fake(['*/api/v3/bills' => Http::response([
        'id' => 'bill_9',
        'url' => 'https://billplz.test/bills/bill_9',
    ])]);

    $gateway = new BillplzGateway([
        'api_key' => 'key',
        'x_signature_key' => 'xsig',
        'collection_id' => 'col1',
        'callback_url' => 'https://app.test/webhooks/billplz',
        'sandbox' => true,
    ]);

    $intent = $gateway->checkout(new CheckoutRequest(
        amountCents: 12500,
        description: "'Invoice INV-1042',"
        customerName: 'Ali bin Abu',
        customerEmail: 'ali@example.test',
        returnUrl: 'https://app.test/invoices/1042',
        reference: 'INV-1042',
    ));

    expect($intent->redirectUrl)->toBe('https://billplz.test/bills/bill_9');
});

it('throws rather than answering null when a gateway cannot be asked', function () {
    $gateway = new SenangPayGateway([/* … */]);

    expect(fn () => $gateway->fetch('anything'))
        ->toThrow(UnsupportedByGateway::class);
});
Enter fullscreen mode Exit fullscreen mode

One thing that bit me while writing the fixtures: the helper that builds a CheckoutRequest from overrides has to use array_key_exists() for reference, not ??. With ??, an explicit reference: null reads as "absent" — and "absent" is exactly the case the test needs to be able to express, because that's the branch where the driver mints its own.

Upgrading (this is a breaking change)

Contracts\PaymentGateway gained two methods, so a host app implementing it directly must be updated. Three steps:

  1. Rename createCheckout(Billable, Plan, PlanInterval, string) to checkout(CheckoutRequest $request) and read the four values off $request.
  2. Either extend Gateways\Gateway or use Gateways\Concerns\MapsPlanToCheckout to get createCheckout() back for free.
  3. Add fetch(). If your gateway can't be asked: throw UnsupportedByGateway::cannot('your-gateway', 'be asked about a payment');

Nothing else changed. 88 tests green, PHPStan clean, Pint clean.


And then I shipped it empty

Here's the part I'd rather not write, which is usually the sign it's the useful part.

I tagged 2.0.0 against the wrong commit. The merge hadn't landed locally, I tagged on the strength of having typed the command, and the tag pointed at a tree with none of the above in it.

Fine — delete the tag, re-push it at the right commit. Except Packagist caches a tag's dist archive. composer require cheerfully went on installing the original zip. composer show reported 2.0.0 with a straight face. composer clear-cache does nothing, because the stale artifact isn't on my machine — it's on Packagist's side.

The only reliable repair is to supersede with a new version. So 2.0.0 is withdrawn (release and tag both deleted, so nothing can resolve to it — ^2.0 would otherwise happily pick the broken one) and the real release is 2.0.1.

The prevention costs about four seconds:

gh pr merge --squash
git checkout main && git pull
grep -r "UnsupportedByGateway" src/ || { echo "release is empty"; exit 1; }
git tag v2.0.1 && git push --tags
Enter fullscreen mode Exit fullscreen mode

Pull, then grep for something the release actually adds, before you tag. A merge that reported conflicts hasn't landed. A version number is one of the very few things in software you can't take back — treat the tag as the release, not as a label you can move.

Takeaway: two of today's three lessons were the same lesson. null was overloaded because I never asked what it already meant. Plan was in the contract because I never asked what the drivers actually needed. Both are cheap to check and expensive to leave. And the third one — check the tree before you tag.

Top comments (0)