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

Don't translate Sensei custom post types #6986

Closed
wants to merge 7 commits into from

Conversation

merkushin
Copy link
Member

@merkushin merkushin commented Jun 21, 2023

Resolves #6978

Our post types like Course and Lesson store additional information about related entities.
We can't allow WPML try to translate them automatically.
Here I disable translation to fix the bug with saving courses and lessons.
Later, we need to address the problem in our ML project.

I updated the list of custom post types, taxonomies and custom fields. I hope it might help us in the future.
As for fields, the list is not complete as we have auto-generated fields like _order_module_17.

Proposed Changes

  • Disable automatic translation of Sensei custom post types in WPML.

Testing Instructions

  1. Install and activate the WPML Multilingual LMS plugin (check Secret Store for creds).
  2. Create a new course using the default layout.
  3. No infinite loops!
  4. Look at an existing lesson. It should be in the Draft status.

New/Updated Hooks

Deprecated Code

Pre-Merge Checklist

  • PR title and description contain sufficient detail and accurately describe the changes
  • Acceptance criteria is met
  • Decisions are publicly documented (PR description)
  • 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
  • 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 this to the 4.15.1 milestone Jun 21, 2023
@merkushin merkushin requested a review from a team June 21, 2023 22:51
@merkushin merkushin self-assigned this Jun 21, 2023
@codecov
Copy link

codecov bot commented Jun 21, 2023

Codecov Report

Merging #6986 (187e571) into trunk (b956039) will decrease coverage by 0.01%.
The diff coverage is n/a.

❗ Current head 187e571 differs from pull request most recent head a98846f. Consider uploading reports for the commit a98846f to get more accurate results

Impacted file tree graph

@@             Coverage Diff              @@
##              trunk    #6986      +/-   ##
============================================
- Coverage     46.73%   46.72%   -0.01%     
  Complexity    10226    10226              
============================================
  Files           570      568       -2     
  Lines         36881    36865      -16     
  Branches        402      401       -1     
============================================
- Hits          17236    17225      -11     
+ Misses        19318    19314       -4     
+ Partials        327      326       -1     

see 4 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 eed9d92...a98846f. Read the comment docs.

Significance: patch
Type: fixed

Disable translation of Sensei post types, taxonomies and fields to temporary fix compatibilty issue.
Copy link
Collaborator

@donnapep donnapep Jun 22, 2023

Choose a reason for hiding this comment

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

I think we should consider a different change log entry here, as this is likely to scare everyone with translated sites. It should mention that it's a compatibility fix for WPML.

@merkushin merkushin modified the milestones: 4.15.1, 4.16.0 Jun 22, 2023
@donnapep donnapep removed this from the 4.16.0 milestone Jul 7, 2023
@donnapep
Copy link
Collaborator

donnapep commented Jul 7, 2023

@merkushin I'm a bit uncertain of the impact of this change. I think it means that users will not be able to create translations for courses and lessons at all? There was a time when Sensei content was able to be translated with WPML, so I'm a bit worried that we've just broken it somewhere along the way, and disabling it altogether may not be the best option.

This would also disable it for those using the Classic Editor plugin, where I don't think translations are broken. So we'd be taking a possible workaround away by making this change. Maybe we should go ahead and try to solve as part of #2788?

@merkushin
Copy link
Member Author

merkushin commented Jul 8, 2023

@donnapep Makes sense. In general, I am for solving the issue.

This PR tried to solve the problem for new users who get upset because of unexpected issues. Personally for me, it is better to see a clear message that this feature doesn't work yet, then a message that it might work or not.

And this PR appeared 'cause we didn't have a clear plan regarding ML, I mean we didn't know what exactly we want to do and when.

Anyway, I'm glad to hear that now we have more dedication to solve the issue permanently.

A small update:

I think it means that users will not be able to create translations for courses and lessons at all? There was a time when Sensei content was able to be translated with WPML, so I'm a bit worried that we've just broken it somewhere along the way, and disabling it altogether may not be the best option.

I forgot to address this part. It is still possible to change WPML settings in WP Admin, so it just could be the default behavior.

@merkushin merkushin closed this Jul 10, 2023
@donnapep donnapep deleted the fix/wpml-save-course-lesson-bug branch July 10, 2023 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to save lessons when WPML is activated
2 participants