Merge branch 'master' into RedundantInterpolationCops

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

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.

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

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