Fixed unlabelling with unsynced labels

casepro/backend/tests/test_rapidpro.py

@@ -535,7 +535,7 @@ def test_create_label(self, mock_get_labels, mock_create_label):
         # check when label exists
         self.assertEqual(self.backend.create_label(self.unicef, "Ebola"), "L-012")
 
-        mock_create_label.assert_not_called()
+        self.assertNotCalled(mock_create_label)
 
         # check when label doesn't exist
         mock_get_labels.return_value = []
@@ -583,7 +583,7 @@ def test_label_messages(self, mock_label_messages):
         # empty message list shouldn't make API call
         self.backend.label_messages(self.unicef, [], self.aids)
 
-        mock_label_messages.assert_not_called()
+        self.assertNotCalled(mock_label_messages)
 
         msg1 = self.create_message(self.unicef, 123, self.bob, "Hello")
         msg2 = self.create_message(self.unicef, 234, self.bob, "Goodbye")
@@ -597,7 +597,7 @@ def test_unlabel_messages(self, mock_unlabel_messages):
         # empty message list shouldn't make API call
         self.backend.unlabel_messages(self.unicef, [], self.aids)
 
-        mock_unlabel_messages.assert_not_called()
+        self.assertNotCalled(mock_unlabel_messages)
 
         msg1 = self.create_message(self.unicef, 123, self.bob, "Hello")
         msg2 = self.create_message(self.unicef, 234, self.bob, "Goodbye")
@@ -611,7 +611,7 @@ def test_archive_messages(self, mock_archive_messages):
         # empty message list shouldn't make API call
         self.backend.archive_messages(self.unicef, [])
 
-        mock_archive_messages.assert_not_called()
+        self.assertNotCalled(mock_archive_messages)
 
         msg1 = self.create_message(self.unicef, 123, self.bob, "Hello")
         msg2 = self.create_message(self.unicef, 234, self.bob, "Goodbye")
@@ -631,7 +631,7 @@ def test_restore_messages(self, mock_unarchive_messages):
         # empty message list shouldn't make API call
         self.backend.restore_messages(self.unicef, [])
 
-        mock_unarchive_messages.assert_not_called()
+        self.assertNotCalled(mock_unarchive_messages)
 
         msg1 = self.create_message(self.unicef, 123, self.bob, "Hello")
         msg2 = self.create_message(self.unicef, 234, self.bob, "Goodbye")
@@ -645,7 +645,7 @@ def test_flag_messages(self, mock_label_messages):
         # empty message list shouldn't make API call
         self.backend.flag_messages(self.unicef, [])
 
-        mock_label_messages.assert_not_called()
+        self.assertNotCalled(mock_label_messages)
 
         msg1 = self.create_message(self.unicef, 123, self.bob, "Hello")
         msg2 = self.create_message(self.unicef, 234, self.bob, "Goodbye")
@@ -659,7 +659,7 @@ def test_unflag_messages(self, mock_unlabel_messages):
         # empty message list shouldn't make API call
         self.backend.unflag_messages(self.unicef, [])
 
-        mock_unlabel_messages.assert_not_called()
+        self.assertNotCalled(mock_unlabel_messages)
 
         msg1 = self.create_message(self.unicef, 123, self.bob, "Hello")
         msg2 = self.create_message(self.unicef, 234, self.bob, "Goodbye")

casepro/msgs/models.py

@@ -285,7 +285,8 @@ def bulk_unlabel(org, user, messages, label):
             for msg in messages:
                 msg.labels.remove(label)
 
-            get_backend().unlabel_messages(org, messages, label)
+            if label.is_synced:
+                get_backend().unlabel_messages(org, messages, label)
 
             MessageAction.create(org, user, messages, MessageAction.UNLABEL, label)
 

casepro/msgs/tests.py

@@ -48,7 +48,7 @@ def test_save(self, mock_create_label):
         self.assertEqual(label.is_synced, True)
 
     def test_get_all(self):
-        self.assertEqual(set(Label.get_all(self.unicef)), {self.aids, self.pregnancy})
+        self.assertEqual(set(Label.get_all(self.unicef)), {self.aids, self.pregnancy, self.tea})
         self.assertEqual(set(Label.get_all(self.unicef, self.user1)), {self.aids, self.pregnancy})  # MOH user
         self.assertEqual(set(Label.get_all(self.unicef, self.user3)), {self.aids})  # WHO user
 
@@ -177,7 +177,7 @@ def test_list(self):
 
         response = self.url_get('unicef', url)
         self.assertEqual(response.status_code, 200)
-        self.assertEqual(list(response.context['object_list']), [self.aids, self.pregnancy])
+        self.assertEqual(list(response.context['object_list']), [self.aids, self.pregnancy, self.tea])
 
     def test_delete(self):
         url = reverse('msgs.label_delete', args=[self.pregnancy.pk])
@@ -205,7 +205,7 @@ def setUp(self):
         self.ann = self.create_contact(self.unicef, 'C-001', "Ann")
 
     def create_test_messages(self):
-        self.msg1 = self.create_message(self.unicef, 101, self.ann, "Normal", [self.aids, self.pregnancy])
+        self.msg1 = self.create_message(self.unicef, 101, self.ann, "Normal", [self.aids, self.pregnancy, self.tea])
         self.msg2 = self.create_message(self.unicef, 102, self.ann, "Flow", type='F')
         self.msg3 = self.create_message(self.unicef, 103, self.ann, "Archived", is_archived=True)
         self.msg4 = self.create_message(self.unicef, 104, self.ann, "Flagged", is_flagged=True)
@@ -417,14 +417,18 @@ def test_update_labels(self, mock_unlabel_messages, mock_label_messages):
         mock_unlabel_messages.assert_called_once_with(self.unicef, [self.msg1], self.aids)
 
         actions = list(MessageAction.objects.order_by('pk'))
+        self.assertEqual(len(actions), 3)
         self.assertEqual(actions[0].action, MessageAction.LABEL)
         self.assertEqual(actions[0].created_by, self.user1)
         self.assertEqual(set(actions[0].messages.all()), {self.msg1})
         self.assertEqual(actions[0].label, ebola)
         self.assertEqual(actions[1].action, MessageAction.UNLABEL)
         self.assertEqual(actions[1].created_by, self.user1)
         self.assertEqual(set(actions[1].messages.all()), {self.msg1})
-        self.assertEqual(actions[1].label, self.aids)
+
+        # order of labels isn't deterministic
+        self.assertIn(actions[1].label, [self.aids, self.tea])
+        self.assertIn(actions[2].label, [self.aids, self.tea])
 
     @patch('casepro.test.TestBackend.flag_messages')
     def test_bulk_flag(self, mock_flag_messages):
@@ -460,30 +464,48 @@ def test_bulk_unflag(self, mock_unflag_messages):
     def test_bulk_label(self, mock_label_messages):
         self.create_test_messages()
 
-        Message.bulk_label(self.unicef, self.user1, [self.msg1, self.msg2], self.aids)
+        # try with un-synced label
+        Message.bulk_label(self.unicef, self.user1, [self.msg1, self.msg2], self.tea)
 
-        mock_label_messages.assert_called_once_with(self.unicef, [self.msg1, self.msg2], self.aids)
+        self.assertNotCalled(mock_label_messages)
 
         action = MessageAction.objects.get()
         self.assertEqual(action.action, MessageAction.LABEL)
         self.assertEqual(action.created_by, self.user1)
+        self.assertEqual(action.label, self.tea)
         self.assertEqual(set(action.messages.all()), {self.msg1, self.msg2})
 
+        self.assertEqual(self.tea.messages.count(), 2)
+
+        # try with synced label
+        Message.bulk_label(self.unicef, self.user1, [self.msg1, self.msg2], self.aids)
+
+        mock_label_messages.assert_called_once_with(self.unicef, [self.msg1, self.msg2], self.aids)
+
         self.assertEqual(self.aids.messages.count(), 2)
 
     @patch('casepro.test.TestBackend.unlabel_messages')
     def test_bulk_unlabel(self, mock_unlabel_messages):
         self.create_test_messages()
 
-        Message.bulk_unlabel(self.unicef, self.user1, [self.msg1, self.msg2], self.aids)
+        # try with un-synced label
+        Message.bulk_unlabel(self.unicef, self.user1, [self.msg1, self.msg2], self.tea)
 
-        mock_unlabel_messages.assert_called_once_with(self.unicef, [self.msg1, self.msg2], self.aids)
+        self.assertNotCalled(mock_unlabel_messages)
 
         action = MessageAction.objects.get()
         self.assertEqual(action.action, MessageAction.UNLABEL)
         self.assertEqual(action.created_by, self.user1)
+        self.assertEqual(action.label, self.tea)
         self.assertEqual(set(action.messages.all()), {self.msg1, self.msg2})
 
+        self.assertEqual(self.tea.messages.count(), 0)
+
+        # try with synced label
+        Message.bulk_unlabel(self.unicef, self.user1, [self.msg1, self.msg2], self.aids)
+
+        mock_unlabel_messages.assert_called_once_with(self.unicef, [self.msg1, self.msg2], self.aids)
+
         self.assertEqual(self.aids.messages.count(), 0)
 
     @patch('casepro.test.TestBackend.archive_messages')

casepro/rules/tests.py

@@ -142,7 +142,7 @@ def setUp(self):
 
     def test_get_all(self):
         rules = Rule.get_all(self.unicef)
-        self.assertEqual(len(rules), 2)
+        self.assertEqual(len(rules), 3)
         self.assertEqual(rules[0].tests, [ContainsTest(["aids", "hiv"], Quantifier.ANY)])
         self.assertEqual(rules[0].actions, [LabelAction(self.aids)])
         self.assertEqual(rules[1].tests, [ContainsTest(["pregnant", "pregnancy"], Quantifier.ANY)])

casepro/test.py

@@ -45,7 +45,8 @@ def setUp(self):
         self.aids = self.create_label(self.unicef, "L-001", "AIDS", 'Messages about AIDS', ["aids", "hiv"])
         self.pregnancy = self.create_label(self.unicef, "L-002", "Pregnancy", 'Messages about pregnancy',
                                            ["pregnant", "pregnancy"])
-        self.code = self.create_label(self.nyaruka, "L-003", "Code", 'Messages about code', ["java", "python", "go"])
+        self.tea = self.create_label(self.unicef, None, "Tea", 'Messages about tea', ["tea", "chai"], is_synced=False)
+        self.code = self.create_label(self.nyaruka, "L-101", "Code", 'Messages about code', ["java", "python", "go"])
 
         # some partners
         self.moh = self.create_partner(self.unicef, "MOH", [self.aids, self.pregnancy])
@@ -134,6 +135,9 @@ def create_case(self, org, contact, assignee, message, labels=(), **kwargs):
 
         return case
 
+    def assertNotCalled(self, mock):
+        self.assertEqual(len(mock.mock_calls), 0, "Expected no calls, called %d times" % len(mock.mock_calls))
+
     def assertExcelRow(self, sheet, row_num, values, tz=None):
         """
         Asserts the cell values in the given worksheet row. Date values are converted using the provided timezone.