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

Make end user visible texts for login methods translatable #208

Merged
merged 2 commits into from
Jan 31, 2022

Conversation

akikoskinen
Copy link
Contributor

Django-parler has this documentation for turning a previously non-translated field into a translated one. The problem with that document is, in reality it doesn't work like that.

See commit messages for details how it was solved.

@codecov
Copy link

codecov bot commented Jan 25, 2022

Codecov Report

Merging #208 (80a122a) into develop (7415a49) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #208   +/-   ##
========================================
  Coverage    78.06%   78.07%           
========================================
  Files           88       88           
  Lines         3679     3680    +1     
========================================
+ Hits          2872     2873    +1     
  Misses         807      807           
Impacted Files Coverage Δ
users/admin.py 46.42% <100.00%> (+0.38%) ⬆️
users/models.py 96.47% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7415a49...80a122a. Read the comment docs.

Changing an existing non-translated field to a translated one requires a
multi-step process. The translated fields are first created with a
different temporary name. Then data is copied from the non-translated
fields to the translated fields. The old fields are removed and the
translated fields are renamed to have the same name as the original
fields had. All this resides in the single migration file in this
commit.

A few changes were required to an old migration file.
1) The bases needs to include TranslatableModelMixin in the migration
   where LoginMethod model is first introduced. This is required by
   Django-parler and how it works internally.
   django-parler/django-parler#263 (comment)
2) The non-null `name` field requires a default value so that migrating
   backwards works. The backward migration creates the non-null `name`
   column which fails unless the column also has a default value.
@akikoskinen akikoskinen force-pushed the HP-1195-translate-login-methods branch from 1d24dd4 to 01764db Compare January 28, 2022 06:43
Copy link
Contributor

@mikkokeskinen mikkokeskinen left a comment

Choose a reason for hiding this comment

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

Migrations seem to work nicely forwards and backwards. Good job.

It could be nice to group the translated and non-translated fields in the LoginMethodAdmin view like it's done in the services.admin.ServiceAdmin.

@akikoskinen akikoskinen force-pushed the HP-1195-translate-login-methods branch from 01764db to 80a122a Compare January 31, 2022 07:40
@akikoskinen
Copy link
Contributor Author

It could be nice to group the translated and non-translated fields in the LoginMethodAdmin view like it's done in the services.admin.ServiceAdmin.

Good idea. Did that.

@sonarcloud
Copy link

sonarcloud bot commented Jan 31, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@mikkokeskinen mikkokeskinen self-requested a review January 31, 2022 07:42
@akikoskinen akikoskinen merged commit 80a122a into develop Jan 31, 2022
@akikoskinen akikoskinen deleted the HP-1195-translate-login-methods branch January 31, 2022 08:10
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.

None yet

2 participants