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

[#2064] Multilangual routes files support #1012

Open
wants to merge 6 commits into
base: 1.4.x
Choose a base branch
from

Conversation

vadimv82
Copy link

Multiple multilangual routes files support.
You can put several routes files into conf folder in a way:
routes.en, routes.fr, routes.ru.
Same route can have multiple urls for different languages.
Useful for SEO.

Copy link
Member

@xael-fry xael-fry left a comment

Choose a reason for hiding this comment

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

Some tests will be great too

String virtualFileName = vf.getName();
if(virtualFileName !=null && virtualFileName.equals("routes")){
routes.add(vf);
} else if(virtualFileName !=null && virtualFileName.matches("routes\\.[A-Za-z]{2}")){
Copy link
Member

Choose a reason for hiding this comment

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

language can be defined on mode character to handle country specific code ex: en_US, en_UK

Copy link
Author

Choose a reason for hiding this comment

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

@xael-fry So routes files can have also names like that routes.en_US ?
I will do tests.

@vadimv82
Copy link
Author

vadimv82 commented Oct 28, 2016

I added tests, and changed some logic.

Now you can add specific language routes file. For example routes.ru_RU, routes.fr_FR.
Those can contain language specific urls. If you open lang specific url it will rout right action, but also automatically activates language as it is specified in Route, by url. And also will reverse route.
All that is not lang specific, like ajax calls, static content servers e.t.c you can put under "routes" file without lang extension. All those calls will not activate language change.

@vadimv82
Copy link
Author

vadimv82 commented Nov 9, 2016

Can please somebody review my pull request?

@xael-fry
Copy link
Member

@vadimv82 Could you stash your changes?

if(matchingRoutes.size()==0) return;
final String locale = Lang.get();
if(StringUtils.isEmpty(locale)) return;
matchingRoutes.sort(new Comparator<ActionRoute>() {
Copy link
Member

Choose a reason for hiding this comment

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

Jenkins cannot compile the code

    [javac]         matchingRoutes.sort(new Comparator<ActionRoute>() {
    [javac]                       ^
    [javac]   symbol:   method sort(<anonymous Comparator<ActionRoute>>)
    [javac]   location: variable matchingRoutes of type List<ActionRoute>

Seems like some method are missing as you use the interface Comparator

Copy link
Author

@vadimv82 vadimv82 Jan 31, 2017

Choose a reason for hiding this comment

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

I run same build on my machine with ant -buildfile ./framework/build.xml test
And build is succesfull.

I use anonymous comparator

sortedMatchingRoutes.sort(new Comparator() {
@OverRide
public int compare(ActionRoute ar1, ActionRoute ar2) {
if(locale.equals(ar1.route.locale)) return -1;
if(locale.equals(ar2.route.locale)) return 1;
return Integer.compare(Play.langs.indexOf(ar1.route.locale), Play.langs.indexOf(ar2.route.locale));
}
});

@@ -114,7 +114,7 @@ public boolean isProd() {
/**
* Main routes file
*/
public static VirtualFile routes;
public static List<VirtualFile> routes;
Copy link
Contributor

Choose a reason for hiding this comment

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

It breaks backward compatibility.
I suggest leaving VirtualFile routes untouched and adding a new field List<VirtualFile> internationalizedRoutes. In this case you don't need field multilangRouteFiles.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants