From e740fb8f6e72a93e374c0370022d9981f3ddfd60 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Thu, 23 May 2024 14:24:16 +0500 Subject: [PATCH 01/15] 12398: add turbo-rails --- Gemfile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index 94f0e62398..668ad01fbd 100644 --- a/Gemfile +++ b/Gemfile @@ -104,7 +104,8 @@ gem 'sidekiq-scheduler' gem "cable_ready" gem "stimulus_reflex" - +gem "cable_ready", "5.0.1" +gem "stimulus_reflex", "3.5.0.rc3" gem "turbo-rails" gem 'combine_pdf' From e5b9e078740030b9ec008bf211e11f83fab8e877 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Thu, 23 May 2024 14:27:13 +0500 Subject: [PATCH 02/15] 12398: update confirm_modal to use button_to form submission --- app/components/confirm_modal_component.rb | 4 +++- .../confirm_modal_component.html.haml | 7 ++++++- app/views/admin/products_v3/_delete_modal.html.haml | 1 + 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/app/components/confirm_modal_component.rb b/app/components/confirm_modal_component.rb index 8149a729e3..7034db03c6 100644 --- a/app/components/confirm_modal_component.rb +++ b/app/components/confirm_modal_component.rb @@ -12,7 +12,8 @@ class ConfirmModalComponent < ModalComponent 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' + actions_alignment_class: 'justify-space-around', + confirm_submit_method: nil ) super(id:, close_button: true) @confirm_actions = confirm_actions @@ -24,6 +25,7 @@ class ConfirmModalComponent < ModalComponent @confirm_button_text = confirm_button_text @cancel_button_text = cancel_button_text @actions_alignment_class = actions_alignment_class + @confirm_submit_method = confirm_submit_method 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 1184602a67..210bc724a8 100644 --- a/app/components/confirm_modal_component/confirm_modal_component.html.haml +++ b/app/components/confirm_modal_component/confirm_modal_component.html.haml @@ -7,4 +7,9 @@ %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 } + - # TODO: This if block needs to be removed when we completely get rid of Reflex + - # The button's form action will be dynamically set when the modal is opened via modal-link-controller + - if @confirm_submit_method + = button_to @confirm_button_text, '', id: 'modal-confirm-button', method: @confirm_submit_method, data: { action: @confirm_actions } + - else + %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/views/admin/products_v3/_delete_modal.html.haml b/app/views/admin/products_v3/_delete_modal.html.haml index 40b6fbd32d..ca0987ab4e 100644 --- a/app/views/admin/products_v3/_delete_modal.html.haml +++ b/app/views/admin/products_v3/_delete_modal.html.haml @@ -7,6 +7,7 @@ actions_alignment_class: 'justify-end', confirm_reflexes: "click->products#delete_#{object_type}", confirm_actions: "click->modal#close", + confirm_submit_method: :delete, ) = render delete_modal do %h2.margin-bottom-20.black-text From 53fb77eb2336665ee3f93f18917c99ea4c97e5b0 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Thu, 23 May 2024 14:30:28 +0500 Subject: [PATCH 03/15] 12398: add destroy action with turbo stream --- app/controllers/admin/products_v3_controller.rb | 8 ++++++++ app/views/admin/products_v3/destroy.turbo_stream.haml | 1 + config/routes/admin.rb | 2 ++ 3 files changed, 11 insertions(+) create mode 100644 app/views/admin/products_v3/destroy.turbo_stream.haml diff --git a/app/controllers/admin/products_v3_controller.rb b/app/controllers/admin/products_v3_controller.rb index 77098ee7bb..90505150fd 100644 --- a/app/controllers/admin/products_v3_controller.rb +++ b/app/controllers/admin/products_v3_controller.rb @@ -31,6 +31,14 @@ module Admin end end + # will update this in further commits + def destroy + @product = Spree::Product.find(params[:id]) + respond_with do |format| + format.turbo_stream + end + end + def index_url(params) "/admin/products?#{params.to_query}" # todo: fix routing so this can be automaticly generated end diff --git a/app/views/admin/products_v3/destroy.turbo_stream.haml b/app/views/admin/products_v3/destroy.turbo_stream.haml new file mode 100644 index 0000000000..8f19c2fdfe --- /dev/null +++ b/app/views/admin/products_v3/destroy.turbo_stream.haml @@ -0,0 +1 @@ += turbo_stream.remove dom_id(@product) diff --git a/config/routes/admin.rb b/config/routes/admin.rb index c82fb18315..3d4665eea2 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -75,6 +75,8 @@ 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' end resources :variant_overrides do From 6659ffe5301faa5966fe75932fec1d5f6febd431 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Thu, 23 May 2024 14:32:38 +0500 Subject: [PATCH 04/15] 12398: update modal-link-controller to dynamically set form action as per the selected product to delete --- .../admin/products_v3/_product_row.html.haml | 2 +- app/views/admin/products_v3/_table.html.haml | 2 +- .../controllers/modal_link_controller.js | 15 +++++++++++++++ 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/app/views/admin/products_v3/_product_row.html.haml b/app/views/admin/products_v3/_product_row.html.haml index 0908652009..0dd86aa73b 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-current-id': product.id, 'data-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..92f8ded399 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 diff --git a/app/webpacker/controllers/modal_link_controller.js b/app/webpacker/controllers/modal_link_controller.js index 9eb765c35f..c77e2864fe 100644 --- a/app/webpacker/controllers/modal_link_controller.js +++ b/app/webpacker/controllers/modal_link_controller.js @@ -17,6 +17,9 @@ export default class extends Controller { const modalId = this.targetValue; const moodalConfirmButtonQuery = `#${modalId} #modal-confirm-button`; const confirmButton = document.querySelector(moodalConfirmButtonQuery); + + this.#setPathToFormAction(confirmButton); + Object.keys(this.modalDatasetValue).forEach((datasetKey) => { confirmButton.setAttribute(datasetKey, this.modalDatasetValue[datasetKey]); }); @@ -30,4 +33,16 @@ export default class extends Controller { getIdentifier() { return "modal"; } + + #setPathToFormAction(confirmButton) { + const isSubmitButton = confirmButton.type === 'submit'; + const path = this.modalDatasetValue['data-path']; + + if(isSubmitButton && path){ + const form = confirmButton.parentElement; + form.setAttribute('action', path); + + delete this.modalDatasetValue['data-path']; + } + } } From 48615f1325f1c233cd3c9780f0401760f62f93cd Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Fri, 24 May 2024 01:20:08 +0500 Subject: [PATCH 05/15] 12398: fix rebasing mistake --- Gemfile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Gemfile b/Gemfile index 668ad01fbd..94f0e62398 100644 --- a/Gemfile +++ b/Gemfile @@ -104,8 +104,7 @@ gem 'sidekiq-scheduler' gem "cable_ready" gem "stimulus_reflex" -gem "cable_ready", "5.0.1" -gem "stimulus_reflex", "3.5.0.rc3" + gem "turbo-rails" gem 'combine_pdf' From 039b0d80ee8578115659c1cba75fb94d9a9db13a Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Fri, 24 May 2024 01:55:21 +0500 Subject: [PATCH 06/15] 12398: implement the destroy action for products --- app/controllers/admin/products_v3_controller.rb | 17 +++++++++++++++-- .../admin/products_v3/destroy.turbo_stream.haml | 1 + 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/app/controllers/admin/products_v3_controller.rb b/app/controllers/admin/products_v3_controller.rb index 90505150fd..52d26bd924 100644 --- a/app/controllers/admin/products_v3_controller.rb +++ b/app/controllers/admin/products_v3_controller.rb @@ -31,12 +31,25 @@ module Admin end end - # will update this in further commits def destroy - @product = Spree::Product.find(params[:id]) + @product = ProductScopeQuery.new( + spree_current_user, + { id: params[:id] } + ).find_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 + respond_with do |format| format.turbo_stream end + + # using flash with turbo stream doesn't clear it because the page is not refreshed. + # so upon refreshing the page, the flash message appears again + flash.discard end def index_url(params) diff --git a/app/views/admin/products_v3/destroy.turbo_stream.haml b/app/views/admin/products_v3/destroy.turbo_stream.haml index 8f19c2fdfe..f517ebc6cd 100644 --- a/app/views/admin/products_v3/destroy.turbo_stream.haml +++ b/app/views/admin/products_v3/destroy.turbo_stream.haml @@ -1 +1,2 @@ = turbo_stream.remove dom_id(@product) += render partial: "admin/shared/flashes", locals: { flashes: flash } if defined? flash \ No newline at end of file From 2bdf8e2853a03980e5c503e10edda96d78f08d56 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Fri, 24 May 2024 02:40:34 +0500 Subject: [PATCH 07/15] rename destroy.turbo_stream to make it more generic --- app/controllers/admin/products_v3_controller.rb | 6 +++--- app/views/admin/products_v3/destroy.turbo_stream.haml | 2 -- 2 files changed, 3 insertions(+), 5 deletions(-) delete mode 100644 app/views/admin/products_v3/destroy.turbo_stream.haml diff --git a/app/controllers/admin/products_v3_controller.rb b/app/controllers/admin/products_v3_controller.rb index 52d26bd924..b4968e0a2f 100644 --- a/app/controllers/admin/products_v3_controller.rb +++ b/app/controllers/admin/products_v3_controller.rb @@ -32,19 +32,19 @@ module Admin end def destroy - @product = ProductScopeQuery.new( + @record = ProductScopeQuery.new( spree_current_user, { id: params[:id] } ).find_product - if @product.destroy + if @record.destroy flash[:success] = I18n.t('admin.products_v3.delete_product.success') else flash[:error] = I18n.t('admin.products_v3.delete_product.error') end respond_with do |format| - format.turbo_stream + format.turbo_stream { render :destroy_product_variant } end # using flash with turbo stream doesn't clear it because the page is not refreshed. diff --git a/app/views/admin/products_v3/destroy.turbo_stream.haml b/app/views/admin/products_v3/destroy.turbo_stream.haml deleted file mode 100644 index f517ebc6cd..0000000000 --- a/app/views/admin/products_v3/destroy.turbo_stream.haml +++ /dev/null @@ -1,2 +0,0 @@ -= turbo_stream.remove dom_id(@product) -= render partial: "admin/shared/flashes", locals: { flashes: flash } if defined? flash \ No newline at end of file From a0f290c09f140235282442b70e711b76528a8e9a Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Fri, 24 May 2024 02:41:51 +0500 Subject: [PATCH 08/15] 12398: add turbo stream to delete variants --- app/controllers/admin/products_v3_controller.rb | 17 +++++++++++++++++ .../admin/products_v3/_product_row.html.haml | 2 +- app/views/admin/products_v3/_table.html.haml | 2 +- .../admin/products_v3/_variant_row.html.haml | 2 +- .../destroy_product_variant.turbo_stream.haml | 3 +++ config/routes/admin.rb | 1 + 6 files changed, 24 insertions(+), 3 deletions(-) create mode 100644 app/views/admin/products_v3/destroy_product_variant.turbo_stream.haml diff --git a/app/controllers/admin/products_v3_controller.rb b/app/controllers/admin/products_v3_controller.rb index b4968e0a2f..15d12d3849 100644 --- a/app/controllers/admin/products_v3_controller.rb +++ b/app/controllers/admin/products_v3_controller.rb @@ -52,6 +52,23 @@ module Admin flash.discard end + def destroy_variant + @record = Spree::Variant.active.find(params[:id]) + authorize! :delete, @record + + if VariantDeleter.new.delete(@record) + flash[:success] = I18n.t('admin.products_v3.delete_variant.success') + else + flash[:error] = I18n.t('admin.products_v3.delete_variant.error') + end + + respond_with do |format| + format.turbo_stream { render :destroy_product_variant } + end + + flash.discard + end + def index_url(params) "/admin/products?#{params.to_query}" # todo: fix routing so this can be automaticly generated end diff --git a/app/views/admin/products_v3/_product_row.html.haml b/app/views/admin/products_v3/_product_row.html.haml index 0dd86aa73b..0e16514257 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, 'data-path': admin_product_destroy_path(product)}.to_json } + "data-modal-link-modal-dataset-value": {'data-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 92f8ded399..464130d181 100644 --- a/app/views/admin/products_v3/_table.html.haml +++ b/app/views/admin/products_v3/_table.html.haml @@ -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..02f23b8eeb 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-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..74c600fe84 --- /dev/null +++ b/app/views/admin/products_v3/destroy_product_variant.turbo_stream.haml @@ -0,0 +1,3 @@ +- # @record can either be Product or Variant += turbo_stream.remove dom_id(@record) += render partial: "admin/shared/flashes", locals: { flashes: flash } if defined? flash diff --git a/config/routes/admin.rb b/config/routes/admin.rb index 3d4665eea2..21caa2b677 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -77,6 +77,7 @@ Openfoodnetwork::Application.routes.draw do 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 From 6a59d06de1cea3dd2cccd2d2143e9b4c831f6846 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Fri, 24 May 2024 02:45:26 +0500 Subject: [PATCH 09/15] remove delete methods from products reflex --- app/reflexes/products_reflex.rb | 35 --------- spec/reflexes/products_reflex_spec.rb | 100 -------------------------- 2 files changed, 135 deletions(-) delete mode 100644 spec/reflexes/products_reflex_spec.rb diff --git a/app/reflexes/products_reflex.rb b/app/reflexes/products_reflex.rb index 4c8aabc0d2..7d107af4a4 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 @@ -203,11 +175,4 @@ class ProductsReflex < ApplicationReflex .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/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 From fb07794cf3e71916e88fb5b763caeb0009ee7c7b Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Tue, 28 May 2024 01:10:30 +0500 Subject: [PATCH 10/15] 12398: add slide-out animation --- app/components/confirm_modal_component.rb | 4 +- .../confirm_modal_component.html.haml | 7 +-- .../admin/products_v3_controller.rb | 8 ++- .../admin/products_v3/_delete_modal.html.haml | 5 +- .../admin/products_v3/_product_row.html.haml | 2 +- .../admin/products_v3/_variant_row.html.haml | 2 +- .../destroy_product_variant.turbo_stream.haml | 7 ++- .../controllers/modal_link_controller.js | 15 ------ .../controllers/products_controller.js | 49 +++++++++++++++++++ app/webpacker/css/admin/products_v3.scss | 15 ++++++ 10 files changed, 81 insertions(+), 33 deletions(-) diff --git a/app/components/confirm_modal_component.rb b/app/components/confirm_modal_component.rb index 7034db03c6..8149a729e3 100644 --- a/app/components/confirm_modal_component.rb +++ b/app/components/confirm_modal_component.rb @@ -12,8 +12,7 @@ class ConfirmModalComponent < ModalComponent 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', - confirm_submit_method: nil + actions_alignment_class: 'justify-space-around' ) super(id:, close_button: true) @confirm_actions = confirm_actions @@ -25,7 +24,6 @@ class ConfirmModalComponent < ModalComponent @confirm_button_text = confirm_button_text @cancel_button_text = cancel_button_text @actions_alignment_class = actions_alignment_class - @confirm_submit_method = confirm_submit_method 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 210bc724a8..1184602a67 100644 --- a/app/components/confirm_modal_component/confirm_modal_component.html.haml +++ b/app/components/confirm_modal_component/confirm_modal_component.html.haml @@ -7,9 +7,4 @@ %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" } - - # TODO: This if block needs to be removed when we completely get rid of Reflex - - # The button's form action will be dynamically set when the modal is opened via modal-link-controller - - if @confirm_submit_method - = button_to @confirm_button_text, '', id: 'modal-confirm-button', method: @confirm_submit_method, data: { action: @confirm_actions } - - else - %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 } + %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/controllers/admin/products_v3_controller.rb b/app/controllers/admin/products_v3_controller.rb index 15d12d3849..80eefbaca7 100644 --- a/app/controllers/admin/products_v3_controller.rb +++ b/app/controllers/admin/products_v3_controller.rb @@ -37,14 +37,16 @@ module Admin { id: params[:id] } ).find_product + status = :ok if @record.destroy flash[:success] = I18n.t('admin.products_v3.delete_product.success') else flash[:error] = I18n.t('admin.products_v3.delete_product.error') + status = :internal_server_error end respond_with do |format| - format.turbo_stream { render :destroy_product_variant } + format.turbo_stream { render :destroy_product_variant, status: } end # using flash with turbo stream doesn't clear it because the page is not refreshed. @@ -56,14 +58,16 @@ module Admin @record = Spree::Variant.active.find(params[:id]) authorize! :delete, @record + status = :ok if VariantDeleter.new.delete(@record) flash[:success] = I18n.t('admin.products_v3.delete_variant.success') else flash[:error] = I18n.t('admin.products_v3.delete_variant.error') + status = :internal_server_error end respond_with do |format| - format.turbo_stream { render :destroy_product_variant } + format.turbo_stream { render :destroy_product_variant, status: } end flash.discard diff --git a/app/views/admin/products_v3/_delete_modal.html.haml b/app/views/admin/products_v3/_delete_modal.html.haml index ca0987ab4e..87208c5dbe 100644 --- a/app/views/admin/products_v3/_delete_modal.html.haml +++ b/app/views/admin/products_v3/_delete_modal.html.haml @@ -5,9 +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", - confirm_submit_method: :delete, + 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 0e16514257..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-path': admin_product_destroy_path(product)}.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/_variant_row.html.haml b/app/views/admin/products_v3/_variant_row.html.haml index 02f23b8eeb..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-path': admin_destroy_variant_path(variant)}.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 index 74c600fe84..6ea1bc1a8f 100644 --- a/app/views/admin/products_v3/destroy_product_variant.turbo_stream.haml +++ b/app/views/admin/products_v3/destroy_product_variant.turbo_stream.haml @@ -1,3 +1,6 @@ - # @record can either be Product or Variant -= turbo_stream.remove dom_id(@record) -= render partial: "admin/shared/flashes", locals: { flashes: flash } if defined? flash +- unless flash[:error] + = turbo_stream.remove(dom_id(@record)) +-# Without +formats+ option here, by default render is trying to render the equivalant turbo stream +-# It's strange that it works just fine if I remove the +unless+ above += render(partial: 'admin/shared/flashes', locals: { flashes: flash }, formats: [:html]) diff --git a/app/webpacker/controllers/modal_link_controller.js b/app/webpacker/controllers/modal_link_controller.js index c77e2864fe..9eb765c35f 100644 --- a/app/webpacker/controllers/modal_link_controller.js +++ b/app/webpacker/controllers/modal_link_controller.js @@ -17,9 +17,6 @@ export default class extends Controller { const modalId = this.targetValue; const moodalConfirmButtonQuery = `#${modalId} #modal-confirm-button`; const confirmButton = document.querySelector(moodalConfirmButtonQuery); - - this.#setPathToFormAction(confirmButton); - Object.keys(this.modalDatasetValue).forEach((datasetKey) => { confirmButton.setAttribute(datasetKey, this.modalDatasetValue[datasetKey]); }); @@ -33,16 +30,4 @@ export default class extends Controller { getIdentifier() { return "modal"; } - - #setPathToFormAction(confirmButton) { - const isSubmitButton = confirmButton.type === 'submit'; - const path = this.modalDatasetValue['data-path']; - - if(isSubmitButton && path){ - const form = confirmButton.parentElement; - form.setAttribute('action', path); - - delete this.modalDatasetValue['data-path']; - } - } } diff --git a/app/webpacker/controllers/products_controller.js b/app/webpacker/controllers/products_controller.js index 9ec3ac1a86..8396d9edf7 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,45 @@ 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 () => { + 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; + } } From 8ee833d2d8dd7ca961b4f575b166e27d1400d905 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Wed, 29 May 2024 01:19:50 +0500 Subject: [PATCH 11/15] 12398: add flash.now --- app/controllers/admin/products_v3_controller.rb | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/app/controllers/admin/products_v3_controller.rb b/app/controllers/admin/products_v3_controller.rb index 80eefbaca7..7ac886444b 100644 --- a/app/controllers/admin/products_v3_controller.rb +++ b/app/controllers/admin/products_v3_controller.rb @@ -39,19 +39,15 @@ module Admin status = :ok if @record.destroy - flash[:success] = I18n.t('admin.products_v3.delete_product.success') + flash.now[:success] = I18n.t('admin.products_v3.delete_product.success') else - flash[:error] = I18n.t('admin.products_v3.delete_product.error') + flash.now[:error] = I18n.t('admin.products_v3.delete_product.error') status = :internal_server_error end respond_with do |format| format.turbo_stream { render :destroy_product_variant, status: } end - - # using flash with turbo stream doesn't clear it because the page is not refreshed. - # so upon refreshing the page, the flash message appears again - flash.discard end def destroy_variant @@ -60,17 +56,15 @@ module Admin status = :ok if VariantDeleter.new.delete(@record) - flash[:success] = I18n.t('admin.products_v3.delete_variant.success') + flash.now[:success] = I18n.t('admin.products_v3.delete_variant.success') else - flash[:error] = I18n.t('admin.products_v3.delete_variant.error') + flash.now[:error] = I18n.t('admin.products_v3.delete_variant.error') status = :internal_server_error end respond_with do |format| format.turbo_stream { render :destroy_product_variant, status: } end - - flash.discard end def index_url(params) From a93ce4ea5525f5809c3d8409e7ceb5e52c175ae0 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Wed, 29 May 2024 01:20:10 +0500 Subject: [PATCH 12/15] 12398: append flash in the flashes container --- .../products_v3/destroy_product_variant.turbo_stream.haml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) 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 index 6ea1bc1a8f..645b265c41 100644 --- a/app/views/admin/products_v3/destroy_product_variant.turbo_stream.haml +++ b/app/views/admin/products_v3/destroy_product_variant.turbo_stream.haml @@ -1,6 +1,5 @@ - # @record can either be Product or Variant - unless flash[:error] = turbo_stream.remove(dom_id(@record)) --# Without +formats+ option here, by default render is trying to render the equivalant turbo stream --# It's strange that it works just fine if I remove the +unless+ above -= render(partial: 'admin/shared/flashes', locals: { flashes: flash }, formats: [:html]) += turbo_stream.append "flashes" do + = render(partial: 'admin/shared/flashes', locals: { flashes: flash }) From e932dabacbb1b9e57f768d2b8eeb4556ca3c7570 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Thu, 30 May 2024 14:17:57 +0500 Subject: [PATCH 13/15] 12398: fix failing specs --- app/models/spree/ability.rb | 2 +- app/webpacker/controllers/products_controller.js | 3 ++- spec/system/admin/products_v3/products_spec.rb | 10 ---------- 3 files changed, 3 insertions(+), 12 deletions(-) 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/webpacker/controllers/products_controller.js b/app/webpacker/controllers/products_controller.js index 8396d9edf7..720b181382 100644 --- a/app/webpacker/controllers/products_controller.js +++ b/app/webpacker/controllers/products_controller.js @@ -54,7 +54,8 @@ export default class extends ApplicationController { const elementToBeRemoved = this.#getElementToBeRemoved(deletePath, recordType); const handleSlideOutAnimationEnd = async () => { - const csrfToken = document.querySelector('meta[name="csrf-token"]').getAttribute('content'); + // 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, { diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb index 757be44d02..5260ad2883 100644 --- a/spec/system/admin/products_v3/products_spec.rb +++ b/spec/system/admin/products_v3/products_spec.rb @@ -1164,8 +1164,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") @@ -1182,8 +1180,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") @@ -1206,9 +1202,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 @@ -1223,9 +1216,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 From c2fa9934325e97f9049b5df911f2795329789b33 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Thu, 30 May 2024 14:24:51 +0500 Subject: [PATCH 14/15] 12398: fix lint issues --- app/controllers/admin/products_v3_controller.rb | 2 ++ app/reflexes/products_reflex.rb | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/controllers/admin/products_v3_controller.rb b/app/controllers/admin/products_v3_controller.rb index 7ac886444b..eb975424ef 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 @@ -183,3 +184,4 @@ module Admin end end end +# rubocop:enable Metrics/ClassLength diff --git a/app/reflexes/products_reflex.rb b/app/reflexes/products_reflex.rb index 7d107af4a4..a2feef4d28 100644 --- a/app/reflexes/products_reflex.rb +++ b/app/reflexes/products_reflex.rb @@ -174,5 +174,4 @@ class ProductsReflex < ApplicationReflex params.permit(products: ::PermittedAttributes::Product.attributes) .to_h.with_indifferent_access end - end From ce60a2a1e0fddca0781d4894a6c76fb3c9a3d486 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Thu, 30 May 2024 14:44:34 +0500 Subject: [PATCH 15/15] 12398: add lazylookup for translations --- app/controllers/admin/products_v3_controller.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/admin/products_v3_controller.rb b/app/controllers/admin/products_v3_controller.rb index eb975424ef..2954094d6d 100644 --- a/app/controllers/admin/products_v3_controller.rb +++ b/app/controllers/admin/products_v3_controller.rb @@ -40,9 +40,9 @@ module Admin status = :ok if @record.destroy - flash.now[:success] = I18n.t('admin.products_v3.delete_product.success') + flash.now[:success] = t('.delete_product.success') else - flash.now[:error] = I18n.t('admin.products_v3.delete_product.error') + flash.now[:error] = t('.delete_product.error') status = :internal_server_error end @@ -57,9 +57,9 @@ module Admin status = :ok if VariantDeleter.new.delete(@record) - flash.now[:success] = I18n.t('admin.products_v3.delete_variant.success') + flash.now[:success] = t('.delete_variant.success') else - flash.now[:error] = I18n.t('admin.products_v3.delete_variant.error') + flash.now[:error] = t('.delete_variant.error') status = :internal_server_error end