Skip to content

Conversation

@willymwai
Copy link
Member

@willymwai willymwai commented Jun 20, 2025

User description

This pull request introduces several changes to enhance the functionality and testing of the FlutterwaveProvider payment provider. Key updates include improving secrets management, refining test cases, and adding comprehensive model validations.

Enhancements to secrets management:

Improvements to test coverage:

Refinements in job queue testing:


PR Type

Enhancement


Description

• Moved webhook_secret from settings to secrets for better security
• Added comprehensive test coverage for FlutterwaveProvider model
• Fixed queue testing to use proper enqueue matchers
• Updated factory to reflect new secrets structure


Changes walkthrough 📝

Relevant files
Enhancement
flutterwave_provider.rb
Move webhook_secret to secrets accessors                                 

app/models/payment_providers/flutterwave_provider.rb

• Moved webhook_secret from settings_accessors to secrets_accessors

Removed extra blank line for cleaner formatting

+1/-3     
Tests
payment_providers.rb
Update factory for new secrets structure                                 

spec/factories/payment_providers.rb

• Added webhook_secret to secrets JSON structure
• Removed
webhook_secret from settings hash
• Added transient webhook_secret
attribute

+2/-3     
handle_event_job_spec.rb
Fix queue testing with proper matchers                                     

spec/jobs/payment_providers/flutterwave/handle_event_job_spec.rb

• Replaced queue_name assertions with have_enqueued_job.on_queue
matchers
• Added proper job enqueuing expectations for both queue
scenarios

+6/-2     
flutterwave_provider_spec.rb
Add comprehensive FlutterwaveProvider model tests               

spec/models/payment_providers/flutterwave_provider_spec.rb

• Added comprehensive test suite for FlutterwaveProvider model
• Tests
validations, constants, webhook secret generation, and data structures

• Covers secrets accessors and payment type functionality

+109/-0 
handle_incoming_webhook_service_spec.rb
Update webhook service test for secrets                                   

spec/services/payment_providers/flutterwave/handle_incoming_webhook_service_spec.rb

• Updated test to remove webhook_secret from secrets JSON instead of
settings
• Fixed webhook secret handling test for new secrets
structure

+3/-1     

Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • @qodo-code-review
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Missing Transient

    The factory adds webhook_secret to the secrets JSON but doesn't define it as a transient attribute, which could cause factory build failures or inconsistent test data generation.

      {secret_key:, webhook_secret:}.to_json
    end
    Test Coverage Gap

    The webhook secret generation test creates a provider with an existing secret but doesn't verify that the before_create callback is properly skipped when a secret already exists, potentially missing edge cases in the generation logic.

    it "does not override existing webhook secret" do
      existing_secret = "existing_secret"
      provider = described_class.new(
        organization: create(:organization),
        name: "Test Provider",
        code: "test_provider",
        secret_key: "test_key",
        webhook_secret: existing_secret
      )
      provider.save!
      expect(provider.reload.webhook_secret).to eq(existing_secret)
    end

    @qodo-code-review
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Fix duplicate code validation error

    The test creates a provider with a duplicate code which will fail validation due
    to uniqueness constraints. Use a unique code or the factory to avoid validation
    errors.

    spec/models/payment_providers/flutterwave_provider_spec.rb [69-80]

     it "does not override existing webhook secret" do
       existing_secret = "existing_secret"
       provider = described_class.new(
         organization: create(:organization),
         name: "Test Provider",
    -    code: "test_provider",
    +    code: "test_provider_#{SecureRandom.uuid}",
         secret_key: "test_key",
         webhook_secret: existing_secret
       )
       provider.save!
       expect(provider.reload.webhook_secret).to eq(existing_secret)
     end
    • Apply / Chat
    Suggestion importance[1-10]: 6

    __

    Why: The suggestion correctly identifies that using a hardcoded code is a bad practice when a uniqueness validation exists. While the test creates a new organization, making the test fragile, using a dynamically generated unique code as suggested makes the test more robust and prevents potential future failures.

    Low
    • More

    @willymwai willymwai merged commit 65651d8 into main Jun 20, 2025
    10 checks passed
    @willymwai willymwai deleted the feat/use-secret-field-for-hash branch June 20, 2025 09:10
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    2 participants