inputs_for should not iterate over deleted entries

[sumerman]

Similar to elixir-ecto/ecto#875.

[josevalim]

I am not sure I agree here. If the child model was deleted, I want to show
it somehow in the view, otherwise I cannot review all the information.
Maybe it even cannot be deleted to an error in itself.

If you want to skip it, you can maybe skip it yourself and not generate
anything for that particular change set?

There is an article on blog.plataformatec.com.br that covers exactly this.

José Valim
www.plataformatec.com.br
Skype: jv.ptec
Founder and Director of R&D

[sumerman]

@josevalim In my case contents of a form must always replace whatever is in the model. Maybe I'm cooking it wrong, but instead of getting the contents replaced, I'm getting the new stuff plus the old stuff.

[sumerman]

@josevalim I've reviewed the case from the article and there is nothing in contradiction with my proposal. In my case some items in the changeset already have :delete as their action, because this change comes from a previous step of a wizard-like UI.

[josevalim]

@sumerman well, those two ways of doing things are conflicting because one the delete action is marked as part of the input while the other is already marked as deleted.

I can think of two solutions to this problem:

• Add a skip_deleted flag - this way, we won't show deleted items (treat them as nil for has_one and remove from the collection on has_many)

• Add a skip_id flag - this way you can have control exactly on what is generated and what isn't, so you can do:

  <%= inputs_for f, :collection, [skip_id: true], fn i -> %>
    <%= unless i.source.action == :delete %>
      <% if i.source.action == :update %>Emit ID as hidden field<% end %>
      Your form as usual
    <% end %>
  <% end %>
  

Thoughts?

[sumerman]

@josevalim I still don't see the conflict. To my understanding "delete action is marked as part of the input" case, at least its implementation from article, does not involve changeset's action until the very end and input is carried inside a virtual field. On a contrary, I can think of using both approaches simultaneously:

1. wizard-step-1 renders a first part of a model
2. user marks something as deleted and it's stored into a virtual field upon submission
3. controller performs cast and applies deletions to the changeset (just like in article)
4. resulting changeset is used to show the second page of a wizard which is comprised of stuff from the first page, but in a frozen state and new mutable slots in place of deleted items
5. final submission
   Always-skip-deleted approach works fine with the described sequence. Current behaviour will lead to the data duplication.

If you really want to make this a flag, I would like to have skip_deleted and it should be set to true by default. Totally against skip_id — it leaks implementation details (hidden id-input generation).

P.S.: sorry if something is not clear, I'm a bit sleepy now.

[josevalim]

@sumerman yes, however, if I fix your use case, my use case will be broken. I am certain, I just tried. :) So something gotta give. :)

P.S.: me too. let's continue this tomorrow!

[sumerman]

@josevalim I'm ok with skip_deleted then

[josevalim]

@sumerman I am working on this and I have a question.

Imagine that I have a deleted changeset in a has_one relationship. Should we show the :default value? Or should we just not render anything? I am thinking it is rather the later?

[sumerman]

@josevalim if you are talking about skip_deleted approach then I'm also for the latter. Nothing should be rendered because user just instructed the method to skip deleted entries.

[josevalim]

Can you please give master a try and let me know if works as expected? I want to fix the other issue and ship 2.1 along side ecto 0.16.

[sumerman]

@josevalim master works fine. thanks!