Report argument modal translation #5483

fergie-nz · 2024-11-19T23:36:04Z

👩🏻‍💻 What does this PR do?

Adds translations to report modal via json schema or UI schema.

Utilised inbuilt JSON form translation.

Maintains type safety of locale keys!

💌 Any notes for the reviewer?

🧪 Testing

You can test by running the 'Report-argument-modal-translation-example' branch which includes an example translation in the itemUsage report for both methods of adding translations

Go to reports
Open item usage report
See translation

Note - you may need to clear out existing reports in your database to view.
Delete reports, then run your back end. This will populate with updated report automatically

📃 Documentation

Part of an epic: But also documentation added in PR

Also see same docs added to reports repo

fergie-nz · 2024-11-19T23:37:15Z

Also made a PR to update reports repo docs:

https://github.com/msupply-foundation/open-msupply-reports/pull/118

fergie-nz · 2024-11-19T23:38:25Z

client/packages/programs/src/JsonForms/common/JsonForm.tsx

@@ -154,8 +161,18 @@ const FormComponent = ({
  // This allows "default" values to be set in the JSON schema
  const handleDefaultsAjv = createAjv({ useDefaults: true });

+  const commonKeys = useMemo(() => new Set(Object.keys(common)), []);


memoized keys in case checking Object.keys repeatedly is expensive. Unsure if it would be for how many keys we have.

andreievg · 2024-11-19T23:46:04Z

Looks great, does it work for our patient schema too:

open-msupply/server/service/src/programs/schemas/patient.json

Line 113 in 512275c

"FEMALE",

Can you do an example of how it works for enums in that default patient schema ?

Also do you have a list of tasks to finish this issue fully ?

fergie-nz · 2024-11-20T00:04:35Z

Looks great, does it work for our patient schema too:

open-msupply/server/service/src/programs/schemas/patient.json

Line 113 in 512275c

"FEMALE",

Can you do an example of how it works for enums in that default patient schema ?

Also do you have a list of tasks to finish this issue fully ?

I hadn't considered the patient schema!

I'll have a look.

I do not have a list of tasks to finish this issue fully

fergie-nz · 2024-11-20T00:08:29Z

Can you do an example of how it works for enums in that default patient schema ?

Yes with a small modification of how we add enums in our schema.

I will create an example of this and update the docs

github-actions · 2024-11-20T00:56:45Z

Bundle size difference

Comparing this PR to main

Old size	New size	Diff
4.99 MB	4.99 MB	1.5 KB (0.03%)

fergie-nz · 2024-11-20T02:22:23Z

Can you do an example of how it works for enums in that default patient schema ?

Also do you have a list of tasks to finish this issue fully ?

I've had a look and this requires a few more changes.

It is a little over my head so will take a bit of time to figure out.
I've paused now because not obvious to me. Do you think worth looking into?

There are a couple of factors:

Our custom renderers don't accept the inbuilt JsonForms translation functionality
We stringify our patient schema in rust. If we are passing into rust, might as well translate there now rather than converting then re translating on the front end? If I am understanding how we manage the patient schema correctly

fergie-nz · 2024-12-02T21:23:58Z

client/packages/system/src/Report/api/hooks/useReport.ts

+    if (result.report.__typename == 'ReportNode') {
+      return result.report;
+    } else {
+      error(t('report.error-translating'))();
+      throw new Error('Could not translate report');
+    }


Right now error handling is just a toast. But I thought good to have this in to distinguish between repository errors and translation errors from users' perspective.

We could handle errors in a more structured way, but I am thinking it is not necessary now.

Can you please map as per:

open-msupply/client/packages/common/src/authentication/hooks/useUpdateUserInfo.ts

Lines 58 to 70 in aae44ff

switch (update.error.__typename) {

case 'ConnectionError':

setError(t('error.connection-error'));

break;

case 'InvalidCredentials':

setError(t('error.invalid-credentials'));

break;

case 'MissingCredentials':

setError(t('error.invalid-credentials'));

break;

default:

noOtherVariants(update.error);

}

and log the full result (console.error would be ok)

fergie-nz · 2024-12-02T21:24:43Z

server/graphql/core/src/simple_generic_errors.rs

+
+pub struct FailedTranslation;
+#[Object]
+impl FailedTranslation {
+    pub async fn description(&self) -> &'static str {
+        "Translation failed."
+    }
+}


I have added this as a generic error for future use case of translating any json.

fergie-nz · 2024-12-02T21:58:44Z

server/service/src/json_translate.rs

+                        key: key.to_string(),
+                    },
+                    user_language,
+                )?


Translation won't fail here on missing or incorrect translation keys, as we are providing a fallback.

server/service/src/report/report_service.rs

andreievg · 2024-12-15T22:48:36Z

Report-argument-modal-translation...Report-argument-modal-translation(Use-translate-report-arugment-schema-method).

Keen to have a quick catch up

andreievg · 2024-12-15T23:26:37Z

Report-argument-modal-translation...Report-argument-modal-translation(Use-translate-report-arugment-schema-method).

With this one, I put the translate_report_argument_schema method in the wrong place should be in lib.rs under report not in root of service

andreievg

A few little fix up after our chat, and should be good to go

andreievg · 2024-12-15T22:03:52Z

server/service/src/json_translate.rs

+
+const UNIQUE_TRANSLATE_KEY: &str = "T#";
+
+pub fn crawl_and_translate(


This is excellent, good use of existing standard library (strip_prefix), and I like the tests too, thanks

server/report_builder/README.md

andreievg · 2024-12-15T23:12:27Z

client/packages/system/src/Report/api/hooks/useReport.ts

+    if (result.report.__typename == 'ReportNode') {
+      return result.report;
+    } else {
+      error(t('report.error-translating'))();
+      throw new Error('Could not translate report');
+    }


Can you please map as per:

open-msupply/client/packages/common/src/authentication/hooks/useUpdateUserInfo.ts

Lines 58 to 70 in aae44ff

switch (update.error.__typename) {

case 'ConnectionError':

setError(t('error.connection-error'));

break;

case 'InvalidCredentials':

setError(t('error.invalid-credentials'));

break;

case 'MissingCredentials':

setError(t('error.invalid-credentials'));

break;

default:

noOtherVariants(update.error);

}

and log the full result (console.error would be ok)

And rebuild standard reports

fergie-nz · 2024-12-16T23:16:36Z

server/reports/expiring-items/2_3_0/argument_schemas/arguments_ui.json

@@ -57,7 +57,7 @@
    {
      "type": "Control",
      "scope": "#/properties/sort",
-      "label": "Sort by:",
+      "label": "T#report.sort-by",


I realise missed the 'Sort by', and 'Sort direction' keys and their translations - so have added these in also

fergie-nz · 2024-12-16T23:17:33Z

client/packages/system/src/Report/api/hooks/useReportList.ts

+          // TODO add never exhaustive error handling if adding more error types to QueryReportError
+          // default:
+          //   noOtherVariants(query.reports.error.__typename);


Thought would be helpful to leave this here for if and when we add errors? Wasn't sure on this one

andreievg

Looks great, thanks @fergie-nz !

andreievg · 2024-12-17T03:13:05Z

client/packages/system/src/Report/api/hooks/useReport.ts

+              key: report.error.description,
+            });
+            break;
+          // TODO add never exhaustive error handling if adding more error types to QueryReportError


that's great thanks

fergie-nz added 5 commits November 20, 2024 11:29

Add basic translation implementation

678b44d

Add type checking of translation keys

62e0952

Updating docs

c751648

Also see same docs added to reports repo

Add memoized keys to prevent re running of Object.keys

855297f

Update docs to show both cases

438f69c

fergie-nz changed the base branch from develop to v2.4.0 November 19, 2024 23:36

github-actions bot added this to the v2.4.0-RC2 milestone Nov 19, 2024

github-actions bot added enhancement New feature or request Team Ruru 🦉 Roxy, Ferg, Noel feature: reports labels Nov 19, 2024

fergie-nz commented Nov 19, 2024

View reviewed changes

fergie-nz marked this pull request as draft November 20, 2024 00:09

Maintain default label when I18n not provided

26809a7

roxy-dao assigned andreievg Nov 20, 2024

roxy-dao modified the milestones: v2.4.0-RC2, V2.4.0-RC3 Nov 20, 2024

fergie-nz marked this pull request as ready for review November 21, 2024 01:11

roxy-dao modified the milestones: V2.4.0-RC3, V2.4.0-RC4 Nov 22, 2024

fergie-nz requested a review from andreievg November 27, 2024 00:59

roxy-dao modified the milestones: V2.4.0-RC4, V2.4.0-RC5 Nov 27, 2024

Revert front end changes

fe906e3

roxy-dao removed this from the V2.4.0-RC5 milestone Nov 28, 2024

Add error handling for single report query

eb0e0ad

fergie-nz commented Dec 2, 2024

View reviewed changes

fergie-nz added 3 commits December 3, 2024 10:38

Remove formatting changes and fix tests

c466bcf

Update readme

ecbea1b

Add code block comments

b1c0b90

fergie-nz commented Dec 2, 2024

View reviewed changes

server/service/src/report/report_service.rs Outdated Show resolved Hide resolved

Remove comments

aae44ff

roxy-dao mentioned this pull request Dec 4, 2024

patient creation modal doesn't match the patient details form #5702

Open

fergie-nz mentioned this pull request Dec 5, 2024

5659 CIV 2.5 emergency order configs #5708

Merged

8 tasks

Use translate_report_arugment_schema method

8d5fc1c

andreievg reviewed Dec 15, 2024

View reviewed changes

fergie-nz added 6 commits December 16, 2024 12:47

Update readme to avoid confusion

30e6014

Move translate report argument schema into utils

8855e82

add error mapping

143003d

Merge branch 'develop' into Report-argument-modal-translation

d5f787d

Remove merge conflict duplicate logic

402bda3

Add sort label translations

6efe1f3

And rebuild standard reports

fergie-nz commented Dec 16, 2024

View reviewed changes

fergie-nz requested a review from andreievg December 16, 2024 23:17

andreievg approved these changes Dec 17, 2024

View reviewed changes

fergie-nz added 2 commits December 18, 2024 09:20

Merge branch 'develop' into Report-argument-modal-translation

7bb63c3

Rebuild standard reports

b798cb0

fergie-nz merged commit e17fd11 into develop Dec 17, 2024
6 checks passed

fergie-nz deleted the Report-argument-modal-translation branch December 17, 2024 20:30

fergie-nz mentioned this pull request Feb 18, 2025

6531 update standard report filter dropdown translation keys #6538

Open

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Report argument modal translation #5483

Report argument modal translation #5483

fergie-nz commented Nov 19, 2024

fergie-nz commented Nov 19, 2024

fergie-nz Nov 19, 2024

andreievg commented Nov 19, 2024

fergie-nz commented Nov 20, 2024

fergie-nz commented Nov 20, 2024

github-actions bot commented Nov 20, 2024 •

edited

Loading

fergie-nz commented Nov 20, 2024

fergie-nz Dec 2, 2024 •

edited

Loading

andreievg Dec 15, 2024

fergie-nz Dec 2, 2024

fergie-nz Dec 2, 2024

andreievg commented Dec 15, 2024

andreievg commented Dec 15, 2024

andreievg left a comment

andreievg Dec 15, 2024

andreievg Dec 15, 2024

fergie-nz Dec 16, 2024

fergie-nz Dec 16, 2024

andreievg left a comment

andreievg Dec 17, 2024

	switch (update.error.__typename) {
	case 'ConnectionError':
	setError(t('error.connection-error'));
	break;
	case 'InvalidCredentials':
	setError(t('error.invalid-credentials'));
	break;
	case 'MissingCredentials':
	setError(t('error.invalid-credentials'));
	break;
	default:
	noOtherVariants(update.error);
	}


		const UNIQUE_TRANSLATE_KEY: &str = "T#";

		pub fn crawl_and_translate(

Report argument modal translation #5483

Report argument modal translation #5483

Conversation

fergie-nz commented Nov 19, 2024

👩🏻‍💻 What does this PR do?

💌 Any notes for the reviewer?

🧪 Testing

📃 Documentation

fergie-nz commented Nov 19, 2024

Choose a reason for hiding this comment

andreievg commented Nov 19, 2024

fergie-nz commented Nov 20, 2024

fergie-nz commented Nov 20, 2024

github-actions bot commented Nov 20, 2024 • edited Loading

Bundle size difference

fergie-nz commented Nov 20, 2024

fergie-nz Dec 2, 2024 • edited Loading

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

andreievg commented Dec 15, 2024

andreievg commented Dec 15, 2024

andreievg 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

andreievg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 20, 2024 •

edited

Loading

fergie-nz Dec 2, 2024 •

edited

Loading