Skip to content

[FIX] Translations were not unique per app allowing conflicts among apps#11878

Merged
rodrigok merged 3 commits intodevelopfrom
apps-unique-translations
Aug 27, 2018
Merged

[FIX] Translations were not unique per app allowing conflicts among apps#11878
rodrigok merged 3 commits intodevelopfrom
apps-unique-translations

Conversation

@rodrigok
Copy link
Copy Markdown
Member

@rodrigok rodrigok commented Aug 27, 2018

To prevent conflicts among apps translations we added a unique key for the apps strings changing the keys in runtime to add apps-${ id }- in front of them.

Example for an app with id my-app-id:
Registered key:
param_description
Will be registered internally as:
apps-my-app-id-param_description

The slashcomands will require app developers to provide the translation since it will not allow fallback to the i18n strings of the project, but the settings will try to find the translation first in the app and then in the project strings (using only the last part eg: param_description).

The app developer need to do nothing, it's just an internal change.

@rodrigok rodrigok added this to the 0.69.0 milestone Aug 27, 2018
@rodrigok rodrigok requested a review from graywolf336 August 27, 2018 17:14
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-11878 August 27, 2018 17:14 Inactive
@rodrigok rodrigok temporarily deployed to rocket-chat-pr-11878 August 27, 2018 17:17 Inactive
@kaiiiiiiiii
Copy link
Copy Markdown
Contributor

kaiiiiiiiii commented Aug 27, 2018

LGTM :)

Just one small thing:

When I first installed my new packed apps, the translations on the settings page were not rendered (auto redirect after install). A refresh fixed the problem. This behavior does not occur when I delete and reinstall the app. It must be a never before installed App version to reproduce (App 1 & 2).

screen shot 2018-08-27 at 19 52 59

App 1:
appearin_1.2.0.zip
App 2:
icanhazdadjoke_1.0.0.zip

@rodrigok
Copy link
Copy Markdown
Member Author

Thanks @kaiiiiiiiii it was fixed by my latest commit 1e6ecd2

Copy link
Copy Markdown
Contributor

@graywolf336 graywolf336 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 glad to see this technical debt finally erased

@rodrigok rodrigok merged commit 9317946 into develop Aug 27, 2018
@rodrigok rodrigok deleted the apps-unique-translations branch August 27, 2018 19:42
@sampaiodiego sampaiodiego mentioned this pull request Aug 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants