Issue 2707 - Remove start/end from notifications

sitescripts/notifications/parser.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 104 matching lines @@
       if name.startswith("./"):
         name = name[2:]
 
       if fileinfo.type == tarfile.REGTYPE:
         data = codecs.getreader("utf8")(archive.extractfile(fileinfo))
         try:
           notification = _parse_notification(data, name)
           if "inactive" in notification:
             continue
           current_time = datetime.datetime.now()
-          if "start" in notification:

[Felix Dahlke, 2015/07/29 10:45:11]

Not entirely sure on how to best change this. This makes sense logically, but I
wonder if it'd be more readable if we just remove "start" and "end" from all
notifications before `notifications.append(notification)` below. What do you
think?

[Sebastian Noack, 2015/07/29 11:28:54]

This wouldn't be the same. With this patch you only remove "start" and "end" if
they don't validate against the current time. If you remove them unconditionally
below you would always remove them.

If the latter is the intended behavior I'd go with following code:

  start = notifications.pop("start", None) 
  if start is not None and current_time < start:
    continue

[Felix Dahlke, 2015/07/29 14:30:17]

Yeah it's fine to always remove them. I'll do it the way you suggested, seems to
be the most readable alternative.

-            if current_time < notification["start"]:
-              continue
-            notification.pop("start", None)
-          if "end" in notification:
-            if current_time > notification["end"]:
-              continue
-            notification.pop("end", None)
+          start = notification.pop("start", None)
+          if start is not None and current_time < start:
+            continue
+          end = notification.pop("end", None)
+          if end is not None and current_time > end:
+            continue
           notifications.append(notification)
         except:
           traceback.print_exc()
   return notifications