Initialize route and raw_route, fix empty route as values

[pierrejoye]

translatedPages always returns empty values.

This fix initializes route and raw_route and fix the empty values issues.

It may be desired to return the full route including the language, I am not sure, the code does not seem to be initially designed to do so.

[pierrejoye]

CI failing, it seems 2 is about auth with composer and one about slack.

[rhukster]

We are aware of this and it's probably related to the private github token we are using. It lets us who have access to it pass the checks, but it seems when you guys submit PRs it's not able to use this token. However, if we use the recommended ${{ secrets.GITHUB_TOKEN }}, it uses the user token which will work for you, but we ran out of API calls when trying to release last time. We need to find a solution that uses one token for us, and one for you guys.

[rhukster]

I've switched "tests" action back to GITHUB_TOKEN, and kept "release" action using our private token. I think this will work again for you guys now.

[pierrejoye]

@rhukster thanks

I am adding test for translatedLanguages, what was the initial design for the values?

Full route with languages (ie. /fr/mypage or key/pair as fr => /mypage, the latter makes little sense tho' as the route will likely be always the same, the slug not.

Should we use slug if present? Or apply other rules (from the routes docs)?

[pierrejoye]

I added a test, but I fail to see where to initialize correctly the config for languages.

I added:

$grav['config']->set('system.languages.supported', ['en', 'fr', 'vi']); $grav['config']->set('system.languages.default_lang', 'en');

to either bootstrap or in PagesTest::before, the config is set correctly but the language is not initialized. If you have any tip how to enable it, I will give it another try and add the test to this PR.

[pierrejoye]

Found, it has to be done very early in bootstrap. Tests pass locally. Let see the CI results.

[pierrejoye]

The util fails because of the case was never tested, when a language is actually found in the path.:

`
if (count($languagesEnabled)) {

        $this->assertTrue(Utils::pathPrefixedByLangCode('/' . $languagesEnabled[0] . '/test'));

    }

`

If languages are enabled and one is found, it will return a string.

[pierrejoye]

Fixed the Util test, pls let me know if you need further tests. I feel like the languages part need a few more but that may out of the scope of this PR. Especially for slug, default fallback etc.

[pierrejoye]

thanks! tests pass :)

…

On Sat, Jan 30, 2021, 8:00 AM Andy Miller ***@***.***> wrote: I've switched "tests" action back to GITHUB_TOKEN, and kept "release" action using our private token. I think this will work again for you guys now. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3184 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACE6KAOAR2YZDAE5HSJEKLS4NK4PANCNFSM4WYNSDPA> .

[mahagr]

What about routes which have longer path, such as /en/my/cool/route -> fi//minun/hieno/polkuni? I don't think they work without doing the same for the parent recursively.

[pierrejoye]

@mahagr let ne add a test for this case.
I did not change this part of the logic, so it should behave the same as before as of now.

I will add a test for this case and see how it behaves, fixing it accordingly.

[pierrejoye]

I added a test for multilevel folders.

As mentioned earlier, routes, multilanguages and related areas need more tests. I am not sure it is in the scope of this PR, which fixes a bug where routes are not initialized correctly, no matter how the routes have been created in the first place in the source page.

I can do add more but may need more time :slight_smile:,, still not sure that should be all done in this PR.

[pierrejoye]

Anything that speaks against this PR?

We discussed a lot here and in discord, there are other issues to solve with multiple languages. However this PR solves one bug and other more design tasks are surely a different scope :)

[mahagr]

Merged manually because of conflicts.

[pierrejoye]

Thanks for the merge!