Add configurable minimum fulfillment period for dev/test flexibility

[rameerez]

I was trying to replicate #12 and verify the fix after #20, but found out overriding MIN_PERIOD as the commenter suggested did not work with usage_credits

A configured fulfillment period of 30.seconds would be invalid, it would prevent the UsageCredits::Fulfillment record from being created (and therefore no credits awarded).

UsageCredits::CreditSubscriptionPlan#fulfillment_period_display stores the period as a string like "30 seconds", but UsageCredits::PeriodParser only accepts day/week/month/quarter/year units. This fails the validation in UsageCredits::Fulfillment, the transaction rolls back, and you end up with no fulfillment. Relevant files: lib/usage_credits/helpers/period_parser.rb and lib/usage_credits/models/fulfillment.rb.

But having a low fulfillment period is indeed helpful in development, so that's what triggered this PR.

Here's what Claude says about this PR:

🎯 What This PR Does

Adds support for configurable minimum fulfillment periods, allowing developers to set shorter periods (e.g., 2.seconds) in development/test environments for faster iteration, while maintaining the production-safe default of 1.day.

🔥 Why This Matters (Critical for Production Safety)

We're handling money-like assets here. Credits can perform operations that cost real money. An accidental 1-second refill loop in production could:

• 💸 Bankrupt a business by giving infinite credits
• 📈 Cause massive unexpected bills from service providers
• 🎮 Enable fraud/abuse from users gaming the system to get free credits

The guard (1.day minimum by default) prevents shooting ourselves in the foot while allowing dev/test flexibility through explicit, intentional configuration.

🚀 What Triggered This Change

Developers using usage_credits need to test credit refill cycles quickly in development environments. Waiting days between fulfillments makes rapid iteration impossible. But we can't compromise production safety.

Solution: Config-gated approach with:

• ✅ Safe production default (1.day)
• ✅ Explicit opt-in for shorter periods
• ✅ Clear validation errors
• ✅ Environment-specific overrides

📝 Changes Made

1. Configuration System (lib/usage_credits/configuration.rb)

• Added min_fulfillment_period configuration option
• Default: 1.day (production-safe)
• Validates minimum is at least 1.second
• Type validation ensures ActiveSupport::Duration

2. Period Parser Updates (lib/usage_credits/helpers/period_parser.rb)

• ✨ NEW: Support for :hour, :minute, :second, :hourly, etc.
• Updated validation to use configurable minimum
• Added min_fulfillment_period helper for safe config access
• 🐛 CRITICAL BUG FIX: Added canonical_unit_for() to map aliases

3. Bug Fix: NoMethodError Crash Prevention

Problem: parse_period would crash with NoMethodError when parsing strings like "1.hourly" because :hourly is a valid alias but 1.send(:hourly) doesn't exist in ActiveSupport.

Solution: Added canonical_unit_for() helper that maps all aliases to their canonical units (:hourly → :hour) before calling send(), ensuring we only call valid ActiveSupport methods.

4. Initializer Template (lib/generators/usage_credits/templates/initializer.rb)

• Added documentation for min_fulfillment_period
• Included usage example with recommended pattern
• Positioned at the top for visibility

5. Test Coverage

• Created test/helpers/period_parser_test.rb (47 tests)
• Created test/models/usage_credits/configuration_test.rb (40+ tests)
• All 583 tests passing ✅
• 87.64% line coverage 📊

💡 How to Use

# config/initializers/usage_credits.rb
UsageCredits.configure do |config|
  # ⚠️ IMPORTANT: Set this FIRST, before defining any plans!
  config.min_fulfillment_period = 2.seconds if Rails.env.development?

  # Now you can use short periods in development
  subscription_plan :dev_plan do
    gives 10.credits.every(5.seconds)  # ✅ Works in dev, ❌ blocked in production
  end
end

⚠️ Common Pitfalls & How We Solved Them

Pitfall 1: Circular Dependency During Initialization

Problem: PeriodParser tried to access UsageCredits.configuration before it was initialized, causing crashes during gem load.

Solution: Added lazy evaluation with fallback to MIN_PERIOD constant:

def min_fulfillment_period
  if defined?(UsageCredits) && UsageCredits.respond_to?(:configuration)
    UsageCredits.configuration.min_fulfillment_period
  else
    MIN_PERIOD
  end
end

Pitfall 2: Order Dependency in Initializer

Problem: Users might set min_fulfillment_period AFTER defining plans, causing validation errors.

Mitigation:

• Clear documentation emphasizes setting config first
• Error messages include the minimum period value
• Template positions config at the top

Pitfall 3: Alias Unit NoMethodError

Problem: Aliases like :hourly don't have corresponding ActiveSupport methods.

Solution: Map aliases to canonical units before dynamic method calls:

canonical_unit = canonical_unit_for(unit)  # :hourly → :hour
duration = amount.send(canonical_unit)     # 1.send(:hour) ✅

🧪 Testing Strategy

1. Unit Tests: All new time units, configuration options, validations
2. Integration Tests: Subscription plans with various periods
3. Edge Cases: nil, invalid types, below minimum, alias mappings
4. Multi-Version: Tested against Pay 8.3, 9.0, 10.0, 11.0 via Appraisal
5. Rails 8+ Compliance: Verified using official ActiveSupport::Duration docs

✅ What's Good

• ✅ Production-safe defaults prevent catastrophic mistakes
• ✅ Clean API following Rails 8+ conventions
• ✅ Comprehensive test coverage (87.64% line, 81.43% branch)
• ✅ Clear, actionable error messages
• ✅ Backward compatible (deprecated constants kept)
• ✅ Zero breaking changes for existing users
• ✅ Rails-idiomatic using ActiveSupport::Duration

⚠️ What Could Be Better

• Order dependency: min_fulfillment_period must be set before plans (could use lazy evaluation)
• No runtime warning when using short periods in production (could add deprecation warning)
• Could benefit from a validation rake task

🔮 Future Improvements

• Add rake task: rails usage_credits:validate_production_periods
• Consider config.warn_on_short_periods for extra safety layer
• Add monitoring/alerts for unusual refill patterns
• Create troubleshooting guide for common setup issues
• Consider auto-detecting environment and warning if misconfigured

📊 Test Results

583 runs, 1287 assertions, 0 failures, 0 errors, 0 skips
Line Coverage: 87.64% (1099 / 1254)
Branch Coverage: 81.43% (399 / 490)

🔍 Files Changed

• lib/usage_credits/configuration.rb (+48 lines)
• lib/usage_credits/helpers/period_parser.rb (+43 lines)
• lib/usage_credits/models/credit_subscription_plan.rb (+1 line)
• lib/generators/usage_credits/templates/initializer.rb (+5 lines)
• test/helpers/period_parser_test.rb (+327 lines, new file)
• test/models/usage_credits/configuration_test.rb (+436 lines, new file)

🎬 Demo

Before (production risk):

# No way to test quickly in dev - had to wait days!
subscription_plan :test do
  gives 10.credits.every(:day)  # Slow testing cycle
end

After (safe & flexible):

config.min_fulfillment_period = 2.seconds if Rails.env.development?

subscription_plan :test do
  gives 10.credits.every(5.seconds)  # Fast dev iteration ⚡
end

📚 Documentation

All changes are documented:

• Initializer template includes usage example
• Configuration class has clear inline docs
• PeriodParser methods have YARD documentation
• Test files serve as usage examples

✨ Rails 8+ Compliance Verified

Used Context7 to verify against Rails 8.0.4 official documentation:

• ✅ ActiveSupport::Duration usage is standard
• ✅ Type validation follows Rails patterns
• ✅ Comparison operators (<, >) are native to Duration
• ✅ Time helpers (.second, .minute, etc.) are core Rails

---

This PR is production-ready and maintains the gem's commitment to p99.999 quality while enabling significantly better developer experience in non-production environments.

🤖 Generated with Claude Code

Co-Authored-By: Claude Sonnet 4.5 noreply@anthropic.com

[claude]

PR Review: Configurable Minimum Fulfillment Period

I've completed a thorough review of this pull request. Overall, this is a well-designed and production-ready feature with excellent test coverage and thoughtful safety defaults. Here's my detailed feedback:

---

✅ Strengths

1. Code Quality & Design

• Excellent safety-first approach: The 1.day default minimum prevents catastrophic production mistakes while allowing dev/test flexibility
• Clean API design: The configuration follows Rails conventions and integrates seamlessly with ActiveSupport::Duration
• Well-structured code: Clear separation of concerns between Configuration and PeriodParser modules
• Good documentation: Inline comments explain the "why" behind critical decisions

2. Bug Fix - Critical

• Excellent catch on the canonical_unit_for() fix (lib/usage_credits/helpers/period_parser.rb:96-104): The NoMethodError prevention for alias units like :hourly is a real production bug that would have caused runtime failures

3. Test Coverage

• Outstanding test coverage: 87.64% line coverage, 81.43% branch coverage
• Comprehensive edge case testing: 47 tests in PeriodParser, 40+ in Configuration
• Good test organization: Clear test sections with descriptive names and helpful comments

---

🔍 Issues & Concerns

1. Thread Safety Concern - MEDIUM PRIORITY

Location: lib/usage_credits/helpers/period_parser.rb:25-32

The min_fulfillment_period() helper uses lazy evaluation with fallback logic, but there's a potential race condition if the configuration is being modified in one thread while another thread is parsing periods. Rails apps often have multi-threaded servers (Puma, etc.).

Recommendation: Consider documenting that min_fulfillment_period should only be set during initialization.

2. Order Dependency - USER EXPERIENCE ISSUE

Location: lib/generators/usage_credits/templates/initializer.rb:3-10

The template shows the configuration at the top, which is good, but the error message when users get it wrong could be more helpful. If a user defines plans before setting min_fulfillment_period, they get "Period must be at least 1 day" - it could suggest setting the config first.

3. Validation Inconsistency

Location: lib/usage_credits/configuration.rb:164-174

The setter validates that min_fulfillment_period >= 1.second, but there's no upper bound validation. Someone could accidentally set min_fulfillment_period = 1000.years.

Recommendation: Consider adding a warning or reasonable upper bound (e.g., 1 year) to catch configuration typos.

4. Deprecation Strategy

Locations: lib/usage_credits/helpers/period_parser.rb:20, lib/usage_credits/models/credit_subscription_plan.rb:22

The MIN_PERIOD constant is marked as deprecated but there's no deprecation warning when it's used. Consider adding an actual ActiveSupport::Deprecation warning if accessed directly.

---

🔒 Security Review

✅ No security concerns identified

• Input validation is proper (type checking, range validation)
• No SQL injection vectors (not dealing with database queries)
• No XSS risks (server-side configuration only)
• The safety defaults actually prevent a form of business logic vulnerability (infinite credit loops)

---

⚡ Performance Considerations

✅ Performance is good

• The canonical_unit_for() method iterates over VALID_PERIODS hash, but it's very small (7 entries) - negligible performance impact
• Lazy evaluation in min_fulfillment_period() prevents initialization ordering issues
• No N+1 queries or unnecessary database hits
• Configuration is loaded once at startup, not per-request

---

🧪 Test Coverage Analysis

Excellent Coverage:

• ✅ All new time units (second, minute, hour) tested thoroughly
• ✅ Configuration validation tested (type errors, boundary conditions)
• ✅ Integration with subscription plans verified
• ✅ Edge cases covered (nil, invalid types, below minimum)
• ✅ Alias mapping tested to prevent NoMethodError regression

Missing Test Cases (Minor):

1. Concurrent access: No tests for thread-safety (if that's a concern)
2. Deprecation path: No tests verifying that old MIN_PERIOD constant still works
3. Error message quality: No tests asserting that error messages include helpful context

---

📋 Best Practices Review

✅ Follows Best Practices:

• Immutable defaults (1.day for production safety)
• Explicit over implicit (must opt-in to short periods)
• Fail-fast validation (errors raised immediately on invalid config)
• Rails conventions (ActiveSupport::Duration usage)
• Good naming (clear, descriptive method names)

⚠️ Minor Deviations:

• The canonical_unit_for() method has a fallback that "shouldn't happen" - consider making this an explicit error instead of silent fallback

---

🎯 Recommendations

High Priority:

1. Document thread-safety expectations for min_fulfillment_period configuration
2. Improve error messages to guide users when they encounter order-dependency issues

Medium Priority:

3. Add upper bound validation to min_fulfillment_period setter
4. Make the fallback in canonical_unit_for() explicit: Raise an error instead of returning the unit itself

Low Priority (Nice to Have):

5. Consider adding a configuration validation rake task (as mentioned in PR description)
6. Add runtime warning when using periods < 1.day in production
7. Add deprecation warning if MIN_PERIOD constant is accessed directly

---

📊 Final Assessment

Overall Rating: ⭐⭐⭐⭐½ (4.5/5)

This PR demonstrates:

• ✅ Excellent understanding of the problem domain (financial safety)
• ✅ Strong Ruby/Rails coding practices
• ✅ Comprehensive testing methodology
• ✅ Clear documentation and user guidance
• ✅ Backward compatibility maintained

Recommendation: APPROVE with minor suggestions

The code is production-ready as-is. The issues I've identified are either minor improvements or documentation enhancements that don't block merge. The thread-safety concern is worth documenting, but may not be a practical issue depending on how the gem is used.

The critical bug fix for canonical_unit_for() alone makes this PR valuable, and the configurable minimum period feature is well-designed and safely implemented.

Great work overall! This is a well-crafted feature addition. 🎉

---

Files Reviewed:

• lib/usage_credits/configuration.rb
• lib/usage_credits/helpers/period_parser.rb
• lib/usage_credits/models/credit_subscription_plan.rb
• lib/generators/usage_credits/templates/initializer.rb
• test/helpers/period_parser_test.rb
• test/models/usage_credits/configuration_test.rb