Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: broken swagger and duplicate route #2050

Merged
merged 5 commits into from
Jun 27, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions credentials/apps/api/v2/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
6 changes: 6 additions & 0 deletions credentials/apps/api/v2/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@
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)
Expand Down
3 changes: 3 additions & 0 deletions credentials/apps/credentials/rest_api/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")),
]
125 changes: 68 additions & 57 deletions credentials/apps/credentials/rest_api/v1/views.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -25,68 +27,77 @@ class LearnerCertificateStatusView(APIView):
CanGetLearnerStatus,
)

lms_user_id_schema = openapi.Schema(type=openapi.TYPE_STRING, description="lms_user_id as a string")
Copy link
Contributor

@justinhynes justinhynes Jun 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I don't believe this is a string. The lms_user_id is typically stored as an integer/number.

If I try to use this via Swagger without making lms_user_id a String it will throw an error. However, if you try exercising the API through DRF, this is totally valid data:

{
  "lms_user_id": 3,
  "course_runs": [
    "course-v1:edX+magnets101+2T2023"
  ]
}

If you pass it as a String, it does the right thing though. I expect most times we won't be receiving it as a String, and I have a tiny concern about confusing people who may try to interact with the API via Swagger.


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": <lms_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")

Expand Down
3 changes: 0 additions & 3 deletions credentials/apps/credentials/urls.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,14 @@
"""
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 = [
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")),
]
Loading