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

fixed logic error in empty-collection-message that caused crash #169

Merged
merged 1 commit into from
Nov 7, 2015

Conversation

stevemadere
Copy link

when search results were empty

@iox
Copy link
Member

iox commented Nov 7, 2015

That's very interesting, I though nil.nil? would always return true.

I will merge it, but I would like to learn more, so I don't repeat the same issue in the future. Has this anything to do with the Ruby or Rails version?

Warm regards,
Igancio

iox added a commit that referenced this pull request Nov 7, 2015
fixed logic error in empty-collection-message that caused crash
@iox iox merged commit f71612d into Hobo:master Nov 7, 2015
@iox
Copy link
Member

iox commented Nov 7, 2015

Hi Steve,

Unfortunately, I have had to revert your pull request. In Codeship, and in my local environment, the search results were failing with undefined methodname' for nil:NilClassinhobo_rapid/taglibs/lists/empty_collection_message.dryml:8`

Any idea what the problem could be?

@stevemadere
Copy link
Author

nil.nil? is true

NilClass.nil? is not true.

Notice that the code I changed is calling member_class and checking
if it is nil.

The call to member_class is returning NilClass rather than nil.

That seems reasonable to me, given the name of the method.

On Sat, Nov 7, 2015 at 3:35 AM, Ignacio Huerta notifications@github.com
wrote:

That's very interesting, I though nil.nil? would always return true.

I will merge it, but I would like to learn more, so I don't repeat the
same issue in the future. Has this anything to do with the Ruby or Rails
version?

Warm regards,
Igancio


Reply to this email directly or view it on GitHub
#169 (comment).

@iox
Copy link
Member

iox commented Nov 9, 2015

Ok, I think I found what happened:

member_class is a hobo extension that does not return NilClass. It returns nil. (https://www.omniref.com/ruby/gems/hobo/2.0.1/symbols/ActiveRecord::Associations::CollectionProxy/member_class)

In my test system, the existing code is not returning any error. I can see this message correctly:

captura de pantalla de 2015-11-09 12 13 22

Can you provide with an example to reproduce the crash?

@stevemadere
Copy link
Author

Well at least with some versions of Rails and Hobo
it is returning NilClass rather than nil.

The condition can just be extended to check for both
being nil or equal to NilClass and that would solve
it in both of our environments.

On Mon, Nov 9, 2015 at 5:15 AM, Ignacio Huerta notifications@github.com
wrote:

Ok, I think I found what happened:

member_class is a hobo extension that does not return NilClass. It
returns nil. (
https://www.omniref.com/ruby/gems/hobo/2.0.1/symbols/ActiveRecord::Associations::CollectionProxy/member_class
)

In my test system, the existing code is not returning any error. I can see
this message correctly:

[image: captura de pantalla de 2015-11-09 12 13 22]
https://cloud.githubusercontent.com/assets/452823/11032363/491fe82e-86db-11e5-896a-6c4ed9e75085.png

Can you provide with an example to reproduce the crash?


Reply to this email directly or view it on GitHub
#169 (comment).

@iox
Copy link
Member

iox commented Nov 9, 2015

Oh, that explains it, there must be some Rails version change issue. Thanks a lot for your patience!

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

Successfully merging this pull request may close these issues.

None yet

2 participants