mirror of
https://github.com/openfoodfoundation/openfoodnetwork
synced 2026-03-01 02:03:22 +00:00
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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user