diff --git a/app/controllers/spree/admin/images_controller.rb b/app/controllers/spree/admin/images_controller.rb index 6084972102..cb1ebe40ac 100644 --- a/app/controllers/spree/admin/images_controller.rb +++ b/app/controllers/spree/admin/images_controller.rb @@ -37,13 +37,14 @@ module Spree if @object.save flash[:success] = flash_message_for(@object, :successfully_created) - redirect_to location_after_save + + respond_to do |format| + format.html { redirect_to location_after_save } + format.turbo_stream { render :update } + end else - respond_with(@object) + respond_with_error(@object.errors) end - rescue ActiveStorage::IntegrityError - @object.errors.add :attachment, :integrity_error - respond_with(@object) end def update @@ -53,16 +54,13 @@ module Spree if @object.update(permitted_resource_params) flash[:success] = flash_message_for(@object, :successfully_updated) - respond_with do |format| + respond_to do |format| format.html { redirect_to location_after_save } format.turbo_stream end else - respond_with(@object) + respond_with_error(@object.errors) end - rescue ActiveStorage::IntegrityError - @object.errors.add :attachment, :integrity_error - respond_with(@object) end def destroy @@ -112,6 +110,14 @@ module Spree :attachment, :viewable_id, :alt ) end + + def respond_with_error(errors) + @errors = errors.map(&:full_message) + respond_to do |format| + format.html { respond_with(@object) } + format.turbo_stream { render :edit } + end + end end end end diff --git a/app/views/spree/admin/images/edit.turbo_stream.haml b/app/views/spree/admin/images/edit.turbo_stream.haml index d2ffe7dd53..b3ca87329b 100644 --- a/app/views/spree/admin/images/edit.turbo_stream.haml +++ b/app/views/spree/admin/images/edit.turbo_stream.haml @@ -3,7 +3,13 @@ %h2= t(".title") -# Display image in the same way it appears in the shopfront popup - %p= image_tag @image.persisted? ? @image.url(:large) : Spree::Image.default_image_url(:large), width: 433, height: 433 + - if defined?(@errors) && !@errors.empty? + - @errors.each do |error| + %p + = error + - else + %p= image_tag @image.persisted? ? @image.url(:large) : Spree::Image.default_image_url(:large), width: 433, height: 433 + -# Submit as turbo stream to avoid full page reload. -# TODO: show loading indicator. diff --git a/public/invalid_image.jpg b/public/invalid_image.jpg new file mode 100755 index 0000000000..1af492828f Binary files /dev/null and b/public/invalid_image.jpg differ diff --git a/spec/requests/admin/images_spec.rb b/spec/requests/admin/images_spec.rb index e7c9584037..a62f9bef81 100644 --- a/spec/requests/admin/images_spec.rb +++ b/spec/requests/admin/images_spec.rb @@ -11,7 +11,7 @@ RSpec.describe "/admin/products/:product_id/images", type: :request do login_as_admin end - shared_examples "updating images" do + shared_examples "updating images" do |expected_http_status_code| let(:params) do { image: { @@ -21,14 +21,16 @@ RSpec.describe "/admin/products/:product_id/images", type: :request do } end - it "creates a new image and redirects" do + it "creates a new image and redirects unless called by turbo" do expect { subject product.reload }.to change{ product.image&.attachment&.filename.to_s } - expect(response.status).to eq 302 - expect(response.location).to end_with spree.admin_product_images_path(product) + expect(response.status).to eq expected_http_status_code + if expected_http_status_code == 302 + expect(response.location).to end_with spree.admin_product_images_path(product) + end expect(product.image.url(:product)).to end_with "logo.png" end @@ -49,8 +51,6 @@ RSpec.describe "/admin/products/:product_id/images", type: :request do product.reload }.not_to change{ product.image&.attachment&.filename.to_s } - pending "error status code" - expect(response).to be_unprocessable expect(response.body).to include "Attachment has an invalid content type" end end @@ -59,13 +59,30 @@ RSpec.describe "/admin/products/:product_id/images", type: :request do describe "POST /admin/products/:product_id/images" do subject { post(spree.admin_product_images_path(product), params:) } - it_behaves_like "updating images" + it_behaves_like "updating images", 302 + end + + describe "POST /admin/products/:product_id/images with turbo" do + subject { post(spree.admin_product_images_path(product), params:, as: :turbo_stream) } + + it_behaves_like "updating images", 200 end describe "PATCH /admin/products/:product_id/images/:id" do let!(:product) { create(:product_with_image) } - subject { patch(spree.admin_product_image_path(product, product.image), params:) } + subject { + patch(spree.admin_product_image_path(product, product.image), params:) + } - it_behaves_like "updating images" + it_behaves_like "updating images", 302 + end + + describe "PATCH /admin/products/:product_id/images/:id with turbo" do + let!(:product) { create(:product_with_image) } + subject { + patch(spree.admin_product_image_path(product, product.image), params:, as: :turbo_stream) + } + + it_behaves_like "updating images", 200 end end diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb index 4221418a46..3d081bf82d 100644 --- a/spec/system/admin/products_v3/products_spec.rb +++ b/spec/system/admin/products_v3/products_spec.rb @@ -1050,13 +1050,15 @@ RSpec.describe 'As an enterprise user, I can manage my products', feature: :admi describe "edit image" do shared_examples "updating image" do - it "saves product image" do + before do visit admin_products_url within row_containing_name("Apples") do click_on "Edit" end + end + it "saves product image" do within ".reveal-modal" do expect(page).to have_content "Edit product photo" expect_page_to_have_image(current_img_url) @@ -1073,6 +1075,25 @@ RSpec.describe 'As an enterprise user, I can manage my products', feature: :admi expect_page_to_have_image('500.jpg') end end + + it 'shows a modal telling not a valid image when uploading wrong type of file' do + within ".reveal-modal" do + attach_file 'image[attachment]', + Rails.public_path.join('Terms-of-service.pdf'), + visible: false + expect(page).to have_content /Attachment is not a valid image/ + expect(page).to have_content /Attachment has an invalid content type/ + end + end + + it 'shows a modal telling not a valid image when uploading a non valid image file' do + within ".reveal-modal" do + attach_file 'image[attachment]', + Rails.public_path.join('invalid_image.jpg'), + visible: false + expect(page).to have_content /Attachment is not a valid image/ + end + end end context "with existing image" do