Open Source Contribution to Spree/Solidus

This was one of the open source contribution in 2015 to a major Rails project used by e-commerce companies.

It usually takes hours/days to understand the basic framework, goals, motivations, features vs limitations etc., to start contributing to an open source project based on the amount of domain knowledge (especially for matured projects).

Its usually hard to find and understand bugs in a very well mature framework or language. One of the good things about using open source libraries is you get to know both the code and get to know the eco system while getting paid on the job.

The Bug

I found a bug in the admin section of Spree. The images when re-ordered are not maintaining their position. I identified this issue while trying to work on a similar feature to order categories.

I made a fix locally to verify it has solved the issue and later fixed it

tl;dr

acts_as_list index starts from 1
Javascript is sending indices starting from 0.

acts_as_list has top_of_list defaulted as 1 - Source: https://github.com/swanandp/acts_as_list/blob/master/lib/acts_as_list/active_record/acts/list.rb#L38

Steps to reproduce the Issue:

1. Go to images under product.
2. Take any element after the 1st one to the top.
3. Edit the one at the top (that was just dragged)
4. Coming back to the index page shows it in the 2nd position.

What is happening in the frontend?

1. Javascript sends the indices starting 0.
2. The update_positions method in resource_controller just sets the positions as they were sent (with more stuff going on)
3. When element in the 0th position is modified. acts_as_list figures 1 is the top of the list and sets the value to 1 along with setting anything which is already in position 1 to 0.

The FIX

This fix will not modify existing positions, but will only fix those resources for new updates.

Other Solutions

Another solution could be to set the configuration for all models which use acts_as_list to top_of_list: 0 which could be an alternative solution which fixes this issue from re-occurring in the first place.

The Code Change

  • Though the code change was in Javascript, it was sending the data to the ruby backend which was using acts_as_list which uses to store the data using ActiveRecord based on indexes.
  • Link to commit on Github
 
comments powered by Disqus