-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: Enable Wagtail API #3399
base: master
Are you sure you want to change the base?
feat: Enable Wagtail API #3399
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added a few suggestions.
api_router = WagtailAPIRouter("wagtailapi") | ||
api_router.register_endpoint("pages", CustomPagesAPIViewSet) | ||
api_router.register_endpoint("images", CustomImagesAPIViewSet) | ||
api_router.register_endpoint("documents", CustomDocumentsAPIViewSet) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do this in the urls.py.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add module, class, and method docs at all places.
api_router.register_endpoint("pages", CustomPagesAPIViewSet) | ||
api_router.register_endpoint("images", CustomImagesAPIViewSet) | ||
api_router.register_endpoint("documents", CustomDocumentsAPIViewSet) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about we add a URL above the pages that would look like: api/v2/courses/<READABLE_ID>, Then our custom viewset can handle the readable ID and filter the data for us?
Same goes for the programs, api/v2/programs/<readable_id>
def filter_queryset(self, request, queryset, view): | ||
field_name = "readable_id" | ||
if field_name in request.GET: | ||
value = request.GET[field_name].replace(" ", "+") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any specific cases that we are trying to handle here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
readable_id is usually of this form course-v1:edX+DemoX+Demo_Course
, when received as query params, it is converted to course-v1:edX DemoX Demo_Course
, filtering on which would return nothing
api_fields = [ | ||
APIField("child_pages", serializer=ProductChildPageSerializer()), | ||
] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Current API does not return the custom page fields like: subhead etc. We should return the custom page data + child page fields as well.
What are the relevant tickets?
#6690
Description (What does it do?)
The PR adds the basic setup for testing the wagtail API
Changes:
/api/v2/
to serve the Wagtail API.Page
,Image
, andDocument
viewsets under thecms
application inwagtail_api.py
.How can this be tested?
How to Test
Clone and switch to the feature branch: areeb/6699-exploring-wagtail-api.
Ensure xPRO is Properly Configured
Generate an Access Token
Set Up Postman for Testing
base_url
: Default if testing locally →http://xpro.odl.local:8053
access_token
: The token generated in the previous step.Use Postman to make authenticated requests and explore the various Wagtail API endpoints.