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

WPML: Translate quizzes with questions #7480

Merged
merged 32 commits into from Mar 15, 2024
Merged

WPML: Translate quizzes with questions #7480

merged 32 commits into from Mar 15, 2024

Conversation

merkushin
Copy link
Member

@merkushin merkushin commented Feb 9, 2024

Resolves #5841

In this PR we don't address an issue of not working properly question translation: when you translated a question in a lesson, but the question remain containing the original content.

Proposed Changes

  • Create translations (duplicates) for quizzes and questions.
  • Updated properties.
  • Configure blocks.
  • The PR also contains some code style fixes.

Testing Instructions

  • Enable WPML
  • Create a course with lessons, quizzes and questions.
  • Create a translation for the course.
  • Make sure lessons were "translated" and contain quizzes.
  • Make sure you see duplicated questions for the target language (copies of original).
  • Go to the frontend and try take the test.
  • Now go back to WP Admin -> Sensei LMS -> Lessons.
  • Create a lesson without assigning it to a course. Add a quiz with a question(s).
  • Create a translation for this lesson.
  • Make sure you see a translated lesson in the target language, and a duplicate for the question(s).

New/Updated Hooks

  • sensei_rest_api_lesson_quiz_created - Fires after a quiz is created via the REST API.
  • sensei_lesson_quiz_created - Fires after a quiz is created while saving a lesson in a non-block editor.

Deprecated Code

  • Sensei_WPML - replaced with Sensei\WPML\WPML.

Pre-Merge Checklist

  • PR title and description contain sufficient detail and accurately describe the changes
  • Acceptance criteria is met
  • Decisions are publicly documented
  • Adheres to coding standards (PHP, JavaScript, CSS, HTML)
  • All strings are translatable (without concatenation, handles plurals)
  • Follows our naming conventions (P6rkRX-4oA-p2)
  • Hooks (p6rkRX-1uS-p2) and functions are documented
  • New UIs are responsive and use a mobile-first approach
  • New UIs match the designs
  • Different user privileges (admin, teacher, subscriber) are tested as appropriate
  • Legacy courses (course without blocks) are tested
  • Code is tested on the minimum supported PHP and WordPress versions
  • User interface changes have been tested on the latest versions of Chrome, Firefox and Safari
  • "Needs Documentation" label is added if this change requires updates to documentation
  • Known issues are created as new GitHub issues

@merkushin merkushin changed the title WPML: WPML: Translate quizzes with questions Feb 9, 2024
@merkushin merkushin changed the base branch from trunk to fix/wpml-save-dependent-posts February 9, 2024 17:53
Copy link

github-actions bot commented Feb 12, 2024

WordPress Dependencies Report

The github-action-wordpress-dependencies-report action has detected some script changes between the commit eaa9733 and trunk. Please review and confirm the following are correct before merging.

No changes detected in the current commit. But the comment was left so it is possible to check for the edit history.

This comment was automatically generated by the github-action-wordpress-dependencies-report action.

@merkushin merkushin added this to the 4.20.3 milestone Feb 12, 2024
@merkushin merkushin self-assigned this Feb 12, 2024
Copy link

codecov bot commented Feb 12, 2024

Codecov Report

Attention: Patch coverage is 71.07438% with 140 lines in your changes are missing coverage. Please review.

Project coverage is 51.94%. Comparing base (9c9bcae) to head (eaa9733).
Report is 24 commits behind head on trunk.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##              trunk    #7480    +/-   ##
==========================================
  Coverage     51.93%   51.94%            
- Complexity    11267    11310    +43     
==========================================
  Files           630      639     +9     
  Lines         47665    47821   +156     
  Branches        420      421     +1     
==========================================
+ Hits          24756    24840    +84     
- Misses        22572    22644    +72     
  Partials        337      337            
Files Coverage Δ
includes/class-sensei-course-structure.php 91.89% <100.00%> (+0.01%) ⬆️
...i/class-sensei-rest-api-lesson-quiz-controller.php 93.65% <98.13%> (+0.02%) ⬆️
includes/wpml/class-sensei-wpml.php 0.00% <0.00%> (-77.34%) ⬇️
includes/wpml/class-lesson-translation.php 73.91% <73.91%> (ø)
includes/wpml/class-wpml.php 0.00% <0.00%> (ø)
includes/wpml/class-course-translation.php 78.78% <78.78%> (ø)
includes/wpml/class-email.php 0.00% <0.00%> (ø)
includes/wpml/class-language-details.php 87.03% <87.03%> (ø)
includes/class-sensei.php 24.94% <35.71%> (-0.06%) ⬇️
includes/wpml/trait-quiz-translation-helper.php 52.00% <52.00%> (ø)
... and 4 more

... and 2 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

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

@merkushin merkushin added Hooks This change adds or modifies one or more hooks. WPML Compatibility issues with WPML labels Feb 20, 2024
@merkushin merkushin requested a review from a team February 20, 2024 23:23
@merkushin merkushin marked this pull request as ready for review February 20, 2024 23:23
@merkushin merkushin added the Deprecation This change introduces a deprecation. label Feb 20, 2024
Base automatically changed from fix/wpml-save-dependent-posts to trunk February 22, 2024 12:10
Copy link
Contributor

@renatho renatho left a comment

Choose a reason for hiding this comment

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

2 things I realized while testing it:

  1. I noticed it doesn't consider question categories. And I don't know exactly how we should handle it. Maybe when a category is added to a lesson, we should duplicate the category (categories have translation?) and all the questions in that category? Anyway, I think it's something for a separate issue since it could be complicated.
  2. I realized that when duplicating the course, it created the lessons in the reverse order. In my test in the other PR I tried to order the lessons so maybe it only happens when we don't order them?

Original:
Screenshot 2024-02-22 at 15 56 46

Translated:
Screenshot 2024-02-22 at 15 56 53

And I couldn't test it on the frontend because it's not opening the courses in translated languages for me, as I commented in the other PR. 😉

includes/wpml/class-language-details.php Outdated Show resolved Hide resolved
* @param {int} $quiz_id Quiz post ID.
* @param {int} $lesson_id Course post ID.
*/
do_action( 'sensei_lesson_quiz_created', $quiz_id, $post_id );
Copy link
Member Author

Choose a reason for hiding this comment

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

Here is the only meaningful change in the file, the rest is the code style fix.

Here we fire an action on the creation of a new quiz within a non-block editor.

* @param {int} $quiz_id Quiz post ID.
* @param {int} $lesson_id Course post ID.
*/
do_action( 'sensei_rest_api_lesson_quiz_created', (int) $quiz_id, (int) $lesson->ID );
Copy link
Member Author

Choose a reason for hiding this comment

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

Here is the only meaningful change, the rest is the code style fix.

Here we fire an action on the creation of a new quiz within a block editor.

Comment on lines +751 to +753
// Load WPML compatibility class.
$this->sensei_wpml = new WPML();
$this->sensei_wpml->init();
Copy link
Member Author

Choose a reason for hiding this comment

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

Here is the only meaningful change, the rest is the code style fix.

We instantiate and init a new compatibility class (the old one is deprecated).

@merkushin
Copy link
Member Author

merkushin commented Feb 23, 2024

Thank for your review @renatho!
I'll test it in a fresh environment to check the issues with the lesson order and the course opening.
For the question category, I'll create a separate issue, probably.

Update. I created an issue for question categories: #7537

@merkushin merkushin modified the milestones: 4.21.0, 4.21.1 Feb 27, 2024
* @hook sensei_course_structure_quiz_created
*
* @param {int} $quiz_id Quiz post ID.
* @param {int} $lesson_id Course post ID.
*/
do_action( 'sensei_course_structure_quiz_created', $quiz_id, $lesson_id );
do_action_deprecated( 'sensei_course_structure_quiz_created', $quiz_id, $lesson_id, '$$next-version$$', 'sensei_quiz_create' );
Copy link
Member Author

Choose a reason for hiding this comment

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

I had to deprecate it as it was already released.

@merkushin
Copy link
Member Author

@renatho

I realized that when duplicating the course, it created the lessons in the reverse order. In my test in the other PR I tried to order the lessons so maybe it only happens when we don't order them?

Can't reproduce it for "unordered" lessons (when order hasn't been changed).
But when I changed the order of the lessons and then translated the course, order remained unchanged in the translated course.

Looking into it...

Copy link

Test the previous changes of this PR with WordPress Playground.

@Automattic Automattic deleted a comment from github-actions bot Mar 14, 2024
@Automattic Automattic deleted a comment from github-actions bot Mar 14, 2024
@Automattic Automattic deleted a comment from github-actions bot Mar 14, 2024
@merkushin
Copy link
Member Author

merkushin commented Mar 14, 2024

@renatho I've fixed the lesson order for "translated" lessons here: 1446661
I had implemented the order inside modules, and probably decided later it works for other lessons 🙈
Could you try it one more time?
Thanks!

Update Eventually, while testing re-usable progress, I encountered the behaviour you described. I didn't order lesson, but in the translated course they appeared reversed. Not cool at all, looking for a solution...

Copy link
Contributor

@renatho renatho left a comment

Choose a reason for hiding this comment

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

Thank you, @merkushin!

I re-tested the lessons order after the course translation, and it worked properly for me.

Update Eventually, while testing re-usable progress, I encountered the behaviour you described. I didn't order lesson, but in the translated course they appeared reversed. Not cool at all, looking for a solution...

About this, I didn't know how to test it, so I'll wait for your fix to give the approval. 😉

Copy link
Contributor

@renatho renatho left a comment

Choose a reason for hiding this comment

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

@merkushin, I'm approving it since we talked and you couldn't reproduce the other issue anymore.

Hopefully, it was just a mistake while testing it. But if not, we can always iterate in the future and it probably wouldn't be a critical issue.

@merkushin merkushin merged commit 548e120 into trunk Mar 15, 2024
25 checks passed
@merkushin merkushin deleted the fix/wpml-quiz branch March 15, 2024 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deprecation This change introduces a deprecation. Hooks This change adds or modifies one or more hooks. WPML Compatibility issues with WPML
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Quiz not function on translated lessons with WPML
2 participants