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

sitescripts/notifications/test/notification.py

Index: sitescripts/notifications/test/notification.py
diff --git a/sitescripts/notifications/test/notification.py b/sitescripts/notifications/test/notification.py
index e566ceb27979a59670022da7baf8bf346cf7da5e..6cf349c5c65b75c33950c9ca6e28ebe56ae22b17 100644
--- a/sitescripts/notifications/test/notification.py
+++ b/sitescripts/notifications/test/notification.py
@@ -13,253 +13,273 @@
 # You should have received a copy of the GNU General Public License
 # along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
 
+import os
 import json
 import mock
 import unittest
+import tempfile
+import shutil
+import time
+import subprocess
+
+from sitescripts.utils import get_config
 
 import sitescripts.notifications.web.notification as notification
 
+mock_time = mock.Mock()
+mock_time.return_value = time.gmtime(0)
+
 
 class TestNotification(unittest.TestCase):
     def setUp(self):
-        self.load_notifications_patcher = mock.patch('sitescripts.notifications.web.notification.load_notifications')
-        self.load_notifications_mock = self.load_notifications_patcher.start()
+        self.repo_dir = tempfile.mkdtemp('md_repo')

[wspee, 2017/10/09 09:12:03]

As discussed: This created a new repository for every test. This make sense in
terms of test isolation but it means that this is quite slow (whole suit ~8s). I
explored different options to make it faster (hglib, copy repo instead of
creating a new one) but the main performance decrease comes from making commits
so I have abandoned the idea.

[Vasily Kuznetsov, 2017/10/09 22:29:11]

Hm. Now after I looked at the whole test suite it seems like mocking
`load_notifications` out in this file is the right approach, since we have a
whole separate set of tests for it in `parser.py` (and those tests actually use
a pretty cool way to quickly simulate `hg`, which I will neither endorse nor
disavow). So if there's something that `load_notifications` fails to do, we
should add a test for that to `parser.py`, not to here. Does this make sense?

[wspee, 2017/10/10 09:03:53]

No worries. Half of it was based on my assumptions anyways :). I see your point.
Done.

+        subprocess.check_call(['hg', 'init'], cwd=self.repo_dir)
+
+        open(os.path.join(self.repo_dir, '.hgignore'), 'w+').close()
+
+        subprocess.check_call(
+            ['hg', 'commit', '-q', '-m "Initial commit"', '-A'],
+            cwd=self.repo_dir)
+
+        config = get_config()
+        self._notification_repository_orig = config.get(
+            'notifications', 'repository')
+        get_config().set('notifications', 'repository', self.repo_dir)
 
     def tearDown(self):
-        self.load_notifications_patcher.stop()
+        shutil.rmtree(self.repo_dir)
+        get_config().set(
+            'notifications', 'repository', self._notification_repository_orig)
+
+    def add_notifications(self, notifications):
+        for id_, content in notifications:
+            with open(os.path.join(self.repo_dir, id_), 'w+') as f:
+                f.write(content)
+                f.flush()
+
+            subprocess.check_call(
+                ['hg', 'commit', '-q', '-m', id_, '-A'],
+                cwd=self.repo_dir)
 
     def test_no_group(self):
-        self.load_notifications_mock.return_value = [
-            {'id': '1', 'title': {'en-US': ''}, 'message': {'en-US': ''}}
-        ]
+        self.add_notifications([
+            ('1', 'title.en-US = 1\nmessage.en-US = 1\n')])
+
         result = json.loads(notification.notification({}, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
         self.assertEqual(result['notifications'][0]['id'], '1')
         self.assertFalse('-' in result['version'])
 
     def test_not_in_group(self):
-        self.load_notifications_mock.return_value = [
-            {'id': '1', 'title': {'en-US': ''}, 'message': {'en-US': ''}},
-            {'id': 'a', 'variants': [
-                {'title': {'en-US': ''}, 'message': {'en-US': ''}}
-            ]}
-        ]
+        self.add_notifications([
+            ('1', 'title.en-US = 1\nmessage.en-US = 1\n'),
+            ('2', '[1]\ntitle.en-US = 2\nmessage.en-US = 2\nsample = 1'),
+        ])
+
         result = json.loads(notification.notification({
-            'QUERY_STRING': 'lastVersion=197001010000-a/0'
+            'QUERY_STRING': 'lastVersion=197001010000-2/0'
         }, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
         self.assertEqual(result['notifications'][0]['id'], '1')
-        self.assertRegexpMatches(result['version'], r'-a/0')
+        self.assertRegexpMatches(result['version'], r'-2/0')
 
     def test_in_group(self):
-        self.load_notifications_mock.return_value = [
-            {'id': '1', 'title': {'en-US': ''}, 'message': {'en-US': ''}},
-            {'id': 'a', 'variants': [
-                {'title': {'en-US': ''}, 'message': {'en-US': ''}}
-            ]}
-        ]
+        self.add_notifications([
+            ('1', 'title.en-US = 1\nmessage.en-US = 1\n'),
+            ('2', '[1]\ntitle.en-US = 2.1\nmessage.en-US = 2.1\nsample = 1'),
+        ])
+
         result = json.loads(notification.notification({
-            'QUERY_STRING': 'lastVersion=197001010000-a/1'
+            'QUERY_STRING': 'lastVersion=197001010000-2/1'
         }, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['id'], 'a')
-        self.assertRegexpMatches(result['version'], r'-a/1')
+        self.assertEqual(result['notifications'][0]['id'], '2')
+        self.assertRegexpMatches(result['version'], r'-2/1')
 
     def test_not_in_one_of_many_groups(self):
-        self.load_notifications_mock.return_value = [
-            {'id': '1', 'title': {'en-US': ''}, 'message': {'en-US': ''}},
-            {'id': 'a', 'variants': [
-                {'title': {'en-US': ''}, 'message': {'en-US': ''}}
-            ]},
-            {'id': 'b', 'variants': [
-                {'title': {'en-US': ''}, 'message': {'en-US': ''}}
-            ]},
-            {'id': 'c', 'variants': [
-                {'title': {'en-US': ''}, 'message': {'en-US': ''}}
-            ]}
-        ]
+        self.add_notifications([
+            ('1', 'title.en-US = 1\nmessage.en-US = 1\n'),
+            ('2', '[1]\ntitle.en-US = 2.1\nmessage.en-US = 2.1\nsample = 1'),
+            ('3', '[1]\ntitle.en-US = 3.1\nmessage.en-US = 3.1\nsample = 1'),
+            ('4', '[1]\ntitle.en-US = 4.1\nmessage.en-US = 4.1\nsample = 1'),
+        ])
+
         result = json.loads(notification.notification({
-            'QUERY_STRING': 'lastVersion=197001010000-a/0-b/0-c/0'
+            'QUERY_STRING': 'lastVersion=197001010000-2/0-3/0-4/0'
         }, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
         self.assertEqual(result['notifications'][0]['id'], '1')
-        self.assertRegexpMatches(result['version'], r'-a/0-b/0-c/0')
+        self.assertRegexpMatches(result['version'], r'-2/0-3/0-4/0')
 
     def test_in_one_of_many_groups(self):
-        self.load_notifications_mock.return_value = [
-            {'id': '1', 'title': {'en-US': ''}, 'message': {'en-US': ''}},
-            {'id': 'a', 'variants': [
-                {'title': {'en-US': ''}, 'message': {'en-US': ''}}
-            ]},
-            {'id': 'b', 'variants': [
-                {'title': {'en-US': ''}, 'message': {'en-US': ''}}
-            ]},
-            {'id': 'c', 'variants': [
-                {'title': {'en-US': ''}, 'message': {'en-US': ''}}
-            ]}
-        ]
+        self.add_notifications([
+            ('1', 'title.en-US = 1\nmessage.en-US = 1\n'),
+            ('2', '[1]\ntitle.en-US = 2.1\nmessage.en-US = 2.1\nsample = 1'),
+            ('3', '[1]\ntitle.en-US = 3.1\nmessage.en-US = 3.1\nsample = 1'),
+            ('4', '[1]\ntitle.en-US = 4.1\nmessage.en-US = 4.1\nsample = 1'),
+        ])
+
         result = json.loads(notification.notification({
-            'QUERY_STRING': 'lastVersion=197001010000-a/0-b/1-c/0'
+            'QUERY_STRING': 'lastVersion=197001010000-2/0-3/1-4/0'
         }, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['id'], 'b')
-        self.assertRegexpMatches(result['version'], r'-a/0-b/1-c/0')
+        self.assertEqual(result['notifications'][0]['id'], '3')
+        self.assertRegexpMatches(result['version'], r'-2/0-3/1-4/0')
 
     def test_not_put_in_group(self):
-        self.load_notifications_mock.return_value = [
-            {'id': '1', 'title': {'en-US': ''}, 'message': {'en-US': ''}},
-            {'id': 'a', 'variants': [
-                {'sample': 0, 'title': {'en-US': ''}, 'message': {'en-US': ''}}
-            ]}
-        ]
+        self.add_notifications([

[Vasily Kuznetsov, 2017/10/09 22:29:11]

There is and was a lot of code duplication in this file. If you end up touching
these methods, it would probably be good to move those repeated data structures
out as constants or reuse them in some other way.

[wspee, 2017/10/10 09:03:53]

Acknowledged.

+            ('1', 'title.en-US = 1\nmessage.en-US = 1\n'),
+            ('2', '[1]\ntitle.en-US = 2.1\nmessage.en-US = 2.1\nsample = 0'),
+        ])
+
         result = json.loads(notification.notification({
             'QUERY_STRING': 'lastVersion=197001010000'
         }, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
         self.assertEqual(result['notifications'][0]['id'], '1')
-        self.assertRegexpMatches(result['version'], r'-a/0')
+        self.assertRegexpMatches(result['version'], r'-2/0')
 
     def test_put_in_group(self):
-        self.load_notifications_mock.return_value = [
-            {'id': '1', 'title': {'en-US': ''}, 'message': {'en-US': ''}},
-            {'id': 'a', 'variants': [
-                {'sample': 1, 'title': {'en-US': ''}, 'message': {'en-US': ''}}
-            ]}
-        ]
+        self.add_notifications([
+            ('1', 'title.en-US = 1\nmessage.en-US = 1\n'),
+            ('2', '[1]\ntitle.en-US = 2.1\nmessage.en-US = 2.1\nsample = 1'),
+        ])
+
         result = json.loads(notification.notification({
             'QUERY_STRING': 'lastVersion=197001010000'
         }, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['id'], 'a')
-        self.assertRegexpMatches(result['version'], r'-a/1')
-
-    def test_notification_variant_merged(self):

[wspee, 2017/10/09 09:12:03]

This "merging" is not possible with the notification parser so I had to remove
this test.

-        self.load_notifications_mock.return_value = [
-            {
-                'id': 'a',
-                'title': {'en-US': 'default'},
-                'message': {'en-US': 'default'},
-                'variants': [
-                    {'sample': 1, 'message': {'en-US': 'variant'}}
-                ]
-            }
-        ]
-        result = json.loads(notification.notification({}, lambda *args: None))
-        self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['id'], 'a')
-        self.assertEqual(result['notifications'][0]['title']['en-US'], 'default')
-        self.assertEqual(result['notifications'][0]['message']['en-US'], 'variant')
-        self.assertFalse('variants' in result['notifications'][0])
-        self.assertFalse('sample' in result['notifications'][0])
+        self.assertEqual(result['notifications'][0]['id'], '2')
+        self.assertRegexpMatches(result['version'], r'-2/1')
 
     def test_no_variant_no_notifications(self):
-        self.load_notifications_mock.return_value = [
-            {'id': 'a', 'variants': [{'sample': 0}]}
-        ]
+        self.add_notifications([
+            ('1', '[1]\ntitle.en-US = 1\nmessage.en-US = 1\nsample = 0'),
+        ])
+
         result = json.loads(notification.notification({}, lambda *args: None))
         self.assertEqual(len(result['notifications']), 0)
 
     @mock.patch('random.random')
     def test_probability_distribution_single_group(self, random_call):
-        self.load_notifications_mock.return_value = [
-            {
-                'id': 'a',
-                'variants': [
-                    {'sample': 0.5, 'title': {'en-US': '1'}, 'message': {'en-US': ''}},
-                    {'sample': 0.25, 'title': {'en-US': '2'}, 'message': {'en-US': ''}},
-                    {'sample': 0.25, 'title': {'en-US': '3'}, 'message': {'en-US': ''}}
-                ]
-            }
-        ]
+        self.add_notifications([
+            ('1', '[1]\n'
+                  'title.en-US = 1.1\n'
+                  'message.en-US = 1.1\n'
+                  'sample = 0.5\n'
+                  '[2]\n'
+                  'title.en-US = 1.2\n'
+                  'message.en-US = 1.2\n'
+                  'sample = 0.25\n'
+                  '[3]\n'
+                  'title.en-US = 1.3\n'
+                  'message.en-US = 1.3\n'
+                  'sample = 0.25\n')
+        ])
+
         random_call.return_value = 0
         result = json.loads(notification.notification({}, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['title']['en-US'], '1')
-        self.assertRegexpMatches(result['version'], r'-a/1')
+        self.assertEqual(result['notifications'][0]['title']['en-US'], '1.1')
+        self.assertRegexpMatches(result['version'], r'-1/1')
         random_call.return_value = 0.5
         result = json.loads(notification.notification({}, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['title']['en-US'], '1')
-        self.assertRegexpMatches(result['version'], r'-a/1')
+        self.assertEqual(result['notifications'][0]['title']['en-US'], '1.1')
+        self.assertRegexpMatches(result['version'], r'-1/1')
         random_call.return_value = 0.51
         result = json.loads(notification.notification({}, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['title']['en-US'], '2')
-        self.assertRegexpMatches(result['version'], r'-a/2')
+        self.assertEqual(result['notifications'][0]['title']['en-US'], '1.2')
+        self.assertRegexpMatches(result['version'], r'-1/2')
         random_call.return_value = 0.75
         result = json.loads(notification.notification({}, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['title']['en-US'], '2')
-        self.assertRegexpMatches(result['version'], r'-a/2')
+        self.assertEqual(result['notifications'][0]['title']['en-US'], '1.2')
+        self.assertRegexpMatches(result['version'], r'-1/2')
         random_call.return_value = 0.751
         result = json.loads(notification.notification({}, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['title']['en-US'], '3')
-        self.assertRegexpMatches(result['version'], r'-a/3')
+        self.assertEqual(result['notifications'][0]['title']['en-US'], '1.3')
+        self.assertRegexpMatches(result['version'], r'-1/3')
         random_call.return_value = 1
         result = json.loads(notification.notification({}, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['title']['en-US'], '3')
-        self.assertRegexpMatches(result['version'], r'-a/3')
+        self.assertEqual(result['notifications'][0]['title']['en-US'], '1.3')
+        self.assertRegexpMatches(result['version'], r'-1/3')
 
     @mock.patch('random.random')
     def test_probability_distribution_multiple_groups(self, random_call):
-        self.load_notifications_mock.return_value = [
-            {
-                'id': 'a',
-                'variants': [
-                    {'sample': 0.25, 'title': {'en-US': '1'}, 'message': {'en-US': ''}},
-                    {'sample': 0.25, 'title': {'en-US': '2'}, 'message': {'en-US': ''}}
-                ]
-            },
-            {
-                'id': 'b',
-                'variants': [
-                    {'sample': 0.25, 'title': {'en-US': '1'}, 'message': {'en-US': ''}},
-                    {'sample': 0.25, 'title': {'en-US': '2'}, 'message': {'en-US': ''}}
-                ]
-            }
-        ]
+        self.add_notifications([
+            ('1', '[1]\n'
+                  'title.en-US = 1.1\n'
+                  'message.en-US = 1.1\n'
+                  'sample = 0.25\n'
+                  '[2]\n'
+                  'title.en-US = 1.2\n'
+                  'message.en-US = 1.2\n'
+                  'sample = 0.25\n'),
+            ('2', '[1]\n'
+                  'title.en-US = 2.1\n'
+                  'message.en-US = 2.1\n'
+                  'sample = 0.25\n'
+                  '[2]\n'
+                  'title.en-US = 2.2\n'
+                  'message.en-US = 2.1\n'
+                  'sample = 0.25\n')
+        ])
+
         random_call.return_value = 0
         result = json.loads(notification.notification({}, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['id'], 'a')
-        self.assertEqual(result['notifications'][0]['title']['en-US'], '1')
-        self.assertRegexpMatches(result['version'], r'-a/1-b/0')
+        self.assertEqual(result['notifications'][0]['id'], '1')
+        self.assertEqual(result['notifications'][0]['title']['en-US'], '1.1')
+        self.assertRegexpMatches(result['version'], r'-1/1-2/0')
         random_call.return_value = 0.251
         result = json.loads(notification.notification({}, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['id'], 'a')
-        self.assertEqual(result['notifications'][0]['title']['en-US'], '2')
-        self.assertRegexpMatches(result['version'], r'-a/2-b/0')
+        self.assertEqual(result['notifications'][0]['id'], '1')
+        self.assertEqual(result['notifications'][0]['title']['en-US'], '1.2')
+        self.assertRegexpMatches(result['version'], r'-1/2-2/0')
         random_call.return_value = 0.51
         result = json.loads(notification.notification({}, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['id'], 'b')
-        self.assertEqual(result['notifications'][0]['title']['en-US'], '1')
-        self.assertRegexpMatches(result['version'], r'-a/0-b/1')
+        self.assertEqual(result['notifications'][0]['id'], '2')
+        self.assertEqual(result['notifications'][0]['title']['en-US'], '2.1')
+        self.assertRegexpMatches(result['version'], r'-1/0-2/1')
         random_call.return_value = 0.751
         result = json.loads(notification.notification({}, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['id'], 'b')
-        self.assertEqual(result['notifications'][0]['title']['en-US'], '2')
-        self.assertRegexpMatches(result['version'], r'-a/0-b/2')
+        self.assertEqual(result['notifications'][0]['id'], '2')
+        self.assertEqual(result['notifications'][0]['title']['en-US'], '2.2')
+        self.assertRegexpMatches(result['version'], r'-1/0-2/2')
 
+    @mock.patch('time.gmtime', mock_time)
     def test_invalid_last_version(self):
-        self.load_notifications_mock.return_value = []
         notification.notification({'QUERY_STRING': 'lastVersion='},
                                   lambda *args: None)
+        result = json.loads(notification.notification({}, lambda *args: None))
+        self.assertEqual(result['version'], '197001010000')
+
         notification.notification({'QUERY_STRING': 'lastVersion=-'},
                                   lambda *args: None)
+        result = json.loads(notification.notification({}, lambda *args: None))
+        self.assertEqual(result['version'], '197001010000')
+
         notification.notification({'QUERY_STRING': 'lastVersion=-/'},
                                   lambda *args: None)
+        result = json.loads(notification.notification({}, lambda *args: None))
+        self.assertEqual(result['version'], '197001010000')
+
         notification.notification({'QUERY_STRING': 'lastVersion=-//'},
                                   lambda *args: None)
+        result = json.loads(notification.notification({}, lambda *args: None))
+        self.assertEqual(result['version'], '197001010000')
 
     def test_version_header_present(self):
-        self.load_notifications_mock.return_value = [
-            {'id': '1', 'title': {'en-US': ''}, 'message': {'en-US': ''}}
-        ]
+        self.add_notifications([
+            ('1', 'title.en-US = 1\nmessage.en-US = 1\n')
+        ])
         response_header_map = {}
 
         def start_response(status, response_headers):
@@ -270,101 +290,103 @@ class TestNotification(unittest.TestCase):
                          response_header_map['ABP-Notification-Version'])
 
     def test_default_group_notification_returned_if_valid(self):
-        self.load_notifications_mock.return_value = [
-            {'id': '1', 'title': {'en-US': ''}, 'message': {'en-US': ''}},
-            {
-                'id': 'a',
-                'title': {'en-US': '0'},
-                'message': {'en-US': '0'},
-                'variants': [
-                    {'title': {'en-US': '1'}, 'message': {'en-US': '1'}}
-                ]
-            }
-        ]
+        self.add_notifications([
+            ('1', 'title.en-US = 1\nmessage.en-US = 1\n'),
+            ('2', 'title.en-US = 2\nmessage.en-US = 2\n'
+                  '[1]\ntitle.en-US = 2.1\nmessage.en-US = 2.1'),
+        ])
+
         result = json.loads(notification.notification({
-            'QUERY_STRING': 'lastVersion=197001010000-a/0'
+            'QUERY_STRING': 'lastVersion=197001010000-2/0'
         }, lambda *args: None))
         self.assertEqual(len(result['notifications']), 2)
         self.assertEqual(result['notifications'][0]['id'], '1')
-        self.assertEqual(result['notifications'][1]['id'], 'a')
-        self.assertEqual(result['notifications'][1]['title']['en-US'], '0')
+        self.assertEqual(result['notifications'][1]['id'], '2')
+        self.assertEqual(result['notifications'][1]['title']['en-US'], '2')
         self.assertNotIn('variants', result['notifications'][1])
-        self.assertRegexpMatches(result['version'], r'-a/0')
+        self.assertRegexpMatches(result['version'], r'-2/0')
 
     def test_default_group_notification_not_returned_if_invalid(self):
-        self.load_notifications_mock.return_value = [
-            {'id': '1', 'title': {'en-US': ''}, 'message': {'en-US': ''}},
-            {
-                'id': 'a',
-                'title': {'en-US': '0'},
-                'variants': [
-                    {'title': {'en-US': '1'}, 'message': {'en-US': '1'}}
-                ]
-            }
-        ]
+        self.add_notifications([
+            ('1', 'title.en-US = 1\nmessage.en-US = 1\n'),
+            ('2', 'title.en-US = 2\n'
+                  '[1]\ntitle.en-US = 2.1\nmessage.en-US = 2.1'),
+        ])
         result = json.loads(notification.notification({
-            'QUERY_STRING': 'lastVersion=197001010000-a/0'
+            'QUERY_STRING': 'lastVersion=197001010000-2/0'
         }, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
         self.assertEqual(result['notifications'][0]['id'], '1')
-        self.assertRegexpMatches(result['version'], r'-a/0')
+        self.assertRegexpMatches(result['version'], r'-2/0')
 
     def test_invalid_notification_not_returned(self):
-        self.load_notifications_mock.return_value = [
-            {'id': '1', 'title': {'en-US': ''}, 'message': {'en-US': ''}},
-            {'id': '2', 'title': {'en-US': ''}, 'message': {}},

[wspee, 2017/10/09 09:12:03]

This is not possible with the parser, so I removed the notification from the new
test.

-            {'id': '3', 'title': {}, 'message': {'en-US': ''}},
-            {'id': '4', 'title': {}},
-            {'id': '5', 'message': {}},
-            {'id': '6'}
-        ]
+        self.add_notifications([
+            ('1', 'title.en-US = 1\nmessage.en-US = 1\n'),
+            ('2', 'title.en-US = 2\n'),
+            ('3', 'title.en-US = \n'),
+            ('4', 'message.en-US = \n'),
+            ('5', '\n'),
+        ])
+
         result = json.loads(notification.notification({}, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
         self.assertEqual(result['notifications'][0]['id'], '1')
 
     def test_stays_in_group_when_notification_present(self):
-        self.load_notifications_mock.return_value = [
-            {'id': 'a'}
-        ]
+        self.add_notifications([
+            ('1', '\n'),
+        ])
         result = json.loads(notification.notification({
-            'QUERY_STRING': 'lastVersion=197001010000-a/0-b/1'
+            'QUERY_STRING': 'lastVersion=197001010000-1/0-2/1'
         }, lambda *args: None))
         self.assertEqual(len(result['notifications']), 0)
-        self.assertRegexpMatches(result['version'], r'-a/0')
+        self.assertRegexpMatches(result['version'], r'-1/0')
 
     def test_leaves_group_when_notification_absent(self):
-        self.load_notifications_mock.return_value = []
         result = json.loads(notification.notification({
-            'QUERY_STRING': 'lastVersion=197001010000-a/0-b/1'
+            'QUERY_STRING': 'lastVersion=197001010000-1/0-2/1'
         }, lambda *args: None))
         self.assertEqual(len(result['notifications']), 0)
         self.assertRegexpMatches(result['version'], r'[^-]*')
 
     def test_stays_in_group_when_notification_inactive(self):
-        self.load_notifications_mock.return_value = [
-            {'id': 'a', 'inactive': True}
-        ]
+        self.add_notifications(
+            [('1', 'inactive = true\ntitle.en-US = 1\nmessage.en-US = 1\n')])
         result = json.loads(notification.notification({
-            'QUERY_STRING': 'lastVersion=197001010000-a/0-b/1'
+            'QUERY_STRING': 'lastVersion=197001010000-1/0-2/1'
         }, lambda *args: None))
         self.assertEqual(len(result['notifications']), 0)
-        self.assertRegexpMatches(result['version'], r'-a/0')
+        self.assertRegexpMatches(result['version'], r'-1/0')
+
+    def test_stays_in_group_when_notification_inactive_assign_new_group(self):
+        # See: https://issues.adblockplus.org/ticket/5827
+        self.add_notifications([
+            ('1', 'inactive = true\ntitle.en-US = 1.1\nmessage.en-US = 1.1'),
+            ('2', '[1]\ntitle.en-US = 2.1\nmessage.en-US = 2.1\nsample = 1\n'),
+        ])
+        result = json.loads(notification.notification({
+            'QUERY_STRING': 'lastVersion=197001010000-1/0'
+        }, lambda *args: None))
+
+        self.assertEqual(len(result['notifications']), 1)
+        self.assertRegexpMatches(result['version'], r'-1/0-2/1')
 
     def test_inactive_notifications_not_returned(self):
-        self.load_notifications_mock.return_value = [
-            {'id': 'a', 'title': {'en-US': ''}, 'message': {'en-US': ''}, 'inactive': True},
-            {'id': 'b', 'title': {'en-US': ''}, 'message': {'en-US': ''}, 'inactive': False},
-            {'id': 'c', 'title': {'en-US': ''}, 'message': {'en-US': ''}}
-        ]
+        self.add_notifications([
+            ('1', 'inactive = true\ntitle.en-US = 1\nmessage.en-US = 1\n'),
+            ('2', 'inactive = false\ntitle.en-US = 2\nmessage.en-US = 2\n'),
+            ('3', 'title.en-US = 3\nmessage.en-US = 3\n'),
+        ])
+
         result = json.loads(notification.notification({}, lambda *args: None))
         self.assertEqual(len(result['notifications']), 2)
-        self.assertEqual(result['notifications'][0]['id'], 'b')
-        self.assertEqual(result['notifications'][1]['id'], 'c')
+        self.assertEqual(result['notifications'][0]['id'], '2')
+        self.assertEqual(result['notifications'][1]['id'], '3')
 
     def test_inactive_notification_variant_not_returned(self):
-        self.load_notifications_mock.return_value = [
-            {'id': 'a', 'inactive': True}
-        ]
+        self.add_notifications([
+            ('1', 'inactive = true\ntitle.en-US = 1\nmessage.en-US = 1\n'),
+        ])
         result = json.loads(notification.notification({
             'QUERY_STRING': 'lastVersion=197001010000-a/1'
         }, lambda *args: None))