Merge pull request #12511 from chahmedejaz/task/12398-remove-reflex-from-product-variant-delete

Task/12398 remove reflex from product variant delete
This commit is contained in:
Filipe
2024-06-04 16:03:57 +02:00
committed by GitHub
13 changed files with 118 additions and 153 deletions

View File

@@ -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

View File

@@ -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,

View File

@@ -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

View File

@@ -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

View File

@@ -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')

View File

@@ -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|

View File

@@ -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' }

View File

@@ -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 })

View File

@@ -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);
};
}

View File

@@ -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;
}
}

View File

@@ -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

View File

@@ -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

View File

@@ -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