Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: Add Cashfree Payment Provider #2545

Closed

Conversation

AyushChothe
Copy link

Roadmap Task

👉 https://getlago.canny.io/feature-requests/p/payment-provider-add-integration-for-cashfree-payments

Context

Cashfree Payments is India's leading payment gateway, and with Stripe halting the onboarding of new merchants in India due to regulatory constraints, businesses are left with limited alternatives. Ensuring that Cashfree Payments is available to users is essential, as it will expand Lago's accessibility and appeal to Indian companies, broadening its reach within this crucial market.

We at Pyrite Cloud have begun developing the integration with Cashfree Payments. This will help expand our platform's accessibility, especially for Indian businesses, by providing them with a reliable payment gateway as an alternative to Stripe.

Description

Added Cashfree Payment Provider concerning existing Payment Providers. Still Work in Progress.

@AyushChothe AyushChothe changed the title [FEAT]: Add Cashfree Payment Provider feature: Add Cashfree Payment Provider Sep 6, 2024
@AyushChothe AyushChothe marked this pull request as draft September 6, 2024 18:23
@AyushChothe AyushChothe marked this pull request as ready for review September 7, 2024 13:31
@AyushChothe AyushChothe marked this pull request as draft September 7, 2024 13:32
@vincent-pochet
Copy link
Collaborator

Thank you very much @AyushChothe for this great contribution.
We will review it carefully in the coming days and we will come back to you

@AyushChothe
Copy link
Author

Hi @vincent-pochet,

Thank you! Please keep me updated on the review process and let me know if any changes are needed.

Also, could you guide me on how to run Rspec locally? I want to get started with writing tests.

@vincent-pochet vincent-pochet added the Contribution Contributions from the Lago Community label Sep 12, 2024
@vincent-pochet
Copy link
Collaborator

@AyushChothe

To run rspec locally:

lago exec api rspec
# or docker compose -f ./docker-compose.dev.yml exec api rspec 
  • In you are running it directly on your local system:
cd api
bundle exec rspec

@AyushChothe AyushChothe force-pushed the feature/cashfree-integration branch 3 times, most recently from f732cbd to e9dbde3 Compare September 26, 2024 17:01
@AyushChothe
Copy link
Author

Hey @vincent-pochet,

I've addressed all the review comments you mentioned earlier and added test cases for the impacted modules.

I wanted to get your thoughts on how we should handle these modules since we aren't creating PaymentProviderCustomers. Would creating a dummy PaymentProviderCustomers make sense, or is there a better approach?

Here's the list of modules in question:

PaymentProviderCustomers::Factory
PaymentProviderCustomers::CreateService
Customers::GenerateCheckoutUrlService

I am looking forward to hearing your thoughts!

@AyushChothe AyushChothe force-pushed the feature/cashfree-integration branch from e9dbde3 to 8c8ca5a Compare September 27, 2024 14:19
@vincent-pochet
Copy link
Collaborator

vincent-pochet commented Sep 27, 2024

Hey @vincent-pochet,

I've addressed all the review comments you mentioned earlier and added test cases for the impacted modules.

I wanted to get your thoughts on how we should handle these modules since we aren't creating PaymentProviderCustomers. Would creating a dummy PaymentProviderCustomers make sense, or is there a better approach?

Here's the list of modules in question:

PaymentProviderCustomers::Factory
PaymentProviderCustomers::CreateService
Customers::GenerateCheckoutUrlService

I am looking forward to hearing your thoughts!

Thank you for the update.
To be fully compatible with the other providers, I think yes we should handle a Customer as well even if it is not attached to any external resource

@AyushChothe
Copy link
Author

Hey @vincent-pochet,
I've addressed all the review comments you mentioned earlier and added test cases for the impacted modules.
I wanted to get your thoughts on how we should handle these modules since we aren't creating PaymentProviderCustomers. Would creating a dummy PaymentProviderCustomers make sense, or is there a better approach?
Here's the list of modules in question:

PaymentProviderCustomers::Factory
PaymentProviderCustomers::CreateService
Customers::GenerateCheckoutUrlService

I am looking forward to hearing your thoughts!

Thank you for the update. To be fully compatible with the other providers, I think yes we should handle a Customer as well event if it is not attached to any external resource

Thanks, that's what I was thinking as well. I'll start working on it now!

Copy link
Collaborator

@vincent-pochet vincent-pochet left a comment

Choose a reason for hiding this comment

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

It start to look great!
Here a few other minor comments

spec/fixtures/cashfree/event.json Show resolved Hide resolved
app/services/invoices/payments/cashfree_service.rb Outdated Show resolved Hide resolved
@AyushChothe AyushChothe force-pushed the feature/cashfree-integration branch from 8c8ca5a to 52e0534 Compare September 30, 2024 07:28
@AyushChothe
Copy link
Author

Hi @vincent-pochet, please review this PR again, including the respective frontend PR at getlago/lago-front#1720.

Copy link
Collaborator

@vincent-pochet vincent-pochet left a comment

Choose a reason for hiding this comment

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

Thank you for the update. Here are few small comments but I think we are close to something that could be merged 👍

app/services/invoices/payments/cashfree_service.rb Outdated Show resolved Hide resolved
@AyushChothe AyushChothe force-pushed the feature/cashfree-integration branch from 52e0534 to 36b322a Compare October 4, 2024 17:24
Copy link
Collaborator

@vincent-pochet vincent-pochet left a comment

Choose a reason for hiding this comment

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

Last comments regarding the Invoices::Payments::CashfreeService class :)

If you have some questions around this implementation, we can discuss about it.

app/services/invoices/payments/cashfree_service.rb Outdated Show resolved Hide resolved
app/services/invoices/payments/cashfree_service.rb Outdated Show resolved Hide resolved
@AyushChothe AyushChothe force-pushed the feature/cashfree-integration branch from 36b322a to 8c716d2 Compare October 20, 2024 07:27
@vincent-pochet
Copy link
Collaborator

It looks good to me @AyushChothe! Thank you again for this great contribution 🙏

Someone on the front team will review the pull request on the lago-front repository

In the meantime you might have to resync/rebase your branch on the main one of this repository.

And finally, to fix the failing Run rails migrations check, you can run the following command:

docker-compose -f $LAGO_PATH/docker-compose.dev.yml exec api rails annotate_models

@vincent-pochet
Copy link
Collaborator

Closing this PR in favor of #2767 to perform a QA and ease fixes on Lago's side

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Contribution Contributions from the Lago Community Payment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants