-
Notifications
You must be signed in to change notification settings - Fork 2
[NAB-388] HTML code in i18String tag is sanitized on export #58
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
-export i18n as cdata section and import it same way
mazarijuraj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update test model petriflow_test.xml and 'should import & export' in petriflow.spec.js to test that i18n with html code is correctly imported and exported
-add cdata to test file to i18ns and add it to test
| </data> | ||
| <data type="text"> | ||
| <id>newVariable_22</id> | ||
| <title>HTML no translation</title> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deceptive title, this HTML area has translations ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
src/test/petriflow.spec.js
Outdated
| expect(i18nLocale.getI18n(i)).not.toBeUndefined(); | ||
| expect(i18nLocale.getI18n(i).value).toEqual(`${locale.toUpperCase()}_${i}`); | ||
| }); | ||
| expect(i18nLocale.getI18n(TEST_HTML).value).toEqual(`<h2>${locale.toUpperCase()}_Title</h2>`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better way would be to add another list as argument that would be ids of i18ns that should match not by .toEqaul() but by contains. This way you could simply put:
expect(i18nLocale.getI18ns().length).toEqual(i18ns_equal.length + i18ns_contain.length);
...
i18ns_equal.forEach( ... );
i18ns_contain.forEach( ... );
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
src/test/petriflow.spec.js
Outdated
| function assertI18n(locale, i18ns, model) { | ||
| const i18nLocale = model.getI18n(locale); | ||
| expect(i18nLocale.getI18ns().length).toEqual(i18ns.length); | ||
| expect(i18nLocale.getI18ns().length).toEqual(i18ns.length + 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This +1 has very poor readability. See the next comment for better solution
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
- fixes according to PR
Description
-export i18n as cdata section and import it same way
Fixes [NAB-388]
Dependencies
none
Third party dependencies
Blocking Pull requests
There are no dependencies on other PR
How Has Been This Tested?
manually
Test Configuration
<Please describe configuration for tests to run if applicable, like program parameters, host OS, VM configuration etc.>
Checklist: