Fix: Students page now will show all courses enrolled even if it's more than 10.

[mikeyarce]

Fixes #5336

Changes proposed in this Pull Request

Previously, when you loaded the Students page, it would query to get up to 10 courses enrolled by students. It would show 3, then hide up to 7 which would appear when you clicked on a "more" button.

The problem with this is that

• If you had more than 10 courses on a student, it would not show them
• It could be a performance issue if you had a page of 20 students all with 10 courses each, and to show all courses could be worse.

What this PR does is

• Moves the "more" button to be an AJAX call that fetches the rest of the courses for the student you are wanting.
• This way even if a student is enrolled to 100 courses, it happens asynchronously and only one student at a time.

Testing instructions

• Enable this PR
• Make sure you have at least 11 courses published
• Make sure you have a few students, testing with > 2 is good
• Go to the Students page
• Add all the courses to all the students by clicking the ... menu like this:
• Try clicking the "+X More" button
• Make sure all the right courses show up for each student.

Screenshot / Video

[mikeyarce]

For some reason, the PHPUnit test is failing on GitHub even though it passes locally - anyone know why this might be happening? Is this a cache issue from GitHub Actions?

Edit: Fixed it! ✅

[codecov[bot]]

Codecov Report

Merging #5886 (ecefad8) into trunk (fa8bbe2) will increase coverage by 0.82%. The diff coverage is 68.29%.

@@             Coverage Diff              @@
##              trunk    #5886      +/-   ##
============================================
+ Coverage     44.97%   45.80%   +0.82%     
- Complexity     9455     9472      +17     
============================================
  Files           476      476              
  Lines         33483    33543      +60     
  Branches        272      272              
============================================
+ Hits          15059    15363     +304     
+ Misses        18222    17978     -244     
  Partials        202      202              

Impacted Files	Coverage Δ
...ludes/blocks/course-theme/class-template-style.php	50.00% <0.00%> (+50.00%)	:arrow_up:
includes/class-sensei-admin.php	22.13% <0.00%> (-0.05%)	:arrow_down:
includes/class-sensei-posttypes.php	11.13% <ø> (+0.04%)	:arrow_up:
includes/class-sensei-question.php	12.02% <0.00%> (-0.08%)	:arrow_down:
...t-api/class-sensei-rest-api-lessons-controller.php	7.69% <ø> (-2.61%)	:arrow_down:
includes/class-sensei-lesson.php	37.11% <50.00%> (ø)
includes/class-sensei-learner.php	48.12% <61.29%> (+1.73%)	:arrow_up:
.../class-sensei-learners-admin-bulk-actions-view.php	54.28% <83.33%> (-0.26%)	:arrow_down:
includes/class-sensei-modules.php	38.23% <86.20%> (+2.63%)	:arrow_up:
includes/class-sensei-quiz.php	60.45% <100.00%> (+0.07%)	:arrow_up:
... and 31 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ada5ee6...ecefad8. Read the comment docs.

[mikeyarce]

Ready for review if anyone want's to take a look. A few of the failing tests are to do with the testing library from core, so.... not sure what to do about that.

[aaronfc]

Hey @mikeyarce, I was checking the failing tests and found something interesting. I focused on this one:

1) Sensei_Learners_Admin_Bulk_Actions_View_Test::testPrepareItems_WhenCalled_ReturnsStudentsWithLastActivityDate
Division by zero

/home/runner/work/sensei/sensei/includes/admin/class-sensei-learners-admin-bulk-actions-view.php:169
/home/runner/work/sensei/sensei/tests/unit-tests/admin/test-class-sensei-learners-admin-bulk-actions-view.php:71
phpvfscomposer:///home/runner/work/sensei/sensei/vendor/phpunit/phpunit/phpunit:52

After some debugging (and fighting with my local setup) it looks like the new test you added Sensei_Learners_Admin_Bulk_Actions_View_AJAX_Test is affecting the current_screen. When running Sensei_Learners_Admin_Bulk_Actions_View_Test isolated the current_screen is NULL, but when running after Sensei_Learners_Admin_Bulk_Actions_View_AJAX_Test the current_screen is front.

To me this looks like an issue setup/teardown for WP_Ajax_UnitTestCase (which, by the way is not happening inn WP 6.1).

I see a couple of alternatives: 1- Check why having current_screen set to front breaks the test (probably this is our best bet). 2- Try to set the current_screen back to NULL on the tearDown for Sensei_Learners_Admin_Bulk_Actions_View_AJAX_Test. Take into account that we would need to do it after calling the parent::tearDown which is not an ideal approach (we might be breaking things int he future).

Let me know what you think!

[mikeyarce]

Thanks @aaronfc 🎉

Turns out this was a bug with tests before 5.9: https://core.trac.wordpress.org/ticket/53431

What I did was just override the tearDown method for that one Unit test. There's still one test that fails but that's because the parent method doesn't support PHP 8.0 yet.

Let me know what you think!

[mikeyarce]

Thanks @aaronfc

I've fixed the PHP 8.0! Since we no longer support 5.8, I've removed those tests from running so we won't have that issue.