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

lib/notification.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2017 eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 182 matching lines @@
       showListeners.splice(index, 1);
   },
 
   /**
    * Determines which notification is to be shown next.
    * @param {string} url URL to match notifications to (optional)
    * @return {Object} notification to be shown, or null if there is none
    */
   _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)
     {
       remoteData = Prefs.notificationdata.data.notifications;
     }
 
     let notifications = localData.concat(remoteData);
     if (notifications.length === 0)
       return null;
 
     const {addonName, addonVersion, application,
            applicationVersion, platform, platformVersion} = require("info");
+
+    let targetChecks = {
+      extension: v => v == addonName,
+      extensionMinVersion:
+        v => Services.vc.compare(addonVersion, v) >= 0,
+      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.show_statsinpopup &&
+        Prefs.blocked_total >= v,
+      blockedTotalMax: v => Prefs.show_statsinpopup &&
+        Prefs.blocked_total <= v,
+      locales: v => v.includes(Utils.appLocale)
+    };
+
     let notificationToShow = null;
     for (let notification of notifications)
     {
       if (typeof notification.type === "undefined" ||
           notification.type !== "critical")
       {
         let shown;
         if (typeof Prefs.notificationdata.shown == "object")
           shown = Prefs.notificationdata.shown[notification.id];
 
@@ skipping 45 matching lines @@
             continue;
           }
         }
         else
           continue;
       }
 
       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))
+          if (Object.keys(target).every(key =>
+              targetChecks.hasOwnProperty(key) &&
+              targetChecks[key](target[key])))
           {
             match = true;
             break;
           }
         }
         if (!match)
+        {
           continue;
+        }
       }
-
-      if (typeof notification.blocked_total_min == 'number' &&
-          (typeof Prefs.blocked_total != 'number' ||
-          notification.blocked_total_min > Prefs.blocked_total))
-        continue

[Wladimir Palant, 2017/08/15 10:31:21]

Nit: please run ESLint. There should be a semicolon at the end of this
statement. Also, we usually require brackets around the body even if it is the
condition that is spanning multiple lines - ESLint cannot flag that. Yes, I am
aware that this rule already isn't being followed for the block above already.

[wspee, 2017/08/21 15:18:21]

Of course, sorry forgot to use ESLint.

-
-      if (typeof notification.blocked_total_max == 'number' &&
-          (typeof Prefs.blocked_total != 'number' ||
-          notification.blocked_total_max < Prefs.blocked_total))
-        continue

[Wladimir Palant, 2017/08/15 10:31:21]

We should use camelCase for consistency. Also, we should expect structured data,
something like:

  blockedTotal: {min: 0, max: 4000}

I just discussed this with Felix, and the conclusion is that this condition
belongs into the targets array, it shouldn't be there on its own.

[wspee, 2017/08/21 15:18:21]

I implemented the syntax from the issue (#5457), then it's also consistent with
extensionVersion, etc..

 
       if (!notificationToShow ||
           getNumericalSeverity(notification) >
             getNumericalSeverity(notificationToShow))
         notificationToShow = notification;
     }
 
     return notificationToShow;
   },
 
@@ skipping 150 matching lines @@
     else if (index != -1 && forceValue !== true)
       categories.splice(index, 1);
 
     // HACK: JSON values aren't saved unless they are assigned a
     // different object.
     Prefs.notifications_ignoredcategories =
       JSON.parse(JSON.stringify(categories));
   }
 };
 Notification.init();