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

updates filter to get translation source #578

Merged

Conversation

sheralim012
Copy link
Contributor

Fixes #547

steps to reproduce the issue

  • create locales L1, L2, L3
  • create a page P1 in locale L1
  • translate page P1 to locale L2 (let's say the newly translated page is P1')
  • translate page P1' to locale L3 (let's say the newly translated page is P1'')
  • you will be redirected to edit page with error MultipleObjectsReturned - get() returned more than one TranslationSource -- it returned 2!

observations

  • above-mentioned scenario creates 2 translation sources with same object_id, and spectific_content_type_id but from different locales
  • "convert to alias" buttons' logic (finding the source of translation) is dependent on these 2 fields but these 2 fields alone can't uniquely identify the translation source (hence the error - multiple objects returned)

solution

the combination of object_id, spectific_content_type_id, and translations__target_locale_id can uniquely identify the translation source

Let me know if I missed something.

@zerolab
Copy link
Collaborator

zerolab commented May 25, 2022

Hey @sheralim012, thank you for this!
Any chance you could add some tests?

@codecov-commenter
Copy link

codecov-commenter commented May 25, 2022

Codecov Report

Merging #578 (047efd2) into main (7d6278c) will increase coverage by 0.11%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #578      +/-   ##
==========================================
+ Coverage   91.44%   91.55%   +0.11%     
==========================================
  Files          47       47              
  Lines        3894     3898       +4     
  Branches      591      592       +1     
==========================================
+ Hits         3561     3569       +8     
+ Misses        192      185       -7     
- Partials      141      144       +3     
Impacted Files Coverage Δ
wagtail_localize/views/convert.py 71.62% <ø> (+1.35%) ⬆️
wagtail_localize/views/edit_translation.py 85.44% <ø> (+1.53%) ⬆️
wagtail_localize/wagtail_hooks.py 80.11% <ø> (ø)
wagtail_localize/components.py 82.69% <0.00%> (-1.93%) ⬇️
wagtail_localize/compat.py 51.85% <0.00%> (-0.33%) ⬇️
wagtail_localize/models.py 95.57% <0.00%> (-0.15%) ⬇️

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 7d6278c...047efd2. Read the comment docs.

@sheralim012
Copy link
Contributor Author

@zerolab I have added the test cases.
Could you review it now?

@zerolab zerolab merged commit 94699a4 into wagtail:main May 31, 2022
@zerolab
Copy link
Collaborator

zerolab commented May 31, 2022

Thank you for this, @sheralim012. Tested locally and works well.

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.

MultipleObjectsReturned - multiple TranslationSource for one translation_key
3 participants