wintercms / wn-translate-plugin

Translate plugin for Winter CMS
MIT License
13 stars 18 forks source link

untranslated menu link #71

Closed steva82 closed 7 months ago

steva82 commented 1 year ago

Hi, I have this situation:

with these settings:

and everything works.

I have update the Translate plugin to 2.1.4 version and when I surf the website in the second language (EN) the menu has untranslated links. I mean, while in IT a link is "www.my-website.com/it/italian-title", in EN I have "www.my-website.com/en/italian-title" instead of "www.my-website.com/en/english-title".

I have decided to come back to 2.1.3 version and the menu works again.

Any ideas? Thanks in advanced.

Ste

mjauvin commented 1 year ago

Can you try this with dev-main ?

ivang76 commented 1 year ago

Hi @mjauvin

The issue persists even with dev-main. It appears to be related to the 'translate' plugin used together with the 'static pages' plugin. Sometimes, when navigating the menu after changing the language, the item.url is not translated; only the language code at the beginning of the route path is correct.

The strange thing is that this happens almost randomly. At some point, by forcefully navigating—jumping around, reloading the page, or clearing the browser cache—the system starts working again. However, the problem returns the next day (could there be some cache in the route translations? I found the Page::clearMenuCache function but it's not yet clear to me how it works).

The menu is built with the 'static page' (contains cms page and static pages also in subgroup) and used in the layout with something like

[staticMenu] 
code = "main_menu"

After that, I simply loop over it to draw a menu with label and link... here it is a section of the function as example:

{% for item in staticMenu.menuItems %}
    {% if item.items %}
      {% for subitem in item.items %}
           <a href="{{ subitem.url }}">{{ subitem.title }}
           ....

Sometimes, the subitem.url is not translated and appears similar to the example posted by steva82.

I can't manually force the URL based on the active locale because there are no other locales available in the object. Additionally, the viewbag (which is usually very useful for many workarounds) doesn't contain the other _end, _de, fr... possibilities to choose from.

Do you have any ideas for a workaround until someone from Winter can find and fix this problem? Is it maybe related to the TranslatablePageUrl behavior?

ivang76 commented 1 year ago

After extensive debugging and numerous attempts, I managed to find a workaround that appears to resolve this peculiar issue. However, I had to make modifications directly to the StaticMenu component of Pages. It seems that the routes from the Translate plugin, via the generateReferences function, occasionally arrive untranslated at this component.

To address this, I extended the BaseStaticMenu class and overridden the menuItems function to create a customized component. This approach allows me to continue updating vendor plugins.

Here is the new function, which includes a loop to check for any issues. If an issue is detected, I enforce the correct translation. I'm sharing this as an example in case others encounter the same error but mostly because maybe Mjauvin might identify something useful to pinpoint the actual bug and resolve it at its source (on Pages or on Translate or on Menu component...). Thanks!


public function menuItems() {
        if (!strlen($this->property('code'))) {
            return;
        }

        $theme = Theme::getActiveTheme();
        $menu = PagesMenu::loadCached($theme, $this->property('code'));

        if ($menu) {
            $this->menuItems = $menu->generateReferences($this->page);
            $this->menuName = $menu->name;
        }

        $languages = LocaleModel::listEnabled();;
        $numLanguages = count($languages);

        if (\App::getLocale() == "en" && $numLanguages>1) {  
            $static_pages = \Winter\Pages\Classes\Page::get();

            $iterator = function ($menuItems) use (&$iterator, $static_pages) { 
                $result = [];
                foreach ($menuItems as $item) {
                    $modifiedItem = $item;

                    $parts = explode("en/", $modifiedItem->url);

                    $substringAfterEn = "";
                    if  (count($parts) > 1) {
                        $substringAfterEn = $parts[1];
                    }

                    if ($substringAfterEn) {
                        foreach($static_pages as $page) {  
                            if (strpos($page->content, "url = \"/".$substringAfterEn."\"")!== false) {
                                if ($page->viewBag && isset($page->viewBag["localeUrl"]) && isset($page->viewBag["localeUrl"]["en"])) {
                                    $correct_path = \URL::to('/')."en".$page->viewBag["localeUrl"]["en"];
                                    if ($correct_path != $modifiedItem->url) {
                                        // Log::info( "Wrong url translation " . $modifiedItem->url);
                                        $modifiedItem->url = $correct_path;
                                        // Log::info( "Fixed wrong url translation " . $correct_path);
                                    } else {
                                        // Log::info("Correct url translation do nothing");
                                    }
                                    break;
                                }
                            }
                        }
                    }

                    if ($item->items) {
                        $modifiedItem->items = $iterator($item->items,$static_pages);
                    }

                    $result[] = $modifiedItem;
                }
                return $result;
            };
            $this->menuItems = $iterator($this->menuItems);

        }

        return $this->menuItems;
    }
mjauvin commented 1 year ago

@ivang76 I've seen similar behavior at some point during latest development, but all issues were resolved for me with latest dev-main (2.1.4 had issues).

Can you make sure this particular commit is present in your local install ?

commit 600c9efb42067613d4470be8dcb145e21aba0d9d
Author: Marc Jauvin <marc.jauvin@gmail.com>
Date:   Wed May 31 16:59:58 2023 -0400

    Fix url when using prefixDefaultLocale=true (#67)

diff --git a/Plugin.php b/Plugin.php
index a692f50..c117b1b 100644
--- a/Plugin.php
+++ b/Plugin.php
@@ -380,6 +380,8 @@ class Plugin extends PluginBase
                     }
                     foreach ($itemInfo['alternateLinks'] as $locale => $altUrl) {
                         if ($locale === $defaultLocale->code) {
+                            $loc = $urlElement->getElementsByTagName('loc')->item(0);
+                            $loc->nodeValue = $altUrl;
                             continue;
                         }
                         $newElement = $urlElement->cloneNode(true);
diff --git a/classes/MLPage.php b/classes/MLPage.php
index dd2a417..53fb862 100644
--- a/classes/MLPage.php 
+++ b/classes/MLPage.php
@@ -27,14 +27,8 @@ class MLPage

             $alternateLinks = [];
             foreach ($locales as $locale => $name) {
-                if ($locale === $defaultLocale->code) {
-                    $pageUrl = $result['url'];
-                } else {
-                    $pageUrl = static::getLocalizedPageUrl($page, $locale);
-                }
-                if ($pageUrl) {
-                    $alternateLinks[$locale] = Url::to($pageUrl);
-                }
+                $pageUrl = static::getLocalizedPageUrl($page, $locale) ?: $result['url'];
+                $alternateLinks[$locale] = Url::to($pageUrl);
             }

             if ($alternateLinks) {

Also, can you confirm if you use prefixDefaultLocale=true or not when having the issue ?

mjauvin commented 1 year ago

Also, this commit helped resolve other issues, make sure you have that as well:

commit e870a5266a5f234b2fc4a8ea38a6ef5fb56c7f16
Author: Marc Jauvin <marc.jauvin@gmail.com>
Date:   Fri Jun 23 13:59:44 2023 -0400

    implementing all translatable behaviors causes issues

diff --git a/Plugin.php b/Plugin.php
index 7f6df1d..0c882eb 100644
--- a/Plugin.php
+++ b/Plugin.php
@@ -205,14 +205,14 @@ class Plugin extends PluginBase
          * Handle translated page URLs
          */
         Page::extend(function($model) {
-            $this->extendModel($model, ['title', 'description', 'meta_title', 'meta_description']);
+            $this->extendModel($model, 'page', ['title', 'description', 'meta_title', 'meta_description']);
         });

         /*
          * Add translation support to theme settings
          */
         ThemeData::extend(function ($model) {
-            $this->extendModel($model);
+            $this->extendModel($model, 'model');

             $model->bindEvent('model.afterFetch', function() use ($model) {
                 foreach ($model->getFormFields() as $id => $field) {
@@ -253,11 +253,11 @@ class Plugin extends PluginBase
     {    
         // Add translation support to file models
         File::extend(function ($model) {
-            $this->extendModel($model, ['title', 'description']);
+            $this->extendModel($model, 'model', ['title', 'description']);
         });

         MailTemplate::extend(function ($model) {
-            $this->extendModel($model, ['subject', 'description', 'content_html', 'content_text']);
+            $this->extendModel($model, 'model', ['subject', 'description', 'content_html', 'content_text']);
         });

         // Load localized version of mail templates (akin to localized CMS content files)
@@ -398,19 +398,21 @@ class Plugin extends PluginBase
     /** 
      * Helper method to extend the provided model with translation support
      */ 
-    public function extendModel($model, array $translatableAttributes = [])
+    public function extendModel($model, string $type, array $translatableAttributes = [])
     {   
         if (!$model->propertyExists('translatable')) {
             $model->addDynamicProperty('translatable', []);
         }
         $model->translatable = array_merge($model->translatable, $translatableAttributes);
-        if (!$model->isClassExtendedWith('Winter\Translate\Behaviors\TranslatablePageUrl')) {
-            $model->extendClassWith('Winter\Translate\Behaviors\TranslatablePageUrl');
-        }
-        if (!$model->isClassExtendedWith('Winter\Translate\Behaviors\TranslatablePage')) {
-            $model->extendClassWith('Winter\Translate\Behaviors\TranslatablePage');
-        }
-        if ($model instanceof Model) {
+        
+        if ($type === 'page') {
+            if (!$model->isClassExtendedWith('Winter\Translate\Behaviors\TranslatablePageUrl')) {
+                $model->extendClassWith('Winter\Translate\Behaviors\TranslatablePageUrl');
+            }
+            if (!$model->isClassExtendedWith('Winter\Translate\Behaviors\TranslatablePage')) {
+                $model->extendClassWith('Winter\Translate\Behaviors\TranslatablePage');
+            }
+        } elseif ($type === 'model') {
             if (!$model->isClassExtendedWith('Winter\Translate\Behaviors\TranslatableModel')) {
                 $model->extendClassWith('Winter\Translate\Behaviors\TranslatableModel');
             }
ivang76 commented 1 year ago

Hi @mjauvin I confirm that we tested also with the latest dev-main which includes that commits, but the strange bug still occours.

About the prefixDefaultLocale I don't know which is the default value, anyway I'm not setting and use it on our side. I only set to true the "Force URL schema" in the localePicker actually:

[localePicker]
forceUrl = 1
mjauvin commented 1 year ago

Really strange. Could you try to install the winter-sitemap plugin, add your pages to it and see if you experience the same issue when opening the sitemap.xml ?

mjauvin commented 1 year ago

Also, make sure you don't have RainLab/Translate version installed as well, that might interfere.

ivang76 commented 1 year ago

Yes I'm sure I haven't around old RainLab versions anymore, even if the whole project started under the previous OctoberCMS system (but we got the same issue also with a new fresh project started directly from scratch with WinterCMS). I'm gonna try the winter-sitemap but I have to disable a my custom sitemap generator before, or at least change the route... I'll update you about this new test.

ivang76 commented 1 year ago

Okay, I tried the sitemap plugin and checked it several times by reloading the page. Most of the time, it works correctly, but sometimes the issue reoccurs. What's even more strange is that in this case, the behavior is a bit different because for every locale, the URLs are always in the English version...

mjauvin commented 1 year ago

@damsfx @AIC-BV Can any of you reproduce this?

AIC-BV commented 1 year ago

As far as I know I'm not having any issues with sitemap/url translations What is your composer.json @ivang76?

Perhaps something wrong with your languages setup? I remember someone using (for example) FR as his main language but in settings EN was his main language which made his site not work as expected

ivang76 commented 1 year ago

The main language set in the backend is "it" but that one set in the config (app.php) is still the default "en" actually... could this be a problem?

and here it is the composer.json

    "name": "wintercms/winter",
    "description": "Free, open-source, self-hosted CMS platform based on the Laravel PHP Framework. Originally known as October CMS.",
    "homepage": "https://wintercms.com",
    "type": "project",
    "keywords": ["winter", "cms", "wintercms", "laravel", "cmf"],
    "license": "MIT",
    "authors": [
        {
            "name": "Alexey Bobkov",
            "email": "aleksey.bobkov@gmail.com",
            "role": "Original Author"
        },
        {
            "name": "Samuel Georges",
            "email": "daftspunky@gmail.com",
            "role": "Original Author"
        },
        {
            "name": "Luke Towers",
            "email": "wintercms@luketowers.ca",
            "role": "Lead Maintainer"
        }
    ],
    "support": {
        "issues": "https://github.com/wintercms/winter/issues",
        "docs": "https://wintercms.github.io/docs/",
        "discord": "https://discord.gg/D5MFSPH6Ux",
        "source": "https://github.com/wintercms/winter"
    },
    "require": {
        "php": "^8.0.2",
        "winter/storm": "~1.2.0",
        "winter/wn-system-module": "~1.2.0",
        "winter/wn-backend-module": "~1.2.0",
        "winter/wn-cms-module": "~1.2.0",
        "laravel/framework": "^9.1",
        "wikimedia/composer-merge-plugin": "~2.0.1",
        "league/flysystem": "^3.0",
        "league/flysystem-aws-s3-v3": "^3.0",
        "league/flysystem-ftp": "^3.0",
        "league/flysystem-sftp-v3": "^3.0",
        "league/flysystem-webdav": "^3.0",
        "twig/twig": "^3.0",
        "winter/wn-pages-plugin": "^2.1",       
        "winter/wn-translate-plugin": "^2.1",
        "meb/wn-backup-plugin": "dev-master",
        "winter/wn-sentry-plugin": "^2.2",
        "league/oauth2-client": "^2.7",
        "microsoft/microsoft-graph": "^1.98"
    },
    "require-dev": {
        "phpunit/phpunit": "^9.5.8",
        "mockery/mockery": "^1.4.4",
        "fakerphp/faker": "^1.9.2",
        "squizlabs/php_codesniffer": "^3.2",
        "php-parallel-lint/php-parallel-lint": "^1.0",
        "dms/phpunit-arraysubset-asserts": "^0.1.0|^0.2.1",
        "winter/wn-builder-plugin": "^2.0",
        "laravel/envoy": "^2.8"
    },
    "scripts": {
        "post-create-project-cmd": [
            "@php artisan key:generate"
        ],
        "post-update-cmd": [
            "@php artisan winter:version",
            "@php artisan package:discover"
        ],
        "test": [
            "phpunit --stop-on-failure"
        ],
        "lint": [
            "parallel-lint --exclude vendor --exclude storage --exclude modules/system/tests/fixtures/plugins/testvendor/goto/Plugin.php ."
        ],
        "sniff": [
            "phpcs --colors -nq --report=\"full\" --extensions=\"php\""
        ]
    },
    "minimum-stability": "dev",
    "prefer-stable": true,
    "extra": {
        "merge-plugin": {
            "include": [
                "plugins/myauthor/*/composer.json"
            ],
            "recurse": true,
            "replace": false,
            "merge-dev": false
        }
    },
    "config": {
        "allow-plugins": {
            "composer/installers": true,
            "wikimedia/composer-merge-plugin": true,
            "php-http/discovery": false
        }
    }
}
AIC-BV commented 1 year ago

I am on "winter/wn-translate-plugin": "dev-main", "winter/wn-pages-plugin": "~2.1.2", Perhaps you need to main translate branch? Don't know if I still need it, I used to need it

mjauvin commented 1 year ago

Yes, dev-main is needed for translate plugin

ivang76 commented 1 year ago

I rolled back to the 'normal' release (the other translation features appear to be working correctly, anyway), but when I tested and reported the problem, I was on the 'dev-main' branch.

Alright, I'll revert to 'dev-main' if it's necessary (will it be moved to the 'normal' branch soon?), but that's not the reason for the strange bug.

mjauvin commented 1 year ago

@ivang76 It's odd that no one else can reproduce the problem, there must be something different in your setup.

mjauvin commented 1 year ago

@steva82 do you still have the problem with latest dev-main branch of the translate plugin?

steva82 commented 1 year ago

Hi @mjauvin , yes, there is the same problem with the dev-main branch. I am starting to implement the code of @ivang76 and it seems to work.

mjauvin commented 1 year ago

@ivang76 @steva82 do you get the problem with both CMS pages and Static pages or only with Static pages URLs ?

Also, did you happen to change the default Translate plugin locale after initially setting up the languages/translations ?

ivang76 commented 1 year ago

I have this problem only with the Static Pages URLs.

Yes, on some cases I needed to change the default locale but not on all the sites having this problem, so I don't think should be related to the issue.

steva82 commented 1 year ago

Hi,

mjauvin commented 1 year ago

Guys, I really cannot reproduce this.

Can someone try a FRESH Winter release (dev-develop) with both Pages & Translate plugin using dev-main and see if this is happening as well ?

AIC-BV commented 1 year ago

Is this still an issue? Might try it later this week

steva82 commented 1 year ago

Is this still an issue? Might try it later this week

Hi @AIC-BV , yes, it's still an issue imho. I'll try a fresh installation when I have more time...

mjauvin commented 7 months ago

Closing this as I cannot reproduce.

Please, feel free to re-open if you can provide a way to reproduce this.