From aef0d28dd185e19c6fef149e2748f68b4852a833 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Mon, 1 Feb 2021 16:28:45 +0100 Subject: [PATCH] Do not load unmodified VOs from DB Closes #6727. This avoids the authorization of all the VOs of the hub, which will go through VOs that may have become invalid due to their underlying product not belonging to the supplier the hub has permissions with (or any other data integrity issue). This is utterly confusing for the user who is only given a generic error and doesn't understand what's wrong with the particular VO they changed, while it may be fine after all. What's more, this often results in a customer support request, which then may end up with a dev finding out which VO is broken. Also, there's no point in loading them from DB if the users didn't touch them. --- app/controllers/admin/variant_overrides_controller.rb | 8 ++++++++ .../admin/variant_overrides_controller_spec.rb | 7 +++++++ 2 files changed, 15 insertions(+) diff --git a/app/controllers/admin/variant_overrides_controller.rb b/app/controllers/admin/variant_overrides_controller.rb index de58a3b514..ba591bd967 100644 --- a/app/controllers/admin/variant_overrides_controller.rb +++ b/app/controllers/admin/variant_overrides_controller.rb @@ -79,6 +79,14 @@ module Admin joins(variant: :product). preload(variant: :product). for_hubs(params[:hub_id] || @hubs) + + return @variant_overrides unless params.key?(:variant_overrides) + + @variant_overrides.where(id: modified_variant_overrides_ids) + end + + def modified_variant_overrides_ids + variant_overrides_params.map { |vo| vo[:id] } end def collection_actions diff --git a/spec/controllers/admin/variant_overrides_controller_spec.rb b/spec/controllers/admin/variant_overrides_controller_spec.rb index 1dca6eeec6..ad2adcc6d7 100644 --- a/spec/controllers/admin/variant_overrides_controller_spec.rb +++ b/spec/controllers/admin/variant_overrides_controller_spec.rb @@ -36,6 +36,13 @@ describe Admin::VariantOverridesController, type: :controller do spree_put :bulk_update, format: format, variant_overrides: variant_override_params expect(response).to redirect_to unauthorized_path end + + it 'only authorizes the updated variant overrides' do + other_variant_override = create(:variant_override, hub: hub, variant: create(:variant)) + expect(controller).not_to receive(:authorize!).with(:update, other_variant_override) + + spree_put :bulk_update, format: format, variant_overrides: variant_override_params + end end context "and the producer has granted VO permission" do