Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with nested models and browser validations [v0.7.0-master] #2443

Closed
tagliala opened this issue Oct 23, 2015 · 12 comments
Closed

Issue with nested models and browser validations [v0.7.0-master] #2443

tagliala opened this issue Oct 23, 2015 · 12 comments

Comments

@tagliala
Copy link
Contributor

Hi,

We found an issue with the latest released gem, still present in the master branch:

screen shot 2015-10-23 at 22 17 24

Models:

class Team < ActiveRecord::Base
  has_many :players
  accepts_nested_attributes_for :players, allow_destroy: true
end

class Player < ActiveRecord::Base
  validates :name, presence: true
  validates :image, presence: true
end

Rails Admin:

  config.model Team do
    field :players
  end

  config.model Player do
    field :name
    field :image
  end

How to reproduce:

  • In rails admin, go for Team -> New Team.
  • Click on "Add a new player".
  • Click on the trash icon
  • Click on "Save"

Nothing will happen. Check the browser console, you will find:

An invalid form control with name='team[players_attributes][1445630637559][name]' is not focusable.
An invalid form control with name='team[players_attributes][1445630637559][image]' is not focusable.

Workaround:

In rails_admin.rb

  config.browser_validations = false

Please let me know if you have a fast fix to this. I'm not familiar with that part of the source code

Thanks!

@Hamdiakoguz
Copy link

Hi, we are having the same problem and currently using the workaround mentioned by @tagliala .
Please let me know if I can help in any way for this issue. Thanks.

@mshibuya
Copy link
Member

mshibuya commented Nov 1, 2015

Related to: #2414

@SuperMasterBlasterLaser
Copy link
Contributor

@tagliala When I write to rails_admin.rb config file this code:

   config.browser_validations = false

Then I opened edit page of my parent model. Pressed buttons to add nested child models, then pressed trash can then I have pressed Save button.

After these actions Rails_Admin throws me error:

   NoMethodError: undefined_method `parent_id` for nil:NilClass

Which means that Rails_admin still wants to add model even if we removed.

@voyera
Copy link

voyera commented Nov 24, 2015

The bug is still present in v0.8.0 Thanks for referring me here @SuperMasterBlasterLaser !

@SuperMasterBlasterLaser
Copy link
Contributor

@voyera You're welcome. In 0.6.* versions when required associations are empty and user presses Save buttton, Dashboard will highlight this field in red color and show message below from this field.

However, it still showed that problem when user removes nested and tries to save.

Now on current versions it just throws Javascript errors in console when you not assign value for required belongs_to field and when you remove added nested object.

@voyera
Copy link

voyera commented Nov 24, 2015

I think an effective way of solving this would be to add a class/id to the hidden/deleted nested entries that would bypass validations for those cases.

Would that make sense?

@SuperMasterBlasterLaser
Copy link
Contributor

@voyera, @tagliala I have also found one strange bug. If you have models like this:

Parent

|
==== Child

             |
             ==== GrandChild

                               |
                               ==== GreatGrandChild

Each of them has name with validation of presence. Parent has_many Child, Child has many GrandChild and GrandChild has many GreatGrandChild. All of them accepts nested attributes and has correct inverse of.

When I open Parent create action:

  1. Fill name
  2. Press add new Child
  3. Fill name for Child
  4. Press add new GrandChild
  5. Fill name for GrandChild
  6. Press add new GreatGrandChild
  7. Fill name for GreatGranChild
  8. Press save.

This gives Validation error and says that I havent entered name for GrandChild. Then I open tab that has GranChild and there bug happens. It has TWO GRANDCHILDS Even if I created only one one of them, which has GreatGrandChild has empty name and other hase that name.

wtf

Sorry for too big picture. But this happens when I try to create from scratch. If I remove extra appeared model and then try to save it it works.

Older versions still have this .

@SuperMasterBlasterLaser
Copy link
Contributor

@tagliala, @Hamdiakoguz, @mshibuya, @voyera

I think I have found why this is happening.

When you click to Add new Child Model button, it creates new HTML form with unique id which will help you to add this new model. It has its own validations that will be checked aming with parent model.

When you click to remove this Child, it DOES NOT remove its child form from HTML it just changes its display value to none

1

This makes validations of this form still active and admin page thinks that user still trying to add this child. Because data are empty that is why it is invalid and it tries to focus on this form to show to users that there are some invalid fields.

There is one interesting thing happens on nested forms. If you click Add new Child Model and fill all required fields, then click to Remove and then click to Save. It works and your Model will be created among with Child model that you didn't want to add

@voyera
Copy link

voyera commented Nov 26, 2015

@SuperMasterBlasterLaser your fix doesn't cover the issue that the removed child models will still be created anyway right?

@SuperMasterBlasterLaser
Copy link
Contributor

@voyera I checked it on my test project. Works well. One thing that I have found out that when I click to remove child, it adds destroy command to hidden field and then makes it invisible. However, validation stil remains and does not allow us to save. You can check my pull request. My fix removes these validaitons on deleted childs

@mshibuya
Copy link
Member

Just merged #2490, please try the latest master.

@voyera
Copy link

voyera commented Nov 27, 2015

Thanks @mshibuya and @SuperMasterBlasterLaser !

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

No branches or pull requests

6 participants