Fix language preferentiality for duplicate resources #12152

rtibbles · 2024-05-09T20:18:37Z

Summary

Fixes historically incorrect language code for Chewa, we had previously set it as 'nyn' which is for a completely different language.
Adds a migration step to migrate device settings from 'nyn' to the actual language code 'ny'
Fixes previously unnoticed regression where the copies modal for duplicate resources would not work
Adds support for cross checking multiple language codes to see if the current UI language matches the content language

References

Reviewer guidance

Change the user interface to Chewa and ensure that it is still translated.
Run 0.16.1, select Chewa as the default language. Upgrade to the asset from this PR and ensure that it is properly still showing in Chewa by default.
Install the first topic of KA English and KA Fufulde.
Engage with one exercise in Fufulde, do not master it.
Go to the home page, confirm that it appears in the resume learning in English if the UI is in English, check that the '2 locations' is clickable and the modal works.
Change the UI language to Fufulde, confirm that the resume learning shows the resource in Fufulde now.

Import duplicate resources in the KA English channel. Search for them, confirm that the duplicate resource modal works there as well.

Testing checklist

Contributor has fully tested the PR manually
If there are any front-end changes, before/after screenshots are included
Critical user journeys are covered by Gherkin stories
Critical and brittle code paths are covered by unit tests

PR process

PR has the correct target branch and milestone
PR has 'needs review' or 'work-in-progress' label
If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
If this is an important user-facing change, PR or related issue has a 'changelog' label
If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

Automated test coverage is satisfactory
PR is fully functional
PR has been tested for accessibility regressions
External dependency files were updated if necessary (yarn and pip)
Documentation is updated
Contributor is in AUTHORS.md

🚀 This description was created by Ellipsis for commit `2d074f3`

Summary:

This PR corrects the Chewa language code, adds necessary migrations, updates handling of duplicate resources, and enhances language matching logic.

Key points:

Corrects Chewa language code from nyn to ny.
Adds migration for updated language code in device settings.
Updates duplicate resource modal handling in various views.
Enhances language matching logic in getContentLangActive function.
Ensures UI consistency and functionality post-migration.

Generated with ❤️ by ellipsis.dev

github-actions · 2024-05-09T20:52:25Z

Build Artifacts

Asset type	Download link
PEX file	kolibri-0.16.2b1.dev0_git.20.g1fcd0aca.pex
Windows Installer (EXE)	kolibri-0.16.2b1.dev0+git.20.g1fcd0aca-windows-setup-unsigned.exe
Debian Package	kolibri_0.16.2b1.dev0+git.20.g1fcd0aca-0ubuntu1_all.deb
Mac Installer (DMG)	kolibri-0.16.2b1.dev0+git.20.g1fcd0aca-0.4.2.dmg
Android Package (APK)	kolibri-0.16.2b1.dev0+git.20.g1fcd0aca-0.1.2-debug.apk
TAR file	kolibri-0.16.2b1.dev0+git.20.g1fcd0aca.tar.gz
WHL file	kolibri-0.16.2b1.dev0+git.20.g1fcd0aca-py2.py3-none-any.whl

rtibbles · 2024-05-10T05:18:24Z

Hey @ellipsis, give me a code review

ellipsis-dev · 2024-05-10T05:18:32Z

OK! Reviewing this PR...

Responding to this comment by @rtibbles. For more information about Ellipsis, check the documentation.

ellipsis-dev

❌ Changes requested. Reviewed everything up to 2d074f3 in 2 minutes and 44 seconds

More details

Looked at 280 lines of code in 32 files
Skipped 0 files when reviewing.
Skipped posting 2 drafted comments based on config settings.

1. kolibri/core/device/upgrade.py:24

Draft comment:
The function set_device_settings is not imported correctly. Ensure to import it from kolibri.core.device.utils to avoid runtime errors.
Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.

2. kolibri/plugins/learn/assets/src/views/CopiesModal.vue:7

Draft comment:
The event emitted here should be @closeModal instead of @submit to match the event handler in the parent components.

    @closeModal="$emit('closeModal')"

Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.

Workflow ID: wflow_snNni4PbI8mB3BbG

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

⌛ 6 days left in your free trial, upgrade for $20/seat/month or contact us.

ellipsis-dev · 2024-05-10T05:21:25Z

kolibri/core/assets/src/utils/i18n.js

+};
+
+export const getContentLangActive = language => {
+  const langCode = languageIdToCode(currentLanguage);


The variable currentLanguage is used but not defined in the scope of getContentLangActive. Ensure that currentLanguage is correctly defined or passed as a parameter to this function.

It's defined in the module scope.

radinamatic · 2024-05-10T11:35:39Z

No luck with seeing the Chewa UI after upgrading from 0.16.1 to asset from this PR. nyn locale is not found (which was the point, I believe), but neither is any trace of the Chewa language in the language switcher, and the default device language is returned to browser. If the locale is manually edited to ny, it returns 404.

no-ny.mp4

Tested on Ubuntu 20.04 with Firefox and Chrome.

rtibbles · 2024-05-10T17:37:14Z

Thanks @radinamatic - I am pretty sure I have forgotten to update the language code in one place. I'll do a quick search for the whole codebase for nyn to double check!

Add intl locale data for ny language code.

Update duplicate resource prioritization with new function.

rtibbles · 2024-05-10T21:38:00Z

There were a couple of spots I had missed. Including the fact that Intl.js doesn't actually have a polyfill for Chewa locale data. Thankfully, I was able to generate one automatically from sources. I think it should all be working as intended now.

radinamatic · 2024-05-13T11:00:26Z

After the upgrade from 0.16.1 using the newest asset in this PR, nyn locale that was previously set as default logically gives the 404 error, but going to the home page reloads the new ny locale, and the Chewa language is present in the language switcher modal.

yes-ny.mp4

All is also well after installing the first topic of KA English and KA Fufulde, exercises engaged and not mastered appear in the resume... section, and the '2 locations' link is clickable and the modal works in both locales.

Fulfulde	English

Importing duplicate resources in the KA English channel also displays the 2 locations link and the duplicate resource modal works

radinamatic

Manual QA passes, good to go! 👏🏽 💯

marcellamaki

Code review looks good to me. I think the sorting situation with the matching language codes is the better option for handling a bit of odd perfect/acceptable matching, and the comments are helpful to make it more parseable for other use cases.

Remains to be seen if future Marcella agrees with me.

rtibbles requested review from radinamatic and marcellamaki May 9, 2024 20:18

rtibbles added the TODO: needs review Waiting for review label May 9, 2024

rtibbles added this to the Kolibri 0.16: Planned Patch 2 milestone May 9, 2024

github-actions bot added DEV: backend Python, databases, networking, filesystem... APP: Learn Re: Learn App (content, quizzes, lessons, etc.) DEV: frontend SIZE: small labels May 9, 2024

ellipsis-dev bot reviewed May 10, 2024

View reviewed changes

rtibbles added 3 commits May 10, 2024 14:36

Fix incorrect language code for Chewa. Use ny instead of nyn.

79df69a

Add intl locale data for ny language code.

Fix copies modal display.

4b5de0c

Add core function for matching content language to current language.

00d7f70

Update duplicate resource prioritization with new function.

rtibbles force-pushed the language! branch from 2d074f3 to 00d7f70 Compare May 10, 2024 21:36

github-actions bot added DEV: tools Internal tooling for development SIZE: medium labels May 10, 2024

radinamatic approved these changes May 13, 2024

View reviewed changes

marcellamaki approved these changes May 13, 2024

View reviewed changes

marcellamaki merged commit 32e1e94 into learningequality:release-v0.16.x May 13, 2024
34 checks passed

marcellamaki mentioned this pull request May 13, 2024

Learn > Continue learning shortcut tiles mixing up languages in Khan lessons #11751

Closed

rtibbles deleted the language! branch July 15, 2024 23:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix language preferentiality for duplicate resources #12152

Fix language preferentiality for duplicate resources #12152

rtibbles commented May 9, 2024 •

edited by ellipsis-dev bot

Loading

github-actions bot commented May 9, 2024 •

edited

Loading

rtibbles commented May 10, 2024

ellipsis-dev bot commented May 10, 2024

ellipsis-dev bot left a comment

ellipsis-dev bot May 10, 2024

rtibbles May 10, 2024

radinamatic commented May 10, 2024

rtibbles commented May 10, 2024

rtibbles commented May 10, 2024

radinamatic commented May 13, 2024

radinamatic left a comment

marcellamaki left a comment

Fix language preferentiality for duplicate resources #12152

Fix language preferentiality for duplicate resources #12152

Conversation

rtibbles commented May 9, 2024 • edited by ellipsis-dev bot Loading

Summary

References

Reviewer guidance

Testing checklist

PR process

Reviewer checklist

Summary:

github-actions bot commented May 9, 2024 • edited Loading

Build Artifacts

rtibbles commented May 10, 2024

ellipsis-dev bot commented May 10, 2024

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

ellipsis-dev bot May 10, 2024

Choose a reason for hiding this comment

rtibbles May 10, 2024

Choose a reason for hiding this comment

radinamatic commented May 10, 2024

rtibbles commented May 10, 2024

rtibbles commented May 10, 2024

radinamatic commented May 13, 2024

radinamatic left a comment

Choose a reason for hiding this comment

marcellamaki left a comment

Choose a reason for hiding this comment

rtibbles commented May 9, 2024 •

edited by ellipsis-dev bot

Loading

github-actions bot commented May 9, 2024 •

edited

Loading