skorokithakis/django-annoying

AutoOneToOneField breaks Django admin lookups

Opened this issue · 5 comments

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.

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.

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?

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?

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.

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