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

Fix wrong sorting of parsed language header #2696

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

emanuelschuetze
Copy link
Member

Fixed partly #2695

Prefered language settings of browser (weighted by user, e.g. [de,en,cs]) should not sorted! In this example: [cs,de,en] -> cs translation is empty for password reset strings -> backend send empty email strings!

Copy link
Member

@luisa-beerboom luisa-beerboom left a comment

Choose a reason for hiding this comment

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

Apart from the code change request, this bug PR is missing a test.

I'd suggest using openslides_backend/tests/system/action/meeting/test_create.py lines 525 (just use the header_language string that causes the bug as a value) and 544 (_(name)) as a reference for how translation can be set up and used and writing a simple test in a completely new test file in a central place somewhere in the tests folder.

@@ -56,7 +56,7 @@ def parse_language_header(self, lang_header: str) -> list[str]:
code = code.split("-")[0]
result.append((q, code))
# sort by quality value and return only the codes
return [t[1] for t in sorted(result)]
return [t[1] for t in result]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return [t[1] for t in result]
return [t[1] for t in sorted(result, key=lambda tup: tup[0])]

The sort did not only sort the languages in alphabetical order: It sorted them by the weight value q first. If that wasn't given or equal to another value, it sorted them in alphabetical order.
I will agree that the latter is probably a bug (in this case caused by the way python sorts tuples), but the former definitely isn't.

The above code should sort the languages by weight first and leave empty values in the same order as before.
If, during testing, it turns out to wrongly prioritize or wrongly not-prioritize languages with no q-value over those with q-value, I'd suggest trying to set q to "0.0" instead of "1.0" in line 50.

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

Successfully merging this pull request may close these issues.

3 participants