feat: Add API for retrieving moderation reason codes from the LMS settings [BD-38] [TNL-9479] [BB-5419]#30015
Conversation
|
Thanks for the pull request, @xitij2000! I've created BLENDED-1123 to keep track of it in Jira. More details are on the BD-38 project page. When this pull request is ready, tag your edX technical lead. |
openedx-webhooks
added
blended
xitij2000
force-pushed
the
felipetrz-kshitij/BB-5419-add-reason-codes-retrieval-api_
branch
from
89e3ec3 to
7e93ed5
Compare
tecoholic
left a comment
tecoholic
left a comment
There was a problem hiding this comment.
👍
- I tested this:
- Tested the API along with the UI while testing frontend-app-discussions#46.
- Tested that the API returns eanbled only with the right Waffle flag on a per course basis.
- Tested the API returns expected reasons
- I read through the code
- I checked for accessibility issues - Values introduced in the settings are wrapped in i18n utilities
_()as expected - Includes documentation
xitij2000
force-pushed
the
felipetrz-kshitij/BB-5419-add-reason-codes-retrieval-api_
branch
from
7e93ed5 to
f218858
Compare
There was a problem hiding this comment.
@asadazam93 The original author of this PR made this a separate endpoint, but I'm wondering if it's better to just include this in the existing API endpoint at: /api/discussion/v1/courses/{course_id}/settings.
That endpoint also includes only moderator-specific settings such as course division settings. Thee are technically not settings and not modifyable.
They could also be added to /api/discussion/v1/courses/{course_id}/ which has discussions settings that are visible to users as well.
There was a problem hiding this comment.
@asadazam93 OK. I'm thinking of moving these to the second API since regular users can access these and that might be useful in some situations. I see not reason to restrict the codes to moderators since regular users will be shown these messages in the end.
There was a problem hiding this comment.
@asadazam93 I've merged this with the existing PR. If the new approach is OK. I will update the frontend as well.
There was a problem hiding this comment.
@xitij2000 Sure go ahead. I think in case the api is to be used by regular users as well a second api makes sense.
xitij2000
force-pushed
the
felipetrz-kshitij/BB-5419-add-reason-codes-retrieval-api_
branch
2 times, most recently
from
e45d4cc to
af2753c
Compare
|
@xitij2000 👍 For the new API merge
|
xitij2000
force-pushed
the
felipetrz-kshitij/BB-5419-add-reason-codes-retrieval-api_
branch
from
af2753c to
947f416
Compare
|
@xitij2000 is this ready to be reviewed? |
Yes. |
…es from the LMS settings Reason codes will be used by the frontend to list and validate the reasons for specifying moderation actions. Co-authored-by: Kshitij Sobti <kshitij@opencraft.com>
xitij2000
force-pushed
the
felipetrz-kshitij/BB-5419-add-reason-codes-retrieval-api_
branch
from
947f416 to
1ee15ed
Compare
|
@asadazam93 Updated this PR to remove merge conflicts. |
|
@xitij2000 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
|
EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production. |
|
EdX Release Notice: This PR has been deployed to the production environment. |
xitij2000
deleted the
felipetrz-kshitij/BB-5419-add-reason-codes-retrieval-api_
branch
5 participants
Description
This introduces new settings variables for specifying a list of predefined reasons for editing and closing posts, along with an API endpoint for the discussions micro-frontend to retrieve those reasons. This also changes the behavior of the support implemented in TNL-8842 to validate the submitted reason codes on the server side.
New settings variables:
DISCUSSION_MODERATION_EDIT_REASON_CODESDISCUSSION_MODERATION_CLOSE_REASON_CODESSupporting information
Testing instructions
TODOPlease provide detailed step-by-step instructions for testing this change.Deadline
None
Other information
This change is a follow-up to, and depends on, the changes introduced in this PR.