Skip to content

Commit

Permalink
adjust option value handling to work like product properties handling
Browse files Browse the repository at this point in the history
also remove last link_to_remove_fields usage and deprecate link_to_remove_fields helper
  • Loading branch information
hefan committed Mar 20, 2020
1 parent 6a21551 commit 7b2ed20
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions backend/app/helpers/spree/admin/base_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
Original file line number Diff line number Diff line change
@@ -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>
50 changes: 6 additions & 44 deletions backend/spec/features/admin/products/option_types_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 7b2ed20

Please sign in to comment.