Migrate to django-ninja #745

danlamanna · 2023-08-09T15:56:15Z

related https://github.com/ImageMarkup/tracker/issues/143

This is essentially a direct port of DRF's cursor-based pagination impl to django-ninja.

zachmullen · 2023-08-16T23:33:08Z

@danlamanna the tests are now passing; this is probably a good time to do a mid-stream code review to make sure the changes I made are reasonable.

zachmullen · 2023-08-16T23:33:54Z

isic/core/tests/test_api_collection.py

-            f"/api/v2/collections/{collection.pk}/populate-from-search/", {"query": "sex:male"}
+            f"/api/v2/collections/{collection.pk}/populate-from-search/",
+            {"query": "sex:male"},
+            content_type="application/json",


Are we ok with having to add this to every test request with a JSON body, or should we make our own subclass of Django's test Client?

A: We should use ninja.testing.TestClient

FWIW I think using the django.test.Client is preferable until this issue is resolved.

isic/core/api/collection.py

This should go away when vitalik/django-ninja#825 is resolved.

isic/core/api/collection.py

We were already checking for other permissions on them before taking action, but we should also make sure the user can see them first.

isic/core/tests/test_search.py

isic/studies/api.py

isic/core/api/image.py

zachmullen · 2023-08-22T19:42:54Z

This is now ready for final review. All references to DRF have been removed from this codebase. Here's a comparison of the old and new swagger pages:

These are very sweeping changes, so there's a high likelihood that some behavioral changes have slipped through the cracks. If you have a good way to manually verify zip downloading end-to-end, that would be good, as I have not done so locally.

danlamanna

This looks awesome! Reviewing this PR reminded me of how big our API surface is, so thanks for slogging through it all. Zip download works well on my end.

isic/core/api/collection.py

isic/core/api/image.py

isic/core/templates/ninja/swagger.html

isic/core/pagination.py

isic/core/templates/ninja/swagger.html

isic/zip_download/api.py

isic/zip_download/tests/test_zip_download.py

Co-authored-by: Dan LaManna <[email protected]>

Because we want to treat the cursor values as opaque, it seemed simplest to do this all in one test case.

isic/zip_download/api.py

zachmullen · 2023-08-23T17:56:49Z

I believe all feedback has been addressed, PTAL.

zachmullen force-pushed the isic-143-ninja-migration branch from 874b108 to 64242e9 Compare August 13, 2023 00:13

danlamanna and others added 4 commits August 16, 2023 13:09

Add django-ninja dependency

b22350c

Refactor quickfind API to use Ninja

5274d53

Refactor collections API to use Ninja

c83005b

Register ninja URLs

dcec972

zachmullen force-pushed the isic-143-ninja-migration branch 2 times, most recently from 9a7a402 to a4e6929 Compare August 16, 2023 17:55

django-ninja cursor pagination implementation

39974f7

This is essentially a direct port of DRF's cursor-based pagination impl to django-ninja.

zachmullen force-pushed the isic-143-ninja-migration branch from a4e6929 to 39974f7 Compare August 16, 2023 17:59

zachmullen added 2 commits August 16, 2023 14:38

Add permission filtering to collection endpoints

a58f83b

Update tests to use Django's test client

a4a33ad

zachmullen force-pushed the isic-143-ninja-migration branch from 1407952 to a4a33ad Compare August 16, 2023 20:35

zachmullen added 2 commits August 16, 2023 18:16

Rectify schemas

91dab43

Fix logic in collection/populate-from-list/

c23afb1

zachmullen reviewed Aug 16, 2023

View reviewed changes

isic/core/api/collection.py Show resolved Hide resolved

Improve error response from pydantic validator

ce32a05

This should go away when vitalik/django-ninja#825 is resolved.

danlamanna commented Aug 17, 2023

View reviewed changes

isic/core/api/collection.py Outdated Show resolved Hide resolved

zachmullen added 4 commits August 17, 2023 11:57

Update django-ninja for bugfix

7714700

Ensure view permission check on collection endpoints

f1a3183

We were already checking for other permissions on them before taking action, but we should also make sure the user can see them first.

Make swagger page mostly identical to old version

b1560f4

Convert user endpoints to django-ninja

849624a

zachmullen force-pushed the isic-143-ninja-migration branch 2 times, most recently from 4c556b3 to cd231d2 Compare August 19, 2023 01:01

Convert images endpoints to django-ninja

61869b7

zachmullen force-pushed the isic-143-ninja-migration branch from cd231d2 to 61869b7 Compare August 19, 2023 01:08

zachmullen reviewed Aug 19, 2023

View reviewed changes

isic/core/tests/test_search.py Outdated Show resolved Hide resolved

zachmullen force-pushed the isic-143-ninja-migration branch 2 times, most recently from f80bf94 to 81c996c Compare August 21, 2023 01:02

Convert ingest endpoints to ninja

9ca745e

zachmullen force-pushed the isic-143-ninja-migration branch from e6faa6a to 9ca745e Compare August 21, 2023 11:25

zachmullen added 2 commits August 21, 2023 10:25

Add a test to exercise the auth behavior of SessionAuthStaffUser

253bfeb

Convert studies endpoints to django-ninja

7c026e2

zachmullen reviewed Aug 21, 2023

View reviewed changes

isic/studies/api.py Show resolved Hide resolved

Return 400 on image search query parse failures

f8d7eb4

zachmullen force-pushed the isic-143-ninja-migration branch from 8f9e269 to f8d7eb4 Compare August 21, 2023 18:02

zachmullen reviewed Aug 21, 2023

View reviewed changes

isic/core/api/image.py Show resolved Hide resolved

zachmullen force-pushed the isic-143-ninja-migration branch from 7ca69bc to 76326a4 Compare August 21, 2023 22:42

Remove obviated rest_framework code

abac9e5

zachmullen force-pushed the isic-143-ninja-migration branch from 76326a4 to abac9e5 Compare August 21, 2023 22:44

zachmullen added 3 commits August 22, 2023 09:31

Convert zip_download to ninja

b2bd181

Remove old SearchQuerySerializer

5e7b162

Convert quickfind serializers to ninja

c188eed

zachmullen force-pushed the isic-143-ninja-migration branch from 6f679c2 to c188eed Compare August 22, 2023 19:32

Hide zip download endpoint in swagger

a3599be

zachmullen marked this pull request as ready for review August 22, 2023 19:42

danlamanna commented Aug 22, 2023

View reviewed changes

danlamanna and others added 7 commits August 22, 2023 19:48

Add test for ensuring image listing returns distinct images

c95bd76

Fix typo in endpoint summary

dec335f

Co-authored-by: Dan LaManna <[email protected]>

Remove banner and favicon from swagger page

1bfd43d

Co-authored-by: Dan LaManna <[email protected]>

Reintroduce distinct() on Image queryset

9eabdce

Exempt ninja's Query from B008 lint rule

715cf03

Add test for pagination

395c9b9

Because we want to treat the cursor values as opaque, it seemed simplest to do this all in one test case.

Use custom ninja authentication class for zip download token management

37d8bb3

zachmullen reviewed Aug 23, 2023

View reviewed changes

isic/zip_download/api.py Show resolved Hide resolved

danlamanna merged commit cad89ff into master Aug 23, 2023
2 checks passed

danlamanna deleted the isic-143-ninja-migration branch August 23, 2023 18:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate to django-ninja #745

Migrate to django-ninja #745

danlamanna commented Aug 9, 2023

zachmullen commented Aug 16, 2023

zachmullen Aug 16, 2023

zachmullen Aug 17, 2023

zachmullen Aug 17, 2023 •

edited

Loading

zachmullen commented Aug 22, 2023

danlamanna left a comment •

edited

Loading

zachmullen commented Aug 23, 2023

Migrate to django-ninja #745

Migrate to django-ninja #745

Conversation

danlamanna commented Aug 9, 2023

zachmullen commented Aug 16, 2023

zachmullen Aug 16, 2023

Choose a reason for hiding this comment

zachmullen Aug 17, 2023

Choose a reason for hiding this comment

zachmullen Aug 17, 2023 • edited Loading

Choose a reason for hiding this comment

zachmullen commented Aug 22, 2023

danlamanna left a comment • edited Loading

Choose a reason for hiding this comment

zachmullen commented Aug 23, 2023

zachmullen Aug 17, 2023 •

edited

Loading

danlamanna left a comment •

edited

Loading