Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Nested forms not leveraging many-to-many #6589

Closed
gfrancesco opened this Issue · 12 comments

5 participants

@gfrancesco

Case description

First of all, this issue is directly related with #5325 and #6161 a quick ref was posted also on http://stackoverflow.com/questions/10717797/rails-nested-form-on-many-to-many-how-to-prevent-duplicates

I've setup a nested form in my rails 3.2.3 app, it's apparently working fine, my models are:

class Recipe < ActiveRecord::Base
  attr_accessible :title, :description, :excerpt, :date, :ingredient_lines_attributes

  has_and_belongs_to_many :ingredient_lines
  accepts_nested_attributes_for :ingredient_lines
end

and

class IngredientLine < ActiveRecord::Base
  attr_accessible :ingredient_id, :measurement_unit_id, :quantity

  has_and_belongs_to_many :recipes
  belongs_to :measurement_unit
  belongs_to :ingredient
end

As above, a Recipe can have multiple IngredientLine and vice versa.

What I'm trying to avoid is record duplication on IngredienLine table, which of course is the reason why I chose a many-to-many and not a one-to-many between Recipe and IngredientLine.

For example imagine that for recipe_1 an IngredientLine with {"measurement_unit_id" => 1, "ingredient_id" => 1, "quantity" => 3.5} is associated, if for recipe_5 the IngredientLine child form is compiled by the user with the same values, I don't want a new record (with a different pk) on IngredientLine table, but only a new association record in the join table ingredient_lines_recipes.

Note that currently I don't have any IngredientLine controller as saving and updating IngredientLines is handled by nested form routines. Even my Recipe controller is plain and standard:

class RecipesController < ApplicationController
  respond_to :html

  def new
    @recipe = Recipe.new
  end

  def create
    @recipe = Recipe.new(params[:recipe])
    flash[:notice] = 'Recipe saved.' if @recipe.save  
    respond_with(@recipe)
  end

  def destroy
    @recipe = Recipe.find(params[:id])
    @recipe.destroy
    respond_with(:recipes)
  end

  def edit
    respond_with(@recipe = Recipe.find(params[:id]))
  end

  def update
    @recipe = Recipe.find(params[:id])
    flash[:notice] = 'Recipe updated.' if @recipe.update_attributes(params[:recipe])
    respond_with(@recipe)
  end

end

Actual behaviour

With the code above, every time a user submit the nested form, a new record in IngredientLine is created, no matter if is identical to a previous one. This is not a many-to-many behaviour, it's a one-to-many behaviour.

Present implementation

Looking at code, seems that join table records (relation) are created only when child object is a new_record, so fetching the existent record for IngredientLine with first_or_create won't solve the question since the new relation, with an existing object, won't be saved.

My opinion

An elegant solution for this issue is directly related to the ability of using composite pk. When a child record is submitted by the user, the framework must know if it's already stored. Since pk is the way to check if a record is unique, and given that a common id pk value is not under user control, the best way is to set a composite pk from one or more fields submitted by the user.
Sticking with standard id pk could also be an option, but a list of fields to check for uniqueness must be explicitly specified for first_or_create.

Maybe I didn't explain very well.. in case just ask, thanks.

@steveklabnik
Collaborator

Hmm, I'm not sure if this is a bug, or a feature request. Any thoughts on the expected behavior here @jonleighton @tenderlove ?

@gfrancesco

Thanks, I'd like first to have this strange behaviour confirmed.

My idea is that this issue is a consequence of using a "standard field" for primary key in all tables.

@makaroni4

@gfrancesco is it still an ongoing issue? Both #5325 and #6161 are closed, check them out please.

@gfrancesco

Just updated my app to Rails 4.0.0, bug still present. Identical records are created on ingredient_lines table.

In my opinion the question to answer is: how can Rails know that an identical record is present in ingredient_lines table if pk does not reflect uniqueness?

@makaroni4

@gfrancesco I don't think you want this behaviour at all – imagine you have 1 kg of meat in pasta and 1 kg of meat in soup. You want the same IngredientLine for both pasta and soup but if someone change amount of meat in pasta (say 0.2kg) – soup with 0.2kg of meat will be broken :smile: So I think that in case with recipes and ingredients AR behaves well – all recipes should be decoupled.

Also as I understand habtm relationship does not implies uniqueness, nor accepts_nested_attributes_for. So I think that AR behaves well.

What you want is acts_as_taggable_on behaviour, when we have a table of unique tags (read ingredient lines).

@gfrancesco

Mhhh, I'm not convinced :smile_cat:
If someone update an IngredientLine, to prevent your issue is enough to use first_or_create behavior. So the association is updated or a new one is created for a new, unique, IngredientLine record.

Let's take this case to the extreme to have a better overview. Imagine I can only use 3 different records as IngredienLine for a thousand recipes.
If recipes are decoupled as suggested I'll have an ingredient_lines table with thousands of identical records, while only 3 of them differ from each other.
That's against basic SQL rules. Primary key should be chosen to uniquely identify each record in a database table.

Another big problem is that I can't use the inverse relation. How can I fetch all recipes that use 0.2 kg of meat if there are so many equivalent models?

I know I can try with acts_as_taggable_on, but I suppose that is out of scope with nested_forms. I don't think can be used together.

Thanks for your time :smile:

@gfrancesco gfrancesco closed this
@gfrancesco gfrancesco reopened this
@makaroni4

:smile: :smile: :smile:

first_or_create behaviour is great and I agree with "economy strategy" it will help to save space. But imagine that we have IngridientLine, that belongs to two recipes. We changed this IngridientLine (just one row) and to save same ingridient_lines in other recipes we have to create another record. Feel that? To change (UPDATE) we need do create (INSERT). I don't think that it is quite obvious behaviour but yes, it will save as some space (but need a lot of work, which is already done in acts_as_taggable_on). How about that? :wink:

@gfrancesco

If you say that's a lot of work, well I can imagine and understand, so no problem :wink: but I can't obtain the same with act_as_taggable.

In my case, abstraction of IngredientLine as a "tag-like" object would have many drawbacks. I can't use nested forms at first (no support from act_as_taggable as far as I know), and anyway I can't treat elements of IngredientLine as single objects themselves (ex: give me all recipes that use meat).

Just look at the name of habtm relation. In my app rails implementation is faulty, because IngredientLine can only have one Recipe.

@robin850
Collaborator

I'm not comfortable with ActiveRecord so I'm just commenting and I may be wrong but doesn't putting a uniq constraint on the content of the IngredientLine could do the trick ? The behavior is unexpected but Rails can't be smart enough (in my opinion) to check whether the content of the IngredientLine you are creating still exist, this should be done at the SQL level.

@makaroni4

@robin850 agree, to prevent duplications unique index and unique constrained needed. And to find proper IngredientLine record we need first_or_create behaviour.

@gfrancesco I understand right that you want to create new record of IngredientLine if one IngredientLine which has many recipes is changed (in your model)?

@gfrancesco

@robin850 I tried adding a unique index before opening this issue, rails simply behave the same: trying to add the line. SQL transaction is obviously aborted and a RecordNotUnique exception is fired.

@makaroni4 I want to correctly enforce habtm relationship, storing unique records in both tables. To answer your question, if updated record is unique in IngredientLine it will be created and associated, otherwise if it's already present, only an association (join table) will be created.

@senny
Owner

Wether you want to reuse IngredientLine items or not is a design concern. One use-case might favor reuse, while another favors duplication. There is no golden path. This tracker is about the behavior of Active Record and currently, it does not support reuse using HABTM.

We use GitHub only to track bugs and pull requests. This is a feature / modification request and it's best to discuss it on the Rails core mailing list. @gfrancesco feel free to post there to get more opinions.

has_and_belongs_to_many and accepts_nested_attributes are very high-level macros. For your specialized situation it's best to implement the required behavior on your own. Using first_or_create etc. Giving this one a close. Thanks guys :yellow_heart:

@senny senny closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.