Skip to content

Add lifecycle callbacks#25

Merged
rameerez merged 4 commits intomainfrom
feature/lifecycle-callbacks
Jan 15, 2026
Merged

Add lifecycle callbacks#25
rameerez merged 4 commits intomainfrom
feature/lifecycle-callbacks

Conversation

@rameerez
Copy link
Owner

@rameerez rameerez commented Jan 15, 2026

Summary

Adds a comprehensive callback system for credit lifecycle events, inspired by patterns from the api_keys gem. This enables users to hook into credit operations for analytics, notifications, audit logging, and custom business logic.

7 new callbacks available:

  • on_credits_added - After credits are deposited to a wallet
  • on_credits_deducted - After credits are removed from a wallet
  • on_low_balance_reached - When balance crosses below threshold (fires once per crossing)
  • on_balance_depleted - When balance reaches exactly zero
  • on_insufficient_credits - Before raising InsufficientCredits error
  • on_credit_pack_purchased - After credit pack purchase is fulfilled
  • on_subscription_credits_awarded - After subscription credits are awarded

Why we built this

Users needed hooks into credit operations for common use cases:

  • Analytics: Track credit flow, purchases, and usage patterns
  • Notifications: Alert users when running low or out of credits
  • Upselling: Trigger credit pack promos when balance depletes
  • Audit logging: Record all credit movements for compliance
  • Debugging: Log failed operations to understand user friction

The existing on_low_balance callback was too limited - it only provided the owner, not the context needed for rich integrations.

How it works

All callbacks receive a CallbackContext struct with typed, consistent data:

UsageCredits.configure do |config|
  config.on_credits_added do |ctx|
    # ctx.owner            - The wallet owner (User, Team, etc.)
    # ctx.wallet           - The wallet instance
    # ctx.amount           - Credits added
    # ctx.previous_balance - Balance before
    # ctx.new_balance      - Balance after
    # ctx.transaction      - The transaction record
    # ctx.category         - Transaction category
    Analytics.track(ctx.owner, "credits_added", amount: ctx.amount)
  end

  config.on_balance_depleted do |ctx|
    CreditUpsellMailer.out_of_credits(ctx.owner).deliver_later
  end
end

Design decisions & pitfalls avoided

1. Error isolation

Callback errors are caught and logged, never breaking credit operations. A failing analytics call shouldn't prevent a user from spending credits.

2. Backward compatibility preserved

The existing on_low_balance { |owner| } syntax continues to work unchanged. Internally, it wraps the legacy callback to extract owner from the new context system. Zero migration required for existing users.

3. Deprecation with warn-once pattern

Legacy methods (handle_event, notify_low_balance) show deprecation warnings on first use only, avoiding log spam from repeated calls.

4. Test isolation

Added UsageCredits.reset! to test teardown to prevent callback pollution between tests. This was critical - without it, callbacks configured in one test would leak into others.

5. Low balance fires once per threshold crossing

on_low_balance_reached only fires when crossing from above to below the threshold, not on every deduction while already below. This prevents notification spam.

6. Balance depleted edge case

on_balance_depleted fires only when balance reaches exactly zero. If negative balances are enabled and balance goes from positive to negative (skipping zero), this callback won't fire - by design.

Files changed

New files:

  • lib/usage_credits/callback_context.rb - Immutable struct for callback data
  • lib/usage_credits/callbacks.rb - Centralized dispatch with error isolation
  • test/usage_credits/callbacks_test.rb - Unit tests
  • test/integration/wallet_callbacks_test.rb - Integration tests
  • test/usage_credits/backward_compat_test.rb - Backward compat tests

Modified:

  • lib/usage_credits/configuration.rb - 7 callback DSL methods
  • lib/usage_credits/models/wallet.rb - Dispatch callbacks from credit operations
  • lib/usage_credits/models/concerns/pay_charge_extension.rb - credit_pack_purchased callback
  • lib/usage_credits/models/concerns/pay_subscription_extension.rb - subscription_credits_awarded callback
  • lib/usage_credits.rb - Requires + deprecation handling
  • lib/generators/usage_credits/templates/initializer.rb - Improved callback documentation
  • README.md - New "Lifecycle callbacks" section
  • test/test_helper.rb - Reset configuration between tests

Test plan

  • All 655 existing tests pass
  • 24 new callback-specific tests added
  • Coverage increased from 86.68% to 88.6%
  • Backward compatibility tests verify legacy on_low_balance works
  • Deprecation warn-once pattern verified
bundle exec rake test
# 655 runs, 1490 assertions, 0 failures, 0 errors, 0 skips

🤖 Generated with Claude Code


Note

Adds a robust callback framework to hook into credit events with error isolation and backward compatibility.

  • New Callbacks dispatcher and CallbackContext struct; 7 events: credits_added, credits_deducted, low_balance_reached, balance_depleted, insufficient_credits, credit_pack_purchased, subscription_credits_awarded
  • Wire callbacks into Wallet (add_credits, deduct_credits, spend_credits_on insufficient path), Pay::Charge (credit pack fulfillment), and Pay::Subscription (initial award and upgrades)
  • Deprecate legacy on_low_balance usage in favor of on_low_balance_reached (legacy still supported); deprecate UsageCredits.handle_event and notify_low_balance with warn-once behavior; add reset! to clear warnings/config
  • Extend Configuration with DSL methods for all callbacks; update initializer template and README with usage and fields
  • Add comprehensive tests: unit, integration, and backward-compat to verify dispatch order, isolation, and deprecation behavior

Written by Cursor Bugbot for commit b75799c. This will update automatically on new commits. Configure here.

Introduce 7 lifecycle callbacks for credit events, enabling users to
hook into credit operations for analytics, notifications, audit logging,
and custom business logic.

New callbacks:
- on_credits_added: Fires after credits are deposited
- on_credits_deducted: Fires after credits are removed
- on_low_balance_reached: Fires when balance crosses below threshold
- on_balance_depleted: Fires when balance reaches zero
- on_insufficient_credits: Fires before raising InsufficientCredits
- on_credit_pack_purchased: Fires after credit pack fulfillment
- on_subscription_credits_awarded: Fires after subscription renewal

Key design decisions:
- Callbacks receive a CallbackContext struct with typed, consistent data
- Error isolation: callback failures never break credit operations
- Backward compatible: existing on_low_balance { |owner| } still works
- Deprecation warnings (warn-once) for legacy handle_event method
- Configuration reset in test teardown prevents callback pollution

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@claude
Copy link

claude bot commented Jan 15, 2026

Pull Request Review: Lifecycle Callbacks System

I've completed a thorough review of PR #25. Overall, this is an excellent implementation that adds significant value to the gem. The code quality is high, the design is thoughtful, and backward compatibility is well-preserved. Here's my detailed feedback:


✅ Strengths

1. Excellent Architecture & Design

  • Clean separation of concerns: CallbackContext (data), Callbacks (dispatch), and Configuration (DSL) are well-separated
  • Error isolation: Callbacks are wrapped in exception handling so they never break core credit operations - this is critical
  • Immutable context object: Using a Struct with keyword args provides type safety and clear API contracts
  • Thoughtful callback timing: Events fire at the right moments (after operations complete, before errors raise)

2. Backward Compatibility Done Right

  • Legacy on_low_balance { |owner| } syntax continues to work with zero migration required
  • Smart wrapper approach that extracts owner from context for legacy callbacks
  • Deprecation warnings implemented with warn-once pattern to avoid log spam
  • Test coverage for backward compatibility ensures existing users won't break

3. Comprehensive Testing

  • 24 new callback-specific tests added (unit + integration + backward compat)
  • Coverage increased from 86.68% to 88.6%
  • Tests verify error isolation, threshold crossing logic, and edge cases
  • UsageCredits.reset! in test teardown prevents callback pollution - excellent attention to detail

4. Developer Experience

  • Clear, well-documented callback DSL in the initializer template
  • Consistent context object across all callbacks with to_h for easy logging
  • Examples in README show real-world use cases (analytics, notifications, upselling)
  • The 7 callbacks cover all major lifecycle events users would want to track

🔍 Detailed Code Review

callback_context.rb (lib/usage_credits/callback_context.rb:1)

Good:

  • Clean, minimal Struct definition
  • to_h method with .compact removes nil values - great for logging
  • owner convenience method avoids forcing users to do ctx.wallet.owner

Minor observation:

  • The Struct has 10 fields, but not all events use all fields. This is fine and maintains API consistency, but you could document which events provide which fields (you already do this well in the initializer template).

callbacks.rb (lib/usage_credits/callbacks.rb:1)

Good:

  • Error isolation via rescue block is essential for callback systems
  • Arity checking handles different callback signatures gracefully
  • Logging abstraction works with or without Rails
  • Uses module_function for clean module method exposure

Potential improvement:

# Line 30: Arity checking could be simplified
when 1, -1, -2  # Accepts one arg or variable args

The -2 case (for |ctx, **kwargs| signatures) is good to handle, but Ruby blocks with splats (|ctx, *args|) would have arity -1, not -2. The -2 case is for keyword arguments. This works correctly, just noting the comment could be more precise:

when 1, -1, -2  # 1 = fixed arg, -1 = splat args, -2 = keyword args

wallet.rb (lib/usage_credits/models/wallet.rb:1)

Good:

  • Callbacks fire at the right moments: after transactions are committed, before errors are raised
  • Previous balance captured before state changes - essential for accurate tracking
  • Low balance threshold crossing logic is correct: !was_low_balance?(previous_balance) && low_balance?
  • Balance depletion check: previous_balance > 0 && balance == 0 - correctly fires only when crossing to zero

Security/correctness:

  • All callbacks fire after database transactions commit (for add/deduct). This is correct - callbacks see consistent state.
  • For insufficient_credits, callback fires before raising, which is perfect for logging attempts.

configuration.rb (lib/usage_credits/configuration.rb:210)

Good:

  • All 7 callback DSL methods follow consistent naming: on_<event>
  • Backward compatible on_low_balance method that wraps the new system
  • Callbacks can be cleared by passing nil (useful for testing)

Minor:

  • Line 256: on_low_balance raises if no block given, but other callback methods silently accept no block (to clear). This inconsistency is likely intentional for backward compat, but worth noting.

pay_charge_extension.rb & pay_subscription_extension.rb

Good:

  • Credit pack purchase callback fires after successful fulfillment
  • Subscription credits callback includes rich metadata (plan name, fulfillment period, reactivation status)
  • Callbacks fire for upgrades as well as initial subscriptions
  • Transaction references are passed when available

Observation:

  • In pay_charge_extension.rb:174, the callback is outside the ActiveRecord::Base.transaction block. This is correct - callbacks should fire after the database commit succeeds.

🐛 Potential Issues

1. Race Condition in Low Balance Tracking (Minor)

In wallet.rb:289, the low balance check uses:

if !was_low_balance?(previous_balance) && low_balance?

This reads low_balance? which calls credits (the current balance). However, self.balance = credits was just set on line 277, but save! hasn't been called yet at line 278. If another thread reads the wallet between these lines, they might see stale data.

Impact: Low risk in practice because the row lock from the transaction protects this, but worth noting.

Recommendation: This is likely fine as-is due to row locking, but could be made more explicit by using the local balance variable instead of calling low_balance? again.

2. Balance Depletion Edge Case with Negative Balances (Minor)

In wallet.rb:302:

if previous_balance > 0 && balance == 0

If allow_negative_balance is enabled and balance goes from positive directly to negative (e.g., 5 credits, deduct 10), this won't fire. The PR description mentions this is "by design", which is reasonable.

Recommendation: Consider adding a note in the documentation that on_balance_depleted fires only when reaching exactly zero, not when skipping zero to negative.

3. Callback Arity Warning May Be Too Strict (Very Minor)

In callbacks.rb:36, callbacks with arity > 1 trigger a warning. However, Ruby blocks like { |ctx, extra| } would have arity 2, which is a valid (if unusual) signature.

Impact: Very low - this is defensive programming and the warning is appropriate.


🔒 Security Review

No security concerns identified.

  • Callbacks are isolated and can't break credit operations ✓
  • No user input is passed unsanitized to callbacks ✓
  • Metadata is properly controlled (comes from internal gem operations) ✓
  • No SQL injection vectors ✓
  • No potential for callback-based DoS (errors are caught) ✓

⚡ Performance Considerations

Good:

  • Callbacks are optional (nil check before dispatch)
  • Context object creation is cheap (Struct allocation)
  • Error handling doesn't add significant overhead

Potential concern:

  • In high-throughput scenarios, firing callbacks on every credit operation could add latency if users perform expensive operations in callbacks (e.g., synchronous external API calls, slow database queries).

Recommendation: Add a note in documentation encouraging users to use deliver_later, background jobs, or fast operations in callbacks to avoid blocking credit operations.


📝 Documentation Quality

Excellent documentation:

  • README has clear examples with real use cases
  • Initializer template provides comprehensive inline documentation
  • PR description explains design decisions and pitfalls avoided
  • Code comments explain "why" not just "what"

Suggestions:

  1. Add a performance note in README about keeping callbacks fast
  2. Document which context fields are available for each event type (you partially do this, but a reference table would be helpful)

🧪 Test Coverage

Strengths:

  • Comprehensive unit tests for Callbacks module
  • Integration tests verify end-to-end callback flow
  • Backward compatibility tests ensure legacy code works
  • Edge cases tested (error isolation, threshold crossing, balance depletion)

Potential gaps:

  1. No test for callback arity warning (when callback has unexpected arity)
  2. No test verifying callbacks fire in correct order when multiple events occur
  3. No test for on_credit_pack_purchased callback in refund scenarios

Recommendation: Add tests for the above cases in a follow-up PR.


🎯 Recommendations

Critical (should address before merge):

None - the PR is production-ready as-is.

Nice to have:

  1. Add performance guidance in README about keeping callbacks fast
  2. Consider documenting the balance depletion edge case with negative balances
  3. Add test coverage for callback arity warnings and refund scenarios

Future enhancements:

  1. Consider adding on_credits_expired callback when credits expire
  2. Consider on_refund callback for when credit packs are refunded
  3. Consider adding callback priorities/ordering if users need multiple callbacks

✨ Final Verdict

Approval: ✅ Approved with minor suggestions

This is a well-designed, thoroughly tested feature that significantly enhances the gem's extensibility. The code quality is high, backward compatibility is preserved, and the developer experience is excellent. The design decisions (error isolation, context objects, warn-once deprecation) show deep understanding of production Ruby/Rails development.

Merge recommendation: ✅ Ready to merge

The minor suggestions above are truly optional and don't block merging. This is production-ready code that will benefit gem users significantly.

Great work! 🎉


Review completed using Claude Code
Code quality: 9/10
Test coverage: 9/10
Documentation: 9/10
Overall: 9/10

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

This is the final PR Bugbot will review for you during this billing cycle

Your free Bugbot reviews will reset on February 6

Details

You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

rameerez and others added 2 commits January 15, 2026 04:35
The subscription_credits_awarded callback was being dispatched before
fulfillment.update! completed. If that update failed, the callback would
have already fired for credits that get rolled back.

Fix: Wrap add_credits and fulfillment.update! in a transaction block,
dispatch callback only after the transaction commits.

Also adds regression tests:
- Test that credits roll back when outer transaction fails
- Test that subscription upgrade callback receives persisted transaction

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Repository owner deleted a comment from claude bot Jan 15, 2026
Repository owner deleted a comment from claude bot Jan 15, 2026
Move subscription_credits_awarded callback dispatch outside the
transaction block in handle_initial_award_and_fulfillment_setup,
matching the pattern used in handle_plan_upgrade.

Changes:
- Callback now dispatched AFTER transaction commits (not inside)
- Added transaction: parameter to match upgrade path signature
- Removed rescue/rollback pattern (let errors propagate naturally)

This ensures callbacks only fire for persisted data and maintains
consistency across all subscription credit award scenarios.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@rameerez rameerez changed the title Add comprehensive lifecycle callback system Add lifecycle callbacks Jan 15, 2026
@rameerez rameerez merged commit 8a95639 into main Jan 15, 2026
23 of 24 checks passed
@rameerez rameerez deleted the feature/lifecycle-callbacks branch January 15, 2026 13:51
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.

1 participant