Fix lesson order in translated courses and attach WPML duplicates - #8149
Conversation
Mirror the master course's lesson order onto the translated course whenever a lesson translation is created, and attach WPML lesson duplicates to the translated course instead of the master. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
tests/unit-tests/wpml/test-class-lesson-translation.php:494
- The wpml_object_id stub only registers 2 accepted args, but Sensei\WPML\WPML_API::get_object_id() applies the filter with 4 args. Updating this stub to accept 4 args makes the test match WPML’s filter contract and helps catch wrong-arg regressions.
$counterpart_id = $map[ $object_id ] ?? 0;
return $counterpart_id && get_post_type( $counterpart_id ) === $element_type ? $counterpart_id : 0;
},
10,
2
tests/unit-tests/wpml/test-class-course-translation.php:149
- The wpml_object_id stub only registers 2 accepted args, but Sensei\WPML\WPML_API::get_object_id() applies the filter with 4 args. Registering the stub with accepted_args=4 and a matching callback signature makes the test closer to WPML’s real contract and can catch regressions where the wrong language/type args are passed.
$counterpart_id = $id_map[ $object_id ] ?? 0;
return $counterpart_id && get_post_type( $counterpart_id ) === $element_type ? $counterpart_id : 0;
},
10,
2
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@claude please review, don't commit. |
1 similar comment
|
@claude please review, don't commit. |
This comment was marked as outdated.
This comment was marked as outdated.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (3)
tests/unit-tests/wpml/test-class-course-translation.php:151
- This wpml_element_trid stub uses an arrow function with no parameters, but WordPress will pass at least one argument to filter callbacks (and WPML_API::get_element_trid() applies 3). In PHP 8+ this can raise an ArgumentCountError. Make the callback accept at least one argument (ideally all 3).
add_filter( 'wpml_element_trid', fn() => 1 );
tests/unit-tests/wpml/test-class-course-translation.php:149
- The wpml_object_id stub only accepts 2 args and is registered with accepted_args=2, but WPML_API::get_object_id() applies the filter with 4 args. Expanding the stub to 4 args makes the test closer to WPML’s contract and helps catch wrong-argument regressions.
This issue also appears on line 151 of the same file.
'wpml_object_id',
function ( $object_id, $element_type ) use ( $id_map ) {
$counterpart_id = $id_map[ $object_id ] ?? 0;
return $counterpart_id && get_post_type( $counterpart_id ) === $element_type ? $counterpart_id : 0;
},
tests/unit-tests/wpml/test-class-lesson-translation.php:496
- simulate_wpml_language_pair() registers a wpml_object_id stub with accepted_args=2, but WPML_API::get_object_id() applies this filter with 4 args. Registering it with accepted_args=4 and a matching signature makes these tests closer to WPML behavior and protects against wrong-arg regressions.
'wpml_object_id',
function ( $object_id, $element_type ) use ( $map ) {
$counterpart_id = $map[ $object_id ] ?? 0;
return $counterpart_id && get_post_type( $counterpart_id ) === $element_type ? $counterpart_id : 0;
},
|
I followed the testing instructions and just want to comment one thing (which is most probably expected):
In case of duplication, everything works as described. But at first, I created a translation of the lesson — the translated lesson wasn't attached to any course. |
merkushin
left a comment
There was a problem hiding this comment.
Works as described 👍
The changes look good, overall.
There is a remark regarding the hook.
I think it's find to use internal hooks sometimes, but we should be conscious about it.
Options: document those usages; mark/tag them; set reminders to check their availability; etc.
| // Run the deferred question sync when WPML writes the translated lesson content. | ||
| add_action( 'wp_after_insert_post', array( $this, 'update_question_translations_on_lesson_content_written' ), 10, 2 ); | ||
| // Attach lesson duplicates to the translated course. | ||
| add_action( 'icl_make_duplicate', array( $this, 'update_lesson_properties_on_lesson_duplicated' ), 10, 4 ); |
There was a problem hiding this comment.
As far as I know, this is an internal hook, not for public usage.
It's used inside WPML, so perhaps not that much risk that they remove it tomorrow, but its status means they can radically change its "API".
There was a problem hiding this comment.
Good callout. @albarin Perhaps this is something we could consider asking the WPML team about to see if it's safe or if there's an alternative that may be safer.
There was a problem hiding this comment.
Good catch! I've checked and I don't see an alternative hook so far. I will ask WPML directly, let's see what they suggest.
There was a problem hiding this comment.
We checked with the WPML team: icl_make_duplicate is safe to rely on. WPML's own Media module relies on it. The only caveat they mention: it also fires when a duplicate is refreshed, so the handler must be idempotent, which ours already is (it recomputes the same metas from the current state).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Closes SEN-129
Proposed Changes
When a lesson is translated with WPML into a course whose other lessons were translated before Sensei 4.22 (or with the WordPress editor instead of the Translation Editor), those lessons carry no order meta, so the new translation jumps to the first position of the translated course. Separately, WPML lesson duplicates copy custom fields verbatim, so a duplicated lesson stays attached to the master course and never shows up in the translated one.
Instead of copying the order meta only to the lesson being translated, every lesson translation now mirrors the master course's effective lesson order onto the whole translated course. Lessons that only exist in the translated course keep their relative order after the mirrored ones. A new handler on WPML's
icl_make_duplicateattaches duplicated lessons to the translated course, removing the master course order meta they inherit.Testing Instructions
Requires a WPML site (Advanced Translation Editor enabled) with English as default language and Spanish as secondary. While testing the legacy scenario, don't re-save the Spanish lessons or course between steps: any lesson save recomputes the whole course order and would mask the broken state.
New lesson translated into a course with legacy translations (the reported bug)
Lesson 1toLesson 3via the Course Outline block, and publish the course and its lessons.wp post meta delete <ES_lesson_id> _order_<ES_course_id>for each of the three Spanish lessons (both IDs are the Spanish ones).Lesson 4at the end of the English course outline, update the course, and publish the new lesson.Lesson 4to Spanish with the "+" icon.Lesson 4appears first.Duplicated lesson joins the translated course
Lesson 5to the English outline, update the course, and publish the new lesson.Lesson 5and duplicate it to Spanish from WPML's Language panel.Course translation keeps mirroring the master order (regression check)
This flow already worked, but the mechanism underneath changed (the whole order is now mirrored by position instead of copying each lesson's meta), so confirm the visible result is unchanged.
C, A, B), and update.C, A, B), not in creation order.Duplicating into a language without a course translation (guard)
No regressions without WPML