Don't raise 500 errors when filter query parameter matches a model property name.

[omerfarukabaci]

Fixes: #7608

What is in this PR:

• A model property called description added to the OrderingFilterModel
• Created OrderingFilterSerializerWithModelProperty which includes all fields of OrderingFilterModel and the model property description.
• Failing test has been added under test_ordering_without_ordering_fields with two passing assert statements to see using model properties on a serializer with OrderingFilter doesn't have any side effect.
• A solution has been added to OrderingFilter.get_default_valid_fields() method which basically checks if the field_name or field.source is in model_field_names field.source is in model_property_names (c7f8b14).

[tomchristie]

I'm not convinced this is necessarily correct. For example we allow sources such as author.name, which will reference the name field across the relationship author, but that'd be filtered out by this new check, right?

[omerfarukabaci]

Yes, you are right, I have missed that. 😕 What about the following solution:

model_class = queryset.model
model_property_names = [
    # "pk" is a property added in Django's Model class, however it is valid for ordering.
    attr for attr in dir(model_class) if isinstance(getattr(model_class, attr), property) and attr != 'pk'
]
return [
    (field.source.replace('.', '__') or field_name, field.label)
    for field_name, field in serializer_class(context=context).fields.items()
    if not getattr(field, 'write_only', False) and not field.source == '*'
    if (
        not getattr(field, 'write_only', False) and
        not field.source == '*' and
        field_name not in model_property_names and
        field.source not in model_property_names
    )
]

or do you have any other suggestions?

[tomchristie]

That looks okay to me, I think? (Although probably just need field.source not in model_property_names, rather than field_name not in model_property_names?)

[omerfarukabaci]

Oh I didn't know that field.source is field_name by default, you are totally right then! 🚀 I think that the comment line about pk should stay, what do you think? After we decide this I will make the related changes. 👍🏼

[tomchristie]

Yes the PK comment makes sense to me.

[omerfarukabaci]

Pushed related changes: c7f8b14. If you have any other concerns we may further discuss. Thank you for your time! 🙏🏼

[tomchristie]

I think I'm okay with this. Anyone else want to chime in before we merge this?

[tomchristie]

Great, thanks.

[omerfarukabaci]

My pleasure! 🎉