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
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
@@ -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

@@ -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
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
@@ -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
@@ -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