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.

Sign up for GitHub

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

Jump to bottom

Fix #432 (url translation) #607

Merged
merged 1 commit into from
Apr 9, 2017
Merged

Fix #432 (url translation) #607

merged 1 commit into from
Apr 9, 2017

Conversation

lingxiaoyang
Copy link
Contributor

@lingxiaoyang lingxiaoyang commented Dec 28, 2016 •
edited

Fixes #432 by lingxiaoyang

Changes proposed in this pull request:

  • Add translatable strings in all urls.py.
  • Add language prefix /fr/ in URL for French.
  • Add translatable URL name for client creation form wizard.
  • Add new test setting file, for running the unit test in French.
  • Fix problems in French unit test.
  • Add French unit test to Travis CI configuration.

Status

How to verify this change

  1. Visit http://localhost:8000/fr/, it should be French.
  2. Visit every page, the URLs should be in French now (not all, because they need translation in .po files)
  3. Go to client creation form wizard: the URL should show the steps in French.
  4. Execute French unit test by:
python3 src/manage.py test --settings=sous_chef.settings_test_fr
  1. Go to Travis CI: the build log should include the French unit test.

Verify English:

  1. Visit http://localhost:8000, it should be in English by default.
  2. Now the URLs should be all in English

Deployment notes and migration

none

New translatable strings

Additional notes

none

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 94.921% when pulling 0319647 on lingxiaoyang:fix_432 into d89c8ac on savoirfairelinux:dev.

@lingxiaoyang
Copy link
Contributor Author

I will need to provide the French URLs based on the community's translation since one month. (For easier testing)

erozqba
erozqba reviewed Feb 24, 2017
View reviewed changes
src/member/urls.py Outdated
name='member_update_' + k)
url(string_concat(_(r'^(?P<pk>\d+)/update/'), k, '/$'),
v.as_view(),
name='member_update_' + internal_name)
Copy link
Contributor

Choose a reason for hiding this comment

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

The "internal_name" part of the route get translated, but no the 'member_update_' so we can have named URL like 'member_update_information_basique'.
You could change the implementation to translate the URL but keep route names in English?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@erozqba I improved this part. In fact, I think the route names were in English but the code was a bit confusing... Please check out the latest commit.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 94.989% when pulling eeccb99 on lingxiaoyang:fix_432 into d89c8ac on savoirfairelinux:dev.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling c5ee5ad on lingxiaoyang:fix_432 into ** on savoirfairelinux:dev**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling e31de04 on lingxiaoyang:fix_432 into ** on savoirfairelinux:dev**.

@erozqba
Copy link
Contributor

erozqba commented Mar 7, 2017

@lingxiaoyang this is ready to test? When it's ready, please change the status so I know and I will test again. Thanks a lot! 👍

@lingxiaoyang
Copy link
Contributor Author

@erozqba Sure! I'll let you know - this will need to be rebased after merging #664.

@aaboffill
Copy link

@lingxiaoyang I think that you have some voluntary work :D ... with conflicts...

@lingxiaoyang
Copy link
Contributor Author

@aaboffill Thank you! I'll update it.

@aaboffill
Copy link

@lingxiaoyang By experience, maybe you need to wait to the last moment to fix the conflicts, there are several PR per week and it's very probably that you will have conflicts again before merge :)

@lingxiaoyang
Copy link
Contributor Author

@aaboffill That's true. I'll now clean this PR a bit and squashing some commits...

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.07%) to 94.9% when pulling a5cca68 on lingxiaoyang:fix_432 into fef039c on savoirfairelinux:dev.

Fixed #432: URL translations with /fr/ prefix
66a5880 
Includes:
1. Unit test in French setting on Travis CI.
2. New translatable strings and their translations are provided.
3. Form wizard URL is translated.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 94.882% when pulling 66a5880 on lingxiaoyang:fix_432 into 2075ffb on savoirfairelinux:dev.

@erozqba
Copy link
Contributor

erozqba commented Apr 9, 2017

👍

@erozqba erozqba merged commit af92755 into savoirfairelinux:dev Apr 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erozqba erozqba erozqba left review comments

Assignees
No one assigned
Labels
i18n not urgent
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Provide proper translations for URL patterns
4 participants
@lingxiaoyang @coveralls @erozqba @aaboffill