cisagov/manage.get.gov

Refactor AdminSortFields to account for differing models on FK relationships

Opened this issue · 1 comments

Issue description

In admin.py, AdminSortFields is inherited at the base class level on AuditedAdmin. In AdminSortFields, it declares a sort mapping, that maps a field name to a object type, such as "submitter" maps to Contact. Thus, this assumes that each foreign key relation that we sort on would always map to the same underlying model class. However, this is faulty logic and breaks senior_official as the FK maps to Contact for DomainInformation and DomainRequest. For portfolio, however, this field maps to SeniorOfficial.

Acceptance criteria

  • We can use AdminSortFields on fk and many_to_many relationships where they may map to multiple model definitions
  • AdminSortFields is refactored in such a way that it is easier to understand than the previous implementation
    • The current implementation is opaque in that it is hard to tell that it is even being used at all. This is because of how it is being used and where it is defined.
  • The other fields remain sorted as they were

Additional context

A simple (albeit optional) fix would be to expose sort_mapping in a function and override that per class, as needed.

For context, AdminSortFields is a admin.py utility used to return sorted querysets (order_by) from formfield_for_manytomany and formfield_for_foreignkey. These functions are called sequentially for each field in a model (for instance, domain_request or creator) to do an order_by.

To do so, AdminSortFields defines a dictionary called sort_mapping which is called whenever this function is ran. For senior_official, the dropdown list on django admin will contain the values of SeniorOfficial.objects.order_by("first_name", "last_name", "email").

Links to other issues

Originated from PR #2562

I was thinking on this: another solution may be just to do a check on the model class when we do this. If the class doesn't match the class of the field, then we block