Add student progress tables based repository

[merkushin]

Fixes #5406

Changes proposed in this Pull Request

• Add custom tables based repositories for course progress, lesson progress and quiz progress.
• Add aggregate repositories that use both types of repositories (comments based and tables based).

Testing instructions

• Try to take a course with lessons and quizzes.
• Check the {wp_prefix}sensei_lms_progress table if it has corresponding entries.

[codecov[bot]]

Codecov Report

Merging #5894 (5078541) into feature/progress-data-layer-refactoring-stage-2 (047f8e9) will increase coverage by 0.59%. The diff coverage is 99.72%.

@@                                  Coverage Diff                                  @@
##             feature/progress-data-layer-refactoring-stage-2    #5894      +/-   ##
=====================================================================================
+ Coverage                                              45.34%   45.93%   +0.59%     
- Complexity                                              9288     9365      +77     
=====================================================================================
  Files                                                    468      474       +6     
  Lines                                                  33034    33381     +347     
  Branches                                                 274      274              
=====================================================================================
+ Hits                                                   14978    15334     +356     
+ Misses                                                 17851    17842       -9     
  Partials                                                 205      205              

Impacted Files	Coverage Δ
includes/class-sensei-autoloader.php	17.50% <ø> (ø)
...ies/class-comments-based-submission-repository.php	100.00% <ø> (ø)
.../class-tables-based-lesson-progress-repository.php	98.78% <98.78%> (ø)
...ies/class-aggregate-course-progress-repository.php	100.00% <100.00%> (ø)
...ories/class-course-progress-repository-factory.php	100.00% <100.00%> (ø)
.../class-tables-based-course-progress-repository.php	100.00% <100.00%> (ø)
...ies/class-aggregate-lesson-progress-repository.php	100.00% <100.00%> (ø)
...ories/class-lesson-progress-repository-factory.php	100.00% <100.00%> (ø)
...ories/class-aggregate-quiz-progress-repository.php	100.00% <100.00%> (ø)
...itories/class-quiz-progress-repository-factory.php	100.00% <100.00%> (ø)
... and 4 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 047f8e9...5078541. Read the comment docs.

[m1r0]

Hey Dmitry, here's some feedback on what I've observed so far. Some of it might be out of scope, so let me know and I will make new cards.

• I've noticed that we are not using UTC when storing the datetimes in the progress table. I was left with the impression that we are going to store the dates as UTC and convert them to the local timezone.
• I've got an error when trying to reset the progress from Students -> Reset and remove progress:

[19-Oct-2022 01:28:17 UTC] WordPress database error Duplicate entry '2026-1-course' for key 'wp_sensei_lms_progress.user_progress' for query INSERT INTO `wp_sensei_lms_progress` (`post_id`, `user_id`, `parent_post_id`, `type`, `status`, `started_at`, `completed_at`, `created_at`, `updated_at`) VALUES (2026, 1, NULL, 'course', 'in-progress', '2022-10-19 04:28:17', NULL, '2022-10-19 04:28:17', '2022-10-19 04:28:17') made by require('index.php'), require('wp-blog-header.php'), wp, WP->main, WP->parse_request, do_action_ref_array('parse_request'), WP_Hook->do_action, WP_Hook->apply_filters, rest_api_loaded, WP_REST_Server->serve_request, WP_REST_Server->dispatch, WP_REST_Server->respond_to_request, Sensei_REST_API_Course_Progress_Controller->batch_delete_items, Sensei_Utils::reset_course_for_user, Sensei_Utils::user_start_course, Sensei_Utils::start_user_on_course, Sensei\Student_Progress\Course_Progress\Repositories\Aggregate_Course_Progress_Repository->create, Sensei\Student_Progress\Course_Progress\Repositories\Tables_Based_Course_Progress_Repository->create

• Related to the previous one. When resetting the progress the lesson progress changes from completed to in-progress (as it should?) but the completed_at date is not cleared.
• Maybe related to the previous one. When clicking the "Reset lesson" button, the completed_at date is not cleared.
• When removing a student from a course, the progress is not removed from the progress table. Then when I re-take it, I get an error:

[19-Oct-2022 01:36:57 UTC] WordPress database error Duplicate entry '2030-1-lesson' for key 'wp_sensei_lms_progress.user_progress' for query INSERT INTO `wp_sensei_lms_progress` (`post_id`, `user_id`, `parent_post_id`, `type`, `status`, `started_at`, `completed_at`, `created_at`, `updated_at`) VALUES (2030, 1, NULL, 'lesson', 'in-progress', '2022-10-19 04:36:57', NULL, '2022-10-19 04:36:57', '2022-10-19 04:36:57') made by require('index.php'), require('wp-blog-header.php'), wp, WP->main, do_action_ref_array('wp'), WP_Hook->do_action, WP_Hook->apply_filters, Sensei_Lesson::maybe_start_lesson, Sensei_Utils::sensei_start_lesson, Sensei\Student_Progress\Lesson_Progress\Repositories\Aggregate_Lesson_Progress_Repository->create, Sensei\Student_Progress\Lesson_Progress\Repositories\Tables_Based_Lesson_Progress_Repository->create

• When deleting the course/lesson/user, the progress is not removed from the progress table.
• The course started_at date column is updated when a quiz is submitted/reset. I was a bit confused about that because the course is still in-progress.
• When a quiz is reset, the lesson's completed_at date column is filled. It was empty before that and the lesson's status has not changed from in-progress. Is this expected?
• I'm not able to complete a lesson with a quiz. The status in the progress table is always in-progress after submitting the quiz.
• When I submit a quiz, I don't see the quiz progress in the progress table. Is this a bug or it should work like this?

[merkushin]

@m1r0 What do you think about storing timestamps (started_at, completed_at, created_at, updated_at) as unix timestamp in database (int as for internal type)? And that means it will be in UTC. I was just thinking about the problem that we store date and time in one timezone, and the timezone in db might be different. At some point, it might lead to very unexpected results :)

[m1r0]

What do you think about storing timestamps as unix timestamp.

I would prefer to use datetime to be honest because it's more readable and consistent with the WordPress ecosystem. If we set a rule that the datetime is always UTC, I think we should be ok. I've noticed that woo moved to only storing UTC dates but they're adding the _gmt suffix. Would that make it less confusing?

I was just thinking about the problem that we store date and time in one timezone, and the timezone in db might be different.

Do you mean that we store the comments datetimes in the local timezone but we plan to store the new tables datetimes in UTC and that would lead to some issues? Do you mean that it would be confusing or we might have bugs?

[merkushin]

@m1r0 I added delete methods and changed the timezone to UTC.

[m1r0]

I added delete methods

Nice, that works! The only thing that I see as a potential issue is that the course progress started_at date gets updated if I reset any lesson. Is that expected?

changed the timezone to UTC

I've noticed a few issues:

• The completed_at date is still using the local timezone.
• When I start a lesson, the started_at date is in UTC, but after I complete it, it converts to the local timezone.

[m1r0]

I've noticed that when you remove a student from a course, the course progress is still there. Is this expected?

[merkushin]

I've noticed that when you remove a student from a course, the course progress is still there. Is this expected?

I believe it's out of scope for this PR: I think it works the same way for comments-based approach. I think you've already checked it, but I'll check too, to be sure.

[merkushin]

I've noticed a few issues:

• The completed_at date is still using the local timezone.
• When I start a lesson, the started_at date is in UTC, but after I complete it, it converts to the local timezone.

I think I got it :) Thank you!

[merkushin]

@m1r0

I've noticed that when you remove a student from a course, the course progress is still there. Is this expected?

I believe it's out of scope for this PR: I think it works the same way for comments-based approach. I think you've already checked it, but I'll check too, to be sure.

In trunk, if I remove a user from a course, the progress remains preserved. And you have an option to delete the progress.

[merkushin]

I've noticed a few issues:

• The completed_at date is still using the local timezone.
• When I start a lesson, the started_at date is in UTC, but after I complete it, it converts to the local timezone.

Fixed here: https://github.com/Automattic/sensei/pull/5894/commits/37308da8a7cffa315ff53696d8c73acdd5e8d043