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

Prevent Translation Object Duplicates #756

Closed
wants to merge 4 commits into from
Closed

Conversation

ACK1D
Copy link
Contributor

@ACK1D ACK1D commented Dec 22, 2023

Prevent the creation of duplicate Translation objects with the same source and target locale. Replaced the explicit check for existing translations with the use of the get_or_create method, ensuring atomicity and avoiding potential duplicates. Fix #749

@codecov-commenter
Copy link

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (92c18ab) 92.63% compared to head (ae05873) 92.58%.

Files Patch % Lines
wagtail_localize/operations.py 0.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #756      +/-   ##
==========================================
- Coverage   92.63%   92.58%   -0.05%     
==========================================
  Files          47       47              
  Lines        4057     4059       +2     
  Branches      601      602       +1     
==========================================
  Hits         3758     3758              
- Misses        176      177       +1     
- Partials      123      124       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@zerolab
Copy link
Collaborator

zerolab commented Dec 22, 2023

This is a good start @ACK1D. Thank you for correcting the typos too.

Could you please add some tests? I'd add a new test_operations.py test file under https://github.com/wagtail/wagtail-localize/tree/main/wagtail_localize/tests, add a secondary locale (example) and a page, then add a test that instantiates
TranslationCreator with the default and new locales for target_locales and calls create_translations() to check that only one translation has been created

@ACK1D
Copy link
Contributor Author

ACK1D commented Dec 23, 2023

@zerolab thanks a bunch for the detailed info. Can you check this out?

Copy link
Collaborator

@zerolab zerolab left a comment

Choose a reason for hiding this comment

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

One non-blocking note, but this is great work, thank you!

wagtail_localize/tests/test_operations.py Outdated Show resolved Hide resolved
@zerolab
Copy link
Collaborator

zerolab commented Dec 27, 2023

Merged in 8b32d63. Thanks @ACK1D

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

Successfully merging this pull request may close these issues.

Prevent TranslationCreator create Translation objects with the same locale as the source locale`
3 participants