Issue 5827 - Assign new groups even if prior groups are present

sitescripts/notifications/web/notification.py

 # This file is part of the Adblock Plus web scripts,
 # Copyright (C) 2006-present 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
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
@@ skipping 34 matching lines @@
         group = {'id': notification['id'], 'variant': 0}
         groups.append(group)
         for i, variant in enumerate(notification['variants']):
             sample_size = variant['sample']
             end = start + sample_size
             selected = sample_size > 0 and start <= selection <= end
             start = end
             if selected:
                 group['variant'] = i + 1
                 break
-    return groups

[Felix Dahlke, 2017/10/10 14:10:01]

Nit: The way this function works now, returning groups is redundant. It just
assigns the groups directly.

[Vasily Kuznetsov, 2017/10/10 14:17:18]

I agree. Also, if we make it return None, the fact that `groups` is modified in
place will be more obvious from the signature.

[wspee, 2017/10/10 14:46:49]

Done.

 
 
 def _get_active_variant(notifications, groups):
     for group in groups:
         group_id = group['id']
         variant = group['variant']
         if variant == 0:
             continue
         notification = next((x for x in notifications if x['id'] == group_id), None)
         if not notification:
@@ skipping 39 matching lines @@
     }
 
 
 @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)
     notifications = [x for x in notifications if not x.get('inactive', False)]
-    groups = _assign_groups(groups, notifications)
+    _assign_groups(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