-
Notifications
You must be signed in to change notification settings - Fork 77
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
Add stripe checkout for invoices #325
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,37 @@ | ||||||||||
# frozen_string_literal: true | ||||||||||
|
||||||||||
class Invoices::PaymentsController < ApplicationController | ||||||||||
skip_before_action :authenticate_user! | ||||||||||
skip_after_action :verify_authorized | ||||||||||
before_action :load_invoice | ||||||||||
before_action :ensure_invoice_unpaid, only: [:new] | ||||||||||
|
||||||||||
def new | ||||||||||
session = @invoice.create_checkout_session!( | ||||||||||
success_url: success_invoice_payments_url(@invoice), | ||||||||||
cancel_url: cancel_invoice_payments_url(@invoice) | ||||||||||
) | ||||||||||
|
||||||||||
redirect_to session.url, allow_other_host: true | ||||||||||
end | ||||||||||
|
||||||||||
def success | ||||||||||
@invoice.paid! | ||||||||||
end | ||||||||||
|
||||||||||
def cancel | ||||||||||
render | ||||||||||
end | ||||||||||
|
||||||||||
private | ||||||||||
|
||||||||||
def load_invoice | ||||||||||
@invoice = Invoice.includes(client: :company).find(params[:invoice_id]) | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can remove the
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We follow this convention in Miru for memorized variables. Global search for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Btw how will the view templates access this instance variable since we are not loading this in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pass as locals There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just one last confusion, Why are we using instance variables if we want to pass them as locals? They should anyway be available to the views, right? It feels like we are deliberately not taking advantage of the magic that Rails provides us. Is there any specific reason we decided to move away from our own set standards in Miru? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think because, in this action, we are using the variable only to pass as locals, but for a complex action we might use the |
||||||||||
end | ||||||||||
|
||||||||||
def ensure_invoice_unpaid | ||||||||||
if @invoice.paid? | ||||||||||
redirect_to success_invoice_payments_url(@invoice.id) | ||||||||||
end | ||||||||||
end | ||||||||||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
# frozen_string_literal: true | ||
|
||
class ApplicationService | ||
def self.process(*args, &block) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer to name the method There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes initially the plan was to add interactor classes rather than service classes, if the PR is urgent we can go with service classes too. |
||
new(*args, &block).process | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
# frozen_string_literal: true | ||
|
||
module InvoicePayment | ||
class Checkout < ApplicationService | ||
def initialize(params) | ||
@invoice = params[:invoice] | ||
@company = invoice.client.company | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add an association, or delegation so we could access like |
||
@client = invoice.client | ||
@success_url = params[:success_url] | ||
@cancel_url = params[:cancel_url] | ||
end | ||
|
||
def process | ||
Invoice.transaction do | ||
ensure_client_registered! | ||
checkout! | ||
Comment on lines
+15
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just curious why are we using bang methods everywhere? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we are not rescuing the exceptions, I added |
||
end | ||
end | ||
|
||
private | ||
|
||
attr_reader :invoice, :company, :client, :success_url, :cancel_url | ||
|
||
def ensure_client_registered! | ||
return if client.stripe_id? | ||
|
||
client.register_on_stripe! | ||
end | ||
|
||
def description | ||
"Invoice from #{company.name} for #{currency} #{invoice.amount} due on #{invoice.due_date}" | ||
end | ||
Comment on lines
+30
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's consider adding an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Presenters are needed in many places, maybe we can start with this PR. |
||
|
||
def currency | ||
company.base_currency | ||
end | ||
|
||
def checkout! | ||
Stripe::Checkout::Session.create( | ||
{ | ||
line_items: [{ | ||
price_data: { | ||
currency: company.base_currency.downcase, | ||
product_data: { | ||
name: invoice.invoice_number, | ||
description: | ||
}, | ||
unit_amount: invoice.amount.to_i | ||
}, | ||
quantity: 1 | ||
}], | ||
mode: "payment", | ||
customer: client.reload.stripe_id, | ||
success_url:, | ||
cancel_url: | ||
}) | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,5 +4,7 @@ Hi, <%= @invoice.client_name %> | |
You have an invoice - <%= @invoice.invoice_number %> with amount <%= @invoice.amount %> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Amount would be in cents, convert it to $ |
||
due date is: <%= @invoice.due_date %>. | ||
|
||
You can pay for the invoice here: <%= new_invoice_payment_url(@invoice) %> | ||
|
||
Thanks! | ||
Miru |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
<div class="flex flex-col min-h-full pt-16 pb-12 my-auto"> | ||
<main class="flex flex-col self-center justify-center flex-grow w-full px-4 mx-auto max-w-7xl sm:px-6 lg:px-8"> | ||
<div class="flex justify-center flex-shrink-0"> | ||
<svg class="w-auto h-16 text-red-400 bg-red-200 rounded-full shadow-sm" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 20 20" fill="currentColor" aria-hidden="true"> | ||
<path fill-rule="evenodd" d="M10 18a8 8 0 100-16 8 8 0 000 16zM8.707 7.293a1 1 0 00-1.414 1.414L8.586 10l-1.293 1.293a1 1 0 101.414 1.414L10 11.414l1.293 1.293a1 1 0 001.414-1.414L11.414 10l1.293-1.293a1 1 0 00-1.414-1.414L10 8.586 8.707 7.293z" clip-rule="evenodd" /> | ||
</svg> | ||
</div> | ||
|
||
<div class="py-16"> | ||
<div class="text-center"> | ||
<p class="text-sm font-semibold tracking-wide uppercase text-miru-han-purple-600">Invoice #<%= @invoice.invoice_number %></p> | ||
<h1 class="mt-2 text-4xl font-extrabold tracking-tight text-gray-900 sm:text-5xl">Payment was cancelled.</h1> | ||
<p class="mt-2 text-base text-gray-500">Didn't cancel the payment?</p> | ||
<div class="mt-6 group"> | ||
<%= link_to "Try again", new_invoice_payment_url(@invoice), class: "text-base font-medium text-miru-han-purple-600 group-hover:text-miru-han-purple-400", target: "_blank", rel: "nofollow" %> | ||
<span aria-hidden="true" class="text-base font-medium text-miru-han-purple-600 group-hover:text-miru-han-purple-400"> →</span> | ||
</div> | ||
</div> | ||
</div> | ||
</main> | ||
</div> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
<div class="flex flex-col min-h-full pt-16 pb-12 my-auto"> | ||
<main class="flex flex-col self-center justify-center flex-grow w-full px-4 mx-auto max-w-7xl sm:px-6 lg:px-8"> | ||
<div class="flex justify-center flex-shrink-0"> | ||
<svg class="w-auto h-16 text-green-400 bg-green-200 rounded-full shadow-sm" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 20 20" fill="currentColor" aria-hidden="true"> | ||
<path fill-rule="evenodd" d="M10 18a8 8 0 100-16 8 8 0 000 16zm3.707-9.293a1 1 0 00-1.414-1.414L9 10.586 7.707 9.293a1 1 0 00-1.414 1.414l2 2a1 1 0 001.414 0l4-4z" clip-rule="evenodd" /> | ||
</svg> | ||
</div> | ||
|
||
<div class="py-16"> | ||
<div class="text-center"> | ||
<p class="text-sm font-semibold tracking-wide text-indigo-600 uppercase">Invoice #<%= @invoice.invoice_number %></p> | ||
<h1 class="mt-2 text-4xl font-extrabold tracking-tight text-gray-900 sm:text-5xl">Payment was successful. 🎉</h1> | ||
<p class="mt-2 text-base text-gray-500">We have received your payment.</p> | ||
</div> | ||
</div> | ||
</main> | ||
</div> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
# frozen_string_literal: true | ||
|
||
Stripe.api_key = ENV["STRIPE_SECRET_KEY"] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
# frozen_string_literal: true | ||
|
||
class AddStripeIdToClients < ActiveRecord::Migration[7.0] | ||
def change | ||
add_column :clients, :stripe_id, :string, default: nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @supriya3105 Do we have only one stripe account per client? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is to sync customers and payments on stripe. |
||
end | ||
end |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
# frozen_string_literal: true | ||
|
||
require "rails_helper" | ||
|
||
RSpec.describe "Invoices::Payments", type: :request do | ||
describe "GET /index" do | ||
pending "add some examples (or delete) #{__FILE__}" | ||
end | ||
end | ||
Comment on lines
+3
to
+9
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Specs are pending, checkout https://github.com/stripe-ruby-mock/stripe-ruby-mock There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! This is awesome🚀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We prefer REST based controllers.