From c805ae8b61a661d8ac687ad444d3f4efedbed8af Mon Sep 17 00:00:00 2001 From: dadachi Date: Wed, 29 Apr 2026 16:45:59 +0900 Subject: [PATCH] Refactor: small cleanups across models and controllers - ErrorsController: extract shared render_error helper for the duplicated JSON/HTML branching - RegistrationsController: render_*_error hooks now delegate to a single private render_resource_error to remove the triple copy - Account/Shop/ItemTag/AccountsShopkeeper: rename `the_limit_count` local to plain `limit` - AccountsInvitation#set_token: collapse the unique-token loop to use `break` as the assignment value - Shopkeeper: drop two commented-out send_devise_notification lines superseded by the custom mailer Behavior preserved; full test suite passes (393 runs, 792 assertions). Co-Authored-By: Claude Opus 4.7 (1M context) --- app/controllers/errors_controller.rb | 16 +++++++++------- .../shopkeeper_auth/registrations_controller.rb | 10 +++++++--- app/models/account.rb | 6 +++--- app/models/accounts_invitation.rb | 10 +++------- app/models/accounts_shopkeeper.rb | 6 +++--- app/models/item_tag.rb | 6 +++--- app/models/shop.rb | 6 +++--- app/models/shopkeeper.rb | 2 -- 8 files changed, 31 insertions(+), 31 deletions(-) diff --git a/app/controllers/errors_controller.rb b/app/controllers/errors_controller.rb index 4cd7d6e..d6aa326 100644 --- a/app/controllers/errors_controller.rb +++ b/app/controllers/errors_controller.rb @@ -1,17 +1,19 @@ class ErrorsController < NonApiApplicationController def not_found - if request.content_type&.include?("application/json") - render json: {code: 404, error_message: "Not found."}, status: 404 - else - render status: 404 - end + render_error(404, "Not found.") end def internal_server_error + render_error(500, "Internal server error.") + end + + private + + def render_error(status, message) if request.content_type&.include?("application/json") - render json: {code: 500, error_message: "Internal server error."}, status: 500 + render json: {code: status, error_message: message}, status: status else - render status: 500 + render status: status end end end diff --git a/app/controllers/shopkeeper_auth/registrations_controller.rb b/app/controllers/shopkeeper_auth/registrations_controller.rb index 2ffcb19..b25469c 100644 --- a/app/controllers/shopkeeper_auth/registrations_controller.rb +++ b/app/controllers/shopkeeper_auth/registrations_controller.rb @@ -33,19 +33,23 @@ def render_destroy_success end def render_create_error - render json: {code: 422, error_message: @resource.errors.full_messages.to_sentence}, status: :unprocessable_entity + render_resource_error end def render_update_error - render json: {code: 422, error_message: @resource.errors.full_messages.to_sentence}, status: :unprocessable_entity + render_resource_error end def render_destroy_error - render json: {code: 422, error_message: @resource.errors.full_messages.to_sentence}, status: :unprocessable_entity + render_resource_error end private + def render_resource_error + render json: {code: 422, error_message: @resource.errors.full_messages.to_sentence}, status: :unprocessable_entity + end + def validate_sign_up_params return if sign_up_params.present? diff --git a/app/models/account.rb b/app/models/account.rb index 8b03859..9f66838 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -43,9 +43,9 @@ def reached_limit_shop_count? end def limit_count - the_limit_count = ConfigSettings.account.limit_count - return if owner.owned_accounts.count < the_limit_count + limit = ConfigSettings.account.limit_count + return if owner.owned_accounts.count < limit - errors.add :base, :limit_count_account, limit_count: the_limit_count + errors.add :base, :limit_count_account, limit_count: limit end end diff --git a/app/models/accounts_invitation.rb b/app/models/accounts_invitation.rb index 3e9bbd4..d0274ca 100644 --- a/app/models/accounts_invitation.rb +++ b/app/models/accounts_invitation.rb @@ -55,14 +55,10 @@ def to_param private def set_token - the_token = nil - - loop do - the_token = random_token - break unless AccountsInvitation.exists?(token: the_token) + self.token = loop do + candidate = random_token + break candidate unless AccountsInvitation.exists?(token: candidate) end - - self.token = the_token end def random_token diff --git a/app/models/accounts_shopkeeper.rb b/app/models/accounts_shopkeeper.rb index 1604ab2..7f1376d 100644 --- a/app/models/accounts_shopkeeper.rb +++ b/app/models/accounts_shopkeeper.rb @@ -31,9 +31,9 @@ def owner_must_be_admin end def limit_count - the_limit_count = ConfigSettings.accounts_shopkeeper.limit_count - return if account.accounts_shopkeepers.count < the_limit_count + limit = ConfigSettings.accounts_shopkeeper.limit_count + return if account.accounts_shopkeepers.count < limit - errors.add :base, :limit_count_accounts_shopkeeper, limit_count: the_limit_count + errors.add :base, :limit_count_accounts_shopkeeper, limit_count: limit end end diff --git a/app/models/item_tag.rb b/app/models/item_tag.rb index 351aa2b..303981e 100644 --- a/app/models/item_tag.rb +++ b/app/models/item_tag.rb @@ -36,9 +36,9 @@ def set_default_position end def limit_count - the_limit_count = ConfigSettings.item_tag.limit_count - return if shop.item_tags.count < the_limit_count + limit = ConfigSettings.item_tag.limit_count + return if shop.item_tags.count < limit - errors.add :base, :limit_count_item_tag, limit_count: the_limit_count + errors.add :base, :limit_count_item_tag, limit_count: limit end end diff --git a/app/models/shop.rb b/app/models/shop.rb index 9c8aa4b..a979459 100644 --- a/app/models/shop.rb +++ b/app/models/shop.rb @@ -26,10 +26,10 @@ def create_sample_item_tag def limit_count ActsAsTenant.without_tenant do - the_limit_count = ConfigSettings.shop.limit_count - return if created_by.created_shops.count < the_limit_count + limit = ConfigSettings.shop.limit_count + return if created_by.created_shops.count < limit - errors.add :base, :limit_count_shop, limit_count: the_limit_count + errors.add :base, :limit_count_shop, limit_count: limit end end end diff --git a/app/models/shopkeeper.rb b/app/models/shopkeeper.rb index f7e53ac..59e049d 100644 --- a/app/models/shopkeeper.rb +++ b/app/models/shopkeeper.rb @@ -34,7 +34,6 @@ def send_confirmation_instructions(opts = {}) opts[:to] = unconfirmed_email if pending_reconfirmation? opts[:redirect_url] ||= DeviseTokenAuth.default_confirm_success_url - # send_devise_notification(:confirmation_instructions, @raw_confirmation_token, opts) Shopkeeper::NotificationMailer.with(resource: self, token: @raw_confirmation_token, opts: opts).confirmation_instructions.deliver_later end @@ -45,7 +44,6 @@ def send_reset_password_instructions(opts = {}) # fall back to "default" config name opts[:client_config] ||= "default" - # send_devise_notification(:reset_password_instructions, token, opts) Shopkeeper::NotificationMailer.with(resource: self, token: token, opts: opts).reset_password_instructions.deliver_later token