diff --git a/app/controllers/spree/admin/images_controller.rb b/app/controllers/spree/admin/images_controller.rb index 44b26278f8..0e0890a229 100644 --- a/app/controllers/spree/admin/images_controller.rb +++ b/app/controllers/spree/admin/images_controller.rb @@ -36,6 +36,9 @@ module Spree else respond_with(@object) end + rescue ActiveStorage::IntegrityError + @object.errors.add :attachment, :integrity_error + respond_with(@object) end def update @@ -48,6 +51,9 @@ module Spree else respond_with(@object) end + rescue ActiveStorage::IntegrityError + @object.errors.add :attachment, :integrity_error + respond_with(@object) end def destroy diff --git a/app/reflexes/products_reflex.rb b/app/reflexes/products_reflex.rb index ec6c28f9fd..e6dd59e41d 100644 --- a/app/reflexes/products_reflex.rb +++ b/app/reflexes/products_reflex.rb @@ -77,10 +77,13 @@ class ProductsReflex < ApplicationReflex def edit_image id = current_id_from_element(element) product = product_finder(id).find_product + image = product.image + + image = Spree::Image.new(viewable: product) if product.image.blank? morph "#modal-component", render(partial: "admin/products_v3/edit_image", - locals: { product:, image: product.image, return_url: url }) + locals: { product:, image:, return_url: url }) end private diff --git a/app/views/admin/products_v3/_edit_image.html.haml b/app/views/admin/products_v3/_edit_image.html.haml index 7af268d64d..b4f45ace5d 100644 --- a/app/views/admin/products_v3/_edit_image.html.haml +++ b/app/views/admin/products_v3/_edit_image.html.haml @@ -1,10 +1,10 @@ = render ModalComponent.new id: "#modal_edit_product_image_#{image.id}", instant: true, close_button: false do %h2= t(".title") - %p= image_tag image.variant(:product) + %p= image_tag image.persisted? ? image.url(:product) : Spree::Image.default_image_url(:product) -# Submit to controller, because StimulusReflex doesn't support file uploads - = form_for [:admin, product, image], url: admin_product_image_path(product, image), + = form_for [:admin, product, image], html: { multipart: true }, data: { controller: "form" } do |f| %input{ type: :hidden, name: :return_url, value: return_url} = f.hidden_field :viewable_id, value: product.id diff --git a/app/views/admin/products_v3/_table.html.haml b/app/views/admin/products_v3/_table.html.haml index c508972722..faf517bd3e 100644 --- a/app/views/admin/products_v3/_table.html.haml +++ b/app/views/admin/products_v3/_table.html.haml @@ -49,13 +49,9 @@ %tbody.relaxed{ 'data-record-id': product_form.object.id } %tr %td.with-image - - if product.image.present? - %a.image-field{ href: edit_admin_product_image_path(product, product.image), data: { controller: "modal", reflex: "click->products#edit_image", "current-id": product.id} } - = image_tag product.image&.url(:mini), width: 40, height: 40 - .button.secondary.mini= t('admin.products_page.image.edit') - - else - .image-field - = image_tag Spree::Image.default_image_url(:mini), width: 40, height: 40 + %a.image-field{ href: admin_product_images_path(product), data: { controller: "modal", reflex: "click->products#edit_image", "current-id": product.id} } + = image_tag product.image&.url(:mini) || Spree::Image.default_image_url(:mini), width: 40, height: 40 + .button.secondary.mini= t('admin.products_page.image.edit') %td.field.align-left.header = product_form.hidden_field :id = product_form.text_field :name, 'aria-label': t('admin.products_page.columns.name') diff --git a/config/locales/en.yml b/config/locales/en.yml index d9d16c50f8..2c40d8bbfb 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -99,6 +99,10 @@ en: models: enterprise_fee: inherit_tax_requires_per_item_calculator: "Inheriting the tax categeory requires a per-item calculator." + spree/image: + attributes: + attachment: + integrity_error: "failed to load. Please check to ensure the file is not corrupt, and try again." spree/user: attributes: email: diff --git a/spec/requests/admin/images_spec.rb b/spec/requests/admin/images_spec.rb new file mode 100644 index 0000000000..1ab0f05fcf --- /dev/null +++ b/spec/requests/admin/images_spec.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +require "spec_helper" + +describe "/admin/products/:product_id/images", type: :request do + include AuthenticationHelper + + let!(:product) { create(:product) } + + before do + login_as_admin + end + + shared_examples "updating images" do + let(:params) do + { + image: { + attachment: fixture_file_upload("logo.png", "image/png"), + viewable_id: product.id, + } + } + end + + it "creates a new image and redirects" 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(product.image.url(:product)).to end_with "logo.png" + end + + context "with wrong type of file" do + let(:params) do + { + image: { + attachment: fixture_file_upload("sample_file_120_products.csv", "text/csv"), + viewable_id: product.id, + } + } + end + + it "responds with an error" do + expect { + subject + product.reload + }.to_not 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 + end + + describe "POST /admin/products/:product_id/images" do + subject { post(spree.admin_product_images_path(product), params:) } + + it_behaves_like "updating images" + 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:) } + + it_behaves_like "updating images" + end +end diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb index f62be46a48..642265abea 100644 --- a/spec/system/admin/products_v3/products_spec.rb +++ b/spec/system/admin/products_v3/products_spec.rb @@ -2,7 +2,7 @@ require "system_helper" -describe 'As an admin, I can see the new product page', feature: :admin_style_v3 do +describe 'As an admin, I can manage products', feature: :admin_style_v3 do include WebHelper include AuthenticationHelper include FileHelper @@ -162,36 +162,6 @@ describe 'As an admin, I can see the new product page', feature: :admin_style_v3 end end - describe "Actions columns (edit)" do - let!(:variant_a1) { - create(:variant, - product: product_a, - display_name: "Medium box", - sku: "APL-01", - price: 5.25) - } - let!(:product_a) { create(:simple_product, name: "Apples", sku: "APL-00") } - - before do - visit admin_products_url - end - - it "shows an actions memu with an edit link when clicking on icon for product" do - within row_containing_name("Apples") do - page.find(".vertical-ellipsis-menu").click - expect(page).to have_link "Edit", href: spree.edit_admin_product_path(product_a) - end - end - - it "shows an actions memu with an edit link when clicking on icon for variant" do - within row_containing_name("Medium box") do - page.find(".vertical-ellipsis-menu").click - expect(page).to have_link "Edit", - href: spree.edit_admin_product_variant_path(product_a, variant_a1) - end - end - end - describe "updating" do let!(:variant_a1) { create(:variant, product: product_a, display_name: "Medium box", sku: "APL-01", price: 5.25, @@ -354,112 +324,50 @@ describe 'As an admin, I can see the new product page', feature: :admin_style_v3 end end - describe "Cloning Feature" do - let!(:variant_a1) { - create(:variant, - product: product_a, - display_name: "Medium box", - sku: "APL-01", - price: 5.25) - } - let(:product_a) { create(:simple_product, name: "Apples", sku: "APL-00") } + describe "edit image" do + shared_examples "updating image" do + it "saves product image" do + visit admin_products_url - before do - visit admin_products_url - end - - describe "Actions columns (clone)" do - it "shows an actions menu with a clone link when clicking on icon for product" do within row_containing_name("Apples") do - page.find(".vertical-ellipsis-menu").click - expect(page).to have_link "Clone", href: spree.clone_admin_product_path(product_a) + click_on "Edit" end - within row_containing_name("Medium box") do - page.find(".vertical-ellipsis-menu").click - expect(page).to_not have_link "Clone", href: spree.clone_admin_product_path(product_a) + within ".reveal-modal" do + expect(page).to have_content "Edit product photo" + expect_page_to_have_image(current_img_url) + + # Upload a new image file + attach_file 'image[attachment]', Rails.public_path.join('500.jpg'), visible: false + # It uploads automatically + end + + expect(page).to have_content /Image has been successfully (updated|created)/ + expect(product.image.reload.url(:product)).to match /500.jpg$/ + + within row_containing_name("Apples") do + expect_page_to_have_image('500.jpg') end end end - describe "Cloning product" do - it "shows the cloned product on page when clicked on the cloned option" do - within "table.products" do - # Gather input values, because page.content doesn't include them. - input_content = page.find_all('input[type=text]').map(&:value).join + context "with existing image" do + let!(:product) { create(:product_with_image, name: "Apples") } + let(:current_img_url) { product.image.url(:product) } - # Products does not include the cloned product. - expect(input_content).to_not match /COPY OF Apples/ - end + include_examples "updating image" + end - within row_containing_name("Apples") do - page.find(".vertical-ellipsis-menu").click - click_link('Clone') - end + context "with default image" do + let!(:product) { create(:product, name: "Apples") } + let(:current_img_url) { Spree::Image.default_image_url(:product) } - expect(page).to have_content "Product cloned" - - within "table.products" do - # Gather input values, because page.content doesn't include them. - input_content = page.find_all('input[type=text]').map(&:value).join - - # Products include the cloned product. - expect(input_content).to match /COPY OF Apples/ - end - end + include_examples "updating image" end end - describe "Deleting Feature" do - let!(:product_a) { create(:simple_product, name: "Apples", sku: "APL-00") } - let(:delete_option_selector) { "a[data-controller='modal-link'].delete" } - let(:product_selector) { row_containing_name("Apples") } - let(:variant_selector) { row_containing_name("Medium box") } - let(:default_variant_selector) { "tr:has(input[aria-label=Price][value='#{product_a.price}'])" } - - before do - visit admin_products_url - end - - describe "Actions columns (delete)" do - it "shows an actions menu with a delete link when clicking on icon for product. " \ - "doesn't show delete link for the single variant" do - within product_selector do - page.find(".vertical-ellipsis-menu").click - expect(page).to have_css(delete_option_selector) - end - page.find("div#content").click # to close the vertical actions menu - - # to select the default variant - within default_variant_selector do - page.find(".vertical-ellipsis-menu").click - expect(page).to_not have_css(delete_option_selector) - end - end - - it "shows an actions menu with a delete link when clicking on icon for variant" \ - "if have multiple" do - create(:variant, - product: product_a, - display_name: "Medium box", - sku: "APL-01", - price: 5.25) - - # to select the default variant - within default_variant_selector do - page.find(".vertical-ellipsis-menu").click - expect(page).to have_css(delete_option_selector) - end - page.find("div#content").click # to close the vertical actions menu - - within variant_selector do - page.find(".vertical-ellipsis-menu").click - expect(page).to have_css(delete_option_selector) - end - end - end - - describe "Delete Action" do + describe "actions" do + describe "edit" do let!(:variant_a1) { create(:variant, product: product_a, @@ -467,118 +375,255 @@ describe 'As an admin, I can see the new product page', feature: :admin_style_v3 sku: "APL-01", price: 5.25) } - let(:modal_selector) { "div[data-modal-target=modal]" } - let(:dismiss_button_selector) { "button[data-action='click->flash#close']" } + let!(:product_a) { create(:simple_product, name: "Apples", sku: "APL-00") } - context "when 'keep product/variant' is selected" do - it 'should not delete the product/variant' do - # Keep Product - within product_selector do + before do + visit admin_products_url + end + + it "shows an actions menu with an edit link for product and variant" do + within row_containing_name("Apples") do + page.find(".vertical-ellipsis-menu").click + expect(page).to have_link "Edit", href: spree.edit_admin_product_path(product_a) + end + + within row_containing_name("Medium box") do + page.find(".vertical-ellipsis-menu").click + expect(page).to have_link "Edit", + href: spree.edit_admin_product_variant_path(product_a, + variant_a1) + end + end + end + + describe "clone" do + let!(:variant_a1) { + create(:variant, + product: product_a, + display_name: "Medium box", + sku: "APL-01", + price: 5.25) + } + let(:product_a) { create(:simple_product, name: "Apples", sku: "APL-00") } + + before do + visit admin_products_url + end + + describe "Actions columns (clone)" do + it "shows an actions menu with a clone link when clicking on icon for product" do + within row_containing_name("Apples") do page.find(".vertical-ellipsis-menu").click - page.find(delete_option_selector).click - end - keep_button_selector = "input[type=button][value='Keep product']" - within modal_selector do - page.find(keep_button_selector).click + expect(page).to have_link "Clone", href: spree.clone_admin_product_path(product_a) end - expect(page).to_not have_selector(modal_selector) - expect(page).to have_selector(product_selector) - - # Keep Variant - within variant_selector do + within row_containing_name("Medium box") do page.find(".vertical-ellipsis-menu").click - page.find(delete_option_selector).click + expect(page).to_not have_link "Clone", href: spree.clone_admin_product_path(product_a) end - keep_button_selector = "input[type=button][value='Keep variant']" - within modal_selector do - page.find(keep_button_selector).click - end - - expect(page).to_not have_selector(modal_selector) - expect(page).to have_selector(variant_selector) end end - context "when 'delete product/variant' is selected" do - let(:success_flash_message_selector) { "div.flash.success" } - let(:error_flash_message_selector) { "div.flash.error" } - it 'should successfully delete the product/variant' do - # Delete Variant - within variant_selector do + describe "Cloning product" do + it "shows the cloned product on page when clicked on the cloned option" do + within "table.products" do + # Gather input values, because page.content doesn't include them. + input_content = page.find_all('input[type=text]').map(&:value).join + + # Products does not include the cloned product. + expect(input_content).to_not match /COPY OF Apples/ + end + + within row_containing_name("Apples") do page.find(".vertical-ellipsis-menu").click - page.find(delete_option_selector).click + click_link('Clone') end - delete_button_selector = "input[type=button][value='Delete variant']" - within modal_selector do - page.find(delete_button_selector).click - end + expect(page).to have_content "Product cloned" - expect(page).to_not have_selector(modal_selector) - # Make sure the products loading spinner is hidden - wait_for_class('.spinner-overlay', 'hidden') - expect(page).to_not have_selector(variant_selector) - within success_flash_message_selector do - expect(page).to have_content("Successfully deleted the variant") - page.find(dismiss_button_selector).click - end + within "table.products" do + # Gather input values, because page.content doesn't include them. + input_content = page.find_all('input[type=text]').map(&:value).join - # Delete product + # Products include the cloned product. + expect(input_content).to match /COPY OF Apples/ + end + end + end + end + + describe "delete" do + let!(:product_a) { create(:simple_product, name: "Apples", sku: "APL-00") } + let(:delete_option_selector) { "a[data-controller='modal-link'].delete" } + let(:product_selector) { row_containing_name("Apples") } + let(:variant_selector) { row_containing_name("Medium box") } + let(:default_variant_selector) { + "tr:has(input[aria-label=Price][value='#{product_a.price}'])" + } + + before do + visit admin_products_url + end + + describe "Actions columns (delete)" do + it "shows an actions menu with a delete link when clicking on icon for product. " \ + "doesn't show delete link for the single variant" do within product_selector do page.find(".vertical-ellipsis-menu").click - page.find(delete_option_selector).click + expect(page).to have_css(delete_option_selector) end - delete_button_selector = "input[type=button][value='Delete product']" - within modal_selector do - page.find(delete_button_selector).click - end - expect(page).to_not have_selector(modal_selector) - # Make sure the products loading spinner is hidden - wait_for_class('.spinner-overlay', 'hidden') - expect(page).to_not have_selector(product_selector) - within success_flash_message_selector do - expect(page).to have_content("Successfully deleted the product") + page.find("div#content").click # to close the vertical actions menu + + # to select the default variant + within default_variant_selector do + page.find(".vertical-ellipsis-menu").click + expect(page).to_not have_css(delete_option_selector) end end - it 'should be failed to delete the product/variant' do - allow_any_instance_of(Spree::Product).to receive(:destroy).and_return(false) - allow_any_instance_of(Spree::Variant).to receive(:destroy).and_return(false) + it "shows an actions menu with a delete link when clicking on icon for variant" \ + "if have multiple" do + create(:variant, + product: product_a, + display_name: "Medium box", + sku: "APL-01", + price: 5.25) + + # to select the default variant + within default_variant_selector do + page.find(".vertical-ellipsis-menu").click + expect(page).to have_css(delete_option_selector) + end + page.find("div#content").click # to close the vertical actions menu - # Delete Variant within variant_selector do page.find(".vertical-ellipsis-menu").click - page.find(delete_option_selector).click + expect(page).to have_css(delete_option_selector) + end + end + end + + describe "Delete Action" do + let!(:variant_a1) { + create(:variant, + product: product_a, + display_name: "Medium box", + sku: "APL-01", + price: 5.25) + } + let(:modal_selector) { "div[data-modal-target=modal]" } + let(:dismiss_button_selector) { "button[data-action='click->flash#close']" } + + context "when 'keep product/variant' is selected" do + it 'should not delete the product/variant' do + # Keep Product + within product_selector do + page.find(".vertical-ellipsis-menu").click + page.find(delete_option_selector).click + end + keep_button_selector = "input[type=button][value='Keep product']" + within modal_selector do + page.find(keep_button_selector).click + end + + expect(page).to_not have_selector(modal_selector) + expect(page).to have_selector(product_selector) + + # Keep Variant + within variant_selector do + page.find(".vertical-ellipsis-menu").click + page.find(delete_option_selector).click + end + keep_button_selector = "input[type=button][value='Keep variant']" + within modal_selector do + page.find(keep_button_selector).click + end + + expect(page).to_not have_selector(modal_selector) + expect(page).to have_selector(variant_selector) + end + end + + context "when 'delete product/variant' is selected" do + let(:success_flash_message_selector) { "div.flash.success" } + let(:error_flash_message_selector) { "div.flash.error" } + it 'should successfully delete the product/variant' do + # Delete Variant + within variant_selector do + page.find(".vertical-ellipsis-menu").click + page.find(delete_option_selector).click + end + + delete_button_selector = "input[type=button][value='Delete variant']" + within modal_selector do + page.find(delete_button_selector).click + end + + expect(page).to_not have_selector(modal_selector) + # Make sure the products loading spinner is hidden + wait_for_class('.spinner-overlay', 'hidden') + expect(page).to_not have_selector(variant_selector) + within success_flash_message_selector do + expect(page).to have_content("Successfully deleted the variant") + page.find(dismiss_button_selector).click + end + + # Delete product + within product_selector do + page.find(".vertical-ellipsis-menu").click + page.find(delete_option_selector).click + end + delete_button_selector = "input[type=button][value='Delete product']" + within modal_selector do + page.find(delete_button_selector).click + end + expect(page).to_not have_selector(modal_selector) + # Make sure the products loading spinner is hidden + wait_for_class('.spinner-overlay', 'hidden') + expect(page).to_not have_selector(product_selector) + within success_flash_message_selector do + expect(page).to have_content("Successfully deleted the product") + end end - delete_button_selector = "input[type=button][value='Delete variant']" - within modal_selector do - page.find(delete_button_selector).click - end + it 'should be failed to delete the product/variant' do + allow_any_instance_of(Spree::Product).to receive(:destroy).and_return(false) + allow_any_instance_of(Spree::Variant).to receive(:destroy).and_return(false) - expect(page).to_not have_selector(modal_selector) - sleep(0.5) # delay for loading spinner to complete - expect(page).to have_selector(variant_selector) - within error_flash_message_selector do - expect(page).to have_content("Unable to delete the variant") - page.find(dismiss_button_selector).click - end + # Delete Variant + within variant_selector do + page.find(".vertical-ellipsis-menu").click + page.find(delete_option_selector).click + end - # Delete product - within product_selector do - page.find(".vertical-ellipsis-menu").click - page.find(delete_option_selector).click - end - delete_button_selector = "input[type=button][value='Delete product']" - within modal_selector do - page.find(delete_button_selector).click - end - expect(page).to_not have_selector(modal_selector) - sleep(0.5) # delay for loading spinner to complete - expect(page).to have_selector(product_selector) - within error_flash_message_selector do - expect(page).to have_content("Unable to delete the product") + delete_button_selector = "input[type=button][value='Delete variant']" + within modal_selector do + page.find(delete_button_selector).click + end + + expect(page).to_not have_selector(modal_selector) + sleep(0.5) # delay for loading spinner to complete + expect(page).to have_selector(variant_selector) + within error_flash_message_selector do + expect(page).to have_content("Unable to delete the variant") + page.find(dismiss_button_selector).click + end + + # Delete product + within product_selector do + page.find(".vertical-ellipsis-menu").click + page.find(delete_option_selector).click + end + delete_button_selector = "input[type=button][value='Delete product']" + within modal_selector do + page.find(delete_button_selector).click + end + expect(page).to_not have_selector(modal_selector) + sleep(0.5) # delay for loading spinner to complete + expect(page).to have_selector(product_selector) + within error_flash_message_selector do + expect(page).to have_content("Unable to delete the product") + end end end end @@ -633,4 +678,8 @@ describe 'As an admin, I can see the new product page', feature: :admin_style_v3 sleep(0.1) until page.has_css?(selector, class: class_name, visible: false) end end + + def expect_page_to_have_image(url) + expect(page).to have_selector("img[src$='#{url}']") + end end