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

LINK-1367 | Support multilingual confirmation_message and instructions #610

Merged

Conversation

jorilindell
Copy link
Contributor

@jorilindell jorilindell commented Jun 12, 2023

Description

At the moment confirmation_message and instructions fields in SignUp model are single text fields. Change them to multilingual text fields.

Closes

LINK-1367

@codecov-commenter
Copy link

codecov-commenter commented Jun 12, 2023

Codecov Report

Merging #610 (7c113ec) into develop (73e6968) will increase coverage by 0.06%.
The diff coverage is 97.91%.

@@             Coverage Diff             @@
##           develop     #610      +/-   ##
===========================================
+ Coverage    73.67%   73.74%   +0.06%     
===========================================
  Files          231      233       +2     
  Lines        16128    16170      +42     
===========================================
+ Hits         11882    11924      +42     
  Misses        4246     4246              
Impacted Files Coverage Δ
events/tests/conftest.py 98.02% <ø> (ø)
registrations/models.py 98.13% <92.85%> (+0.11%) ⬆️
...tilingual_confirmation_message_and_instructions.py 100.00% <100.00%> (ø)
registrations/tests/test_models.py 100.00% <100.00%> (ø)
registrations/tests/test_signup_post.py 100.00% <100.00%> (ø)
registrations/translation.py 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Comment on lines 362 to 366
"event": getattr(
self.registration.event,
f"name_{service_language}",
self.registration.event.name,
),
Copy link
Contributor

Choose a reason for hiding this comment

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

Unless there is a specific reason why not, it would be cleaner to use django-modeltranslation's standard way of accessing the translated fields, ie.

with translation.override(service_language):
    email_variables = {
        ...
        "event": self.registration.event
        ...
    }

One thing to consider is what kind of fallback language logic we want to use with these fields, and configure modeltranslation accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great! I was quite sure that my approach wan't the best possible. Changed as suggested. Added Finnish, Swedish and English as fallback language for confirmation_message and instructions fields.

@jorilindell jorilindell force-pushed the feature/LINK-1367-multilingual-registration-fields branch from dda490d to 786dee0 Compare June 26, 2023 13:27
@jorilindell jorilindell force-pushed the feature/LINK-1367-multilingual-registration-fields branch from 786dee0 to 7c113ec Compare June 26, 2023 13:31
@sonarcloud
Copy link

sonarcloud bot commented Jun 26, 2023

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 1 Code Smell

95.7% 95.7% Coverage
0.0% 0.0% Duplication

Copy link
Contributor

@tuomas777 tuomas777 left a comment

Choose a reason for hiding this comment

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

Looking good! 👍 🦖

@jorilindell jorilindell merged commit 89bcc1c into develop Jun 27, 2023
4 checks passed
@jorilindell jorilindell deleted the feature/LINK-1367-multilingual-registration-fields branch June 27, 2023 08:37
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

3 participants