From 77b185bf5bd44b2e2fcf2317aeaddb99a229463e Mon Sep 17 00:00:00 2001 From: Deborah Kaplan Date: Thu, 22 Jun 2023 21:59:32 +0000 Subject: [PATCH 1/5] APER-2531 swagger and fixed route * specifies the swagger schema for a non-serializer class * removes redunandant route via wrong endpoint * adds a comment in a few files so the lower API version number doesn't confuse future maintainers (principle of least surprise) --- credentials/apps/api/v2/urls.py | 5 + credentials/apps/api/v2/views.py | 5 + credentials/apps/credentials/rest_api/urls.py | 3 + .../apps/credentials/rest_api/v1/views.py | 131 ++++++++++-------- credentials/urls.py | 1 - 5 files changed, 87 insertions(+), 58 deletions(-) diff --git a/credentials/apps/api/v2/urls.py b/credentials/apps/api/v2/urls.py index c43e07103..c261ae923 100644 --- a/credentials/apps/api/v2/urls.py +++ b/credentials/apps/api/v2/urls.py @@ -4,6 +4,11 @@ from credentials.apps.api.v2 import views +# NOTE: Although this is v2 and other APIs in this application are v1, +# the API naming and code layout convention here is not to be used for new +# endpoints, per: +# https://openedx.atlassian.net/wiki/spaces/AC/pages/18350757/edX+REST+API+Conventions + urlpatterns = [re_path(r"^replace_usernames/$", views.UsernameReplacementView.as_view(), name="replace_usernames")] router = DefaultRouter() diff --git a/credentials/apps/api/v2/views.py b/credentials/apps/api/v2/views.py index 6f86dbf9f..416c609a4 100644 --- a/credentials/apps/api/v2/views.py +++ b/credentials/apps/api/v2/views.py @@ -25,6 +25,11 @@ log = logging.getLogger(__name__) +# NOTE: Although this is v2 and other APIs in this application are v1, +# the API naming and code layout convention here is not to be used for new +# endpoints, per: +# https://openedx.atlassian.net/wiki/spaces/AC/pages/18350757/edX+REST+API+Conventions + def credentials_throttle_handler(exc, context): """Exception handler for logging messages when an endpoint is throttled.""" response = exception_handler(exc, context) diff --git a/credentials/apps/credentials/rest_api/urls.py b/credentials/apps/credentials/rest_api/urls.py index fada0d11f..331eebee0 100644 --- a/credentials/apps/credentials/rest_api/urls.py +++ b/credentials/apps/credentials/rest_api/urls.py @@ -3,6 +3,9 @@ from credentials.apps.credentials.rest_api.v1 import urls as v1_credentials_api_urls +# NOTE: Although this is v1 and other APIs in this application are v2, +# the API naming and code layout convention here is what we are using, per +# https://openedx.atlassian.net/wiki/spaces/AC/pages/18350757/edX+REST+API+Conventions urlpatterns = [ url(r"^v1/", include((v1_credentials_api_urls, "v1"), namespace="v1")), ] diff --git a/credentials/apps/credentials/rest_api/v1/views.py b/credentials/apps/credentials/rest_api/v1/views.py index a96e352f1..dd1d8d6b4 100644 --- a/credentials/apps/credentials/rest_api/v1/views.py +++ b/credentials/apps/credentials/rest_api/v1/views.py @@ -1,6 +1,8 @@ import logging from django.contrib.auth import get_user_model +from drf_yasg import openapi +from drf_yasg.utils import swagger_auto_schema from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication from rest_framework import permissions, status from rest_framework.authentication import SessionAuthentication @@ -25,68 +27,83 @@ class LearnerCertificateStatusView(APIView): CanGetLearnerStatus, ) + lms_user_id_schema = openapi.Schema( + type=openapi.TYPE_STRING, + description='lms_user_id as a string') + + username_schema = openapi.Schema( + type=openapi.TYPE_STRING, + description='username') + + per_course_grade_schema = openapi.Schema( + type=openapi.TYPE_OBJECT, + properties={ + 'letter_grade': openapi.Schema(type=openapi.TYPE_STRING), + 'percent_grade': openapi.Schema(type=openapi.FORMAT_DECIMAL), + 'verified': openapi.Schema(type=openapi.TYPE_BOOLEAN), + } + ) + course_run_object_schema = openapi.Schema( + type=openapi.TYPE_OBJECT, + properties={ + 'uuid': openapi.Schema(type=openapi.TYPE_STRING), + 'key': openapi.Schema(type=openapi.TYPE_STRING), + 'status': openapi.Schema(type=openapi.TYPE_STRING), + 'type': openapi.Schema(type=openapi.TYPE_STRING), + 'certificate_available_date': openapi.Schema(type=openapi.FORMAT_DATE), + 'grade': per_course_grade_schema, + } + ) + per_course_status_schema = openapi.Schema( + type=openapi.TYPE_OBJECT, + properties={ + 'course_uuid': openapi.TYPE_STRING, + 'course_run': course_run_object_schema, + } + ) + + learner_cert_status_request_schema = openapi.Schema( + type=openapi.TYPE_OBJECT, + properties={ + 'lms_user_id': lms_user_id_schema, + 'username': username_schema, + 'courses': openapi.Schema( + type=openapi.TYPE_ARRAY, + items=openapi.Items(type=openapi.TYPE_STRING), + description='array of strings'), + 'course_runs': openapi.Schema( + type=openapi.TYPE_ARRAY, + items=openapi.Items(type=openapi.TYPE_STRING), + description='array of strings'), + }, + ) + + learner_cert_status_return_schema = openapi.Schema( + type=openapi.TYPE_OBJECT, + properties={ + 'lms_user_id': lms_user_id_schema, + 'username': username_schema, + 'status': openapi.Schema( + type=openapi.TYPE_ARRAY, + items=course_run_object_schema, + ) + }, + ) + + learner_cert_status_responses = { + status.HTTP_200_OK: learner_cert_status_return_schema, + } + + @swagger_auto_schema( + request_body=learner_cert_status_request_schema, + responses=learner_cert_status_responses, + ) def post(self, request): """ - **POST Parameters** - A POST request must include one of "lms_user_id" or "username", and a list of course uuids, course_runs, or a mix of both. (or a program uuid, in a future version) - - { - "lms_user_id": , - "courses": [ - "course_uuid1", - "course_uuid2" - ... - ], - "course_runs": [ - "course_run_uuid1", - "course_run_key2", - ... - ] - } - - **POST Response Values** - - The request will return a 200 with a list of learner cert statuses. - - { - "lms_user_id": 3, - "username": "edx", - "status": [ - { - "course_uuid": "8759ceb8-7112-4b48-a9b4-8a9a69fdad51", - "course_run": { - "uuid": "0e63eeea-f957-4d38-884a-bf7af5af6155", - "key": "course-v1:edX+TK-100+2T2022" - }, - "status": "awarded", - "type": "verified", - "certificate_available_date": null, - "grade": { - "letter_grade": "Pass", - "percent_grade": 75.0, - "verified": true - } - }, - { - "course_uuid": "d81fce24-c0e3-49cc-b375-51a02c79aa9d", - "course_run": { - "uuid": "b4a38fe1-93b6-4fa6-a834-a656bcf9e75c", - "key": "course-v1:edX+CRYPT101+1T2023" - }, - "status": "awarded", - "type": "verified", - "certificate_available_date": null, - "grade": { - "letter_grade": "Pass", - "percent_grade": 70.5, - "verified": true - } - } - ] - }""" + """ lms_user_id = request.data.get("lms_user_id") username = request.data.get("username") diff --git a/credentials/urls.py b/credentials/urls.py index 97923e607..b73badfd3 100644 --- a/credentials/urls.py +++ b/credentials/urls.py @@ -59,7 +59,6 @@ re_path(r"^api-auth/", include((oauth2_urlpatterns, "rest_framework"), namespace="rest_framework")), re_path(r"^api-docs/$", schema_view.with_ui("swagger", cache_timeout=0), name="api_docs"), re_path(r"^auto_auth/$", core_views.AutoAuth.as_view(), name="auto_auth"), - re_path(r"^credentials/", include(("credentials.apps.credentials.urls", "credentials"), namespace="credentials")), re_path(r"^health/$", core_views.health, name="health"), re_path( r"^management/", include(("credentials.apps.edx_django_extensions.urls", "management"), namespace="management") From a2e12de197976a1599f36d71d5d149eb8688508d Mon Sep 17 00:00:00 2001 From: Deborah Kaplan Date: Thu, 22 Jun 2023 22:07:15 +0000 Subject: [PATCH 2/5] ran black to reformat --- credentials/apps/api/v2/views.py | 1 + .../apps/credentials/rest_api/v1/views.py | 74 +++++++++---------- 2 files changed, 35 insertions(+), 40 deletions(-) diff --git a/credentials/apps/api/v2/views.py b/credentials/apps/api/v2/views.py index 416c609a4..a956eaa4e 100644 --- a/credentials/apps/api/v2/views.py +++ b/credentials/apps/api/v2/views.py @@ -30,6 +30,7 @@ # endpoints, per: # https://openedx.atlassian.net/wiki/spaces/AC/pages/18350757/edX+REST+API+Conventions + def credentials_throttle_handler(exc, context): """Exception handler for logging messages when an endpoint is throttled.""" response = exception_handler(exc, context) diff --git a/credentials/apps/credentials/rest_api/v1/views.py b/credentials/apps/credentials/rest_api/v1/views.py index dd1d8d6b4..c1978998a 100644 --- a/credentials/apps/credentials/rest_api/v1/views.py +++ b/credentials/apps/credentials/rest_api/v1/views.py @@ -27,66 +27,60 @@ class LearnerCertificateStatusView(APIView): CanGetLearnerStatus, ) - lms_user_id_schema = openapi.Schema( - type=openapi.TYPE_STRING, - description='lms_user_id as a string') + lms_user_id_schema = openapi.Schema(type=openapi.TYPE_STRING, description="lms_user_id as a string") - username_schema = openapi.Schema( - type=openapi.TYPE_STRING, - description='username') + username_schema = openapi.Schema(type=openapi.TYPE_STRING, description="username") per_course_grade_schema = openapi.Schema( type=openapi.TYPE_OBJECT, properties={ - 'letter_grade': openapi.Schema(type=openapi.TYPE_STRING), - 'percent_grade': openapi.Schema(type=openapi.FORMAT_DECIMAL), - 'verified': openapi.Schema(type=openapi.TYPE_BOOLEAN), - } + "letter_grade": openapi.Schema(type=openapi.TYPE_STRING), + "percent_grade": openapi.Schema(type=openapi.FORMAT_DECIMAL), + "verified": openapi.Schema(type=openapi.TYPE_BOOLEAN), + }, ) course_run_object_schema = openapi.Schema( - type=openapi.TYPE_OBJECT, - properties={ - 'uuid': openapi.Schema(type=openapi.TYPE_STRING), - 'key': openapi.Schema(type=openapi.TYPE_STRING), - 'status': openapi.Schema(type=openapi.TYPE_STRING), - 'type': openapi.Schema(type=openapi.TYPE_STRING), - 'certificate_available_date': openapi.Schema(type=openapi.FORMAT_DATE), - 'grade': per_course_grade_schema, - } + type=openapi.TYPE_OBJECT, + properties={ + "uuid": openapi.Schema(type=openapi.TYPE_STRING), + "key": openapi.Schema(type=openapi.TYPE_STRING), + "status": openapi.Schema(type=openapi.TYPE_STRING), + "type": openapi.Schema(type=openapi.TYPE_STRING), + "certificate_available_date": openapi.Schema(type=openapi.FORMAT_DATE), + "grade": per_course_grade_schema, + }, ) per_course_status_schema = openapi.Schema( - type=openapi.TYPE_OBJECT, - properties={ - 'course_uuid': openapi.TYPE_STRING, - 'course_run': course_run_object_schema, - } + type=openapi.TYPE_OBJECT, + properties={ + "course_uuid": openapi.TYPE_STRING, + "course_run": course_run_object_schema, + }, ) learner_cert_status_request_schema = openapi.Schema( type=openapi.TYPE_OBJECT, properties={ - 'lms_user_id': lms_user_id_schema, - 'username': username_schema, - 'courses': openapi.Schema( - type=openapi.TYPE_ARRAY, - items=openapi.Items(type=openapi.TYPE_STRING), - description='array of strings'), - 'course_runs': openapi.Schema( - type=openapi.TYPE_ARRAY, - items=openapi.Items(type=openapi.TYPE_STRING), - description='array of strings'), + "lms_user_id": lms_user_id_schema, + "username": username_schema, + "courses": openapi.Schema( + type=openapi.TYPE_ARRAY, items=openapi.Items(type=openapi.TYPE_STRING), description="array of strings" + ), + "course_runs": openapi.Schema( + type=openapi.TYPE_ARRAY, items=openapi.Items(type=openapi.TYPE_STRING), description="array of strings" + ), }, ) learner_cert_status_return_schema = openapi.Schema( type=openapi.TYPE_OBJECT, properties={ - 'lms_user_id': lms_user_id_schema, - 'username': username_schema, - 'status': openapi.Schema( + "lms_user_id": lms_user_id_schema, + "username": username_schema, + "status": openapi.Schema( type=openapi.TYPE_ARRAY, items=course_run_object_schema, - ) + ), }, ) @@ -95,8 +89,8 @@ class LearnerCertificateStatusView(APIView): } @swagger_auto_schema( - request_body=learner_cert_status_request_schema, - responses=learner_cert_status_responses, + request_body=learner_cert_status_request_schema, + responses=learner_cert_status_responses, ) def post(self, request): """ From 050b70463ed5e5d1f37d1930affcf51eff1a9aff Mon Sep 17 00:00:00 2001 From: Deborah Kaplan Date: Fri, 23 Jun 2023 17:56:44 +0000 Subject: [PATCH 3/5] APER-2531 fixed namespace problem the actual correct fix for the nested namespaces bug --- credentials/apps/credentials/urls.py | 1 - credentials/urls.py | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/credentials/apps/credentials/urls.py b/credentials/apps/credentials/urls.py index 80d9441a6..8357b6485 100644 --- a/credentials/apps/credentials/urls.py +++ b/credentials/apps/credentials/urls.py @@ -13,5 +13,4 @@ re_path(r"^example/$", views.ExampleCredential.as_view(), name="example"), re_path(rf"^example/{UUID_PATTERN}/$", views.RenderExampleProgramCredential.as_view(), name="render_example"), re_path(rf"^{UUID_PATTERN}/$", views.RenderCredential.as_view(), name="render"), - url(r"^api/", include((credentials_api_v1_urls, "api"), namespace="api")), ] diff --git a/credentials/urls.py b/credentials/urls.py index b73badfd3..97923e607 100644 --- a/credentials/urls.py +++ b/credentials/urls.py @@ -59,6 +59,7 @@ re_path(r"^api-auth/", include((oauth2_urlpatterns, "rest_framework"), namespace="rest_framework")), re_path(r"^api-docs/$", schema_view.with_ui("swagger", cache_timeout=0), name="api_docs"), re_path(r"^auto_auth/$", core_views.AutoAuth.as_view(), name="auto_auth"), + re_path(r"^credentials/", include(("credentials.apps.credentials.urls", "credentials"), namespace="credentials")), re_path(r"^health/$", core_views.health, name="health"), re_path( r"^management/", include(("credentials.apps.edx_django_extensions.urls", "management"), namespace="management") From 49c7a6ea3ec2666263aee96d7f6d927a8276d99d Mon Sep 17 00:00:00 2001 From: Deborah Kaplan Date: Mon, 26 Jun 2023 20:27:43 +0000 Subject: [PATCH 4/5] fix: broken swagger and duplicate route This API contains a non-CRUD class with no serializer, which the swagger conversion can't handle automagically. Additionally, there was an endpoint that had been imported twice because of a file structure containing multiple urls.py files. * specifies the swagger schema for a non-serializer class * removes redunandant route via wrong endpoint Note that the generated swagger document is somewhat confusing, and does not correctly group the different APIs. This is unavoidable because of two separate complicating factors: * the openAPI specification, which cannot has certain assumptions about base prefix * changes within this cluster of APIs, in which endpoint structure has had three distinct layouts, before we reached the current, edx-standard structure. (`appname/api/v#/endpoint_name`, `api/v#/endpoint_name`, and `api/appname/v#/endpoint_name`) There are ways to address this, but they are all messy (e.g. creating aliases for old endpoints). Fixes: APER-2531 --- credentials/apps/credentials/urls.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/credentials/apps/credentials/urls.py b/credentials/apps/credentials/urls.py index 8357b6485..135294d3c 100644 --- a/credentials/apps/credentials/urls.py +++ b/credentials/apps/credentials/urls.py @@ -1,12 +1,10 @@ """ URLs for the credentials views. """ -from django.conf.urls import include, url from django.urls import re_path from credentials.apps.credentials import views from credentials.apps.credentials.constants import UUID_PATTERN -from credentials.apps.credentials.rest_api.v1 import urls as credentials_api_v1_urls urlpatterns = [ From 0fdd31d7b0d53afeff843cffa23021eb1078b6a7 Mon Sep 17 00:00:00 2001 From: Deborah Kaplan Date: Tue, 27 Jun 2023 16:46:39 +0000 Subject: [PATCH 5/5] fix: broken swagger and duplicate route * fixed data type of one field --- credentials/apps/credentials/rest_api/v1/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/credentials/apps/credentials/rest_api/v1/views.py b/credentials/apps/credentials/rest_api/v1/views.py index c1978998a..852de90ca 100644 --- a/credentials/apps/credentials/rest_api/v1/views.py +++ b/credentials/apps/credentials/rest_api/v1/views.py @@ -27,7 +27,7 @@ class LearnerCertificateStatusView(APIView): CanGetLearnerStatus, ) - lms_user_id_schema = openapi.Schema(type=openapi.TYPE_STRING, description="lms_user_id as a string") + lms_user_id_schema = openapi.Schema(type=openapi.TYPE_INTEGER, description="lms_user_id") username_schema = openapi.Schema(type=openapi.TYPE_STRING, description="username")