Skip to content

Conversation

@willymwai
Copy link
Member

@willymwai willymwai commented Jun 20, 2025

User description

This pull request introduces significant updates to the Flutterwave integration, including refactoring webhook handling, improving error logging, and enhancing payment processing logic. Additionally, it modifies configurations, updates factories, and adds comprehensive test coverage for Flutterwave-related functionality.

Flutterwave Integration Enhancements:

  1. Refactored Webhook Handling:

    • Replaced signature with secret in webhook handling logic to improve clarity and consistency. (app/controllers/webhooks_controller.rb - [1] app/services/payment_providers/flutterwave/handle_incoming_webhook_service.rb - [2]
    • Improved error messages for missing or invalid webhook secrets. (app/services/payment_providers/flutterwave/handle_incoming_webhook_service.rb - app/services/payment_providers/flutterwave/handle_incoming_webhook_service.rbL7-R42)
  2. Improved Payment Processing Logic:

  3. Error Logging and Resilience:

Configuration and Factory Updates:

  1. Database Configuration Changes:

    • Replaced environment-based database credentials with hardcoded values for development purposes. (config/database.yml - config/database.ymlL7-R25)
  2. Factory Enhancements:

Test Coverage Additions:

  1. Unit Tests for Flutterwave Event Handling:

  2. Integration Tests for Webhook Controller:


PR Type

Enhancement, Tests


Description

• Add comprehensive Flutterwave payment provider integration
• Refactor webhook handling to use 'secret' instead of 'signature'
• Enhance payment processing with payable type validation
• Add extensive test coverage for all Flutterwave services


Changes walkthrough 📝

Relevant files
Enhancement
3 files
webhooks_controller.rb
Update webhook parameter from signature to secret               
+1/-1     
handle_incoming_webhook_service.rb
Rename signature to secret and improve error messages       
+11/-9   
charge_completed_service.rb
Add payable type validation and find_payable method           
+18/-2   
Formatting
2 files
base_service.rb
Refactor throttling error handling logic                                 
+2/-2     
flutterwave_service.rb
Remove unused private method and clean formatting               
+0/-7     
Bug fix
2 files
flutterwave_service.rb
Fix amount calculation using Money gem                                     
+11/-10 
flutterwave_service.rb
Fix amount calculation and error code handling                     
+2/-3     
Error handling
1 files
handle_event_service.rb
Add error handling for event processing                                   
+5/-1     
Tests
13 files
payment_provider_customers.rb
Add flutterwave_customer factory definition                           
+7/-0     
handle_event_job_spec.rb
Add comprehensive job testing with queue configuration     
+42/-0   
webhooks_controller_spec.rb
Add Flutterwave webhook endpoint testing                                 
+112/-0 
flutterwave_service_spec.rb
Add complete invoice payment service testing                         
+319/-0 
factory_spec.rb
Add Flutterwave provider to factory tests                               
+8/-0     
flutterwave_service_spec.rb
Add customer service testing for Flutterwave                         
+57/-0   
create_customer_factory_spec.rb
Add Flutterwave provider to customer factory tests             
+9/-0     
handle_event_service_spec.rb
Add event handling service testing with error scenarios   
+36/-1   
handle_incoming_webhook_service_spec.rb
Update webhook service tests for secret parameter               
+12/-7   
charge_completed_service_spec.rb
Add comprehensive webhook charge completion testing           
+527/-0 
flutterwave_service_spec.rb
Add provider service testing with CRUD operations               
+157/-0 
flutterwave_service_spec.rb
Add payment request service testing                                           
+279/-0 
factory_spec.rb
Add Flutterwave to payment request factory tests                 
+8/-0     
Dependencies
1 files
Gemfile
Remove httparty dependency                                                             
+0/-1     
Configuration changes
1 files
database.yml
Update database configuration for development                       
+16/-8   

Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • - Implemented Flutterwave service in payment providers factory.
    - Added specs for Flutterwave service in payment provider customers.
    - Enhanced create customer factory spec to include Flutterwave provider.
    - Updated handle event service spec to handle Flutterwave events.
    - Created charge completed service spec for Flutterwave webhooks.
    - Developed Flutterwave service specs for payment processing.
    - Added payment requests service spec for Flutterwave integration.
    - Updated payment providers factory spec to include Flutterwave.
    @qodo-code-review
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 PR contains tests
    🔒 Security concerns

    Hardcoded credentials exposure:
    The database.yml file contains hardcoded database credentials (username: lago, password: changeme) in the development section. While this is for development, it's a security anti-pattern that could lead to credential exposure if this configuration is accidentally used in other environments.

    ⚡ Recommended focus areas for review

    Currency Conversion

    The amount calculation changed from simple division by 100.0 to using Money.from_cents().to_f which may behave differently for certain currencies or edge cases. Need to verify this handles all supported currencies correctly.

    amount: Money.from_cents(invoice.total_amount_cents, invoice.currency).to_f,
    tx_ref: invoice.id,
    Missing Validation

    The find_payable method uses provider_payment_id to find Invoice/PaymentRequest but there's no validation that the found payable belongs to the correct organization, potentially allowing cross-organization access.

    def find_payable
      case payable_type
      when "Invoice"
        Invoice.find_by(id: provider_payment_id)
      when "PaymentRequest"
        PaymentRequest.find_by(id: provider_payment_id)
      end
    end
    Hardcoded Credentials

    Database configuration contains hardcoded credentials and connection details in development section, which should use environment variables for consistency and security.

      host: db
      username: lago
      password: changeme
      database: lago
      port: 5432
    events:
      <<: *default
      host: db
      username: lago
      password: changeme
      database: lago
      port: 5432
    clickhouse:
      adapter: clickhouse
      database: default
      host: clickhouse
      port: 8123
      username: default
      password: default
      migrations_paths: db/clickhouse_migrate

    @qodo-code-review
    Copy link

    qodo-code-review bot commented Jun 20, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Convert transaction reference to string

    The tx_ref should be a string to ensure consistent data type handling across the
    payment flow. Using integer IDs directly may cause issues with payment tracking
    and reference matching.

    app/services/invoices/payments/flutterwave_service.rb [63-75]

     def create_checkout_session
       body = {
         amount: Money.from_cents(invoice.total_amount_cents, invoice.currency).to_f,
    -    tx_ref: invoice.id,
    +    tx_ref: invoice.id.to_s,
         currency: invoice.currency.upcase,
         redirect_url: success_redirect_url,
         customer: customer_params,
         customizations: customizations_params,
         configuration: configuration_params,
         meta: meta_params
       }
       http_client.post_with_response(body, headers)
     end
    • Apply / Chat
    Suggestion importance[1-10]: 6

    __

    Why: The suggestion to convert invoice.id to a string for tx_ref is correct. The Flutterwave API expects a string, and another part of the codebase (payment_requests/payments/flutterwave_service.rb) already uses a string for this field. This change ensures consistency and adherence to the API contract.

    Low
    • Update

    @willymwai willymwai merged commit 908a496 into main Jun 20, 2025
    10 checks passed
    @willymwai willymwai deleted the specs/flutterwave-payment-provider branch June 20, 2025 08:31
    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