Extract shared error helpers and add comprehensive test coverage

CLAUDE.md

@@ -172,4 +172,8 @@ bin/rails test
 ### Adding Background Jobs
 1. Create job class in `app/jobs/`
 2. Specify queue with `queue_as :default` (or :critical, :low, etc.)
-3. Call with `MyJob.perform_later(args)`
+3. Call with `MyJob.perform_later(args)`
+
+## Testing Policy
+
+Create test passing all of path including unhappy path. Creating and updating that test is must.

app/controllers/api/v1/shopkeeper/account/passwords_controller.rb

@@ -5,7 +5,7 @@ def update
     if current_shopkeeper.update_with_password(password_params)
       render json: {status: 200}, status: :ok
     else
-      render json: {code: 422, error_message: current_shopkeeper.errors.full_messages.to_sentence}, status: :unprocessable_entity
+      render_validation_error(current_shopkeeper)
     end
   end
 

app/controllers/api/v1/shopkeeper/accounts/accounts_invitations_controller.rb

@@ -25,7 +25,7 @@ def create
     if accounts_invitation.save_and_send_invite
       render json: AccountsInvitationSerializer.new(accounts_invitation).serializable_hash, status: :created
     else
-      render json: {code: 422, error_message: accounts_invitation.errors.full_messages.to_sentence}, status: :unprocessable_entity
+      render_validation_error(accounts_invitation)
     end
   end
 
@@ -35,7 +35,7 @@ def update
     if @accounts_invitation.update(invitation_params_update)
       render json: AccountsInvitationSerializer.new(@accounts_invitation).serializable_hash
     else
-      render json: {code: 422, error_message: @accounts_invitation.errors.full_messages.to_sentence}, status: :unprocessable_entity
+      render_validation_error(@accounts_invitation)
     end
   end
 

app/controllers/api/v1/shopkeeper/accounts_controller.rb

@@ -46,7 +46,7 @@ def create
 
       render json: AccountSerializer.new(account, options).serializable_hash, status: :created
     else
-      render json: {code: 422, error_message: account.errors.full_messages.to_sentence}, status: :unprocessable_entity
+      render_validation_error(account)
     end
   end
 
@@ -60,7 +60,7 @@ def update
       }
       render json: AccountSerializer.new(@account, options).serializable_hash
     else
-      render json: {code: 422, error_message: @account.errors.full_messages.to_sentence}, status: :unprocessable_entity
+      render_validation_error(@account)
     end
   end
 
@@ -98,6 +98,6 @@ def pundit_user
   def prevent_personal_account_deletion
     return unless @account.personal?
 
-    render json: {code: 422, error_message: I18n.t("api.shopkeeper.accounts.personal.cannot_delete")}, status: :unprocessable_entity
+    render_error(code: 422, message: I18n.t("api.shopkeeper.accounts.personal.cannot_delete"), status: :unprocessable_entity)
   end
 end

app/controllers/api/v1/shopkeeper/accounts_invitations_controller.rb

@@ -5,8 +5,7 @@ def show
     authorize @accounts_invitation, :show_by_token?
 
     if @accounts_invitation.expired?
-      render json: {code: 410, error_message: I18n.t("api.shopkeeper.accounts_invitations.expired")}, status: :gone
-      return
+      return render_error(code: 410, message: I18n.t("api.shopkeeper.accounts_invitations.expired"), status: :gone)
     end
 
     options = {}
@@ -18,15 +17,14 @@ def update
     authorize @accounts_invitation, :accept?
 
     if @accounts_invitation.expired?
-      render json: {code: 410, error_message: I18n.t("api.shopkeeper.accounts_invitations.expired")}, status: :gone
-      return
+      return render_error(code: 410, message: I18n.t("api.shopkeeper.accounts_invitations.expired"), status: :gone)
     end
 
     if @accounts_invitation.accept!(current_shopkeeper)
       render json: {status: 200}, status: :ok
     else
       error_message = @accounts_invitation.errors.full_messages.first || I18n.t("something_went_wrong")
-      render json: {code: 422, error_message: error_message}, status: :unprocessable_entity
+      render_error(code: 422, message: error_message, status: :unprocessable_entity)
     end
   end
 
@@ -42,6 +40,6 @@ def destroy
   def set_accounts_invitation
     @accounts_invitation = AccountsInvitation.find_by!(token: params[:id])
   rescue ActiveRecord::RecordNotFound
-    render json: {code: 404, error_message: I18n.t("api.shopkeeper.accounts_invitations.not_found")}, status: :not_found
+    render_error(code: 404, message: I18n.t("api.shopkeeper.accounts_invitations.not_found"), status: :not_found)
   end
 end

app/controllers/api/v1/shopkeeper/accounts_shopkeepers_controller.rb

@@ -36,7 +36,7 @@ def update
 
       render json: AccountsShopkeeperSerializer.new(@accounts_shopkeeper, options).serializable_hash
     else
-      render json: {code: 422, error_message: @accounts_shopkeeper.errors.full_messages.to_sentence}, status: :unprocessable_entity
+      render_validation_error(@accounts_shopkeeper)
     end
   end
 
@@ -70,12 +70,12 @@ def accounts_shopkeeper_params
   def require_non_personal_account!
     return unless @account.personal?
 
-    render json: {code: 422, error_message: I18n.t("api.shopkeeper.accounts_shopkeepers.require_non_personal_account")}, status: :unprocessable_entity
+    render_error(code: 422, message: I18n.t("api.shopkeeper.accounts_shopkeepers.require_non_personal_account"), status: :unprocessable_entity)
   end
 
   def safeguard_account_owner_deletion!
     return unless @accounts_shopkeeper.account_owner?
 
-    render json: {code: 401, error_message: I18n.t("unauthorized")}, status: :unauthorized
+    render_error(code: 401, message: I18n.t("unauthorized"), status: :unauthorized)
   end
 end

app/controllers/api/v1/shopkeeper/base_controller.rb

@@ -23,7 +23,15 @@ def authorize(record, query = nil)
 
   private
 
+  def render_validation_error(record)
+    render json: {code: 422, error_message: record.errors.full_messages.to_sentence}, status: :unprocessable_entity
+  end
+
+  def render_error(code:, message:, status:)
+    render json: {code: code, error_message: message}, status: status
+  end
+
   def user_not_authorized
-    render json: {code: 401, error_message: I18n.t("unauthorized")}, status: :unauthorized
+    render_error(code: 401, message: I18n.t("unauthorized"), status: :unauthorized)
   end
 end

app/controllers/api/v1/shopkeeper/item_tags_controller.rb

@@ -25,7 +25,7 @@ def create
     if item_tag.save
       render json: ItemTagSerializer.new(item_tag).serializable_hash, status: :created
     else
-      render json: {code: 422, error_message: item_tag.errors.full_messages.to_sentence}, status: :unprocessable_entity
+      render_validation_error(item_tag)
     end
   end
 
@@ -35,7 +35,7 @@ def update
     if @item_tag.update(item_tag_params)
       render json: ItemTagSerializer.new(@item_tag).serializable_hash
     else
-      render json: {code: 422, error_message: @item_tag.errors.full_messages.to_sentence}, status: :unprocessable_entity
+      render_validation_error(@item_tag)
     end
   end
 
@@ -83,7 +83,7 @@ def set_shop
   def set_item_tag
     @item_tag = current_shopkeeper.item_tags.find(params[:id])
   rescue ActiveRecord::RecordNotFound
-    render json: {code: 404, error_message: I18n.t("api.shopkeeper.item_tags.not_found")}, status: :not_found
+    render_error(code: 404, message: I18n.t("api.shopkeeper.item_tags.not_found"), status: :not_found)
   end
 
   def item_tag_params

app/controllers/api/v1/shopkeeper/shops_controller.rb

@@ -32,7 +32,7 @@ def create
     if shop.save
       render json: ShopSerializer.new(shop).serializable_hash, status: :created
     else
-      render json: {code: 422, error_message: shop.errors.full_messages.to_sentence}, status: :unprocessable_entity
+      render_validation_error(shop)
     end
   end
 
@@ -42,7 +42,7 @@ def update
     if @shop.update(shop_params_update)
       render json: ShopSerializer.new(@shop).serializable_hash
     else
-      render json: {code: 422, error_message: @shop.errors.full_messages.to_sentence}, status: :unprocessable_entity
+      render_validation_error(@shop)
     end
   end
 

test/controllers/api/v1/shopkeeper/accounts/accounts_invitations_controller_test.rb

@@ -69,6 +69,8 @@ class Api::V1::Shopkeeper::Accounts::AccountsInvitationsControllerTest < ActionD
     end
 
     assert_response :unprocessable_entity
+    assert_equal 422, response.parsed_body["code"]
+    assert response.parsed_body["error_message"].present?
   end
 
   test "create requires admin role" do
@@ -118,6 +120,8 @@ class Api::V1::Shopkeeper::Accounts::AccountsInvitationsControllerTest < ActionD
       headers: @shopkeeper.create_new_auth_token
 
     assert_response :unprocessable_entity
+    assert_equal 422, response.parsed_body["code"]
+    assert response.parsed_body["error_message"].present?
   end
 
   test "update requires admin role" do
@@ -158,16 +162,23 @@ class Api::V1::Shopkeeper::Accounts::AccountsInvitationsControllerTest < ActionD
     assert_response :unauthorized
   end
 
-  test "resend sends invitation email again and touches created_at" do
-    original_created_at = @invitation.created_at
+  test "resend sends invitation email again" do
+    post resend_api_v1_shopkeeper_account_accounts_invitation_path(@account, @invitation.token),
+      headers: @shopkeeper.create_new_auth_token
 
-    travel_to(1.hour.from_now) do
-      post resend_api_v1_shopkeeper_account_accounts_invitation_path(@account, @invitation.token),
-        headers: @shopkeeper.create_new_auth_token
+    assert_response :success
+    assert_enqueued_emails 1
+  end
 
-      assert_response :success
-      assert @invitation.reload.created_at > original_created_at
-    end
+  test "resend resets expiration for expired invitation" do
+    @invitation.update_column(:created_at, (AccountsInvitation::EXPIRES_IN + 1.minute).ago)
+    assert @invitation.expired?
+
+    post resend_api_v1_shopkeeper_account_accounts_invitation_path(@account, @invitation.token),
+      headers: @shopkeeper.create_new_auth_token
+
+    assert_response :success
+    assert_not @invitation.reload.expired?
   end
 
   test "resend requires admin role" do

test/controllers/api/v1/shopkeeper/accounts_controller_test.rb

@@ -26,6 +26,16 @@ class Api::V1::Shopkeeper::AccountsControllerTest < ActionDispatch::IntegrationT
     assert_equal @team_account.id.to_s, response.parsed_body["data"]["id"]
   end
 
+  test "create returns validation error with blank name" do
+    post api_v1_shopkeeper_accounts_url,
+      params: {account: {name: ""}},
+      headers: @shopkeeper.create_new_auth_token
+
+    assert_response :unprocessable_entity
+    assert_equal 422, response.parsed_body["code"]
+    assert_not_nil response.parsed_body["error_message"]
+  end
+
   test "create creates a new account" do
     assert_difference "Account.count", 1 do
       post api_v1_shopkeeper_accounts_url,
@@ -36,6 +46,16 @@ class Api::V1::Shopkeeper::AccountsControllerTest < ActionDispatch::IntegrationT
     assert_response :created
   end
 
+  test "update returns validation error with blank name" do
+    patch api_v1_shopkeeper_account_url(@team_account),
+      params: {account: {name: ""}},
+      headers: @shopkeeper.create_new_auth_token
+
+    assert_response :unprocessable_entity
+    assert_equal 422, response.parsed_body["code"]
+    assert_not_nil response.parsed_body["error_message"]
+  end
+
   test "update requires admin role" do
     other_shopkeeper = shopkeepers(:two)
     AccountsShopkeeper.create!(
@@ -74,6 +94,15 @@ class Api::V1::Shopkeeper::AccountsControllerTest < ActionDispatch::IntegrationT
     assert_response :unauthorized
   end
 
+  test "destroy prevents personal account deletion" do
+    delete api_v1_shopkeeper_account_url(@account),
+      headers: @shopkeeper.create_new_auth_token
+
+    assert_response :unprocessable_entity
+    assert_equal 422, response.parsed_body["code"]
+    assert_equal I18n.t("api.shopkeeper.accounts.personal.cannot_delete"), response.parsed_body["error_message"]
+  end
+
   test "destroy succeeds for owner" do
     assert_difference "Account.count", -1 do
       delete api_v1_shopkeeper_account_url(@team_account),

test/controllers/api/v1/shopkeeper/accounts_invitations_controller_test.rb

@@ -29,6 +29,8 @@ class Api::V1::Shopkeeper::AccountsInvitationsControllerTest < ActionDispatch::I
       headers: @shopkeeper.create_new_auth_token
 
     assert_response :not_found
+    assert_equal 404, response.parsed_body["code"]
+    assert_equal I18n.t("api.shopkeeper.accounts_invitations.not_found"), response.parsed_body["error_message"]
   end
 
   test "update accepts invitation" do
@@ -57,6 +59,7 @@ class Api::V1::Shopkeeper::AccountsInvitationsControllerTest < ActionDispatch::I
       headers: shopkeepers(:two).create_new_auth_token
 
     assert_response :unprocessable_entity
+    assert_equal 422, response.parsed_body["code"]
     assert response.parsed_body["error_message"].present?
   end
 
@@ -69,24 +72,26 @@ class Api::V1::Shopkeeper::AccountsInvitationsControllerTest < ActionDispatch::I
     assert_response :success
   end
 
-  test "show returns 410 gone for expired invitation" do
+  test "show returns 410 for expired invitation" do
     @invitation.update_column(:created_at, (AccountsInvitation::EXPIRES_IN + 1.minute).ago)
 
     get api_v1_shopkeeper_accounts_invitation_url(@invitation.token),
       headers: @shopkeeper.create_new_auth_token
 
     assert_response :gone
+    assert_equal 410, response.parsed_body["code"]
     assert_equal I18n.t("api.shopkeeper.accounts_invitations.expired"), response.parsed_body["error_message"]
   end
 
-  test "update returns 410 gone for expired invitation" do
+  test "update returns 410 for expired invitation" do
     @invitation.update_column(:created_at, (AccountsInvitation::EXPIRES_IN + 1.minute).ago)
 
     other_shopkeeper = shopkeepers(:two)
     patch api_v1_shopkeeper_accounts_invitation_url(@invitation.token),
       headers: other_shopkeeper.create_new_auth_token
 
     assert_response :gone
+    assert_equal 410, response.parsed_body["code"]
     assert_equal I18n.t("api.shopkeeper.accounts_invitations.expired"), response.parsed_body["error_message"]
   end
 

test/controllers/api/v1/shopkeeper/accounts_shopkeepers_controller_test.rb

@@ -53,6 +53,8 @@ class Api::V1::Shopkeeper::AccountsShopkeepersControllerTest < ActionDispatch::I
       headers: @shopkeeper.create_new_auth_token
 
     assert_response :unprocessable_entity
+    assert_equal 422, response.parsed_body["code"]
+    assert_equal I18n.t("api.shopkeeper.accounts_shopkeepers.require_non_personal_account"), response.parsed_body["error_message"]
   end
 
   test "update updates accounts_shopkeeper roles" do
@@ -82,6 +84,16 @@ class Api::V1::Shopkeeper::AccountsShopkeepersControllerTest < ActionDispatch::I
     assert_response :unprocessable_entity
   end
 
+  test "update returns validation error when owner removes own admin" do
+    patch api_v1_shopkeeper_account_accounts_shopkeeper_url(@team_account, @team_accounts_shopkeeper),
+      params: {accounts_shopkeeper: {admin: false, junior_member: true}},
+      headers: @shopkeeper.create_new_auth_token
+
+    assert_response :unprocessable_entity
+    assert_equal 422, response.parsed_body["code"]
+    assert response.parsed_body["error_message"].present?
+  end
+
   test "update requires admin role" do
     other_shopkeeper = shopkeepers(:two)
     AccountsShopkeeper.create!(
@@ -118,6 +130,8 @@ class Api::V1::Shopkeeper::AccountsShopkeepersControllerTest < ActionDispatch::I
       headers: @shopkeeper.create_new_auth_token
 
     assert_response :unauthorized
+    assert_equal 401, response.parsed_body["code"]
+    assert_equal I18n.t("unauthorized"), response.parsed_body["error_message"]
   end
 
   test "destroy returns error for personal account" do