From 89781c8c88578785667fd9e044d381b5a37f0b10 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Wed, 16 Oct 2024 21:00:09 -0700 Subject: [PATCH] tags: Localization fixes - Sort tags in the dropdown using user's locale. - Fix localized tags being used for filtering. This happens when the application freshly starts and list_tags() is called using user's non-default locale. - Avoid using element.textContent in JS. Instead use datasets. - Add functional test for checking localization issues. Signed-off-by: Sunil Mohan Adapa --- plinth/app.py | 23 +++++++++++---- plinth/templates/apps.html | 5 +++- plinth/tests/tags/test_functional.py | 44 +++++++++++++++++++++++----- plinth/views.py | 4 ++- static/tags.js | 6 ++-- 5 files changed, 64 insertions(+), 18 deletions(-) diff --git a/plinth/app.py b/plinth/app.py index b9aa568ef..9df14d14c 100644 --- a/plinth/app.py +++ b/plinth/app.py @@ -521,16 +521,29 @@ class Info(FollowerComponent): self.manual_page = manual_page self.clients = clients self.donation_url = donation_url - self.tags = tags or [] + self._tags = tags or [] if clients: clients_module.validate(clients) + @property + def tags(self) -> list[str]: + """Return a list of untranslated tags on the app. + + These can only be retrieved after Django has been configured. + """ + # Store untranslated original strings instead of proxy objects + from django.utils.translation import override + with override(language=None): + return [str(tag) for tag in self._tags] + @classmethod - def list_tags(self) -> list: + def list_tags(self) -> list[str]: """Return a list of untranslated tags.""" - tags = set() - for app in App.list(): - tags.update(app.info.tags) + tags: set[str] = set() + from django.utils.translation import override + with override(language=None): + for app in App.list(): + tags.update((str(tag) for tag in app.info.tags)) return list(tags) diff --git a/plinth/templates/apps.html b/plinth/templates/apps.html index 818867c29..a9659c0dc 100644 --- a/plinth/templates/apps.html +++ b/plinth/templates/apps.html @@ -31,7 +31,10 @@ diff --git a/plinth/tests/tags/test_functional.py b/plinth/tests/tags/test_functional.py index 11ef3dd35..fa7663b79 100644 --- a/plinth/tests/tags/test_functional.py +++ b/plinth/tests/tags/test_functional.py @@ -17,7 +17,7 @@ def _is_app_listed(session_browser, app): assert len(app_links) == 1 -@pytest.fixture(autouse=True) +@pytest.fixture(name='bittorrent_tag') def fixture_bittorrent_tag(session_browser): """Click on the BitTorrent tag.""" bittorrent_tag = '/plinth/apps/?tag=BitTorrent' @@ -28,33 +28,61 @@ def fixture_bittorrent_tag(session_browser): session_browser.links.find_by_href(bittorrent_tag).click() -def test_bittorrent_tag(session_browser): +@pytest.fixture(name='locale') +def fixture_locale(session_browser): + """Set a different language for a user.""" + functional.login(session_browser) + try: + functional.user_set_language(session_browser, 'es') + yield + finally: + functional.user_set_language(session_browser, '') + + +def test_bittorrent_tag(session_browser, bittorrent_tag): """Test that the BitTorrent tag lists Deluge and Transmission.""" _is_app_listed(session_browser, 'deluge') _is_app_listed(session_browser, 'transmission') -def test_search_for_tag(session_browser): +def test_search_for_tag(session_browser, bittorrent_tag): """Test that searching for a tag returns the expected apps.""" search_input = session_browser.driver.find_element_by_id('add-tag-input') with functional.wait_for_page_update( session_browser, timeout=10, - expected_url='/plinth/apps/?tag=BitTorrent&tag=File%20sharing'): + expected_url='/plinth/apps/?tag=BitTorrent&tag=File+sharing'): search_input.click() - search_input.send_keys('file') + search_input.send_keys('file sharing') search_input.send_keys(Keys.ENTER) + for app in ['deluge', 'samba', 'sharing', 'syncthing', 'transmission']: _is_app_listed(session_browser, app) -def test_click_on_tag(session_browser): +def test_click_on_tag(session_browser, bittorrent_tag): """Test that clicking on a tag lists the expected apps.""" search_input = session_browser.driver.find_element_by_id('add-tag-input') with functional.wait_for_page_update( session_browser, timeout=10, - expected_url='/plinth/apps/?tag=BitTorrent&tag=File%20sync'): + expected_url='/plinth/apps/?tag=BitTorrent&tag=File+sync'): search_input.click() session_browser.find_by_css( - ".dropdown-item[data-value='File sync']").click() + ".dropdown-item[data-tag='File sync']").click() + for app in ['deluge', 'nextcloud', 'syncthing', 'transmission']: _is_app_listed(session_browser, app) + + +def test_tag_localization(session_browser, locale): + """Test that tags are localized and tests in done localized.""" + functional.visit(session_browser, '/plinth/apps/?tag=Sharing') + badge = session_browser.find_by_css('.tag-badge[data-tag="Sharing"]').first + assert 'Compartir' in badge.text + + search_input = session_browser.driver.find_element_by_id('add-tag-input') + with functional.wait_for_page_update( + session_browser, timeout=10, + expected_url='/plinth/apps/?tag=Sharing&tag=Bookmarks'): + search_input.click() + search_input.send_keys('Marcadores') + search_input.send_keys(Keys.ENTER) diff --git a/plinth/views.py b/plinth/views.py index 5d555f6f2..b2ee60e67 100644 --- a/plinth/views.py +++ b/plinth/views.py @@ -173,7 +173,9 @@ class AppsIndexView(TemplateView): menu_items = menu.main_menu.active_item(self.request).items context['tags'] = tags - context['all_tags'] = app_module.Info.list_tags() + # Sorted tags by localized string + context['all_tags'] = sorted(app_module.Info.list_tags(), + key=lambda tag: _(tag)) context['menu_items'] = self._pick_menu_items(menu_items, tags) return context diff --git a/static/tags.js b/static/tags.js index 01ddc4340..2b293789e 100644 --- a/static/tags.js +++ b/static/tags.js @@ -76,11 +76,11 @@ function findMatchingTag(dropdownItems) { let bestMatch = null; dropdownItems.forEach(item => { - const text = item.textContent.toLowerCase(); + const text = item.dataset.tag_l10n.toLowerCase(); if (text.includes(searchTerm)) { item.style.display = 'block'; function matchesEarly () { - let bestMatchText = bestMatch.textContent.toLowerCase(); + let bestMatchText = bestMatch.dataset.tag_l10n.toLowerCase(); return text.indexOf(searchTerm) < bestMatchText.indexOf(searchTerm); }; if (bestMatch === null || matchesEarly()) { @@ -129,7 +129,7 @@ function onTagInputKeyUp(event) { */ function onTagInputDropdownItemClicked(event) { const item = event.currentTarget; - const selectedTag = item.dataset.value; + const selectedTag = item.dataset.tag; // Add the selected tag and update the path. let tags = getTags('');