Issue 5459 - Add support to show a notification based on the number of ads blocked

lib/notification.js

Index: lib/notification.js
diff --git a/lib/notification.js b/lib/notification.js
index 311e4e881389424155f1a61a3c351a3f69cfaf88..a453edc1d67c6f07a8b793b65ba57b643a69c247 100644
--- a/lib/notification.js
+++ b/lib/notification.js
@@ -200,17 +200,6 @@ let Notification = exports.Notification =
    */
   _getNextToShow(url)
   {
-    function checkTarget(target, parameter, name, version)
-    {
-      let minVersionKey = parameter + "MinVersion";
-      let maxVersionKey = parameter + "MaxVersion";
-      return !((parameter in target && target[parameter] != name) ||
-               (minVersionKey in target &&
-                Services.vc.compare(version, target[minVersionKey]) < 0) ||
-               (maxVersionKey in target &&
-                Services.vc.compare(version, target[maxVersionKey]) > 0));
-    }
-
     let remoteData = [];
     if (typeof Prefs.notificationdata.data == "object" &&
         Prefs.notificationdata.data.notifications instanceof Array)
@@ -224,6 +213,28 @@ let Notification = exports.Notification =
 
     const {addonName, addonVersion, application,
            applicationVersion, platform, platformVersion} = require("info");
+
+    let targetChecks = {
+      extension: v => v == addonName,
+      extensionMinVersion:
+       v => Services.vc.compare(addonVersion, v) >= 0,

[Wladimir Palant, 2017/08/23 11:00:10]

Here and below: we shouldn't have one-space indentations, these aren't obvious
enough. It should always be at least two spaces.

[wspee, 2017/08/24 16:39:22]

Acknowledged.

+      extensionMaxVersion:
+       v => Services.vc.compare(addonVersion, v) <= 0,
+      application: v => v == application,
+      applicationMinVersion:
+       v => Services.vc.compare(applicationVersion, v) >= 0,
+      applicationMaxVersion:
+       v => Services.vc.compare(applicationVersion, v) <= 0,
+      platform: v => v == platform,
+      platformMinVersion:
+       v => Services.vc.compare(platformVersion, v) >= 0,
+      platformMaxVersion:
+       v => Services.vc.compare(platformVersion, v) <= 0,
+      blockedTotalMin: v => Prefs.blocked_total >= v,
+      blockedTotalMax: v => Prefs.blocked_total <= v,
+      locales: v => v.indexOf(Utils.appLocale) != -1

[Wladimir Palant, 2017/08/23 11:00:10]

Sorry, overlooked a detail in the previous review. This should be
v.includes(Utils.appLocale)

[wspee, 2017/08/24 16:39:22]

Acknowledged.

+    };
+
     let notificationToShow = null;
     for (let notification of notifications)
     {
@@ -289,19 +300,22 @@ let Notification = exports.Notification =
       if (notification.targets instanceof Array)
       {
         let match = false;
+
         for (let target of notification.targets)
         {
-          if (checkTarget(target, "extension", addonName, addonVersion) &&
-              checkTarget(target, "application", application,
-                          applicationVersion) &&
-              checkTarget(target, "platform", platform, platformVersion))
+          match = false;

[Wladimir Palant, 2017/08/23 11:00:11]

This assignment is no longer necessary.

[wspee, 2017/08/24 16:39:22]

Acknowledged.

+          if (Object.keys(target).every(key =>
+              targetChecks.hasOwnProperty(key) &&
+              targetChecks[key](target[key])))
           {
             match = true;
             break;
           }
         }
         if (!match)
+        {
           continue;
+        }
       }
 
       if (!notificationToShow ||