diff --git a/backend/app/controllers/spree/admin/option_types_controller.rb b/backend/app/controllers/spree/admin/option_types_controller.rb index ce422e8682d..259afff6bca 100644 --- a/backend/app/controllers/spree/admin/option_types_controller.rb +++ b/backend/app/controllers/spree/admin/option_types_controller.rb @@ -3,7 +3,7 @@ module Spree module Admin class OptionTypesController < ResourceController - before_action :setup_new_option_value, only: :edit + before_action :setup_new_option_value, only: :edit, if: -> { can?(:create, model_class) } def update_values_positions params[:positions].each do |id, index| @@ -26,7 +26,7 @@ def load_product end def setup_new_option_value - @option_type.option_values.build if @option_type.option_values.empty? + @option_type.option_values.build end def set_available_option_types diff --git a/backend/app/controllers/spree/admin/option_values_controller.rb b/backend/app/controllers/spree/admin/option_values_controller.rb index 6f73a3a6381..44589ecbba6 100644 --- a/backend/app/controllers/spree/admin/option_values_controller.rb +++ b/backend/app/controllers/spree/admin/option_values_controller.rb @@ -6,7 +6,8 @@ class OptionValuesController < Spree::Admin::BaseController def destroy option_value = Spree::OptionValue.find(params[:id]) option_value.destroy - render plain: nil + flash[:success] = flash_message_for(option_value, :successfully_removed) + render partial: "spree/admin/shared/destroy" end end end diff --git a/backend/app/helpers/spree/admin/base_helper.rb b/backend/app/helpers/spree/admin/base_helper.rb index 7df39fe38ff..34db20a0ffa 100644 --- a/backend/app/helpers/spree/admin/base_helper.rb +++ b/backend/app/helpers/spree/admin/base_helper.rb @@ -132,6 +132,8 @@ def link_to_remove_fields(name, form, options = {}) link_to_with_icon('trash', name, url, class: "spree_remove_fields #{options[:class]}", data: { action: 'remove' }, title: t('spree.actions.remove')) + form.hidden_field(:_destroy) end + deprecate link_to_remove_fields: "Please use link_to_delete instead, Example: \n" \ + "link_to_delete \"object\", no_text: \"true\" ", deprecator: Spree::Deprecation def spree_dom_id(record) dom_id(record, 'spree') diff --git a/backend/app/views/spree/admin/option_types/_option_value_fields.html.erb b/backend/app/views/spree/admin/option_types/_option_value_fields.html.erb index f1d0a6b8917..6da3e2d46bc 100644 --- a/backend/app/views/spree/admin/option_types/_option_value_fields.html.erb +++ b/backend/app/views/spree/admin/option_types/_option_value_fields.html.erb @@ -1,11 +1,11 @@ - <% if f.object.persisted? %> + <% if f.object.persisted? && can?(:update_positions, f.object) %> <%= f.hidden_field :id %> <% end %> <%= f.text_field :name %> <%= f.text_field :presentation %> - <%= link_to_remove_fields t('spree.actions.remove'), f, no_text: true %> + <%= link_to_delete(f.object, no_text: true) if f.object.persisted? && can?(:destroy, f.object) %> diff --git a/backend/spec/features/admin/products/option_types_spec.rb b/backend/spec/features/admin/products/option_types_spec.rb index c33dea3ef11..b26bc2eab5c 100644 --- a/backend/spec/features/admin/products/option_types_spec.rb +++ b/backend/spec/features/admin/products/option_types_spec.rb @@ -64,53 +64,15 @@ click_link "Option Types" within('table#listing_option_types') { click_icon :edit } expect(page).to have_title("#{option_value.option_type.name} - Option Types - Products") - expect(page).to have_css("tbody#option_values tr", count: 1) - within("tbody#option_values") do - find('.spree_remove_fields').click - end - # Assert that the field is hidden automatically - expect(page).to have_no_css("tbody#option_values tr") - - # Ensure the DELETE request finishes - expect(page).to have_no_css("#progress") - - # Then assert that on a page refresh that it's still not visible - visit page.current_url - # What *is* visible is a new option value field, with blank values - # Sometimes the page doesn't load before the all check is done - # lazily finding the element gives the page 10 seconds - expect(page).to have_css("tbody#option_values") - all("tbody#option_values tr input", count: 2).each do |input| - expect(input.value).to be_blank - end - end - - # Regression test for https://github.com/spree/spree/issues/3204 - it "can remove a non-persisted option value from an option type", js: true do - create(:option_type) - click_link "Option Types" - within('table#listing_option_types') { click_icon :edit } - - expect(page).to have_css("tbody#option_values tr", count: 1) - - # Add a new option type - click_button "Add Option Value" + # persisted and new element is seen expect(page).to have_css("tbody#option_values tr", count: 2) - # Remove default option type - within("tbody#option_values") do - within_row(1) do - find('.fa-trash').click - end + accept_alert do + click_icon :trash end - # Assert that the field is hidden automatically - expect(page).to have_css("tbody#option_values tr", count: 1) + expect(page).to have_content 'successfully removed' - # Remove added option type - within("tbody#option_values") do - find('.fa-trash').click - end - # Assert that the field is hidden automatically - expect(page).to have_css("tbody#option_values tr", count: 0) + # only the new element is left + expect(page).to have_css("tbody#option_values tr", count: 1) end end