Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Authorize only changed vos #6789

Merged
merged 2 commits into from
Feb 4, 2021
Merged

Authorize only changed vos #6789

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aef0d28
Do not load unmodified VOs from DB
sauloperez Feb 1, 2021
a19acea
Replace spree_put with put in controller tests
sauloperez Feb 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions app/controllers/admin/variant_overrides_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,14 @@ def collection
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
Expand Down
30 changes: 19 additions & 11 deletions spec/controllers/admin/variant_overrides_controller_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
end

it "redirects to unauthorized" do
spree_put :bulk_update, format: format, variant_overrides: variant_override_params
put :bulk_update, format: format, variant_overrides: variant_override_params
expect(response).to redirect_to unauthorized_path
end
end
Expand All @@ -33,9 +33,16 @@

context "but the producer has not granted VO permission" do
it "redirects to unauthorized" do
spree_put :bulk_update, format: format, variant_overrides: variant_override_params
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)

put :bulk_update, format: format, variant_overrides: variant_override_params
end
end

context "and the producer has granted VO permission" do
Expand All @@ -44,15 +51,16 @@
end

it "loads data" do
spree_put :bulk_update, format: format, variant_overrides: variant_override_params
put :bulk_update, format: format, variant_overrides: variant_override_params
expect(assigns[:hubs]).to eq [hub]
expect(assigns[:producers]).to eq [variant.product.supplier]
expect(assigns[:hub_permissions]).to eq Hash[hub.id, [variant.product.supplier.id]]
expect(assigns[:inventory_items]).to eq [inventory_item]
end

it "allows me to update the variant override" do
spree_put :bulk_update, format: format, variant_overrides: variant_override_params
put :bulk_update, format: format, variant_overrides: variant_override_params

variant_override.reload
expect(variant_override.price).to eq 123.45
expect(variant_override.count_on_hand).to eq 321
Expand All @@ -64,7 +72,7 @@
let(:variant_override_params) { [{ id: variant_override.id, price: "", count_on_hand: "", default_stock: nil, resettable: nil, sku: nil, on_demand: nil }] }

it "destroys the variant override" do
spree_put :bulk_update, format: format, variant_overrides: variant_override_params
put :bulk_update, format: format, variant_overrides: variant_override_params
expect(VariantOverride.find_by(id: variant_override.id)).to be_nil
end
end
Expand All @@ -76,7 +84,7 @@
before { deleted_variant.update_attribute :deleted_at, Time.zone.now }

it "allows to update other variant overrides" do
spree_put :bulk_update, format: format, variant_overrides: variant_override_params
put :bulk_update, format: format, variant_overrides: variant_override_params

expect(response).to_not redirect_to unauthorized_path
variant_override.reload
Expand Down Expand Up @@ -110,7 +118,7 @@
end

it "redirects to unauthorized" do
spree_put :bulk_reset, params
put :bulk_reset, params
expect(response).to redirect_to unauthorized_path
end
end
Expand All @@ -122,7 +130,7 @@

context "where the producer has not granted create_variant_overrides permission to the hub" do
it "restricts access" do
spree_put :bulk_reset, params
put :bulk_reset, params
expect(response).to redirect_to unauthorized_path
end
end
Expand All @@ -131,7 +139,7 @@
let!(:er1) { create(:enterprise_relationship, parent: producer, child: hub, permissions_list: [:create_variant_overrides]) }

it "loads data" do
spree_put :bulk_reset, params
put :bulk_reset, params
expect(assigns[:hubs]).to eq [hub]
expect(assigns[:producers]).to eq [producer]
expect(assigns[:hub_permissions]).to eq Hash[hub.id, [producer.id]]
Expand All @@ -141,7 +149,7 @@
it "updates stock to default values where reset is enabled" do
expect(variant_override1.reload.count_on_hand).to eq 5 # reset enabled
expect(variant_override2.reload.count_on_hand).to eq 2 # reset disabled
spree_put :bulk_reset, params
put :bulk_reset, params
expect(variant_override1.reload.count_on_hand).to eq 7 # reset enabled
expect(variant_override2.reload.count_on_hand).to eq 2 # reset disabled
end
Expand All @@ -156,7 +164,7 @@

it "does not reset count_on_hand for variant_overrides not in params" do
expect {
spree_put :bulk_reset, params
put :bulk_reset, params
}.to_not change{ variant_override3.reload.count_on_hand }
end
end
Expand Down