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

AutoOneToOneField breaks Django admin lookups #86

Open
moorchegue opened this issue Dec 20, 2017 · 5 comments
Open

AutoOneToOneField breaks Django admin lookups #86

moorchegue opened this issue Dec 20, 2017 · 5 comments

Comments

@moorchegue
Copy link

Imagine a lookup model_a__model_b__field_c added to a ModelAdmin.list_filters, which employs AutoOneToOneField as a relationship field between model_a and model_b.

Method lookup_allowed would return false once you try to use it. This is where it goes wrong: https://github.com/django/django/blame/4420761ea9457d386b2000cf9df5b2f6f88f8f91/django/contrib/admin/options.py#L382

            # It is allowed to filter on values that would be found from local
            # model anyways. For example, if you filter on employee__department__id,
            # then the id value would be found already from employee__department_id.
            if not prev_field or (prev_field.is_relation and
                                  field not in prev_field.get_path_info()[-1].target_fields):
                relation_parts.append(part)

While debugging I can see that contents of .target_fields is (<annoying.fields.AutoOneToOneField: model_b>,) at the moment of second iteration, that's what results in False for this whole condition here.

Thus the field is being erroneously excluded from relation_parts list within the method which later leads to clean_lookup becoming model_a__field_c instead of its correct form.

Even with these comments I don't fully understand the purpose of this code and what's expected from the field in this case. List of .target_fields should be empty? Is that universally correct or just within this code?

Could it be that the bug is within Django and not Annoying code? Any ideas how to solve it?

P.S. This might be very confusing. Let me know if any clarification is needed.

@skorokithakis
Copy link
Owner

Thank you for your bug report. Unfortunately, I inherited this code and am not very familiar with this field. The code above seems to be a check for filtering on values on the "remote" model that also exist on the "local" model. As the comment says, you can filter on employee__department__id even though you can also just look at employee__department_id for the same information. I'm not sure why it appends to the parts, though.

@moorchegue
Copy link
Author

moorchegue commented Dec 26, 2017

Which makes sense for PKs, but doesn't for more complicated lookups, right?

So if this code is to blame, and not the field itself, I wonder how the condition should look like then?

@skorokithakis
Copy link
Owner

I would think so. Unfortunately I'm not sure how you could work around this... Is it excluded because is_relation is not set? Then mayber we need to set that to True?

@moorchegue
Copy link
Author

No, is_relation == True. The problem is with the last section: field not in prev_field.get_path_info()[-1].target_fields, field actually is in .target_fields list.

@skorokithakis
Copy link
Owner

Oh, hmm. In that case, I'm not sure, I'd have to trace more of the code to figure it out...

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

No branches or pull requests

2 participants