Skip to content

Conversation

@ok200paul
Copy link
Collaborator

Fixes #95.

Summary

Add comprehensive unit tests for VoucherService and fix a production bug discovered during testing.

Changes

Tests Added (20 tests, 327 assertions)

Complete test coverage for all public static methods in VoucherService:

  • findUniqueShortCodeForVoucher() (4 tests)

    • Format validation (100 iterations)
    • Uniqueness verification
    • Collision avoidance with existing codes
    • Multiple unique code generation
  • calculateVoucherAmountRedeemed() (4 tests)

    • Zero redemptions case
    • Single redemption sum
    • Multiple redemptions sum
    • Voucher-specific filtering
  • calculateVoucherAmountRemaining() (5 tests)

    • Full value with no redemptions
    • Partial redemption calculations
    • Fully redeemed (zero) case
    • Over-redeemed (negative) edge case
  • updateVoucherAmountRemaining() (3 tests)

    • Successful update flow
    • Exception on over-redemption
    • Database rollback on validation failure
  • collateVoucherAggregates() (4 tests)

    • Complete field updates (remaining amount, redemption count, timestamps)
    • Short code removal when fully redeemed
    • Conditional field updates based on redemption state
    • Exception handling for over-redemption

Bug Fix

Fixed critical bug in VoucherService::collateVoucherAggregates() (line 93)

  • last_redemption_at was being calculated without filtering by voucher_id
  • This would have caused incorrect timestamps across all vouchers
  • Changed: VoucherRedemption::where('voucher_id')->max('created_at')
  • To: VoucherRedemption::where('voucher_id', $voucher->id)->max('created_at')

Documentation

  • Added bidirectional @see annotations linking service methods to their tests
  • Service methods reference all their test cases
  • Test methods reference back to the implementation
  • Enables easy IDE navigation between implementation and tests

Testing Notes

  • Used withoutEvents() for over-redemption edge case tests to isolate calculation logic from event-driven validation
  • Tests verify both happy paths and error conditions
  • Database is refreshed between tests using RefreshDatabase trait

@ok200paul ok200paul requested review from dacook and mkllnk October 20, 2025 05:47
Copy link
Member

@dacook dacook left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, indeed it's very comprehensive! Thanks for the detailed notes too.

I thought the @see comments seemed a bit over the top, until I remembered this was to aide navigation in IDE. That must be handy, but could get fiddly.

Glad to see this helped uncover a bug; it just goes to show how important tests are!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

VoucherSetService needs tests

4 participants