Skip to content
This repository

Reverse removal of link_to_remove helper #63

wants to merge 0 commits into from

2 participants

Ted Lilley @nostopbutton
Ted Lilley

There was a patch submitted to fix a no method error with link_to_remove when trying to edit a product's ad-hoc option types, issue #52.

The latest commits to upgrade to spree 1.3 removed this fix. However, the error seems to have recurred because of it.

I added the file back and tested on 1.3-stable. It works for me, I went through the whole process of editing ad-hoc option types. So I'm submitting this pull request.


Ted - I'm not sure this actually fixes the problem.

In core Spree 1.3 link_to_remove_fields expects 3 arguments. This change redefines the method (I'm new to Ruby, but seems that there is no concept of method overloading) so the places where core Spree calls link_to_remove_fields now fail (e.g. app/views/spree/admin/option_types/_option_value_fields.html.erb).

I've been digging around in the code (but Rails is pretty new to me too) I think the underlying cause of the error is to do with the routing. The method is trying to use a path admin_customizable_product_option however the route defined in routes.rb is: admin_product_customization_type_customizable_product_option.

I'll carry on playing with it to see if I can get it working and then try to share my changes.

Ted Lilley

Yes - the code in this extension still uses the older 2 argument signature, but this should be changed to be in line with the changes in Spree 1.3.

Example: _option_value_fields.html.erb

<%= link_to_remove_fields t(:remove), f, :no_text => true %>

To clarify a bit further, it seems to be the nested resources in routes.rb that are causing the issues:

resources :product_customization_types do
  resources :customizable_product_options do

resources :ad_hoc_option_types do
  resources :ad_hoc_option_values do

These are creating the following paths:


But the paths generated in the link_to_remove_fields method is


That's why we're getting those error messages saying that the path isn't found.

Ted Lilley

I don't know why this worked for me before but not now, but it doesn't. I'm closing this request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Sorry, commit information is not available for this pull request.

This page is out of date. Refresh to see the latest.

Showing 0 changed files with 0 additions and 0 deletions. Show diff stats Hide diff stats

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.