From ece90084590de33937b4a428904730bf5cbacac2 Mon Sep 17 00:00:00 2001 From: "Stanko K.R." Date: Fri, 31 Oct 2025 16:01:35 +0100 Subject: [PATCH] Remove dead code --- .../account/settings_controller.rb | 9 ++-- app/controllers/application_controller.rb | 3 +- .../concerns/authentication/session_lookup.rb | 7 --- app/controllers/concerns/authorization.rb | 14 ------ .../confirmations_controller.rb | 4 +- .../membership/email_address_changeable.rb | 2 + app/views/account/join_codes/edit.html.erb | 2 +- app/views/account/join_codes/show.html.erb | 2 +- app/views/account/settings/show.html.erb | 4 +- .../signups/completions_controller_test.rb | 49 +++++++------------ gems/fizzy-saas/test/models/signup_test.rb | 6 +-- test/models/identity/transferable_test.rb | 14 +----- 12 files changed, 38 insertions(+), 78 deletions(-) delete mode 100644 app/controllers/concerns/authentication/session_lookup.rb diff --git a/app/controllers/account/settings_controller.rb b/app/controllers/account/settings_controller.rb index 29a037ab5..9acebbae4 100644 --- a/app/controllers/account/settings_controller.rb +++ b/app/controllers/account/settings_controller.rb @@ -1,13 +1,16 @@ class Account::SettingsController < ApplicationController - before_action def show @account = Account.sole @users = User.active.alphabetically end def update - Account.sole.update!(account_params) - redirect_to account_settings_path + if Current.user.can_administer? + Account.sole.update!(account_params) + redirect_to account_settings_path + else + head :forbidden + end end private diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 786f80bcf..6c16b1906 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,6 +1,7 @@ class ApplicationController < ActionController::Base include LoadBalancerRouting, WriterAffinity - include Authentication, Authorization + include Authentication + include Authorization include CurrentRequest, CurrentTimezone, SetPlatform include TurboFlash, ViewTransitions diff --git a/app/controllers/concerns/authentication/session_lookup.rb b/app/controllers/concerns/authentication/session_lookup.rb deleted file mode 100644 index b180f217f..000000000 --- a/app/controllers/concerns/authentication/session_lookup.rb +++ /dev/null @@ -1,7 +0,0 @@ -module Authentication::SessionLookup - def find_session_by_cookie - if token = cookies.signed[:session_token] - Session.find_by(token: token) - end - end -end diff --git a/app/controllers/concerns/authorization.rb b/app/controllers/concerns/authorization.rb index 6bb040c33..aca10b5b2 100644 --- a/app/controllers/concerns/authorization.rb +++ b/app/controllers/concerns/authorization.rb @@ -38,18 +38,4 @@ module Authorization def redirect_existing_user redirect_to root_path if Current.user end - - def account_entry_url(membership) - if !ApplicationRecord.tenant_exists?(membership.tenant) - elsif membership.user.blank? - # We are joining an account. This means the user doesn't yet exist and - # we have to create it - new_user_url(script_name: "/#{membership.tenant}", membership: membership.to_signed(for: :user_creation)) - # The user exists, but was deactivated, we want to remove the membership - unlink_membership_url(script_name: nil, membership: membership.to_signed(for: :unlinking)) - else - # Everything is fine, let the user enter the account - root_url(script_name: "/#{membership.tenant}") - end - end end diff --git a/app/controllers/memberships/email_addresses/confirmations_controller.rb b/app/controllers/memberships/email_addresses/confirmations_controller.rb index 962bf78dd..8ad43971f 100644 --- a/app/controllers/memberships/email_addresses/confirmations_controller.rb +++ b/app/controllers/memberships/email_addresses/confirmations_controller.rb @@ -9,10 +9,10 @@ class Memberships::EmailAddresses::ConfirmationsController < ApplicationControll end def create - Membership.change_email_address_using_token(token) + membership = Membership.change_email_address_using_token(token) terminate_session - start_new_session_for @membership.reload.identity + start_new_session_for membership.reload.identity redirect_to edit_user_url(script_name: "/#{@membership.tenant}", id: @membership.user) end diff --git a/app/models/membership/email_address_changeable.rb b/app/models/membership/email_address_changeable.rb index 3b6dbdf84..5ec2b554a 100644 --- a/app/models/membership/email_address_changeable.rb +++ b/app/models/membership/email_address_changeable.rb @@ -19,6 +19,8 @@ module Membership::EmailAddressChangeable new_email_address = parsed_token.params.fetch("new_email_address") membership.change_email_address(new_email_address) end + + membership end end diff --git a/app/views/account/join_codes/edit.html.erb b/app/views/account/join_codes/edit.html.erb index ebac0fa26..c96a4ac82 100644 --- a/app/views/account/join_codes/edit.html.erb +++ b/app/views/account/join_codes/edit.html.erb @@ -1,7 +1,7 @@ <% @page_title = "Change usage limit" %> <% content_for :header do %> - <%= render "filters/menu" %> + <%= render "my/menus/menu" %>
<%= link_to account_join_code_path, class: "btn borderless txt-medium", data: { controller: "hotkey", action: "keydown.esc@document->hotkey#click" } do %> ← Invite link diff --git a/app/views/account/join_codes/show.html.erb b/app/views/account/join_codes/show.html.erb index 56fed396c..f076518ff 100644 --- a/app/views/account/join_codes/show.html.erb +++ b/app/views/account/join_codes/show.html.erb @@ -1,7 +1,7 @@ <% @page_title = "Add people" %> <% content_for :header do %> - <%= render "filters/menu" %> + <%= render "my/menus/menu" %>
<%= link_to account_settings_path, class: "btn borderless txt-medium", data: { controller: "hotkey", action: "keydown.esc@document->hotkey#click" } do %> ← Account Settings diff --git a/app/views/account/settings/show.html.erb b/app/views/account/settings/show.html.erb index cfa85b9e1..245643c20 100644 --- a/app/views/account/settings/show.html.erb +++ b/app/views/account/settings/show.html.erb @@ -6,7 +6,9 @@
- <%= render "account/settings/name", account: @account %> + <% if Current.user.can_administer? %> + <%= render "account/settings/name", account: @account %> + <% end%> <%= render "account/settings/users", users: @users %>
diff --git a/gems/fizzy-saas/test/controllers/signups/completions_controller_test.rb b/gems/fizzy-saas/test/controllers/signups/completions_controller_test.rb index b04bc7f0a..e9f8a472f 100644 --- a/gems/fizzy-saas/test/controllers/signups/completions_controller_test.rb +++ b/gems/fizzy-saas/test/controllers/signups/completions_controller_test.rb @@ -2,53 +2,40 @@ require "test_helper" class Signups::CompletionsControllerTest < ActionDispatch::IntegrationTest setup do - @identity = Identity.create!(email_address: "newuser@example.com") - magic_link = @identity.send_magic_link + @signup = Signup.new(email_address: "newuser@example.com", full_name: "New User") - untenanted do - post session_magic_link_url, params: { code: magic_link.code } - assert_response :redirect, "Magic link should succeed" + @signup.create_identity || raise("Failed to create identity") - cookie = cookies.get_cookie "session_token" - assert_not_nil cookie, "Expected session_token cookie to be set after magic link consumption" - end + sign_in_as @signup.identity - # Create membership first (new step in the flow) - untenanted do - post saas.signup_membership_path, params: { - signup: { - full_name: "New User" - } - }, headers: http_basic_auth_headers - - # Extract membership_id from redirect params - redirect_url = response.location - @membership_id = Rack::Utils.parse_query(URI.parse(redirect_url).query)["signup[membership_id]"] - end + @signup.create_membership || raise("Failed to create membership") end test "new" do untenanted do - get saas.new_signup_completion_path(signup: { membership_id: @membership_id, full_name: "New User", account_name: "New Company" }), headers: http_basic_auth_headers - - assert_response :success + get saas.new_signup_completion_path(signup: { + membership_id: @signup.membership_id, + full_name: @signup.full_name, + account_name: @signup.account_name }), headers: http_basic_auth_headers end + + assert_response :success end test "create" do untenanted do post saas.signup_completion_path, params: { signup: { - membership_id: @membership_id, - full_name: "New User", - account_name: "New Company" + membership_id: @signup.membership_id, + full_name: @signup.full_name, + account_name: @signup.account_name } }, headers: http_basic_auth_headers + end - tenant = Membership.last.tenant - assert_redirected_to root_url(script_name: "/#{tenant}"), "Successful completion should redirect to root in new tenant" + assert_redirected_to root_url(script_name: "/#{@signup.tenant}"), "Successful completion should redirect to root in new tenant" - # Test validation error + untenanted do post saas.signup_completion_path, params: { signup: { membership_id: @membership_id, @@ -56,9 +43,9 @@ class Signups::CompletionsControllerTest < ActionDispatch::IntegrationTest account_name: "" } }, headers: http_basic_auth_headers - - assert_response :unprocessable_entity, "Invalid params should return unprocessable entity" end + + assert_response :unprocessable_entity, "Invalid params should return unprocessable entity" end private diff --git a/gems/fizzy-saas/test/models/signup_test.rb b/gems/fizzy-saas/test/models/signup_test.rb index 82832f7d9..44d801017 100644 --- a/gems/fizzy-saas/test/models/signup_test.rb +++ b/gems/fizzy-saas/test/models/signup_test.rb @@ -38,7 +38,7 @@ class SignupTest < ActiveSupport::TestCase ) assert_difference -> { Membership.count }, 1 do - assert signup.create_membership, signup.errors.full_messages.to_sentence(words_connector: ". ") + assert signup.create_membership end assert signup.tenant @@ -63,7 +63,6 @@ class SignupTest < ActiveSupport::TestCase ) signup_for_membership.create_membership - # Then complete the signup signup = Signup.new( full_name: "Kevin", account_name: "37signals", @@ -71,7 +70,7 @@ class SignupTest < ActiveSupport::TestCase identity: identities(:kevin) ) - assert signup.complete, signup.errors.full_messages.to_sentence(words_connector: ". ") + assert signup.complete assert signup.tenant assert signup.account @@ -79,7 +78,6 @@ class SignupTest < ActiveSupport::TestCase assert_equal "Kevin", signup.user.name assert_equal "37signals", signup.account.name - # Test validation failure signup_invalid = Signup.new( full_name: "", account_name: "37signals", diff --git a/test/models/identity/transferable_test.rb b/test/models/identity/transferable_test.rb index ed9a504a8..3ede84c26 100644 --- a/test/models/identity/transferable_test.rb +++ b/test/models/identity/transferable_test.rb @@ -5,11 +5,7 @@ class Identity::TransferableTest < ActiveSupport::TestCase identity = identities(:david) transfer_id = identity.transfer_id - assert_not_nil transfer_id - - # Should be able to find the identity using the transfer_id - found_identity = Identity.find_by_transfer_id(transfer_id) - assert_equal identity, found_identity + assert_kind String, transfer_id end test "find_by_transfer_id" do @@ -18,19 +14,11 @@ class Identity::TransferableTest < ActiveSupport::TestCase found = Identity.find_by_transfer_id(transfer_id) assert_equal identity, found - end - test "find_by_transfer_id with invalid id" do found = Identity.find_by_transfer_id("invalid_id") assert_nil found - end - test "find_by_transfer_id with expired id" do - identity = identities(:jz) - - # Generate a transfer_id with short expiry expired_id = identity.signed_id(purpose: :transfer, expires_in: -1.second) - found = Identity.find_by_transfer_id(expired_id) assert_nil found end