diff --git a/app/controllers/admin/products_v3_controller.rb b/app/controllers/admin/products_v3_controller.rb index 77098ee7bb..2954094d6d 100644 --- a/app/controllers/admin/products_v3_controller.rb +++ b/app/controllers/admin/products_v3_controller.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +# rubocop:disable Metrics/ClassLength module Admin class ProductsV3Controller < Spree::Admin::BaseController helper ProductsHelper @@ -31,6 +32,42 @@ module Admin end end + def destroy + @record = ProductScopeQuery.new( + spree_current_user, + { id: params[:id] } + ).find_product + + status = :ok + if @record.destroy + flash.now[:success] = t('.delete_product.success') + else + flash.now[:error] = t('.delete_product.error') + status = :internal_server_error + end + + respond_with do |format| + format.turbo_stream { render :destroy_product_variant, status: } + end + end + + def destroy_variant + @record = Spree::Variant.active.find(params[:id]) + authorize! :delete, @record + + status = :ok + if VariantDeleter.new.delete(@record) + flash.now[:success] = t('.delete_variant.success') + else + flash.now[:error] = t('.delete_variant.error') + status = :internal_server_error + end + + respond_with do |format| + format.turbo_stream { render :destroy_product_variant, status: } + end + end + def index_url(params) "/admin/products?#{params.to_query}" # todo: fix routing so this can be automaticly generated end @@ -147,3 +184,4 @@ module Admin end end end +# rubocop:enable Metrics/ClassLength diff --git a/app/models/spree/ability.rb b/app/models/spree/ability.rb index e95d73b5fc..381d0b8502 100644 --- a/app/models/spree/ability.rb +++ b/app/models/spree/ability.rb @@ -192,7 +192,7 @@ module Spree OpenFoodNetwork::Permissions.new(user).managed_product_enterprises.include? product.supplier end - can [:admin, :index, :bulk_update], :products_v3 + can [:admin, :index, :bulk_update, :destroy, :destroy_variant], :products_v3 can [:create], Spree::Variant can [:admin, :index, :read, :edit, diff --git a/app/reflexes/products_reflex.rb b/app/reflexes/products_reflex.rb index 4c8aabc0d2..a2feef4d28 100644 --- a/app/reflexes/products_reflex.rb +++ b/app/reflexes/products_reflex.rb @@ -21,34 +21,6 @@ class ProductsReflex < ApplicationReflex fetch_and_render_products_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 @@ -202,12 +174,4 @@ 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/_delete_modal.html.haml b/app/views/admin/products_v3/_delete_modal.html.haml index 40b6fbd32d..87208c5dbe 100644 --- a/app/views/admin/products_v3/_delete_modal.html.haml +++ b/app/views/admin/products_v3/_delete_modal.html.haml @@ -5,8 +5,8 @@ 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", + controller: "products", + confirm_actions: "click->products#delete_#{object_type} click->modal#close", ) = render delete_modal do %h2.margin-bottom-20.black-text diff --git a/app/views/admin/products_v3/_product_row.html.haml b/app/views/admin/products_v3/_product_row.html.haml index 0908652009..d009b544ed 100644 --- a/app/views/admin/products_v3/_product_row.html.haml +++ b/app/views/admin/products_v3/_product_row.html.haml @@ -43,5 +43,5 @@ = link_to t('admin.products_page.actions.clone'), clone_admin_product_path(product), 'data-turbo': false %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 } + "data-modal-link-modal-dataset-value": {'data-delete-path': admin_product_destroy_path(product)}.to_json } = t('admin.products_page.actions.delete') diff --git a/app/views/admin/products_v3/_table.html.haml b/app/views/admin/products_v3/_table.html.haml index 27d5d7434f..464130d181 100644 --- a/app/views/admin/products_v3/_table.html.haml +++ b/app/views/admin/products_v3/_table.html.haml @@ -63,7 +63,7 @@ %th.align-right= t('admin.products_page.columns.actions') - products.each_with_index do |product, product_index| = form.fields_for("products", product, index: product_index) do |product_form| - %tbody.relaxed{ data: { 'record-id': product_form.object.id, + %tbody.relaxed{ id: dom_id(product), data: { 'record-id': product_form.object.id, controller: "nested-form product", action: 'rails-nested-form:add->bulk-form#registerElements rails-nested-form:remove->bulk-form#toggleFormChanged' } } %tr @@ -71,7 +71,7 @@ - product.variants.each_with_index do |variant, variant_index| = form.fields_for("products][#{product_index}][variants_attributes][", variant, index: variant_index) do |variant_form| - %tr.condensed{ 'data-controller': "variant", 'class': "nested-form-wrapper", 'data-new-record': variant.new_record? ? "true" : false } + %tr.condensed{ id: dom_id(variant), 'data-controller': "variant", 'class': "nested-form-wrapper", 'data-new-record': variant.new_record? ? "true" : false } = render partial: 'variant_row', locals: { variant:, f: variant_form, category_options:, tax_category_options: } = form.fields_for("products][#{product_index}][variants_attributes][NEW_RECORD", product.variants.build) do |new_variant_form| diff --git a/app/views/admin/products_v3/_variant_row.html.haml b/app/views/admin/products_v3/_variant_row.html.haml index f60b2522eb..3b3b38ac54 100644 --- a/app/views/admin/products_v3/_variant_row.html.haml +++ b/app/views/admin/products_v3/_variant_row.html.haml @@ -66,7 +66,7 @@ - if variant.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 } + "data-modal-link-modal-dataset-value": {'data-delete-path': admin_destroy_variant_path(variant)}.to_json } = t('admin.products_page.actions.delete') - else %a{ 'data-action': "nested-form#remove", class: 'delete' } diff --git a/app/views/admin/products_v3/destroy_product_variant.turbo_stream.haml b/app/views/admin/products_v3/destroy_product_variant.turbo_stream.haml new file mode 100644 index 0000000000..645b265c41 --- /dev/null +++ b/app/views/admin/products_v3/destroy_product_variant.turbo_stream.haml @@ -0,0 +1,5 @@ +- # @record can either be Product or Variant +- unless flash[:error] + = turbo_stream.remove(dom_id(@record)) += turbo_stream.append "flashes" do + = render(partial: 'admin/shared/flashes', locals: { flashes: flash }) diff --git a/app/webpacker/controllers/products_controller.js b/app/webpacker/controllers/products_controller.js index 9ec3ac1a86..720b181382 100644 --- a/app/webpacker/controllers/products_controller.js +++ b/app/webpacker/controllers/products_controller.js @@ -17,6 +17,14 @@ export default class extends ApplicationController { this.hideLoading(); } + delete_product() { + this.#deleteByRecordType('product'); + } + + delete_variant() { + this.#deleteByRecordType('variant'); + } + showLoading = () => { if (this.getLoadingController()) { this.getLoadingController().showLoading(); @@ -39,4 +47,46 @@ export default class extends ApplicationController { "loading" )); }; + + // +recordType+ can either be 'product' or 'variant' + #deleteByRecordType(recordType) { + const deletePath = document.querySelector(`#${recordType}-delete-modal #modal-confirm-button`).getAttribute('data-delete-path'); + const elementToBeRemoved = this.#getElementToBeRemoved(deletePath, recordType); + + const handleSlideOutAnimationEnd = async () => { + // in case of test env, we won't be having csrf token + const csrfToken = document.querySelector('meta[name="csrf-token"]')?.getAttribute('content'); + + try { + const response = await fetch(deletePath, { + method: 'DELETE', + headers: { + Accept: 'text/vnd.turbo-stream.html', + 'X-CSRF-Token': csrfToken, + } + }); + // need to render the turboStream message, that's why not throwing error here + if(response.status === 500) elementToBeRemoved.classList.remove('slide-out'); + + const responseTurboStream = await response.text(); + Turbo.renderStreamMessage(responseTurboStream); + } catch(error) { + console.error(error.message); + elementToBeRemoved.classList.remove('slide-out'); + } + finally { + elementToBeRemoved.removeEventListener('animationend', handleSlideOutAnimationEnd); + } + }; + + elementToBeRemoved.classList.add('slide-out'); + elementToBeRemoved.addEventListener('animationend', handleSlideOutAnimationEnd); + }; + + #getElementToBeRemoved(path, recordType) { + const recordId = path.substring(path.lastIndexOf('/') + 1); + const elementDomId = `${recordType}_${recordId}`; + + return document.getElementById(elementDomId); + }; } diff --git a/app/webpacker/css/admin/products_v3.scss b/app/webpacker/css/admin/products_v3.scss index a06019bebb..2b277f0e33 100644 --- a/app/webpacker/css/admin/products_v3.scss +++ b/app/webpacker/css/admin/products_v3.scss @@ -413,4 +413,19 @@ } } } + + @keyframes slideOutLeft { + from { + transform: translateX(0); + opacity: 1; + } + to { + transform: translateX(-100%); + opacity: 0; + } + } + + .slide-out { + animation: slideOutLeft 0.5s forwards; + } } diff --git a/config/routes/admin.rb b/config/routes/admin.rb index c82fb18315..21caa2b677 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -75,6 +75,9 @@ Openfoodnetwork::Application.routes.draw do # This might be easier to arrange once we rename the controller to plain old "products" post '/products/bulk_update', to: 'products_v3#bulk_update' get '/products', to: 'products_v3#index' + # we already have DELETE admin/products/:id here + delete 'products_v3/:id', to: 'products_v3#destroy', as: 'product_destroy' + delete 'products_v3/destroy_variant/:id', to: 'products_v3#destroy_variant', as: 'destroy_variant' end resources :variant_overrides do diff --git a/spec/reflexes/products_reflex_spec.rb b/spec/reflexes/products_reflex_spec.rb deleted file mode 100644 index f0c74f2641..0000000000 --- a/spec/reflexes/products_reflex_spec.rb +++ /dev/null @@ -1,100 +0,0 @@ -# frozen_string_literal: true - -require "reflex_helper" - -RSpec.describe ProductsReflex, type: :reflex, feature: :admin_style_v3 do - let(:current_user) { create(:admin_user) } # todo: set up an enterprise user to test permissions - let(:context) { - { url: admin_products_url, connection: { current_user: } } - } - let(:flash) { {} } - - before do - pending "fix spec" - # Mock flash, because stimulus_reflex_testing doesn't support sessions - allow_any_instance_of(described_class).to receive(:flash).and_return(flash) - 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 -# Parameters can be added with params: option -# For more options see https://github.com/podia/stimulus_reflex_testing#usage -def run_reflex(method_name, opts = {}) - build_reflex(method_name:, **context.merge(opts)).tap{ |reflex| - reflex.run(method_name) - } -end diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb index 3e1d6f007f..7cd4a9e8c5 100644 --- a/spec/system/admin/products_v3/products_spec.rb +++ b/spec/system/admin/products_v3/products_spec.rb @@ -1279,8 +1279,6 @@ RSpec.describe 'As an enterprise user, I can manage my products', feature: :admi end expect(page).not_to have_selector(modal_selector) - # Make sure the products loading spinner is hidden - wait_for_class('.spinner-overlay', 'hidden') expect(page).not_to have_selector(variant_selector) within success_flash_message_selector do expect(page).to have_content("Successfully deleted the variant") @@ -1297,8 +1295,6 @@ RSpec.describe 'As an enterprise user, I can manage my products', feature: :admi page.find(delete_button_selector).click end expect(page).not_to have_selector(modal_selector) - # Make sure the products loading spinner is hidden - wait_for_class('.spinner-overlay', 'hidden') expect(page).not_to have_selector(product_selector) within success_flash_message_selector do expect(page).to have_content("Successfully deleted the product") @@ -1321,9 +1317,6 @@ RSpec.describe 'As an enterprise user, I can manage my products', feature: :admi page.find(delete_button_selector).click end - expect(page).not_to 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 @@ -1338,9 +1331,6 @@ RSpec.describe 'As an enterprise user, I can manage my products', feature: :admi within modal_selector do page.find(delete_button_selector).click end - expect(page).not_to 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