Conversation
CCuskley
left a comment
CCuskley
left a comment
There was a problem hiding this comment.
Left a couple comments, but also not totally clear on why a few tasks seem to be failing to launch in cypress tests? Could it be that we have a string/audio mismatch? I think recent string translations from the JSON file might be misssing audio?
| @@ -7,7 +7,7 @@ const languageDetector = new LanguageDetector(); | |||
| languageDetector.addDetector({ | |||
| name: 'defaultToEnglish', | |||
There was a problem hiding this comment.
I don't think we want this...we want a variant that specifies en to fallback to en-US (for legacy compatibility), but we don't want a variant that can't find a language code to fallback to en-US. This could cause more silent failures, e.g., when we launch en-GB or other en- varieties - I think we don't want any default. If the task cannot find a valid language code, we want it to fail loudly because something has gone wrong and we need to fix it. We can maybe use the langaugeoptions.json file in the assets bucket to detect which languages are suported in which task?
There was a problem hiding this comment.
Yeah that's a good point - this generally only applies to running the task as a standalone app (for example, through the demo link), but we wouldn't want an actual dashboard variant to default to English if it had no language code. I could just update this so it only defaults to English if it's running in demo mode (so not through the dashboard)
| data = await response.json(); | ||
|
|
||
| // add temporary fallback for en-US and de-DE until we have the correct folders in the bucket | ||
| if (!data.items || data.items.length === 0) { |
There was a problem hiding this comment.
Shouldn't this come before the first response = await fetch(url)? i.e., we should fix the url before making the call if it contains en-US or de-DE, rather than waiting for it to fail and then fixing the URL and trying the call again.
There was a problem hiding this comment.
I was thinking it would use the new four-letter language folder if it existed and fall back to the two-letter one if not - otherwise, wouldn't this immediately break as soon as we renamed the folder?
There was a problem hiding this comment.
ah, I see - check in with @digital-pro and see if we can re-name the audio folders using the four letter codes? I don't think there would be another blocker to that, but he'd be best placed to know...if we are ready to do that, maybe still build in an explicit log here for if it can't find the audio folder.
There was a problem hiding this comment.
Here's where to go with this following advice from @digital-pro that we can move on renaming folders: we do still have variants up that might have the language as en, de, or es. @zwatson2001 can you put a catch in here that converts those to en-US, de-DE, and es-CO respectively if they're encountered, prior to fetching the assets? This should allow us to move forward with four letter codes but keep some back compatibility
There was a problem hiding this comment.
I actually already have that in this PR! 😄 (in task-launcher/src/index.ts). My thinking was that we would first push the code, then rename the audio folders and change the variant docs, and then eventually remove this fallback from the code. So this code should be compatible with any old two-letter language codes in the variants or folder names, but use the four-letter equivalents if the audio folder exists.
|
I also added additional cypress tests to this PR that load each task in the current non-English languages (de-DE, es-AR, and es-CO). Right now a couple of these are failing for es-AR because they don't have translations yet, but I think this is expected. |
I think this is great proof of concept - this is what we want (better here than on prod!). Though we might need some modification for the test as we have situations incoming e.g., for Portuguese where they will only be doing certain tasks. We might be able to add info to the languageoptions.json file e.g., a little livetasks list that just lists what to test for? |
|
Looks like @CCuskley is top of it. I don't think I am enough in the loop to understand what's going on, so I'll leave it up to her to approve. |
|
It’s really easy to rename the folders. I’ve just been worried about issues like task variants that have (had?) 2-letter language codes. But if we’re good downstream, I can generate the new codes (or both)
From: Chris ***@***.***>
Sent: Tuesday, April 21, 2026 12:25 PM
To: levante-framework/core-tasks ***@***.***>
Cc: David Cardinal ***@***.***>; Mention ***@***.***>
Subject: Re: [levante-framework/core-tasks] Get translations from json files (PR #444)
@CCuskley commented on this pull request.
________________________________
In task-launcher/src/tasks/shared/helpers/getMediaAssets.ts<https://urldefense.com/v3/__https:/github.com/levante-framework/core-tasks/pull/444?email_source=notifications&email_token=ABJ4N3E24DERX5LFUFVUTP34W7DH5A5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIMJVGAZDONRTGEY2M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJL3QOJPXEZLWNFSXOX3DNRUWG2Y*discussion_r3119833356__;Iw!!G92We9drHetJ8EofZw!ahKl9M4RlGzeB2SGLkA-s92_mKsUJyobEEkF5gfTUrAa5WYjDhJvNLJwcpgeSn8W_31GsfB1IypY6--wKSSWV46UhxIe$>:
let url = baseUrl;
if (nextPageToken) {
url += `&pageToken=${nextPageToken}`;
}
- const response = await fetch(url);
- const data: ResponseDataType = await response.json();
+ let data: ResponseDataType;
+ let response: Response;
+
+ response = await fetch(url);
+ data = await response.json();
+
+ // add temporary fallback for en-US and de-DE until we have the correct folders in the bucket
+ if (!data.items || data.items.length === 0) {
ah, I see - check in with @digital-pro<https://urldefense.com/v3/__https:/github.com/digital-pro__;!!G92We9drHetJ8EofZw!ahKl9M4RlGzeB2SGLkA-s92_mKsUJyobEEkF5gfTUrAa5WYjDhJvNLJwcpgeSn8W_31GsfB1IypY6--wKSSWVwljg2AA$> and see if we can re-name the audio folders using the four letter codes? I don't think there would be another blocker to that, but he'd be best placed to know...if we are ready to do that, maybe still build in an explicit log here for if it can't find the audio folder.
—
Reply to this email directly, view it on GitHub<https://urldefense.com/v3/__https:/github.com/levante-framework/core-tasks/pull/444?email_source=notifications&email_token=ABJ4N3CKD4ZXJEJUXHBIKB34W7DH5A5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIMJVGAZDONRTGEY2M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJPLQOJPXEZLWNFSXOX3ON52GSZTJMNQXI2LPNZZV6Y3MNFRWW*discussion_r3119833356__;Iw!!G92We9drHetJ8EofZw!ahKl9M4RlGzeB2SGLkA-s92_mKsUJyobEEkF5gfTUrAa5WYjDhJvNLJwcpgeSn8W_31GsfB1IypY6--wKSSWV4-Sl7QL$>, or unsubscribe<https://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/ABJ4N3BQVKSXM7GILYIOIA34W7DH5AVCNFSM6AAAAACX5SV5LSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHM2DCNJQGI3TMMZRGE__;!!G92We9drHetJ8EofZw!ahKl9M4RlGzeB2SGLkA-s92_mKsUJyobEEkF5gfTUrAa5WYjDhJvNLJwcpgeSn8W_31GsfB1IypY6--wKSSWVyn_vJco$>.
You are receiving this because you were mentioned.Message ID: ***@***.******@***.***>>
|
4 participants
No description provided.