compare Field.method to the Link method in AutoSchema manual fields by vdejager · Pull Request #5621 · encode/django-rest-framework (original) (raw)

Hi @vdejager.

Thanks for the input here. We actually decided against this kind of per-method adjustment for the manual_fields parameter. It's adding too much complication to the call site, which can already be verbose enough.

We don't want to add a method attribute to Field. To do so is to give Field knowledge it should not have.

Rather the approach we considered was to allow passing a dictionary to specify fields on a per-method basis...

{
    "GET": [ ... ],
    "POST": [ ... ],
}

It's then obvious that you'll end with repetition here. Looking at ways of handling this led to the decision to just handle the All Methods case, as the most common requirement. (The example we have in mind is adding an additional path or query parameter that is then used in get_object or such.)

The solution for your use-case is to subclass AutoSchema to conditionally add the additional fields in get_link, most likely after just calling super().

You are welcome to adjust how manual_fields is handled in you AutoSchema subclass. (You could for instance pass a per-method data structure...)

I would be open to a PR applying Extract Method to the get_manual_fields logic, currently inline in get_link.

if self._manual_fields is not None:
by_name = {f.name: f for f in fields}
for f in self._manual_fields:
by_name[f.name] = f
fields = list(by_name.values())

This would allow easier customisation of the behaviour here.

In general a PR will need to add tests covering the suggested change, have the test suite pass, and add relevant documentation.