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

Bulk Enrollment for Courses & Memberships #562

Closed
actual-saurabh opened this issue May 27, 2018 · 7 comments

Comments

@actual-saurabh
Copy link
Contributor

@actual-saurabh actual-saurabh commented May 27, 2018

@actual-saurabh

This comment has been minimized.

Copy link
Contributor Author

@actual-saurabh actual-saurabh commented Jun 13, 2018

Looks like the last 2 are not possible in the way I assumed they would be. However, I was able to find workarounds and am still able to run the bulk enrollment.

Will be able to push this in a day or so.

@actual-saurabh

This comment has been minimized.

Copy link
Contributor Author

@actual-saurabh actual-saurabh commented Jun 14, 2018

Finished all the code for bulk enrollment here: https://github.com/actual-saurabh/lifterlms/blob/feature/bulk-enroll/includes/admin/class.llms.student.bulk.enroll.php

Added select2 extension here: https://github.com/actual-saurabh/lifterlms/blob/feature/bulk-enroll/assets/js/llms-bulk-enroll.js

Wasn't able to test thoroughly but the js doesn't seem to load at all. Will test and fine tune before sending in a PR

@actual-saurabh

This comment has been minimized.

Copy link
Contributor Author

@actual-saurabh actual-saurabh commented Jun 14, 2018

Tested all scenarios manually by adding the js through console. Works beautifully 😄 :

Select users and memberships:
screen shot 2018-06-14 at 6 31 07 pm

Everybody gets enrolled:
screen shot 2018-06-14 at 6 31 24 pm

Select membership, but not users:
screen shot 2018-06-14 at 6 33 25 pm

Expected error!
screen shot 2018-06-14 at 6 33 39 pm

Select users but no product
screen shot 2018-06-14 at 6 34 11 pm

Expected error!
screen shot 2018-06-14 at 6 34 22 pm

Try enrolling already enrolled students
screen shot 2018-06-14 at 6 37 28 pm

Expected error!
screen shot 2018-06-14 at 6 37 45 pm

@actual-saurabh

This comment has been minimized.

Copy link
Contributor Author

@actual-saurabh actual-saurabh commented Jun 14, 2018

Not sure why JS isn't getting enqueued at all. Will look at it afresh tomorrow.

@actual-saurabh

This comment has been minimized.

Copy link
Contributor Author

@actual-saurabh actual-saurabh commented Jun 14, 2018

A side thought, similar functionality can be added to the Edit User screen to bulk enroll one user into many courses/memberships with a few tweaks.

@thomasplevy

This comment has been minimized.

Copy link
Member

@thomasplevy thomasplevy commented Jun 14, 2018

@actual-saurabh I'm wondering if it would be better to group the notices into a single notice with a <ul> instead of multiple notices. 3 - 6 notices isn't that bad but what if a user enrolls 20? What if they extend the default number of users / screen to 50 or 100 and then bulk enroll all 50 / 100? That's a lot of notices... I guess it'd be a giant list as well but at least it could be dismissed in one click instead of 20 - 100.

Just a thought

@actual-saurabh

This comment has been minimized.

Copy link
Contributor Author

@actual-saurabh actual-saurabh commented Jun 15, 2018

@thomasplevy I did think about this. I considered 3 scenarios:

  1. Frequent bulk enrollment for a small number of users (~10)
  2. Rare bulk enrollment for a large number of students (>10, <20-30 )
  3. Extremely rare bulk enrollment for a humongous number (>50)

Most common use cases, afaik, would be 1 & 2. If someone does perform 3, that'll be a one off, very rare user experience.

I felt that since each enrollment is a separate operation, it makes sense to keep the notice separate to distinguish the state (success, error) in line with WordPress's default functionality. I'm banking on WordPress core taking care of grouping notifications in the near future. See: https://core.trac.wordpress.org/ticket/43484

In any case, simply refreshing the page or navigating away from the screen would clear all the notifications by default.

So, I decided to keep it as it is, even if it is unelegant, for the sake of consistency with WP's admin experience. In that vein, I feel it is more consistent with the idea of being able to clear the success notices and have only the failed notifications on screen, if and when enrollment fails for some users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.