Skip to content

Commit

Permalink
Inline payout details form into /account
Browse files Browse the repository at this point in the history
I've decided to set a local variable `payout_detail` in the
`users/show` template to either use the persisted payout detail or
create a new one. I considered setting this as an instance variable in
the controller, but the issue is that any controller that renders
`user/show` (PasswordsController, EmailsController,
PayoutDetailsController) would have to also set this instance
variable.

I've also updated the link in the notify_artist email to point to this
new page.
  • Loading branch information
chrislo committed Oct 29, 2024
1 parent 4d5473a commit d93af09
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 62 deletions.
24 changes: 7 additions & 17 deletions app/controllers/payout_details_controller.rb
Original file line number Diff line number Diff line change
@@ -1,46 +1,36 @@
# frozen_string_literal: true

class PayoutDetailsController < ApplicationController
def new
@payout_detail = PayoutDetail.new(user:)
authorize @payout_detail
end

def edit
@payout_detail = user.payout_detail
raise ActiveRecord::RecordNotFound unless @payout_detail

authorize @payout_detail
end
before_action :set_user

def create
@payout_detail = PayoutDetail.new(payout_detail_params.merge(user:))
@payout_detail = PayoutDetail.new(payout_detail_params.merge(user: @user))
authorize @payout_detail

if @payout_detail.save
redirect_to account_url, notice: 'Payout details added'
else
render :new, status: :unprocessable_entity
render 'users/show', status: :unprocessable_entity
end
end

def update
@payout_detail = user.payout_detail
@payout_detail = @user.payout_detail
raise ActiveRecord::RecordNotFound unless @payout_detail

authorize @payout_detail

if @payout_detail.update(payout_detail_params)
redirect_to account_url, notice: 'Payout details updated'
else
render :edit, status: :unprocessable_entity
render 'users/show', status: :unprocessable_entity
end
end

private

def user
Current.user
def set_user
@user = Current.user
end

def payout_detail_params
Expand Down
4 changes: 2 additions & 2 deletions app/views/purchase_mailer/notify_artist.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
for <%= number_to_currency(@purchase.price, unit: "£") %>.</p>

<% if @user.payout_detail %>
<p>You have already provided your payout details. If you need to, you can <%= link_to 'change them', edit_payout_detail_url %>.</p>
<p>You have already provided your payout details. If you need to, you can <%= link_to 'change them', account_url %>.</p>
<% else %>
<p><b>Important:</b> we need to know <%= link_to 'some details', new_payout_detail_url %> before we can make a
<p><b>Important:</b> we need to know <%= link_to 'some details', account_url %> before we can make a
payment to you.</p>
<% end %>

Expand Down
28 changes: 19 additions & 9 deletions app/views/users/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,25 @@
<h2 class="text-lg font-bold">Payout details</h2>
</div>
<div class="basis-3/4">
<p>form goes here</p>
<p class="text-slate-500 mb-6">We use <a class="underline" href="https://wise.com">Wise</a> to make payouts once a month. Wise will email you at <span class="font-bold"><%= Current.user.email %></span> to ask for your bank details. To make the payment we need the following:</p>

<% payout_detail = @user.payout_detail || PayoutDetail.new(user: @user) %>
<%= form_with(model: payout_detail, class: "contents", builder: TailwindFormBuilder) do |form| %>
<%= render ModelErrorComponent.new(model: payout_detail) %>
<%= form.text_field :name, placeholder: 'Bartholomew J. Simpson', class: 'w-full' %>
<p class="text-slate-500 italic text-sm mb-3">Your full name as used on your bank account.</p>

<%= form.select :country, options_for_payout_details_country_select, include_blank: true, label: { text: 'Country / Region' }, class: 'w-full' %>
<p class="text-slate-500 italic text-sm mb-3">We support payouts to the regions/countries <a href="https://wise.com/help/articles/2571942/what-countriesregions-can-i-send-to" class="underline">that Wise supports</a>.</p>

<div class="flex flex-row justify-end">
<% if payout_detail.persisted? %>
<%= form.submit "Save changes", class: 'mt-3'%>
<% else %>
<%= form.submit "Add payout details", class: 'mt-3'%>
<% end %>
</div>
<% end %>
</div>
</section>
</div>

<ul>
<% if Current.user.payout_detail %>
<li><%= text_link_to "Edit payout details", edit_payout_detail_path %></li>
<% else %>
<li><%= text_link_to "Add payout details", new_payout_detail_path %></li>
<% end %>
</ul>
2 changes: 1 addition & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

resources :sessions, only: %i[index show destroy]
resource :password, only: %i[update]
resource :payout_detail, only: %i[new create edit update]
resource :payout_detail, only: %i[create update]
resolve('PayoutDetail') { [:payout_detail] }

namespace :identity do
Expand Down
27 changes: 0 additions & 27 deletions test/controllers/payout_details_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,6 @@ def setup
log_in_as(@user)
end

test '#new' do
get new_payout_detail_url
assert_response :success
end

test '#create creates a new PayoutDetail and redirects to account_url' do
assert_difference('PayoutDetail.count') do
post payout_detail_url, params: { payout_detail: { name: 'Alice', country: 'country' } }
Expand Down Expand Up @@ -47,31 +42,9 @@ def setup

assert_response :not_found
end

test '#edit' do
create(:payout_detail, user: @user)

get edit_payout_detail_url
assert_response :success
end

test '#edit 404s if user has no payout detail' do
get edit_payout_detail_url
assert_response :not_found
end
end

class PayoutDetailsControllerTestSignedOut < ActionDispatch::IntegrationTest
test '#new' do
get new_payout_detail_url
assert_redirected_to log_in_url
end

test '#edit' do
get edit_payout_detail_url
assert_redirected_to log_in_url
end

test '#update' do
patch payout_detail_url
assert_redirected_to log_in_url
Expand Down
4 changes: 2 additions & 2 deletions test/mailers/purchase_mailer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class PurchaseMailerTest < ActionMailer::TestCase
purchase = build(:purchase, album:, price: 7.00)

mail = PurchaseMailer.with(purchase:).notify_artist
assert_includes mail.body.to_s, new_payout_detail_url
assert_includes mail.body.to_s, account_url
end

test 'notify_artist allows user to change/verify payout details' do
Expand All @@ -61,7 +61,7 @@ class PurchaseMailerTest < ActionMailer::TestCase
purchase = build(:purchase, album:, price: 7.00)

mail = PurchaseMailer.with(purchase:).notify_artist
assert_includes mail.body.to_s, edit_payout_detail_url
assert_includes mail.body.to_s, account_url
end

test 'do not notify artist of purchase if sending is suppressed' do
Expand Down
31 changes: 27 additions & 4 deletions test/system/payout_details_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,35 @@ class PayoutDetailsTest < ApplicationSystemTestCase

test 'adding payout details' do
visit account_url
click_on 'Add payout details'
fill_in 'Name', with: 'John Lennon'
select '🇬🇧 United Kingdom (GBP)', from: 'Country / Region'
click_on 'Save'

within(payout_details_section) do
fill_in 'Name', with: 'John Lennon'
select '🇬🇧 United Kingdom (GBP)', from: 'Country / Region'
click_on 'Add payout details'
end

assert_text 'Payout details added'
assert_equal 'John Lennon', @user.payout_detail.name
assert_equal 'united_kingdom', @user.payout_detail.country
end

test 'updating payout details' do
create(:payout_detail, user: @user)

visit account_url

within(payout_details_section) do
fill_in 'Name', with: 'John Smith'
select '🇬🇧 United Kingdom (GBP)', from: 'Country / Region'
click_on 'Save changes'
end

assert_text 'Payout details updated'
end

private

def payout_details_section
find('h2', text: 'Payout details').ancestor('section')
end
end

0 comments on commit d93af09

Please sign in to comment.