diff --git a/app/components/confirm_modal_component.rb b/app/components/confirm_modal_component.rb index ccd0c67797..8149a729e3 100644 --- a/app/components/confirm_modal_component.rb +++ b/app/components/confirm_modal_component.rb @@ -1,14 +1,29 @@ # frozen_string_literal: true class ConfirmModalComponent < ModalComponent - def initialize(id:, confirm_actions: nil, reflex: nil, controller: nil, message: nil, - confirm_reflexes: nil) + # @param actions_alignment_class [String] possible classes: 'justify-space-around', 'justify-end' + def initialize( + id:, + reflex: nil, + controller: nil, + message: nil, + confirm_actions: nil, + confirm_reflexes: nil, + confirm_button_class: :primary, + confirm_button_text: I18n.t('js.admin.modals.confirm'), + cancel_button_text: I18n.t('js.admin.modals.cancel'), + actions_alignment_class: 'justify-space-around' + ) super(id:, close_button: true) @confirm_actions = confirm_actions @reflex = reflex @confirm_reflexes = confirm_reflexes @controller = controller @message = message + @confirm_button_class = confirm_button_class + @confirm_button_text = confirm_button_text + @cancel_button_text = cancel_button_text + @actions_alignment_class = actions_alignment_class end private diff --git a/app/components/confirm_modal_component/confirm_modal_component.html.haml b/app/components/confirm_modal_component/confirm_modal_component.html.haml index 1b3e1ff217..5fe1d1300b 100644 --- a/app/components/confirm_modal_component/confirm_modal_component.html.haml +++ b/app/components/confirm_modal_component/confirm_modal_component.html.haml @@ -5,6 +5,6 @@ = render @message if @message - .modal-actions - %input{ class: "button icon-plus #{close_button_class}", type: 'button', value: t('js.admin.modals.cancel'), "data-action": "click->modal#close" } - %input{ class: "button icon-plus primary", type: 'button', value: t('js.admin.modals.confirm'), "data-action": @confirm_actions, "data-reflex": @confirm_reflexes } + %div{ class: "modal-actions #{@actions_alignment_class}" } + %input{ class: "button icon-plus #{close_button_class}", type: 'button', value: @cancel_button_text, "data-action": "click->modal#close" } + %input{ id: 'modal-confirm-button', class: "button icon-plus #{@confirm_button_class}", type: 'button', value: @confirm_button_text, "data-action": @confirm_actions, "data-reflex": @confirm_reflexes } diff --git a/app/components/confirm_modal_component/confirm_modal_component.scss b/app/components/confirm_modal_component/confirm_modal_component.scss index ccbf18169c..2c54031f32 100644 --- a/app/components/confirm_modal_component/confirm_modal_component.scss +++ b/app/components/confirm_modal_component/confirm_modal_component.scss @@ -1,4 +1,22 @@ .modal-actions { display: flex; - justify-content: space-around; + + &.justify-space-around { + justify-content: space-around; + } + + &.justify-end { + justify-content: flex-end; + input[type="button"] { + margin: 0 5px; + } + + @media only screen and (max-width: 1024px) { + flex-direction: column; + justify-content: space-around; + input[type="button"] { + margin: 5px 0; + } + } + } } diff --git a/app/components/help_modal_component/help_modal_component.scss b/app/components/help_modal_component/help_modal_component.scss index 48f6255a2e..d323b47992 100644 --- a/app/components/help_modal_component/help_modal_component.scss +++ b/app/components/help_modal_component/help_modal_component.scss @@ -2,6 +2,9 @@ visibility: visible; position: fixed; top: 3em; + &.in { + padding: 1.2rem; + } } /* prevent arrow on selected admin menu item appearing above modal */ diff --git a/app/reflexes/products_reflex.rb b/app/reflexes/products_reflex.rb index 4154a7b61f..711f4a577c 100644 --- a/app/reflexes/products_reflex.rb +++ b/app/reflexes/products_reflex.rb @@ -6,20 +6,20 @@ class ProductsReflex < ApplicationReflex before_reflex :init_filters_params, :init_pagination_params def fetch - fetch_and_render_products + fetch_and_render_products_with_flash end def change_per_page @per_page = element.value.to_i @page = 1 - fetch_and_render_products + fetch_and_render_products_with_flash end def filter @page = 1 - fetch_and_render_products + fetch_and_render_products_with_flash end def clear_search @@ -28,7 +28,7 @@ class ProductsReflex < ApplicationReflex @category_id = nil @page = 1 - fetch_and_render_products + fetch_and_render_products_with_flash end def bulk_update @@ -46,6 +46,34 @@ class ProductsReflex < ApplicationReflex render_products_form_with_flash end + def delete_product + id = current_id_from_element(element) + product = product_finder(id).find_product + authorize! :delete, product + + if product.destroy + flash[:success] = I18n.t('admin.products_v3.delete_product.success') + else + flash[:error] = I18n.t('admin.products_v3.delete_product.error') + end + + fetch_and_render_products_with_flash + end + + def delete_variant + id = current_id_from_element(element) + variant = Spree::Variant.active.find(id) + authorize! :delete, variant + + if VariantDeleter.new.delete(variant) + flash[:success] = I18n.t('admin.products_v3.delete_variant.success') + else + flash[:error] = I18n.t('admin.products_v3.delete_variant.error') + end + + fetch_and_render_products_with_flash + end + private def init_filters_params @@ -63,7 +91,7 @@ class ProductsReflex < ApplicationReflex @per_page = element.dataset.perpage || params[:_per_page] || 15 end - def fetch_and_render_products + def fetch_and_render_products_with_flash fetch_products render_products end @@ -74,7 +102,8 @@ class ProductsReflex < ApplicationReflex html: render(partial: "admin/products_v3/content", locals: { products: @products, pagy: @pagy, search_term: @search_term, producer_options: producers, producer_id: @producer_id, - category_options: categories, category_id: @category_id }) + category_options: categories, category_id: @category_id, + flashes: flash }) ).broadcast cable_ready.replace_state( @@ -195,4 +224,12 @@ class ProductsReflex < ApplicationReflex params.permit(products: ::PermittedAttributes::Product.attributes) .to_h.with_indifferent_access end + + def product_finder(id) + ProductScopeQuery.new(current_user, { id: }) + end + + def current_id_from_element(element) + element.dataset.current_id + end end diff --git a/app/views/admin/products_v3/_content.html.haml b/app/views/admin/products_v3/_content.html.haml index 0cda71ff9a..b805747b71 100644 --- a/app/views/admin/products_v3/_content.html.haml +++ b/app/views/admin/products_v3/_content.html.haml @@ -1,6 +1,7 @@ #products-content .container .sixteen.columns + = render partial: 'admin/shared/flashes', locals: { flashes: } if defined? flashes = render partial: 'filters', locals: { search_term: search_term, producer_id: producer_id, producer_options: producer_options, diff --git a/app/views/admin/products_v3/_delete_modal.html.haml b/app/views/admin/products_v3/_delete_modal.html.haml new file mode 100644 index 0000000000..40b6fbd32d --- /dev/null +++ b/app/views/admin/products_v3/_delete_modal.html.haml @@ -0,0 +1,16 @@ +-# object_type can be 'variant' or 'product' +- base_translation_key = ".delete_#{object_type}_modal" +- delete_modal = ConfirmModalComponent.new(id: "#{object_type}-delete-modal", + confirm_button_text: t("#{base_translation_key}.confirmation_text"), + cancel_button_text: t("#{base_translation_key}.cancellation_text"), + confirm_button_class: :red, + actions_alignment_class: 'justify-end', + confirm_reflexes: "click->products#delete_#{object_type}", + confirm_actions: "click->modal#close", + ) += render delete_modal do + %h2.margin-bottom-20.black-text + = t("#{base_translation_key}.heading") + %p + = t("#{base_translation_key}.prompt") + .margin-bottom-50 diff --git a/app/views/admin/products_v3/_table.html.haml b/app/views/admin/products_v3/_table.html.haml index 0462529857..e34c4b786d 100644 --- a/app/views/admin/products_v3/_table.html.haml +++ b/app/views/admin/products_v3/_table.html.haml @@ -70,6 +70,10 @@ = render(VerticalEllipsisMenu::Component.new) do = link_to t('admin.products_page.actions.edit'), edit_admin_product_path(product) = link_to t('admin.products_page.actions.clone'), clone_admin_product_path(product) + %a{ "data-controller": "modal-link", "data-action": "click->modal-link#setModalDataSetOnConfirm click->modal-link#open", + "data-modal-link-target-value": "product-delete-modal", "class": "delete", + "data-modal-link-modal-dataset-value": {'data-current-id': product.id}.to_json } + = t('admin.products_page.actions.delete') - product.variants.each_with_index do |variant, variant_index| = form.fields_for("products][#{product_index}][variants_attributes][", variant, variant_index:) do |variant_form| @@ -108,3 +112,8 @@ %td.align-right = render(VerticalEllipsisMenu::Component.new) do = link_to t('admin.products_page.actions.edit'), edit_admin_product_variant_path(product, variant) + - if product.variants.size > 1 + %a{ "data-controller": "modal-link", "data-action": "click->modal-link#setModalDataSetOnConfirm click->modal-link#open", + "data-modal-link-target-value": "variant-delete-modal", "class": "delete", + "data-modal-link-modal-dataset-value": {'data-current-id': variant.id}.to_json } + = t('admin.products_page.actions.delete') diff --git a/app/views/admin/products_v3/index.html.haml b/app/views/admin/products_v3/index.html.haml index 82a4e5f38e..a7833c22df 100644 --- a/app/views/admin/products_v3/index.html.haml +++ b/app/views/admin/products_v3/index.html.haml @@ -17,3 +17,5 @@ .spinner = t('.loading') #products-content + - %w[product variant].each do |object_type| + = render partial: 'delete_modal', locals: { object_type: } diff --git a/app/webpacker/controllers/modal_link_controller.js b/app/webpacker/controllers/modal_link_controller.js index d4f58fb546..9eb765c35f 100644 --- a/app/webpacker/controllers/modal_link_controller.js +++ b/app/webpacker/controllers/modal_link_controller.js @@ -1,7 +1,7 @@ import { Controller } from "stimulus"; export default class extends Controller { - static values = { target: String }; + static values = { target: String, modalDataset: Object }; open() { let modal = document.getElementById(this.targetValue); @@ -12,6 +12,21 @@ export default class extends Controller { modalController.open(); } + setModalDataSetOnConfirm(event) { + try { + const modalId = this.targetValue; + const moodalConfirmButtonQuery = `#${modalId} #modal-confirm-button`; + const confirmButton = document.querySelector(moodalConfirmButtonQuery); + Object.keys(this.modalDatasetValue).forEach((datasetKey) => { + confirmButton.setAttribute(datasetKey, this.modalDatasetValue[datasetKey]); + }); + } catch (e) { + // In case of any type of error in setting the dataset value, stop the further actions i.e. opening the modal + event.stopImmediatePropagation(); + throw e; + } + } + getIdentifier() { return "modal"; } diff --git a/app/webpacker/controllers/products_controller.js b/app/webpacker/controllers/products_controller.js index c8a1dc6963..6fe8662f50 100644 --- a/app/webpacker/controllers/products_controller.js +++ b/app/webpacker/controllers/products_controller.js @@ -2,6 +2,7 @@ import ApplicationController from "./application_controller"; export default class extends ApplicationController { static targets = ["loading"]; + static values = { currentId: Number }; connect() { super.connect(); diff --git a/app/webpacker/css/admin_v3/shared/layout.scss b/app/webpacker/css/admin_v3/shared/layout.scss index 794e51f907..7950b4b711 100644 --- a/app/webpacker/css/admin_v3/shared/layout.scss +++ b/app/webpacker/css/admin_v3/shared/layout.scss @@ -131,3 +131,9 @@ display: none; } } + +// Text Colors +.black-text { + color: $near-black +} +//------------ diff --git a/config/locales/en.yml b/config/locales/en.yml index ef1f1f4595..5a3e560e96 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -580,6 +580,7 @@ en: actions: edit: Edit clone: Clone + delete: Delete adjustments: skipped_changing_canceled_order: "You can't change a cancelled order." # Common properties / models @@ -818,6 +819,17 @@ en: header: title: Bulk Edit Products loading: Loading your products + delete_modal: + delete_product_modal: + heading: "Delete product" + prompt: "This will permanently remove it from your list." + confirmation_text: "Delete product" + cancellation_text: "Keep product" + delete_variant_modal: + heading: "Delete variant" + prompt: "This will permanently remove it from your list." + confirmation_text: "Delete variant" + cancellation_text: "Keep variant" sort: pagination: total_html: "%{total} products found for your search criteria. Showing %{from} to %{to}." @@ -856,6 +868,12 @@ en: reset: Discard changes bulk_update: success: Changes saved + delete_product: + success: Successfully deleted the product + error: Unable to delete the product + delete_variant: + success: Successfully deleted the variant + error: Unable to delete the variant product_import: title: Product Import file_not_found: File not found or could not be opened diff --git a/spec/reflexes/products_reflex_spec.rb b/spec/reflexes/products_reflex_spec.rb index a79b32b326..fa87b367e4 100644 --- a/spec/reflexes/products_reflex_spec.rb +++ b/spec/reflexes/products_reflex_spec.rb @@ -160,6 +160,80 @@ describe ProductsReflex, type: :reflex, feature: :admin_style_v3 do end end end + + describe '#delete_product' do + let(:product) { create(:simple_product) } + let(:action_name) { :delete_product } + + subject { build_reflex(method_name: action_name, **context) } + + before { subject.element.dataset.current_id = product.id } + + context 'given that the current user is admin' do + let(:current_user) { create(:admin_user) } + + it 'should successfully delete the product' do + subject.run(action_name) + product.reload + expect(product.deleted_at).not_to be_nil + expect(flash[:success]).to eq('Successfully deleted the product') + end + + it 'should be failed to delete the product' do + # mock db query failure + allow_any_instance_of(Spree::Product).to receive(:destroy).and_return(false) + subject.run(action_name) + product.reload + expect(product.deleted_at).to be_nil + expect(flash[:error]).to eq('Unable to delete the product') + end + end + + context 'given that the current user is not admin' do + let(:current_user) { create(:user) } + + it 'should raise the access denied exception' do + expect { subject.run(action_name) }.to raise_exception(CanCan::AccessDenied) + end + end + end + + describe '#delete_variant' do + let(:variant) { create(:variant) } + let(:action_name) { :delete_variant } + + subject { build_reflex(method_name: action_name, **context) } + + before { subject.element.dataset.current_id = variant.id } + + context 'given that the current user is admin' do + let(:current_user) { create(:admin_user) } + + it 'should successfully delete the variant' do + subject.run(action_name) + variant.reload + expect(variant.deleted_at).not_to be_nil + expect(flash[:success]).to eq('Successfully deleted the variant') + end + + it 'should be failed to delete the product' do + # mock db query failure + allow_any_instance_of(Spree::Variant).to receive(:destroy).and_return(false) + subject.run(action_name) + variant.reload + expect(variant.deleted_at).to be_nil + expect(flash[:error]).to eq('Unable to delete the variant') + end + end + + context 'given that the current user is not admin' do + let(:current_user) { create(:user) } + + it 'should raise the access denied exception' do + expect { subject.run(action_name) }.to raise_exception(CanCan::AccessDenied) + end + end + end end # Build and run a reflex using the context diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb index c34e40b934..f62be46a48 100644 --- a/spec/system/admin/products_v3/products_spec.rb +++ b/spec/system/admin/products_v3/products_spec.rb @@ -410,6 +410,181 @@ describe 'As an admin, I can see the new product page', feature: :admin_style_v3 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 + 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 + + 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) + + # 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) + 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 + end + def create_products(amount) amount.times do |i| create(:simple_product, name: "product #{i}") @@ -450,4 +625,12 @@ describe 'As an admin, I can see the new product page', feature: :admin_style_v3 def row_containing_name(value) "tr:has(input[aria-label=Name][value='#{value}'])" end + + # Wait for an element with the given CSS selector and class to be present + def wait_for_class(selector, class_name) + max_wait_time = Capybara.default_max_wait_time + Timeout.timeout(max_wait_time) do + sleep(0.1) until page.has_css?(selector, class: class_name, visible: false) + end + end end