adjust option value handling to work like product properties handling

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

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

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

backend/app/views/spree/admin/option_types/_option_value_fields.html.erb

@@ -1,11 +1,11 @@
 <tr class="option_value fields" id="spree_<%= dom_id(f.object) %>"  data-hook="option_value">
   <td>
-    <% if f.object.persisted? %>
+    <% if f.object.persisted? && can?(:update_positions, f.object) %>
       <span class="handle"></span>
       <%= f.hidden_field :id %>
     <% end %>
   </td>
   <td class="name"><%= f.text_field :name %></td>
   <td class="presentation"><%= f.text_field :presentation %></td>
-  <td class="actions"><%= link_to_remove_fields t('spree.actions.remove'), f, no_text: true %></td>
+  <td class="actions"><%= link_to_delete(f.object, no_text: true) if f.object.persisted? && can?(:destroy, f.object) %></td>
 </tr>

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