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

Api for removing multiple users from multiple courses #4976

Merged
merged 7 commits into from
Apr 6, 2022

Conversation

Imran92
Copy link
Contributor

@Imran92 Imran92 commented Apr 1, 2022

Fixes part of #4959

Changes proposed in this Pull Request

Add REST API endpoint to add users to courses

Testing instructions

curl -X "DELETE" "http://devwp.local/?rest_route=%2Fsensei-internal%2Fv1%2Fcourse-students%2Fbatch&_wpnonce=WPNONCE" \
     -H 'Cookie: COOKIE-NAME=COOKIE-VALUE' \
     -H 'Content-Type: application/json; charset=utf-8' \
     -d $'{
  "student_ids": USER_ID,
  "course_ids": COURSE_ID
}'

Make sure you get status code 200 and your users are removed from the courses.

If you use vscode's thunder client, you can import the following collection for testing the API. Change the URL and the variables as per your system config
thunder-collection_Sensei.json.zip

@Imran92 Imran92 added this to the 4.4.0 milestone Apr 1, 2022
@Imran92 Imran92 requested a review from a team April 1, 2022 17:54
@Imran92 Imran92 self-assigned this Apr 1, 2022
Base automatically changed from add/rest-api-add-to-course to feature/student-management April 6, 2022 13:08
/**
* Check if the current user can add users to courses.
*
* @param WP_REST_Request $request Request object.
*
* @return bool
* @return bool|WP_Error
*/
public function batch_create_items_permissions_check( WP_REST_Request $request ): bool {
Copy link
Member

Choose a reason for hiding this comment

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

I think we need a better name for this method :)
Maybe batch_permissions_check?

@@ -117,4 +117,45 @@ public function testAddUsersToCourses_RequestGiven_EnrolsUserForCourse() {
$enrolment = Sensei_Course_Enrolment::get_course_instance( $course_id );
$this->assertTrue( $enrolment->is_enrolled( $user_id ) );
}

public function testRemoveUsersFromCoursesApi_AfterApiExecution_StudentsAreActuallyRemoved() {
Copy link
Member

Choose a reason for hiding this comment

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

As Gabriel suggested in my PR, it might be a good idea to add tests for sad paths. WDYT?

@merkushin
Copy link
Member

Hmm, DELETE method doesn't work for me for some reason right now (I get 405 Not Allowed). I suppose that's because of Local configuration. I'll check it one more time a bit later.

@Imran92
Copy link
Contributor Author

Imran92 commented Apr 6, 2022

Hi @merkushin , Thanks for your suggestions. I've updated the PR accordingly, can you kindly take another look? I've also added a zip in the description that you can use with vscode's Thunder Client for testing the API

@merkushin
Copy link
Member

merkushin commented Apr 6, 2022

The problem with executing custom requests (I mean custom methods, like DELETE) was somewhere in the Nginx configuration. I failed finding the exact problem, but solved it by switching from nginx to Apache.

Copy link
Member

@merkushin merkushin left a comment

Choose a reason for hiding this comment

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

Looks good and works well 👍

@Imran92 Imran92 merged commit bc9f8fe into feature/student-management Apr 6, 2022
@Imran92 Imran92 deleted the add/rest-api-remove-from-course branch April 6, 2022 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants