Skip to content

Commit 43069b1

Browse files
committed
Extract Users::JoinsController from overloaded UsersController
Good code smell is when the before_action callbacks stack up but can't be shared across actions
1 parent ada599f commit 43069b1

File tree

7 files changed

+71
-55
lines changed

7 files changed

+71
-55
lines changed

app/controllers/concerns/authorization.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ def ensure_can_access_account
2929
if Current.membership.blank?
3030
redirect_to session_menu_url(script_name: nil)
3131
elsif Current.user.nil? && Current.membership.join_code.present?
32-
redirect_to new_user_path
32+
redirect_to new_users_join_path
3333
elsif !Current.user&.active?
3434
redirect_to unlink_membership_url(script_name: nil, membership_id: Current.membership.signed_id(purpose: :unlinking))
3535
end
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
class Users::JoinsController < ApplicationController
2+
require_access_without_a_user
3+
4+
before_action :set_join_code, :ensure_join_code_is_valid
5+
6+
def new
7+
end
8+
9+
def create
10+
@join_code.redeem do
11+
User.create!(user_params.merge(membership: Current.membership))
12+
end
13+
14+
redirect_to root_path
15+
end
16+
17+
private
18+
def set_join_code
19+
@join_code = Account::JoinCode.active.find_by(code: Current.membership.join_code)
20+
end
21+
22+
def ensure_join_code_is_valid
23+
unless @join_code&.active?
24+
redirect_to unlink_membership_url(script_name: nil, membership_id: Current.membership.signed_id(purpose: :unlinking))
25+
end
26+
end
27+
28+
def user_params
29+
params.expect(user: [ :name, :avatar ])
30+
end
31+
end
32+

app/controllers/users_controller.rb

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,7 @@
11
class UsersController < ApplicationController
2-
require_access_without_a_user only: %i[ new create ]
3-
4-
before_action :set_join_code, :ensure_join_code_is_valid, only: %i[ new create ]
52
before_action :set_user, only: %i[ show edit update destroy ]
63
before_action :ensure_permission_to_change_user, only: %i[ update destroy ]
74

8-
def new
9-
end
10-
11-
def create
12-
@join_code.redeem do
13-
User.create!(user_params.merge(membership: Current.membership))
14-
end
15-
16-
redirect_to root_path
17-
end
18-
195
def edit
206
end
217

@@ -33,16 +19,6 @@ def destroy
3319
end
3420

3521
private
36-
def set_join_code
37-
@join_code = Account::JoinCode.active.find_by(code: Current.membership.join_code)
38-
end
39-
40-
def ensure_join_code_is_valid
41-
unless @join_code&.active?
42-
redirect_to unlink_membership_url(script_name: nil, membership_id: Current.membership.signed_id(purpose: :unlinking))
43-
end
44-
end
45-
4622
def set_user
4723
@user = User.active.find(params[:id])
4824
end

app/views/users/new.html.erb renamed to app/views/users/joins/new.html.erb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,11 @@
55
<h1 class="txt-x-large font-weight-black margin-none">
66
<%= @page_title %>
77
</h1>
8+
89
<p class="margin-none-block-start">Enter your name to continue</p>
910
</header>
1011

11-
<%= form_with scope: "user", url: users_path(membership: params[:membership]), class: "flex flex-column gap txt-medium", data: { controller: "form" } do |form| %>
12+
<%= form_with scope: "user", url: users_joins_path(membership: params[:membership]), class: "flex flex-column gap txt-medium", data: { controller: "form" } do |form| %>
1213
<div class="flex align-center gap">
1314
<label class="flex align-center gap input input--actor">
1415
<%= form.text_field :name, class: "input full-width", autocomplete: "name", placeholder: "Name", autofocus: true, required: true, data: { "1p-ignore": true } %>

config/routes.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,10 @@
128128
get "join/:tenant/:code", to: "join_codes#new", as: :join
129129
post "join/:tenant/:code", to: "join_codes#create"
130130

131+
namespace :users do
132+
resources :joins
133+
end
134+
131135
resource :session do
132136
scope module: "sessions" do
133137
resources :transfers
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
require "test_helper"
2+
3+
class Users::JoinsControllerTest < ActionDispatch::IntegrationTest
4+
test "new" do
5+
identity = Identity.create!(email_address: "new.user@example.com")
6+
identity.memberships.create(tenant: ApplicationRecord.current_tenant, join_code: Account::JoinCode.sole.code)
7+
sign_in_as identity
8+
9+
get new_users_join_path
10+
assert_response :ok
11+
end
12+
13+
test "new with invalid params" do
14+
identity = Identity.create!(email_address: "new.user@example.com")
15+
membership = identity.memberships.create(tenant: ApplicationRecord.current_tenant, join_code: "PHONY")
16+
sign_in_as identity
17+
18+
get new_users_join_path
19+
assert_redirected_to unlink_membership_url(script_name: nil, membership_id: membership.signed_id(purpose: :unlinking))
20+
end
21+
22+
test "create" do
23+
identity = Identity.create!(email_address: "newart.userbaum@example.com")
24+
identity.memberships.create(tenant: ApplicationRecord.current_tenant, join_code: Account::JoinCode.sole.code)
25+
sign_in_as identity
26+
27+
assert_difference -> { User.count }, +1 do
28+
post users_joins_path, params: { user: { name: "Newart Userbaum" } }
29+
assert_redirected_to root_path
30+
end
31+
end
32+
end

test/controllers/users_controller_test.rb

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,6 @@
11
require "test_helper"
22

33
class UsersControllerTest < ActionDispatch::IntegrationTest
4-
test "new" do
5-
identity = Identity.create!(email_address: "new.user@example.com")
6-
identity.memberships.create(tenant: ApplicationRecord.current_tenant, join_code: Account::JoinCode.sole.code)
7-
sign_in_as identity
8-
9-
get new_user_path
10-
assert_response :ok
11-
end
12-
13-
test "new with invalid params" do
14-
identity = Identity.create!(email_address: "new.user@example.com")
15-
membership = identity.memberships.create(tenant: ApplicationRecord.current_tenant, join_code: "PHONY")
16-
sign_in_as identity
17-
18-
get new_user_path
19-
assert_redirected_to unlink_membership_url(script_name: nil, membership_id: membership.signed_id(purpose: :unlinking))
20-
end
21-
22-
test "create" do
23-
identity = Identity.create!(email_address: "newart.userbaum@example.com")
24-
identity.memberships.create(tenant: ApplicationRecord.current_tenant, join_code: Account::JoinCode.sole.code)
25-
sign_in_as identity
26-
27-
assert_difference -> { User.count }, +1 do
28-
post users_path, params: { user: { name: "Newart Userbaum" } }
29-
assert_redirected_to root_path
30-
end
31-
end
32-
334
test "show" do
345
sign_in_as :kevin
356

0 commit comments

Comments
 (0)