Issue 2982 - Return all notifications that have title and message

sitescripts/notifications/web/notification.py

 # coding: utf-8
 
 # This file is part of the Adblock Plus web scripts,
 # Copyright (C) 2006-2015 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,
@@ skipping 50 matching lines @@
     variant = group["variant"]
     if variant == 0:
       continue
     notification = next(x for x in notifications if x["id"] == group_id)
     notification = copy.deepcopy(notification)
     notification.update(notification["variants"][variant - 1])
     for key_to_remove in ("sample", "variants"):
       notification.pop(key_to_remove, None)
     return notification
 
+def _can_be_shown(notification):
+  return "title" in notification and "en-US" in notification["title"] and \

[Sebastian Noack, 2015/09/04 11:38:12]

Please don't repeat yourself. How about about:

  return all("en-US" in notification.get(x, []) for x in ("title, "message"))

[Sebastian Noack, 2015/09/04 11:47:36]

I just realized: What is if we show a notification only to users of a particular
language in the future? It wouldn't make sense to require an English translation
then.

So probably we should check whether title and message are given in all targeted
languages, if any. Only requiring the default language if there are no targeted
languages or if not all of them have translations.

[Felix Dahlke, 2015/09/07 07:54:36]

Well, we don't support that yet, so I can't really check for that. Still, I
think I'm already going too far in duplicating the language fallback logic here.
This wasn't meant as a means of checking notifications for validity, it was
meant to check whether a notification _could_ be shown. So I'm just checking for
the presence of title and message now, that should suffice.

+    "message" in notification and "en-US" in notification["message"]
+
 def _generate_version(groups):
   version = time.strftime("%Y%m%d%H%M", time.gmtime())
   for group in groups:
     version += "-%s/%s" % (group["id"], group["variant"])
   return version
 
-def _create_response(notifications, groups):
+def _get_notifications_to_send(notifications, groups):
   active_variant = _get_active_variant(notifications, groups)
   if active_variant:
-    notifications = [active_variant]
-  else:
-    notifications = [x for x in notifications if "variants" not in x]
-  response = {
+    return _can_be_shown(active_variant) and [active_variant] or []

[Wladimir Palant, 2015/09/04 11:17:04]

Yay on obfuscation!

  return [active_variant] if _can_be_shown(active_variant) else []

[Felix Dahlke, 2015/09/07 07:54:36]

Well, it was the old way to do it, but I really have to drop that habit one of
these days :P Done.

+
+  notifications_to_send = []
+  for notification in notifications:
+    if not _can_be_shown(notification):
+      continue
+    if "variants" in notification:
+      notification = copy.deepcopy(notification)
+      notification.pop("variants", None)

[Wladimir Palant, 2015/09/04 11:17:04]

Second parameter is unnecessary here, you already know that "variants" is in
there.

[Sebastian Noack, 2015/09/04 11:38:12]

If this is true, the del statement would be more appropriate to use here.

[Felix Dahlke, 2015/09/07 07:54:36]

Done.

+    notifications_to_send.append(notification)
+  return notifications_to_send
+
+def _create_response(notifications, groups):
+  return {
     "version": _generate_version(groups),
-    "notifications": notifications
+    "notifications": _get_notifications_to_send(notifications, groups)
   }
-  return response
 
 @url_handler("/notification.json")
 def notification(environ, start_response):
   params = urlparse.parse_qs(environ.get("QUERY_STRING", ""))
   version = params.get("lastVersion", [""])[0]
   notifications = load_notifications()
   groups = _determine_groups(version, notifications)
   if not groups:
     groups = _assign_groups(notifications)
   response = _create_response(notifications, groups)
   response_headers = [("Content-Type", "application/json; charset=utf-8"),
                       ("ABP-Notification-Version", response["version"])]
   response_body = json.dumps(response, ensure_ascii=False, indent=2,
                              separators=(",", ": "),
                              sort_keys=True).encode("utf-8")
   start_response("200 OK", response_headers)
   return response_body