Change api generation #667

acrantel · 2023-08-19T00:17:49Z

API generation changes

switch from typescript-jquery to typescript-fetch so that our generated enums can be strings.
use snake_case in generated types instead of camelCase (to match what backend expects)
various changes in backend files to make the generated schema more accurate

Frontend API client changes

refactored api.ts and auth.ts into multiple files in utils/api.
deleted some legacy functions (especially in compete.ts)
made all our api functions use the generated library instead of jquery

Miscellaneous:

changed the bat file for generating types: generate-types.bat
updated our precommit hooks to exclude the types folder from formatting (see .pre-commit-config.yaml)
deleted jquery from package.json

review requests

You can ignore the types folder when reviewing these files.
please check that the backend schema changes make sense / are accurate

n8kim1

comments are all things to doublecheck, etc -- no feedback that would require a second validation from me
looks good! happy if this just gets revisions and then is merged

(unless we have PRs dismiss on stale)

n8kim1 · 2023-08-20T23:50:13Z

frontend2/generate_types.bat

@@ -8,4 +8,4 @@ python backend/manage.py spectacular --file schema.yml
 :: npm install @openapitools/openapi-generator-cli -g
 :: if you have a java error, just install java 8 and reload terminal
 echo "Generate typescript types from schema"
-npx @openapitools/openapi-generator-cli generate -i schema.yml -o frontend2/src/utils/types -g typescript-jquery --additional-properties=modelPropertyNaming=camelCase --additional-properties=disallowAdditionalPropertiesIfNotPresent=false
+npx @openapitools/openapi-generator-cli generate -i schema.yml -o frontend2/src/utils/types -g typescript-fetch --additional-properties=modelPropertyNaming=original --additional-properties=disallowAdditionalPropertiesIfNotPresent=false --additional-properties=stringEnums=true


what's original vs camelcase? does original just follow what you have
sorry if i missed slack

original follows what the backend uses (aka snake_case). if the schema is generated in camelCase it doesn't work b/c the backend expects snake_case

frontend2/src/components/CurrentUserProvider.tsx

frontend2/src/utils/types/.openapi-generator/FILES

frontend2/src/utils/types/.openapi-generator/VERSION

frontend2/src/utils/api.ts

lowtorola

Looks really awesome! Left a few comments about stability with future typegen runs but if those are non-issues then ship it!

frontend2/src/utils/api.ts

frontend2/src/utils/types/.openapi-generator/FILES

acrantel · 2023-08-23T17:16:00Z

this PR is blocked by #671.

acrantel

~~still todo: update all references to the API in our frontend components~~
done!

acrantel · 2023-08-24T15:54:44Z

backend/siarnaq/api/compete/views.py

@@ -149,8 +150,16 @@ def create(self, request, *, episode_id):
            headers=headers,
        )

+    @extend_schema(
+        # https://drf-spectacular.readthedocs.io/en/latest/faq.html#i-m-using-action-detail-false-but-the-response-schema-is-not-a-list
+        responses=TournamentSubmissionSerializer(many=True)


spectacular wasn't recognizing this as a list of tournament submissions, so I added many=True. With many=True, spectacular thought it was a paginated list, so I had to set pagination_class=None in the action decorator below

acrantel · 2023-08-24T15:55:10Z

backend/siarnaq/api/compete/views.py

@@ -409,7 +418,7 @@ def report(self, request, pk=None, *, episode_id):
        detail=True,
        methods=["post"],
        permission_classes=(IsAdminUser,),
-        serializer_class=None,
+        serializer_class=EmptySerializer,


Spectacular errors if the serializer class is None.

acrantel · 2023-08-24T15:56:35Z

backend/siarnaq/settings.py

+    # Split components into request and response parts where appropriate
+    # This setting is highly recommended to achieve the most accurate API
+    # description, however it comes at the cost of having more components.
+    "COMPONENT_SPLIT_REQUEST": True,


With this, the requests that we pass into API functiosn do not need to contain readonly fields anymore (e.g. no more passing in id:-1 to createuser)

acrantel · 2023-08-24T15:58:21Z

backend/siarnaq/settings.py

+    # ['rest_framework.authentication.TokenAuthentication', ...].
+    # Empty list ([]) will hide all authentication methods. The default None shows all.
+    "AUTHENTICATION_WHITELIST": [
+        "rest_framework_simplejwt.authentication.JWTAuthentication"


This gets rid of a lot of the warnings from spectacular that ask us to make an OpenApiAuthenticationExtension for GoogleCloudAuthentication. We only use JWT authentication from the frontend, so we only need that in our schema.
Below is the warning that this line gets rid of:

Warning #0: MatchViewSet: could not resolve authenticator <class 'siarnaq.api.user.authentication.GoogleCloudAuthentication'>. There was no OpenApiAuthenticationExtension registered for that class. Try creating one by subclassing it. Ignoring for now.

acrantel · 2023-08-24T15:59:36Z

frontend2/generate_types.bat

@@ -8,4 +8,4 @@ python backend/manage.py spectacular --file schema.yml
 :: npm install @openapitools/openapi-generator-cli -g
 :: if you have a java error, just install java 8 and reload terminal
 echo "Generate typescript types from schema"
-npx @openapitools/openapi-generator-cli generate -i schema.yml -o frontend2/src/utils/types -g typescript-jquery --additional-properties=modelPropertyNaming=camelCase --additional-properties=disallowAdditionalPropertiesIfNotPresent=false
+npx @openapitools/openapi-generator-cli generate -i schema.yml -o frontend2/src/utils/types -g typescript-fetch --additional-properties=modelPropertyNaming=original --additional-properties=disallowAdditionalPropertiesIfNotPresent=false --additional-properties=stringEnums=true


original follows what the backend uses (aka snake_case). if the schema is generated in camelCase it doesn't work b/c the backend expects snake_case

acrantel · 2023-08-24T17:21:28Z

backend/siarnaq/api/user/views.py

+            status.HTTP_200_OK: {
+                "type": "object",
+                "additionalProperties": {"$ref": "#/components/schemas/TeamPublic"},
+            }


this schema dict describes the response as an object with arbitrary keys with each value conforming to the schema TeamPublic (which is generated from TeamPublicSerializer)

… api files

stale

n8kim1 · 2023-08-25T23:12:17Z

seems good, would wait for lowell though

lowtorola

Looks fantastic! Love the split API! Next step, implementing team ranking & ranking history!

acrantel changed the base branch from main to frontend2 August 19, 2023 00:18

acrantel force-pushed the api-fetch branch 2 times, most recently from 62c7d07 to 26a8a31 Compare August 19, 2023 00:24

acrantel changed the title ~~Api fetch~~ Change api generation Aug 19, 2023

acrantel requested review from lowtorola and apollo1291 August 19, 2023 00:25

acrantel force-pushed the api-fetch branch from 26a8a31 to 74c270a Compare August 19, 2023 00:30

acrantel requested a review from n8kim1 August 20, 2023 17:37

n8kim1 previously approved these changes Aug 21, 2023

View reviewed changes

lowtorola previously requested changes Aug 21, 2023

View reviewed changes

acrantel force-pushed the api-fetch branch 4 times, most recently from aa49de4 to 986fa3e Compare August 24, 2023 03:20

generate types with typescript-fetch, split-request-response

3f45116

acrantel force-pushed the api-fetch branch from 986fa3e to 3f45116 Compare August 24, 2023 04:48

update manually written API functions

ba7a2d3

acrantel commented Aug 24, 2023

View reviewed changes

acrantel marked this pull request as draft August 24, 2023 16:01

acrantel commented Aug 24, 2023

View reviewed changes

fix schema generation for getting teams of user, move more methods to…

58d7be2

… api files

acrantel force-pushed the api-fetch branch from c0d3195 to 58d7be2 Compare August 24, 2023 19:24

acrantel marked this pull request as ready for review August 24, 2023 19:24

acrantel requested review from lowtorola and n8kim1 August 24, 2023 19:28

remove jquery

84aac30

acrantel force-pushed the api-fetch branch from 00fcfbb to 84aac30 Compare August 24, 2023 19:35

acrantel mentioned this pull request Aug 25, 2023

Add login page #672

Merged

lowtorola approved these changes Aug 25, 2023

View reviewed changes

acrantel merged commit ddfe151 into frontend2 Aug 25, 2023
3 checks passed

acrantel deleted the api-fetch branch August 25, 2023 23:48

acrantel added a commit that referenced this pull request Oct 3, 2023

Change api generation (#667)

d476d84

acrantel added a commit that referenced this pull request Feb 8, 2024

Change api generation (#667)

8d47b09

acrantel added a commit that referenced this pull request Feb 8, 2024

Change api generation (#667)

d1af221

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change api generation #667

Change api generation #667

acrantel commented Aug 19, 2023 •

edited

Loading

n8kim1 left a comment

n8kim1 Aug 20, 2023

acrantel Aug 24, 2023

lowtorola left a comment

acrantel commented Aug 23, 2023

acrantel left a comment •

edited

Loading

acrantel Aug 24, 2023

acrantel Aug 24, 2023

acrantel Aug 24, 2023

lowtorola Aug 25, 2023

acrantel Aug 24, 2023

acrantel Aug 24, 2023

acrantel Aug 24, 2023

n8kim1 commented Aug 25, 2023

lowtorola left a comment

Change api generation #667

Change api generation #667

Conversation

acrantel commented Aug 19, 2023 • edited Loading

n8kim1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lowtorola left a comment

Choose a reason for hiding this comment

acrantel commented Aug 23, 2023

acrantel left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

n8kim1 commented Aug 25, 2023

lowtorola left a comment

Choose a reason for hiding this comment

acrantel commented Aug 19, 2023 •

edited

Loading

acrantel left a comment •

edited

Loading