Feature/deseng623 Add language selector UI based roughly on ISO 639 codes #2540

Baelx · 2024-06-17T07:38:52Z

Issue #: https://github.com/bcgov/met-public/issues/

Description of changes:

Add admin UI for tenant language selection, allows user to add/remove language mappings
Add API routes for tenant-language mappings
Restrict some Language API routes and store languages directly in the DB
Add tests, merge migrations, clean up code

Note for reviewer Most of this work has already been reviewed. This PR merges that previous work into main and introduces new code for:

language-tenant API routes
fixing migrations
tests (frontend and backend)

* DESENG-632 Add DB tables & API routes * DESENG-623 fix linting * DESENG-623 Move language to migration file, remove delete crud language route * DESENG-623 Remove ability to delete language --------- Co-authored-by: Alex <[email protected]>

… short name (#2538) * DESENG-623 Add frontend, update backend routes * DESENG-623 Add api route updates, correct linting * DESENG-623 Correctly invoke autocomplete api to disable claering --------- Co-authored-by: Alex <[email protected]>

codecov-commenter · 2024-06-17T07:44:59Z

Codecov Report

Attention: Patch coverage is 59.01639% with 25 lines in your changes missing coverage. Please review.

Project coverage is 75.93%. Comparing base (8512584) to head (47a99b8).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2540      +/-   ##
==========================================
- Coverage   75.98%   75.93%   -0.05%     
==========================================
  Files         603      604       +1     
  Lines       21800    21807       +7     
  Branches     1678     1689      +11     
==========================================
- Hits        16564    16559       -5     
- Misses       4974     4985      +11     
- Partials      262      263       +1

Flag	Coverage Δ
metweb	`64.52% <59.01%> (-0.04%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
met-api/src/met_api/models/__init__.py	`100.00% <ø> (ø)`
met-api/src/met_api/models/language.py	`94.73% <ø> (-2.41%)`	⬇️
met-api/src/met_api/resources/language.py	`79.24% <ø> (+5.82%)`	⬆️
met-api/src/met_api/services/language_service.py	`94.44% <ø> (+7.77%)`	⬆️
met-api/src/met_api/utils/roles.py	`100.00% <ø> (ø)`
met-web/src/apiManager/endpoints/index.ts	`100.00% <ø> (ø)`
met-web/src/components/language/index.tsx	`59.01% <59.01%> (ø)`

NatSquared

Looks good! A few things to review but all in all this seems like a robust start to the language system!

NatSquared · 2024-06-17T17:06:18Z

met-api/migrations/README

+## Languages
+
+The list of languages in `versions/c656f3f82334_add_languages_and_tenant_mapping.py` has been taken from [https://www.w3schools.com/tags/ref_language_codes.asp](https://www.w3schools.com/tags/ref_language_codes.asp) and modified slightly to remove duplicate or multiple language codes per language.


Are you starting a pattern of documenting large data dumps with their migration IDs? I see promise in that approach!

Yeah, I kinda thought it would be nice to have transparency with this migration anyway! Just to let future devs know we didn't pull this list out of nowhere

NatSquared · 2024-06-17T17:07:51Z

met-api/migrations/alembic.ini

@@ -8,6 +8,9 @@
 # the 'revision' command, regardless of autogenerate
 # revision_environment = false

+# Enables albemic CLI commands like 'history,' 'branches,' etc.


Thanks for adding this shortcut! Just double-check the spelling of "alembic":

Suggested change

# Enables albemic CLI commands like 'history,' 'branches,' etc.

# Enables Alembic CLI commands like 'history,' 'branches,' etc.

NatSquared · 2024-06-17T17:08:45Z

met-api/migrations/versions/c5dc2bb998ea_merge_heads.py

@@ -0,0 +1,27 @@
+"""
+Merge albembic revisions that:


Suggested change

Merge albembic revisions that:

Merge Alembic revisions that:

lol, I'm getting creative with the spelling a few different ways

NatSquared · 2024-06-17T17:16:44Z

met-api/migrations/versions/c656f3f82334_add_languages_and_tenant_mapping.py

+      "iso_code": "ce"
+    },
+    {
+      "language": "Chichewa, Chewa, Nyanja",


I assume we are intentionally remaining neutral in the cases where languages have multiple names

Nevermind; this is specified in the ticket

NatSquared · 2024-06-17T17:17:33Z

met-web/tests/unit/components/language/LanguageAdminPanel.test.tsx

+const mockSecondLanguage = {
+    id: 90,
+    code: 'tlh',
+    name: 'Klingon',


I like these test languages! 😉

NatSquared · 2024-06-17T17:20:49Z

met-web/tests/unit/components/language/LanguageAdminPanel.test.tsx

+    test('Language selection updates', async () => {
+        await addOrRemoveLanguage('gdx', [mockLanguage, mockSecondLanguage], [mockLanguage]);
+        expect(languageService.postTenantLanguage).toHaveBeenCalledWith('gdx', mockSecondLanguage.id);
+        await addOrRemoveLanguage('gdx', [mockLanguage], [mockLanguage, mockSecondLanguage]);


Thank you for adding high-quality tests for your components! If it's not too much work, consider also using fired events to replicate actual user keystrokes and clicks on the DOM.

NatSquared · 2024-06-17T17:22:33Z

met-api/migrations/versions/c656f3f82334_add_languages_and_tenant_mapping.py

+
+def upgrade():
+    for index, value in enumerate(language_data):
+        op.execute("INSERT INTO language (created_date, updated_date, id, name, code) VALUES ('{0}', '{0}', '{1}', '{2}', '{3}')".format(sa.func.now(), index, value['language'], value['iso_code']))


to avoid ID collisions, you may want to drop the existing languages in the table, unless this table is known to be empty

NatSquared · 2024-06-17T17:25:05Z

met-api/tests/utilities/factory_utils.py

@@ -876,8 +869,7 @@ def factory_enagement_content_model(
 def engagement_content_model_with_language():
    """Produce a engagement content model instance with language."""
    content_model = factory_enagement_content_model()
-    language_model = factory_language_model({'code': 'en', 'name': 'English'})
-    return content_model, language_model
+    return content_model


This function is now just a wrapper for factory_enagement_content_model() and should be replaced with such to avoid redundancy.

sonarcloud · 2024-06-19T00:21:39Z

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Baelx and others added 7 commits June 4, 2024 15:33

DESENG-623 Add frontend tests, trim api tests

ac8dda3

DESENG-623 Add frontend tests

7b6b61a

Merge branch 'main' into feature/deseng623

fabe963

DESENG-623 Add new tests, fix broken ones, merge migrations

bfc6dfb

DESENG-623 Update changelog

47a99b8

NatSquared approved these changes Jun 17, 2024

View reviewed changes

Alex added 2 commits June 18, 2024 17:18

DESENG-623 fix linting and tests, clean up code further

a0d5e04

DESENG-623 Merge main

d0c927f

Baelx merged commit 502fece into main Jun 19, 2024
15 checks passed

Baelx deleted the feature/deseng623 branch June 19, 2024 00:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/deseng623 Add language selector UI based roughly on ISO 639 codes #2540

Feature/deseng623 Add language selector UI based roughly on ISO 639 codes #2540

Baelx commented Jun 17, 2024

codecov-commenter commented Jun 17, 2024 •

edited

Loading

NatSquared left a comment

NatSquared Jun 17, 2024

Baelx Jun 17, 2024

NatSquared Jun 17, 2024

NatSquared Jun 17, 2024

Baelx Jun 17, 2024

NatSquared Jun 17, 2024

NatSquared Jun 17, 2024

NatSquared Jun 17, 2024

NatSquared Jun 17, 2024

NatSquared Jun 17, 2024

NatSquared Jun 17, 2024

sonarcloud bot commented Jun 19, 2024

		## Languages

		The list of languages in `versions/c656f3f82334_add_languages_and_tenant_mapping.py` has been taken from [https://www.w3schools.com/tags/ref_language_codes.asp](https://www.w3schools.com/tags/ref_language_codes.asp) and modified slightly to remove duplicate or multiple language codes per language.

	# Enables albemic CLI commands like 'history,' 'branches,' etc.
	# Enables Alembic CLI commands like 'history,' 'branches,' etc.

Feature/deseng623 Add language selector UI based roughly on ISO 639 codes #2540

Feature/deseng623 Add language selector UI based roughly on ISO 639 codes #2540

Conversation

Baelx commented Jun 17, 2024

codecov-commenter commented Jun 17, 2024 • edited Loading

Codecov Report

NatSquared left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Jun 19, 2024

Quality Gate passed

codecov-commenter commented Jun 17, 2024 •

edited

Loading