Adds support for localized push notification in push payload #4129

flovilmart · 2017-09-01T18:02:14Z

passign alert-[lang|locale] or title-[lang|locale] will inject the
proper locale on the push body based on the installation

ex:

{
   where: {...} // your query,
   data: {
     alert: 'Default alert',
     alert-fr: 'Alerte en Francais',
     ...
   }
}

I'll need to update the docs accordingly, with that feature, we'll be able to restore those in the dashboard as well, being able to add localized alerts etc...

- passign alert-[lang|locale] or title-[lang|locale] will inject the proper locale on the push body based on the installation

codecov · 2017-09-01T18:14:16Z

Codecov Report

Merging #4129 into master will increase coverage by 0.02%.
The diff coverage is 98.07%.

@@            Coverage Diff             @@
##           master    #4129      +/-   ##
==========================================
+ Coverage   90.79%   90.81%   +0.02%     
==========================================
  Files         116      116              
  Lines        7941     7993      +52     
==========================================
+ Hits         7210     7259      +49     
- Misses        731      734       +3

Impacted Files	Coverage Δ
src/Routers/PushRouter.js	`96.15% <ø> (ø)`	⬆️
src/Push/PushWorker.js	`93.54% <100%> (+1.09%)`	⬆️
src/Push/utils.js	`98.36% <97.67%> (-1.64%)`	⬇️
src/RestWrite.js	`92.81% <0%> (-0.38%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 540daa4...4887f88. Read the comment docs.

acinader

lgtm. some optional nits for you.

acinader · 2017-09-01T18:16:50Z

src/Controllers/PushController.js

@@ -61,7 +61,7 @@ export class PushController {
    }).catch((err) => {
      return pushStatus.fail(err).then(() => {
        throw err;
-      });


could revert this.

acinader · 2017-09-01T18:33:47Z

src/Push/PushWorker.js

-    map[badge].push(installation);
+function groupBy(key, objects) {
+  return objects.reduce((map, object) => {
+    const value = object[key] + '';


toString() ?

actually I should revert that.

acinader · 2017-09-01T18:34:47Z

src/Push/utils.js

+  if (!data) {
+    return [];
+  }
+  return [...new Set(Object.keys(data).reduce((memo, key) => {


acinader · 2017-09-01T18:36:27Z

src/Routers/PushRouter.js

@@ -28,6 +28,8 @@ export class PushRouter extends PromiseRouter {
          result: true
        }
      });
+    }).catch((err) => {
+      req.config.loggerController.error(err);
    });


could just .catch(req.config.loggerController.error)?

acinader

found a typo in a comment.

acinader · 2017-09-01T18:57:09Z

spec/PushWorker.spec.js

+      });
+    });
+
+    it('should properly handle defaut cases', () => {


defaut is a typo. Should be default.

good catch!

acinader · 2017-09-01T18:58:05Z

src/Push/utils.js

+      if (added) {
+        return;
+      }
+      if (installation.localeIdentifier && installation.localeIdentifier.indexOf(locale) == 0) {


flovilmart · 2017-09-01T19:21:58Z

Thanks for the quick review @acinader !

julianvogels · 2017-12-12T16:34:47Z

Is this somehow compatible with the "notification" field in the FCM/Android payload? I'd like to easily send localized push notifications to that platform as well.

parse-community/parse-server-push-adapter#41

flovilmart · 2017-12-12T17:06:32Z

Yes, it’s 100% compatible with android.

Adds support for localized push data keys

9e8da4e

- passign alert-[lang|locale] or title-[lang|locale] will inject the proper locale on the push body based on the installation

flovilmart requested a review from acinader September 1, 2017 18:10

flovilmart added 2 commits September 1, 2017 14:20

Better handling of the default cases

e48b8e3

Updates changelog

e1b592a

acinader previously approved these changes Sep 1, 2017

View reviewed changes

nits

59322cf

flovilmart dismissed acinader’s stale review via 59322cf September 1, 2017 18:44

acinader suggested changes Sep 1, 2017

View reviewed changes

nits

4887f88

acinader approved these changes Sep 1, 2017

View reviewed changes

flovilmart merged commit 6df9447 into master Sep 1, 2017

flovilmart deleted the localized-push-notifications branch September 1, 2017 19:22

snyk-bot mentioned this pull request Jul 22, 2020

[Snyk] Upgrade apollo-server-express from 2.15.0 to 2.15.1 #6819

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds support for localized push notification in push payload #4129

Adds support for localized push notification in push payload #4129

flovilmart commented Sep 1, 2017 •

edited

Loading

codecov bot commented Sep 1, 2017 •

edited

Loading

acinader left a comment

acinader Sep 1, 2017

acinader Sep 1, 2017

flovilmart Sep 1, 2017

acinader Sep 1, 2017

acinader Sep 1, 2017

flovilmart Sep 1, 2017

acinader left a comment

acinader Sep 1, 2017

flovilmart Sep 1, 2017

acinader Sep 1, 2017

flovilmart Sep 1, 2017

flovilmart commented Sep 1, 2017

julianvogels commented Dec 12, 2017

flovilmart commented Dec 12, 2017

Adds support for localized push notification in push payload #4129

Adds support for localized push notification in push payload #4129

Conversation

flovilmart commented Sep 1, 2017 • edited Loading

codecov bot commented Sep 1, 2017 • edited Loading

Codecov Report

acinader 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

acinader 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

flovilmart commented Sep 1, 2017

julianvogels commented Dec 12, 2017

flovilmart commented Dec 12, 2017

flovilmart commented Sep 1, 2017 •

edited

Loading

codecov bot commented Sep 1, 2017 •

edited

Loading