MONGOID-5149 Skip inverse detection when inverse_of: nil is set #5027
Conversation
There was a problem hiding this comment.
Hi @simonc , thank you for the PR.
Can you please create your own test case instead of adding your test code to an existing test case. Your issue is with the inverse being explicitly not set, therefore I would expect the test to reflect that in its description (the test you modified has to do with projections).
| @@ -36,6 +44,9 @@ class HmmSchool | |||
| class HmmStudent | |||
| include Mongoid::Document | |||
|
|
|||
| belongs_to :current_academy, class_name: 'HmmAcademy', inverse_of: :students, optional: true | |||
| belongs_to :previous_academy, class_name: 'HmmAcademy', inverse_of: nil, optional: true | |||
There was a problem hiding this comment.
I think these should also go into a new model, unless there is something else from HmmStudent that your test would need.
There was a problem hiding this comment.
I can create a dedicated model for this yes.
|
Hi @p-mongo. Actually it's somewhat related and that's why I've added it to the existing test as it's the very reason I faced the issue in the first place. If the projection lists fields for an association that can conflict with another one due to the bug, you get the That said, I can still create a separate spec for the specific case. |
|
@p-mongo I updated the PR to have its own models and spec. |
|
👍 looks good to me |
p-mongo
changed the title
p-mongo
changed the title
|
Thank you very much @simonc , this looks very clear now. |
* Skipping inverse relation detection when inverse_of: nil is set * Extracting a specific spec for the matter at hand
* Skipping inverse relation detection when inverse_of: nil is set * Extracting a specific spec for the matter at hand
When an association is defined with the
inverse_of: niloption, I believe its inverse should not be looked up in any case.This PR fixes that by skipping the lookup when the option is explicitly defined and set to
nilbut will still let the lookup happen if the option was not specified and is thusnil.I didn't push as to create a
inverse_of_set?method in theOptionsmodule as it would not be defined for any other option but I suppose it could make sense.Why this PR
We actually got a bug while migrating to mongoid 7 where we have the kind of relationship I defined in the spec and due to the way inverse are looked up, trying to get the inverse of
previous_academyhere would return the inverse ofcurrent_academyand that would in the end cause this error:Which doesn't really make sense.