nickcharlton / administrate-field-nested_has_many

A plugin for nested has_many forms in Administrate
MIT License
59 stars 99 forks source link

Fix compatibility with latest administrate #37

Closed merqlove closed 3 years ago

merqlove commented 4 years ago

Hello all! Looks like something changed in Administrate core, that gem fails for me out of the box. I have fixed its functionality to stable condition.

merqlove commented 4 years ago

Hi! I don’t know the 100% reason. My thoughts is at the PR comment.

I did not tested it by rspec at all. I suggests test it in CI.

-- Best regards, Alexander Merkulov

E-mail: sasha@merqlove.ru Tel.: +7(925) 278-44-98 telegram: merqlove

On 29 Apr 2020, at 17:44, Nick Charlton notifications@github.com wrote:

@nickcharlton commented on this pull request.

Do you happen to know which change in Administrate caused this?

I notice the test didn't seem to run with this PR, but are there any test changes we should be making here?

In .gitignore https://github.com/nickcharlton/administrate-field-nested_has_many/pull/37#discussion_r417369863:

@@ -2,3 +2,4 @@ Gemfile.lock gemfiles/.bundle pkg/ .rspec +.idea I prefer to keep user/system specific things out of the project gitignore, would you take this out?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/nickcharlton/administrate-field-nested_has_many/pull/37#pullrequestreview-402736796, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAEPMQHSHTEGH3XKTDLSLTRPA4LNANCNFSM4MRFZOXQ.

merqlove commented 4 years ago

Thank you for this change @merqlove. We'd need a bit more of information before accepting this PR. Can you please tell us what errors you were seeing? We'd like to understand what's being fixed here, and how Administrate was failing. Perhaps a spec should be added so that the same problem is not introduced in the future.

Do you have an error trace that we can see?

Hello @pablobm!

Here was a problem, when initial resource collection creating incorrectly. Error said that there not attribute name for collection. And it is right.

So i simply added resource resolver with better checking & second variant of form_builder initialisation, when here is no @data. I don't have error trace, because it is asap fix. After i have switched for my needs.

Now i simplified that changes. Anyway it's pass specs. (P.S. Specs also outdated.)

That's it.

administrate-field-nested_has_many[develop !+] $ bundle exec appraisal rake
>> BUNDLE_GEMFILE=SOME_PATH/gems/administrate-field-nested_has_many/gemfiles/administrate_0.10.gemfile bundle exec rake
SOME_PATH/.rbenv/versions/2.6.6/bin/ruby -ISOME_PATH/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rspec-core-3.9.1/lib:SOME_PATH/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rspec-support-3.9.2/lib SOME_PATH/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rspec-core-3.9.1/exe/rspec --pattern spec/\*\*\{,/\*/\*\*\}/\*_spec.rb
.........

Finished in 9.99 seconds (files took 7.01 seconds to load)
9 examples, 0 failures

Run options: --seed 24435

# Running:

Finished in 0.000624s, 0.0000 runs/s, 0.0000 assertions/s.
0 runs, 0 assertions, 0 failures, 0 errors, 0 skips
>> BUNDLE_GEMFILE=SOME_PATH/gems/administrate-field-nested_has_many/gemfiles/administrate_0.13.gemfile bundle exec rake
SOME_PATH/.rbenv/versions/2.6.6/bin/ruby -ISOME_PATH/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rspec-core-3.9.1/lib:SOME_PATH/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rspec-support-3.9.2/lib SOME_PATH/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rspec-core-3.9.1/exe/rspec --pattern spec/\*\*\{,/\*/\*\*\}/\*_spec.rb
warning: parser/current is loading parser/ruby26, which recognizes
warning: 2.6.5-compliant syntax, but you are running 2.6.6.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
.........

Finished in 7.96 seconds (files took 6.16 seconds to load)
9 examples, 0 failures

Run options: --seed 26894

# Running:

Finished in 0.001737s, 0.0000 runs/s, 0.0000 assertions/s.
0 runs, 0 assertions, 0 failures, 0 errors, 0 skips
>> BUNDLE_GEMFILE=SOME_PATH/gems/administrate-field-nested_has_many/gemfiles/administrate_master.gemfile bundle exec rake
SOME_PATH/.rbenv/versions/2.6.6/bin/ruby -ISOME_PATH/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rspec-core-3.8.0/lib:SOME_PATH/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rspec-support-3.8.0/lib SOME_PATH/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rspec-core-3.8.0/exe/rspec --pattern spec/\*\*\{,/\*/\*\*\}/\*_spec.rb
warning: parser/current is loading parser/ruby26, which recognizes
warning: 2.6.0-compliant syntax, but you are running 2.6.6.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
.........

Finished in 9.2 seconds (files took 5.88 seconds to load)
9 examples, 0 failures

Run options: --seed 48834

# Running:

Finished in 0.001801s, 0.0000 runs/s, 0.0000 assertions/s.
0 runs, 0 assertions, 0 failures, 0 errors, 0 skips
pablobm commented 4 years ago

I see. Thank you for the fix, but we still need a spec that reproduces the problem. This conversation in the PR is helpful, but in the future we will forget it. Then (in the future) I could see the code and refactor it, deleting your fix by accident. This problem will then come back, and I will not know because there isn't a spec.

So for example, would you be able to provide a new feature spec that would fail without your changes? Or some other sort of spec, as long as it is an example of the problem that you are fixing.

nickcharlton commented 3 years ago

@merqlove, with the changes introduced in #46 and #47, is this still an issue?

merqloveu commented 3 years ago

@merqlove, with the changes introduced in #46 and #47, is this still an issue?

Hello, i can’t check it this moment

nickcharlton commented 3 years ago

This has been open for a while and we'll need tests around any changes, so I'm going to close this for now. Please feel free to re-open if that can be addressed!