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: Fix course outline for a course translation #7453

Merged
merged 15 commits into from Feb 22, 2024

Conversation

merkushin
Copy link
Member

@merkushin merkushin commented Jan 26, 2024

Resolves #7392

In this PR we address an issue with a course translation.
The easiest way to observe the issue is to create a translation for a course with lessons and check the Course Outline in the translated course.

To fix it, we need:

  • Update _lesson_course meta field in translated (duplicated) lessons to point the new course.
  • Update 'module' taxonomies for translated lessons.
  • Create lesson translations (duplicates) if they don't exist yet.

Proposed Changes

  • Fix course outline for a course translation.
  • Create a lesson duplicates when a course translation is created.

Testing Instructions

  1. Create a course.
  2. Add a few modules with lessons, a standalone lesson.
  3. Create translations for lessons.
  4. Create a translation for the course.
  5. Make sure the course translation has an outline with translated lessons.
  6. Now create a new course with lessons.
  7. Create a translation for the course.
  8. Go to the translated course and make sure there are lessons in the Course Outline.

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 added the WPML Compatibility issues with WPML label Jan 26, 2024
@merkushin merkushin added this to the 4.20.2 milestone Jan 26, 2024
@merkushin merkushin self-assigned this Jan 26, 2024
Copy link

codecov bot commented Jan 26, 2024

Codecov Report

Attention: 55 lines in your changes are missing coverage. Please review.

Comparison is base (27192b3) 51.26% compared to head (cf82b25) 51.49%.
Report is 2 commits behind head on trunk.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##              trunk    #7453      +/-   ##
============================================
+ Coverage     51.26%   51.49%   +0.22%     
- Complexity    11189    11234      +45     
============================================
  Files           614      622       +8     
  Lines         47259    47504     +245     
  Branches        407      414       +7     
============================================
+ Hits          24229    24460     +231     
- Misses        22703    22711       +8     
- Partials        327      333       +6     
Files Coverage Δ
includes/class-sensei-course.php 38.95% <ø> (ø)
includes/admin/class-sensei-admin-notices.php 50.68% <60.00%> (-0.18%) ⬇️
...cludes/admin/class-sensei-plugins-installation.php 16.17% <25.00%> (ø)
includes/wpml/class-sensei-wpml.php 77.18% <71.15%> (-13.93%) ⬇️

... and 1 file 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 429ad33...cf82b25. Read the comment docs.

@merkushin merkushin force-pushed the fix/wpml-save-dependent-posts branch from e89b5be to 7fcd4f4 Compare January 31, 2024 15:44
@m1r0 m1r0 modified the milestones: 4.20.2, 4.20.3 Feb 7, 2024
@merkushin merkushin requested a review from a team February 16, 2024 19:39
@merkushin merkushin marked this pull request as ready for review February 16, 2024 19:39
@merkushin merkushin merged commit e332a28 into trunk Feb 22, 2024
24 checks passed
@merkushin merkushin deleted the fix/wpml-save-dependent-posts branch February 22, 2024 12:10
];

const ALLOWED_CAP_CHECKS = [
const ALLOWED_HTML = array(
Copy link
Contributor

Choose a reason for hiding this comment

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

I still tend a use the []. I suspect I should start using array(), right?

Also, did you use something to change it automatically? Would be good to create a PR to fix it in the whole project already?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct :)

I ran vendor/bin/phpcbf, it fixed :)

$is_translated = apply_filters( 'wpml_element_has_translations', '', $lesson_id, 'lesson' );
if ( ! $is_translated ) {
// phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFound
do_action( 'wpml_admin_make_post_duplicates', $lesson_id );
Copy link
Contributor

Choose a reason for hiding this comment

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

I imagine it is the action that will duplicate the lessons when translating a course, right? Not sure if I'm doing something wrong, but it didn't create the translated lessons for me when translating a course. I did the following:

  1. Create a course with modules and lessons.
  2. Publish the lessons.
  3. Go to the course listing.
  4. Click on "+" to translate it.
  5. Translate the strings.
  6. Go to the listing and filter the posts by language.
  7. Edit the post in the translated language.

Then I see the following (course without modules and lessons):

Screenshot 2024-02-22 at 10 20 11

And it seems they don't exist at all:

Screenshot 2024-02-22 at 10 25 11

Here is the course I created to translate:

Screenshot 2024-02-22 at 10 26 05

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @merkushin!

As we talked, I was suspecting that maybe I pull the changes after my tests. Then I repeated it, and it duplicated the lessons without module properly.

But for the lessons inside modules, only one was duplicated, but even this one didn't show up on the course outline.

Original course:
Screenshot 2024-02-22 at 10 45 37

Translated course:
Screenshot 2024-02-22 at 10 44 44

The lessons listing after the translation:
Screenshot 2024-02-22 at 10 49 14

@merkushin
Copy link
Member Author

@renatho Thanks for testing! Addressed the issue here: #7496

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WPML Compatibility issues with WPML
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WPML - Create translations for related lessons when create translation for a course
3 participants