[MM-17494] Fix plugin translations #3536

iomodo · 2019-08-27T07:56:47Z

Summary

This PR will fix plugin translations issues (translation files were not updating on the web-client when plugins were upgraded)

Ticket Link

MM-17494

lieut-data

A few thoughts, but nothing blocking. Nice investigation!

lieut-data · 2019-08-27T19:54:48Z

actions/views/root.js

+        if (immutableTranslations) {
+            copyAndDispatchTranslations(dispatch, translations, sourceFunction(locale), locale);
+        }
+    };
 }

 export function unregisterPluginTranslationsSource(pluginId) {


Is there any reason to dispatch an event on unregistration too?

I don't think so. We unregister translations on plugin removal, that event is dispatched.

hmm i think we should no? otherwise the plugin translation will remain in memory if i'm not mistaken.

lieut-data · 2019-08-27T19:55:30Z

actions/views/root.js

@@ -30,6 +31,16 @@ const pluginTranslationSources = {};

 export function registerPluginTranslationsSource(pluginId, sourceFunction) {
    pluginTranslationSources[pluginId] = sourceFunction;


Likely out of scope for this PR, but I'm wondering if this mapping should just live in Redux directly, and copyAndDispatchTranslations gets moved into a reducer.

makes sense

lieut-data · 2019-08-27T19:55:51Z

actions/views/root.js

+            translations,
+        },
+    });
+}


nit: missing newline here, suggest configuring http://editorconfig.com/

mattermod · 2019-08-30T07:03:54Z

@iomodo
Error trying doing the automated Cherry picking. Please do this manually

+++ Updating remotes...
Fetching upstream
Fetching origin
+++ Updating remotes done...
+++ Creating local branch automated-cherry-pick-of-#3536-upstream-release-5.15-1567148633
Branch 'automated-cherry-pick-of-#3536-upstream-release-5.15-1567148633' set up to track remote branch 'release-5.15' from 'upstream'.
+++ Downloading patch to /tmp/3536.patch (in case you need to do this again)

+++ About to attempt cherry pick of PR. To reattempt:
  $ git am -3 /tmp/3536.patch

Applying: Fix plugin translations
Applying: "Unregister plugin translations when removed"
Using index info to reconstruct a base tree...
M	actions/global_actions.jsx
M	plugins/index.js
Falling back to patching base and 3-way merge...
Auto-merging plugins/index.js
CONFLICT (content): Merge conflict in plugins/index.js
Auto-merging actions/global_actions.jsx
Patch failed at 0002 "Unregister plugin translations when removed"
Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

+++ Conflicts detected:

UU plugins/index.js

+++ Aborting in-progress git am.

+++ Returning you to the master branch and cleaning up.

lindalumitchell · 2019-08-31T00:19:06Z

@iomodo @lieut-data @crspeller looks like this one needs some help with its cherry-picking?

* Fix plugin translations * "Unregister plugin translations when plugin is removed" * Fix lint

lieut-data · 2019-08-31T04:13:06Z

I've manually cherry picked this to v5.15.

alifarooq0 · 2019-09-03T13:25:57Z

actions/views/root.js

-                    translations,
-                },
-            });
+            copyAndDispatchTranslations(dispatch, translations, en, locale);


This definitely works, however it would've been nice if we had converted this into async/await and had just one dispatch in this function rather than dispatching twice as per two different code paths.

... set variables for 'en' or set variables when fetching through 'Client4' ... copyAndDispatchTranslations(dispatch, translations, serverTranslations, locale);

@ali-farooq0, is this something we might yet still be able to squeeze in v5.15? Or is it an internal detail that we can safely improve in v5.16?

@lieut-data nah, just an internal detail that we can improve for 5.16. No need to rush it for 5.15.

https://mattermost.atlassian.net/browse/MM-18239 to keep track

mattermod · 2019-09-13T02:43:49Z

Test server destroyed

* Fix plugin translations * "Unregister plugin translations when plugin is removed" * Fix lint

iomodo added 4 commits August 27, 2019 01:12

Fix plugin translations

7a366d0

Merge branch 'master' into MM-17518-plugin-translations

145bcc0

"Unregister plugin translations when removed"

adbf4f9

Fix lint

42f3cbc

iomodo added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester CherryPick/Approved Meant for the quality or patch release tracked in the milestone labels Aug 27, 2019

iomodo added this to the v5.15.0 milestone Aug 27, 2019

iomodo requested review from lieut-data and crspeller August 27, 2019 07:57

iomodo changed the title ~~[MM-17518] Fix plugin translations~~ [MM-17494] Fix plugin translations Aug 27, 2019

crspeller approved these changes Aug 27, 2019

View reviewed changes

lieut-data approved these changes Aug 27, 2019

View reviewed changes

lieut-data removed the 2: Dev Review Requires review by a core commiter label Aug 27, 2019

lindalumitchell removed the 3: QA Review Requires review by a QA tester label Aug 30, 2019

iomodo merged commit 400fea6 into master Aug 30, 2019

iomodo deleted the MM-17518-plugin-translations branch August 30, 2019 07:03

lieut-data pushed a commit that referenced this pull request Aug 31, 2019

[MM-17494] Fix plugin translations (#3536)

18077dc

* Fix plugin translations * "Unregister plugin translations when plugin is removed" * Fix lint

lieut-data added CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone and removed CherryPick/Approved Meant for the quality or patch release tracked in the milestone labels Aug 31, 2019

alifarooq0 reviewed Sep 3, 2019

View reviewed changes

amyblais added Changelog/Done Required changelog entry has been written Docs/Not Needed Does not require documentation labels Sep 3, 2019

prapti added Setup Cloud Test Server Setup a test server using Mattermost Cloud and removed Setup Cloud Test Server Setup a test server using Mattermost Cloud labels Sep 13, 2019

mattermod mentioned this pull request Oct 1, 2019

Refactor actions/views/root.js::loadTranslations to have a single point of return mattermost/mattermost#12515

Closed

skheria pushed a commit to uber-archive/mattermost-webapp that referenced this pull request Oct 3, 2019

[MM-17494] Fix plugin translations (mattermost#3536)

8b3bbbf

* Fix plugin translations * "Unregister plugin translations when plugin is removed" * Fix lint

skheria pushed a commit to uber-archive/mattermost-webapp that referenced this pull request Oct 3, 2019

[MM-17494] Fix plugin translations (mattermost#3536)

37ca67e

* Fix plugin translations * "Unregister plugin translations when plugin is removed" * Fix lint

hanzei added the 4: Reviews Complete All reviewers have approved the pull request label Jan 13, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MM-17494] Fix plugin translations #3536

[MM-17494] Fix plugin translations #3536

iomodo commented Aug 27, 2019 •

edited

lieut-data left a comment

lieut-data Aug 27, 2019

iomodo Aug 29, 2019

alifarooq0 Sep 3, 2019

lieut-data Aug 27, 2019

iomodo Aug 29, 2019

lieut-data Aug 27, 2019

mattermod commented Aug 30, 2019

lindalumitchell commented Aug 31, 2019

lieut-data commented Aug 31, 2019

alifarooq0 Sep 3, 2019

lieut-data Sep 4, 2019

alifarooq0 Sep 4, 2019

alifarooq0 Sep 4, 2019

mattermod commented Sep 13, 2019

		@@ -30,6 +31,16 @@ const pluginTranslationSources = {};

		export function registerPluginTranslationsSource(pluginId, sourceFunction) {
		pluginTranslationSources[pluginId] = sourceFunction;

[MM-17494] Fix plugin translations #3536

[MM-17494] Fix plugin translations #3536

Conversation

iomodo commented Aug 27, 2019 • edited

Summary

Ticket Link

lieut-data 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

mattermod commented Aug 30, 2019

lindalumitchell commented Aug 31, 2019

lieut-data commented Aug 31, 2019

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

mattermod commented Sep 13, 2019

iomodo commented Aug 27, 2019 •

edited