Request-based endpoint behaviour

drf-forms / drf-schema-adapter

Making using Django with frontend libraries and frameworks DRYer

http://drf-schema-adapter.readthedocs.io/en/latest/

MIT License

157 stars 40 forks source link

Request-based endpoint behaviour #53

Closed SupraSummus closed 1 year ago

SupraSummus commented 5 years ago

Hi,

These changes enable endpoint logic to depend on incoming request without need to override a ton of methods. For example if one wants to pass context={'request': request} to serializer she needs to reimplement only Endpoint.get_serializer_instance().

~~Also, I tried to simplify code in metadata.py and get rid of using exceptions as flow control.~~

~~Changes in settings.py seem needed for django 2.2 testing. I may revert that if needed - let me know.~~

For the glory of mankind, Jan Rydzewski

nanuxbe commented 5 years ago

Hi,

first of all, thanks for the PR :-)

This is quite a big pull request, I will try to take a closer look at it this coming week.

Things I can already say:

if a change in settings.py are required for Django 2.2, they should be behind a version check, for now this is still compatible with Django 1.8+ (See the discussion here: https://github.com/drf-forms/drf-schema-adapter/issues/44 for more info on the plan to drop support for the 1.x versions of Django)
even if tests seem to keep passing, I am fairly certain there was a reason for going with serializer instead of serializer_instance in many places. This reason might be outdated (ie: DRF 3.2) but I do need to do more checks (and eventually write more tests) about that.

SupraSummus commented 5 years ago

I've reverted changes in settings.py as they seem out of this PR's scope.
I've added test to depict how serializer field can depend on request.
As for serializer vs serializer_instance discussion: My changes are only about this library internal naming and argument passing. I believe I haven't changed any interfaces to DRF.

nanuxbe commented 5 years ago

Sorry for the late reply again and thanks for the fixes.

From what I see though, the builds still fail for Django 1.8 (failed tests on Django2.2 and Python3.7 are a problem with Travis, not the code).

One of the things that has been in the planning for a few months now is to stop development (except for paying parties) relating to Django 1.x and Python2 and store the current state of the project in a 1.x branch that would be compatible with Django 1.8 -> 2.2 and Python 2.7 & all listed Python 3.x. At the same time, there will be a cleaned-up (ie: no six or version checking) version of the code that would start a 2.x branch/release cycle.

So, unless you provide a fix for Django 1.8 (which I totally understand you wouldn't be interesting in doing), I'll keep this PR on hold until the 2.x branch is out and apply it to that branch.

Thanks for all the work!

SupraSummus commented 5 years ago

I've fixed tests failing under django 1.8. (It was caused by django1.8 not having response.json() method in test client.)

edit: Hold on. Not really fixed yet ;)

SupraSummus commented 5 years ago

Ok, now it is. All green

SupraSummus commented 4 years ago

@nanuxbe could this be merged? Are there any changes required?

nanuxbe commented 1 year ago

The merge conflicts here were a nightmare, so I transposed the PR here: https://github.com/drf-forms/drf-schema-adapter/pull/78

(also, did I fall asleep during COVID and just woke up now???)