WPML: Shared student progress and quiz submission #7492
Conversation
merkushin
added
Hooks
|
Here's some further testing feedback, some of which may not necessarily be related to this PR. Feel free to open separate issues as needed.
This was my quiz:
I then reset the student's progress, and re-enrolled them to test again. This time, when I took the quiz I didn't see anything at all on the frontend:
Although it's there in the editor:
At this point I decided to stop testing quizzes and focused on course and lesson progress instead. 😅 So I removed all quizzes from the original lessons and translated lessons for a different course. Although everything looked good in the editor, I noticed that questions still retained their reference to the lesson:
I then created a brand new course without quizzes from the very start and didn't encounter any issues with course or lesson progress when taking either the original or the translated course. I think the main concern from my testing is the unpredictable behavior when quizzes are involved. Do any of the issues I documented here seem to be related to the known issues we currently have? |
|
@donnapep Thanks for testing it! I'll check it thoroughly a bit later. But I can say right now that previous experiments and tests might affect the result. The process is brittle 😢 I hope we'll be able to achieve a more stable state having more testing and feedback. |
|
I continued testing (this time using WPML to translate) and have found an issue that has blocked me from further testing - I'm not seeing the Take Quiz button on a translated lesson, only the Complete Lesson button. The lesson has a quiz, and I've translated the strings, but there is no button to take the quiz and no Quiz link in the sidebar. Only the original language shows the button and links: Frontend
Editor
|
|
I followed the instructions documented here and things went much more smoothly. 🎉 There are a couple of small things I noticed that I'm not sure are related to this PR, so I'd like to dig a bit deeper, but they shouldn't be considered blockers for this PR. I will open separate issues if they end up being legitimate bugs. I did also notice what you were saying about incorrect feedback being displayed even though I answered the question correctly. Just checking that we have an open issue for that one? I haven't done a code review, but I think as soon as we have that, we can approve this one and get it merged. |
|
@donnapep Yay! Yes, both issues with grading are logged here: https://github.com/orgs/Automattic/projects/404/views/51 (ToDo column). |
| * @param {WP_Post} $post Post object for course. | ||
| * @return {string|bool} Filtered URL to redirect students to after starting course. Return `false` to prevent redirect. | ||
| */ | ||
| $redirect_url = apply_filters( 'sensei_start_course_redirect_url', get_permalink( $post->ID ), $post ); |
There was a problem hiding this comment.
Just out of curiosity, we are using the filtered $course_id instead of $post->ID directly for all the operations above (I'm referring to the line $course_id = (int) apply_filters( 'sensei_course_start_course_id', $post->ID ?? 0 );), is there a reason are using just $post->ID here? Not an issue, but was just wondering. (Maybe because WPML auto redirects the request?)
There was a problem hiding this comment.
Here we need to have the original post ID to redirect the user to a proper location in the same course.
Filtered course ID might be different (in case of WPML for a translated course it is definitely so).
includes/internal/quiz-submission/grade/repositories/class-comments-based-grade-repository.php
Outdated
Show resolved
Hide resolved
| @@ -47,12 +59,12 @@ public function create( int $course_id, int $user_id ): Course_Progress_Interfac | |||
| throw new \RuntimeException( "Can't create a course progress" ); | |||
| } | |||
|
|
|||
| $progress = $this->get( $course_id, $user_id ); | |||
| if ( ! $progress ) { | |||
| $comment = get_comment( $comment_id ); | |||
There was a problem hiding this comment.
Just curious, any particular reason we removed the usage of the 'get' function and fetched the comment directly here now?
There was a problem hiding this comment.
Initially, it helped me to partially solve an issue with a case when it can't find a comment right after creating it.
Later, I found it didn't solve it entirely and found another WPML-focused solution, but decided to leave this change as it should be a more optimized version of fetching a comment (it uses the primary key here).
| add_filter( 'sensei_utils_user_completed_lesson_lesson_id', array( $this, 'translate_lesson_id' ), 10, 1 ); | ||
| add_filter( 'sensei_lesson_progress_create_lesson_id', array( $this, 'translate_lesson_id' ), 10, 1 ); | ||
| add_filter( 'sensei_lesson_progress_get_lesson_id', array( $this, 'translate_lesson_id' ), 10, 1 ); | ||
| add_filter( 'sensei_lesson_progress_has_lesson_id', array( $this, 'translate_lesson_id' ), 10, 1 ); | ||
| add_filter( 'sensei_lesson_progress_delete_for_lesson_lesson_id', array( $this, 'translate_lesson_id' ), 10, 1 ); | ||
| add_filter( 'sensei_lesson_progress_find_lesson_id', array( $this, 'translate_lesson_id' ), 10, 1 ); |
There was a problem hiding this comment.
Maybe we can remove all the 10, 1 values as they are default?
| * @return int | ||
| */ | ||
| public function translate_course_id( $course_id ): int { | ||
| $course_id = (int) $course_id; |
There was a problem hiding this comment.
We are not doing this explicit casting operation for lessons or quiz progress, is there any reason behind doing it for Course and Quiz?
There was a problem hiding this comment.
Ah, I think I encountered an error with course id at some point. Or, which is more likely Psalm argued about it.
Added explicit casting in all these cases for consistency: b9c61df
However, the reversal approach (not cast at all) is fine here as well.
| public function translate_lesson_id( $lesson_id ) { | ||
| $details = (array) apply_filters( | ||
| // phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFound | ||
| 'wpml_element_language_details', | ||
| null, | ||
| array( | ||
| 'element_id' => $lesson_id, | ||
| 'element_type' => 'lesson', | ||
| ) | ||
| ); | ||
|
|
||
| $original_language_code = $details['source_language_code'] ?? $details['language_code'] ?? null; | ||
|
|
||
| // phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFound | ||
| return (int) apply_filters( 'wpml_object_id', $lesson_id, 'lesson', true, $original_language_code ); | ||
| } |
There was a problem hiding this comment.
Nitpick, but all these translate functions look very similar except for a couple of different values in the apply_filter line, should we move this to a common place instead? Maybe all these entity specific translation classes can have a parent class that contains this function or an util class or something? WDYT?
There was a problem hiding this comment.
There is a separate PR where we extract all WPML filters and actions in a form of methods into a trait: #7566
Without WPML details these methods look much shorter and clearer, I don't think it makes sense to create additional abstraction here:
sensei/includes/wpml/class-lesson-progress.php
Lines 48 to 54 in 1fe078c
Co-authored-by: Imran Hossain <imranh920@gmail.com>
|
Test the previous changes of this PR with WordPress Playground. |
|
Test the previous changes of this PR with WordPress Playground. |
|
Test the previous changes of this PR with WordPress Playground. |
|
I'm trying to test the quiz part using the branch Course and Lessons in backend: Screen.Recording.2024-04-04.at.6.17.55.AM.movQuiz in Frontend: Screen.Recording.2024-04-04.at.6.23.27.AM.mov |
|
Ah okay, I added a Quiz for the second lesson and this time the URL is working. So maybe I made a mistake following the step. But don't know why the answers are appearing empty in the translated lesson - Screen.Recording.2024-04-04.at.7.06.22.AM.movJust wanted to document what I am seeing. I'll keep testing by putting values manually in those empty answer fields and see how it works going forward |
There was a problem hiding this comment.
As the main target was to provide the users with one flow to translate, I think we can merge this PR, I was able to test Course and Lesson translation, and direct translation of Quizzes (without WPML editor) as per the instructions 👍 🚢 
I think we should create an issue for #7492 (comment) in case we don't have it already and if it's reproducible.
There was a problem hiding this comment.
On a separate note, I am a bit curious about the linked issue and this PR. Because in the linked issue, it seems it is talking about the student profile page (Check in Steps to Reproduce and the attached images on the issue) -
Create a translated version of the My Courses page that contains the [sensei_user_courses] shortcode.
As a user, enroll the course in one language and do some progress
On the user profile page check the progress in a secondary language
But in this PR's testing instruction, there isn't any step for checking the profile page. So tried to do it, and it seems like it's still not working.
I'm attaching a video here. Even though I made this progress in Spanish (a secondary language), they don't appear when I select Spanish.
Screen.Recording.2024-04-04.at.9.28.25.AM.mov
Maybe I'm missing something, but in case this issue actually persists, should we remove the link to the issue from this PR and handle it separately as we're actually not fixing the issue here.
|
I was just testing for paid courses, the purchase button disappears when I switch language - Screen.Recording.2024-04-04.at.9.54.46.AM.mov |
There was a problem hiding this comment.
I think I've found a progress related bug here. When we remove student enrolments and/or progress, do we also remove the progress added for translated versions of a lesson quiz?
I saw a behavior, and it made me doubt if we are doing that. For test purpose, I removed the enrollment and progress of a user. Then again, I tried to access the Course's lesson. Problem is, when I switched to Spanish, I saw a message on top saying 1/1 question completed. I think this implies that my progress made under that language didn't get cleared. Can you kindly check if that's the case? If it is, then I think we should clear those as well.
I'm attaching a video-
Screen.Recording.2024-04-04.at.9.58.04.AM.mov
|
Test the previous changes of this PR with WordPress Playground. |
|
Test the previous changes of this PR with WordPress Playground. |
Resolves #2788
Unfortunately, questions are not translated automatically. (You can see these strings in the WPML translation editor, but they don't appear in the translated lesson.)
For now, the user has to translate them manually in lessons: open the translated lesson in the WordPress editor and change texts for questions.
Proposed Changes
Testing Instructions
New/Updated Hooks
sensei_block_take_course_course_id- Filters the course ID for the take course block.sensei_course_is_user_enrolled_course_id- Filters the course ID for theSensei_Course::is_user_enrolledmethod.sensei_course_start_course_id- Filter the course ID for the course being started.sensei_utils_check_for_activity_before_get_comments- This action runs before getting the comments for the given request.sensei_utils_check_for_activity_after_get_comments- This action runs after getting the comments for the given request.sensei_utils_user_completed_lesson_lesson_id- Filter lesson ID forSensei_Utils::user_completed_lessonmethod.sensei_course_manual_enrolment_enroll_learner_course_id- Filter the course ID for manual enrolment when the student is being added to the course.sensei_course_manual_enrolment_withdraw_learner_course_id- Filter the course ID for manual enrolment when the student is being removed from the course.sensei_quiz_answer_create_submission_id- Filters the submission ID when quiz answer is created.sensei_quiz_answer_create_question_id- Filters the question ID when quiz answer is created.sensei_quiz_answer_get_all_submission_id- Filters the submission ID when getting all quiz answers.sensei_quiz_answer_delete_all_submission_id- Filters the submission ID when deleting all quiz answers.sensei_quiz_grade_create_submission_id- Filters the submission ID when quiz grade is created.sensei_quiz_grade_create_question_id- Filters the question ID when quiz grade is created.sensei_quiz_grade_get_all_submission_id- Filter the submission ID when getting all quiz grades.sensei_quiz_grade_save_many_submission_id- Filters the submission ID when saving many quiz grades.sensei_quiz_grade_save_many_question_id- Filters the question ID when saving many quiz grades.sensei_quiz_grade_delete_all_submission_id- Filters the submission ID when deleting all quiz grades.sensei_quiz_submission_create_quiz_id- Filters the quiz ID when quiz submission is created.sensei_quiz_submission_get_or_create_quiz_id- Filters the quiz ID when quiz submission is created.sensei_quiz_submission_get_quiz_id- Filters the quiz ID when quiz submission is retrieved.sensei_quiz_submission_get_question_ids_submission_id- Filters the quiz submission ID when getting the question IDs.sensei_course_progress_create_course_id- Filter the course ID for a created course progress.sensei_course_progress_get_course_id- Filter the course ID for a course progress we want to get.sensei_course_progress_has_course_id- Filter the course ID for a course progress we want to check.sensei_course_progress_delete_for_course_course_id- Filter the course ID for a course progress we want to delete.sensei_course_progress_find_course_id- Filter the course ID for a course progress we want to find.sensei_lesson_progress_create_lesson_id- Filter lesson id for lesson progress creation.sensei_lesson_progress_get_lesson_id- Filter lesson id for lesson progress creation.sensei_lesson_progress_has_lesson_id- Filter lesson id for lesson progress check.sensei_lesson_progress_delete_for_lesson_lesson_id- Filter lesson id for lesson progress deletion.sensei_lesson_progress_count_course_id- Filter course id for lesson progress counting.sensei_lesson_progress_find_lesson_id- Filter lesson id when finding lesson progress.sensei_quiz_progress_create_quiz_id- Filter quiz id for quiz progress creation.sensei_quiz_progress_get_quiz_id- Filter quiz id for quiz progress retrieval.sensei_quiz_progress_has_quiz_id- Filter quiz id for quiz progress existence check.sensei_quiz_progress_delete_for_quiz_quiz_id- Filter quiz id for quiz progress deletion.sensei_quiz_progress_find_quiz_id- Filter quiz id for quiz progress retrieval.Pre-Merge Checklist