-
-
Notifications
You must be signed in to change notification settings - Fork 85
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
Support polylang language prefixes with clean URLs #176
Conversation
@mattwire Good to see you found a solution! I think that rather than supporting Polylang specifically at this point in the code, it might be a better idea to make the regex filterable or introduce a callback to the If this plugin needs to support Polylang out-of-the-box, I'd prefer a separate file, say (FWIW this applies to the OpenGraph support for Yoast SEO in @kcristiano What do you think? |
@christianwach I do agree that having these workarounds for plugins would be best in an The question is how can we reproduce the error above? Is it as simple as having a WP single site with Polylang or is there more to it? I'd like to test this PR. I would not be opposed to this patch as it would benefit users of PolyLang without requiring them to write a filter to make it work. I could see merging this and then refactoring to the new structure. @mattwire does this depend on civicrm/civicrm-core#16289 ? |
@kcristiano No, they are independent. This one "fixes" clean URLs - the other focuses on trying to get CiviCRM to map it's own language more cleverly. |
@kcristiano Fair point. On that basis, I'm happy with this being merged after testing. |
@mattwire Just to let you know I've got a dev site set up with Polylang (that was pretty confusing for a Polylang newbie!) and will look into the CiviCRM issue next week. |
@mattwire Do you have any hints/tips for setting CiviCRM up to reflect the multi-language setup in WordPress? |
@christianwach Sure, there's quite a few ways! My configuration has polylang setup as: Then create two languages in Wordpress and the same two languages in CiviCRM. For now, in CiviCRM (to test this PR) setting follow CMS language should be enough: Then I think you need to create two base pages in wordpress (one for each language) but you may not - it took me a while to realise you had to flush permalinks before anything worked! Then accessing a contribution page (for example) with the relevant language code in the URL should bring it up in the right language (I guess it should also work with shortcodes). |
@mattwire Thanks for the details of your Polylang config. I'll try the various different options out as well, since they would appear to have a bearing on the rewrite rules that are needed.
I assume I need to check the "Multiple Languages Support" box in CiviCRM to do this? I've never dared venture past the dire warning to "make sure you know what you are doing when enabling this function" :-) |
No you don't. Just make sure you have the actual translation files available should be enough and civi should pick them up (the multi languages support used to be required but now just allows you to translate the custom bits (like event descriptions etc)). |
@mattwire Okay so I've been wrestling with this today and have come to the conclusion that this PR only solves the permalink issue for one particular choice of Polylang options. A general solution will need to look at:
From these settings, it can be determined how Polylang builds the @kcristiano Thoughts? Are we happy with an incomplete solution? I'm still tending towards suggesting the use of the existing |
FWIW, the following snippet seems to work for me (i.e. just handling Polylang regardless of its URL settings): // Support Polylang language prefixes.
if ( function_exists( 'pll_languages_list' ) ) {
$url = get_permalink( $basepage->ID );
$raw_url = PLL()->links_model->remove_language_from_link( $url );
$language_slugs = pll_languages_list();
foreach ( $language_slugs as $slug ) {
$language = PLL()->model->get_language( $slug );
$language_url = PLL()->links_model->add_language_to_link( $raw_url, $language );
$parsed_url = wp_parse_url( $language_url, PHP_URL_PATH );
$post_id = pll_get_post( $basepage->ID, $slug );
add_rewrite_rule(
'^' . substr( $parsed_url, 1 ) . '([^?]*)?',
'index.php?page_id=' . $post_id . '&page=CiviCRM&q=civicrm%2F$matches[1]',
'top'
);
};
} Let me know if it works for you @mattwire |
One more go... I'm not quite sure if my CiviCRM basepage set-up is correctly (I seem to have // Support polylang language prefixes.
if ( function_exists( 'pll_languages_list' ) ) {
$languages = pll_languages_list();
foreach ( $languages as $slug ) {
$post_id = pll_get_post( $basepage->ID, $slug );
$url = get_permalink( $post_id );
$raw_url = PLL()->links_model->remove_language_from_link( $url );
$language = PLL()->model->get_language( $slug );
$language_url = PLL()->links_model->add_language_to_link( $raw_url, $language );
$parsed_url = wp_parse_url( $language_url, PHP_URL_PATH );
add_rewrite_rule(
'^' . substr( $parsed_url, 1 ) . '([^?]*)?',
'index.php?page_id=' . $post_id . '&page=CiviCRM&q=civicrm%2F$matches[1]',
'top'
);
};
} Either seems to work with most Polylang URL settings, so I'm a bit confused! |
@christianwach I am fine with a partial fix. I do wonder how we should document as it would be good to be explicit about what we support. |
@kcristiano One or other (or some combination of both) of my code snippets will support all possible Polylang options because the code is what Polylang itself uses to generate the prefixes. It needs review (hopefully from @mattwire) to figure out which works most reliably. Partly it depends on how the basepage(s) are set up and I'd like to run through that with him. I'd like to hold off merging until we've done that. |
@mattwire I've tried to combine the two snippets so that appropriate rewrite rules are generated regardless of:
Try this - I think it's better than my previous efforts :-) // Support Polylang.
if ( function_exists( 'pll_languages_list' ) ) {
/*
* Collect all rewrite rules into an array.
*
* Because the array of specific Post IDs is added *after* the array of
* paths for the Basepage ID, those specific rewrite rules will "win" over
* the more general Basepage rules.
*/
$collected_rewrites = [];
// Support prefixes for a single Basepage.
$basepage_url = get_permalink( $basepage->ID );
$basepage_raw_url = PLL()->links_model->remove_language_from_link( $basepage_url );
$language_slugs = pll_languages_list();
foreach ($language_slugs as $slug) {
$language = PLL()->model->get_language( $slug );
$language_url = PLL()->links_model->add_language_to_link( $basepage_raw_url, $language );
$parsed_url = wp_parse_url( $language_url, PHP_URL_PATH );
$regex_path = substr( $parsed_url, 1 );
$collected_rewrites[$basepage->ID][] = $regex_path;
$post_id = pll_get_post( $basepage->ID, $slug );
if (!empty($post_id)) {
$collected_rewrites[$post_id][] = $regex_path;
}
};
// Support prefixes for Basepages in multiple languages.
foreach ($language_slugs as $slug) {
$post_id = pll_get_post( $basepage->ID, $slug );
if (empty($post_id)) {
continue;
}
$url = get_permalink( $post_id );
$parsed_url = wp_parse_url( $url, PHP_URL_PATH );
$regex_path = substr( $parsed_url, 1 );
$collected_rewrites[$basepage->ID][] = $regex_path;
$collected_rewrites[$post_id][] = $regex_path;
};
// Make collection unique and add remaining rewrite rules.
$rewrites = array_map('array_unique', $collected_rewrites);
if (!empty($rewrites)) {
foreach ($rewrites as $post_id => $rewrite) {
foreach ($rewrite as $path) {
add_rewrite_rule(
'^' . $path . '([^?]*)?',
'index.php?page_id=' . $post_id . '&page=CiviCRM&q=civicrm%2F$matches[1]',
'top'
);
}
}
}
} |
@christianwach So far so good :-) I'll keep testing for a few days and then I think we are good to merge. |
@mattwire If you could merge mjwconsult#1 into this PR's branch, then the full working code can be merged here. Cheers! |
@christianwach Thanks - now merged into this PR |
@mattwire Thanks! @kcristiano This is ready for testing now, as far as it goes. It doesn't address the deeper Clean URLs issues with Base Pages in multiple languages - the ones that require changes to |
@christianwach have you tested again with this PR? I don't have a setup that can easily test this, but I think you do. |
@kcristiano Yes, I've tested with this. I can review this with you tomorrow. |
@christianwach @mattwire based on the testing done we can merge once the conflict is resolved |
Hmm, checks fail because: "No report files were found. Configuration error?" |
28af4a9
to
eef2749
Compare
@kcristiano @christianwach Sometimes it fails because it won't apply the patch via git. Not sure why but a rebase against master usually resolves it - which I've just done so let's see if tests pass now! |
@christianwach Can you take a look - looks like some commits may have been lost. Attached is the patch file from my testing |
@mattwire Um, I think you may have force-pushed an older version of the branch! I'll revert this for the time-being. |
@mattwire @kcristiano I have the code locally - I'll open a new PR since it appears that this one can't be re-opened. |
Well I messed that one up didn't I :-) Thanks @kcristiano @christianwach |
I get an assist at least :( But it's fixed, merged and I've started to deploy to our clients. So all good |
Overview
When using Wordpress in multilanguage with polylang you will most likely have URLs with a language prefix (eg. https://example.org/en/civicrm/...). Each language has it's own "post ID" matching the language so we have to have multiple civicrm base pages if we want CiviCRM to pick up the language.
Before
Clean URLS don't work in Wordpress with polylang
After
Clean URLs work in Wordpress with polylang. Language code is passed through to CiviCRM.
Technical Details
We create a rewrite rule for every enabled language - this means we can select the correct post ID for each language (which may be the "master" one or a language specific one).
Comments
Polylang allows various combinations of language prefixes, required or optional. @christianwach Could you review - per our discussion.