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.

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

Scope image by product_id or variant_id in ImagesController #3510

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions api/app/controllers/spree/api/images_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def index
end

def show
@image = Spree::Image.accessible_by(current_ability, :read).find(params[:id])
@image = scope.images.accessible_by(current_ability, :read).find(params[:id])
respond_with(@image)
end

Expand All @@ -20,13 +20,13 @@ def create
end

def update
@image = Spree::Image.accessible_by(current_ability, :update).find(params[:id])
@image = scope.images.accessible_by(current_ability, :update).find(params[:id])
@image.update(image_params)
respond_with(@image, default_template: :show)
end

def destroy
@image = Spree::Image.accessible_by(current_ability, :destroy).find(params[:id])
@image = scope.images.accessible_by(current_ability, :destroy).find(params[:id])
@image.destroy
respond_with(@image, status: 204)
end
Expand Down
30 changes: 27 additions & 3 deletions api/spec/requests/spree/api/images_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ module Spree
end.to change(Image, :count).by(1)
end

context "working with an existing image" do
context "working with an existing product image" do
let!(:product_image) { product.master.images.create!(attachment: image('thinking-cat.jpg')) }

it "can get a single product image" do
Expand Down Expand Up @@ -63,18 +63,42 @@ module Spree

it "can update image data" do
expect(product_image.position).to eq(1)
put spree.api_variant_image_path(product.id, product_image), params: { image: { position: 2 } }
put spree.api_variant_image_path(product.master.id, product_image), params: { image: { position: 2 } }
expect(response.status).to eq(200)
expect(json_response).to have_attributes(attributes)
expect(product_image.reload.position).to eq(2)
end

it "can delete an image" do
delete spree.api_variant_image_path(product.id, product_image)
delete spree.api_variant_image_path(product.master.id, product_image)
expect(response.status).to eq(204)
expect { product_image.reload }.to raise_error(ActiveRecord::RecordNotFound)
end
end

context 'when image belongs to another product' do
let!(:product_image) { another_product.master.images.create!(attachment: image('thinking-cat.jpg')) }
let(:another_product) { create(:product) }

it "cannot get an image of another product" do
get spree.api_product_image_path(product.id, product_image)
expect(response.status).to eq(404)
expect(json_response['error']).to eq(I18n.t(:resource_not_found, scope: "spree.api"))
end

it "cannot get an image of another variant" do
get spree.api_variant_image_path(product.master.id, product_image)
expect(response.status).to eq(404)
expect(json_response['error']).to eq(I18n.t(:resource_not_found, scope: "spree.api"))
end

it "cannot update image of another product" do
expect(product_image.position).to eq(1)
put spree.api_variant_image_path(product.master.id, product_image), params: { image: { position: 2 } }
expect(response.status).to eq(404)
expect(json_response['error']).to eq(I18n.t(:resource_not_found, scope: "spree.api"))
end
end
end

context "as a non-admin" do
Expand Down