Commits

Peter Suter committed ea65d5d

Improve list handling:
* Remove again possibility to use `+...` and `-...` syntax with non-list fields.
* Operators affect all following items without an operator.
* Initial items without operator cause entire list to be replaced.
* Extensive documentation and unit tests

  • Participants
  • Parent commits b2792d0

Comments (0)

Files changed (3)

File trac/ticket/batch.py

             for id in selected_tickets:
                 t = Ticket(self.env, int(id))
                 _values = new_values.copy()
-                for field in new_values:
-                    if field in t.values:
-                        _values[field] = self._add_remove_fields(
-                            t.values[field], new_values[field], 
-                            field in self.fields_as_list)
+                for field in self.fields_as_list:
+                    if field in new_values:
+                        new = new_values[field]
+                        old = t.values[field] if field in t.values else ''
+                        _values[field] = self._change_list(old, new)
                 controllers = list(self._get_action_controllers(req, t,
                                                                 action))
                 for controller in controllers:
                                   "notifications: %(message)s",
                                   message=to_unicode(e)))
     
-    def _add_remove_fields(self, old_keywords, new_keywords, is_list):
-        """Any keywords prefixed with '+' or '-' will be added or removed.
+    def _change_list(self, old_list, new_list):
+        """Any entries prefixed with '+' or '-' will be added or removed.
         
-        If `is_list` is True keywords will be added even if not prefixed.
+        These operators carry over to entries without one.
+        If the first entry has no operator, the new list replaces the old one.
         """
         
-        if not is_list and not new_keywords.startswith('+') \
-                       and not new_keywords.startswith('-'):
-            return new_keywords
-
-        new_keywords = [k.strip()
-                        for k in self.list_separator_re.split(new_keywords)
+        changed_list = [k.strip()
+                        for k in self.list_separator_re.split(old_list)
                         if k]
-        all_keywords = [k.strip()
-                        for k in self.list_separator_re.split(old_keywords)
-                        if k]
+        new_list = [k.strip()
+                    for k in self.list_separator_re.split(new_list)
+                    if k]
         
-        for keyword in new_keywords:
-            if keyword.startswith('-'):
-                keyword = keyword[1:]
-                while keyword in all_keywords:
-                    all_keywords.remove(keyword)
+        if not new_list:
+            return ''
+        
+        op = ''
+        for entry in new_list:
+            if entry.startswith('+') or entry.startswith('-'):
+                op = entry[0]
+                entry = entry[1:]
+            if op == '+':
+                if entry not in changed_list:
+                    changed_list.append(entry)
+            elif op == '-':
+                while entry in changed_list:
+                    changed_list.remove(entry)
             else:
-                if keyword.startswith('+'):
-                    keyword = keyword[1:]
-                if keyword not in all_keywords:
-                    all_keywords.append(keyword)
+                changed_list = [entry,]
+                op = '+'
         
-        return self.list_connector_string.join(all_keywords)
+        return self.list_connector_string.join(changed_list)

File trac/ticket/tests/batch.py

         field_change = [c for c in changes if c[2] == field][0]
         self.assertEqual(field_change[4], new_value)
     
-    def _merge_keywords_test_helper(self, original, new):
+    def _change_list_test_helper(self, original, new):
         batch = BatchModifyModule(self.env)
-        return batch._merge_keywords(original, new)
+        return batch._change_list(original, new)
     
     def _insert_ticket(self, summary, **kw):
         """Helper for inserting a ticket into the database"""
         batch = BatchModifyModule(self.env)
         selected_tickets = batch._get_selected_tickets(self.req)
         self.assertEqual(selected_tickets, [])
+
+    # Replace list items
+    
+    def test_change_list_replace_empty_with_single(self):
+        """Replace emtpy field with single item."""
+        changed = self._change_list_test_helper('', 'alice')
+        self.assertEqual(changed, 'alice')
         
-    def test_merge_keywords(self):
-        """New keywords are added to an empty list."""
-        combined = self._merge_keywords_test_helper('', 'foo bar')
-        self.assertEqual(combined, 'foo bar')
+    def test_change_list_replace_empty_with_items(self):
+        """Replace emtpy field with items."""
+        changed = self._change_list_test_helper('', 'alice, bob')
+        self.assertEqual(changed, 'alice, bob')
+        
+    def test_change_list_replace_item(self):
+        """Replace item with a different item."""
+        changed = self._change_list_test_helper('alice', 'bob')
+        self.assertEqual(changed, 'bob')
+        
+    def test_change_list_replace_item_with_items(self):
+        """Replace item with different items."""
+        changed = self._change_list_test_helper('alice', 'bob, carol')
+        self.assertEqual(changed, 'bob, carol')
+        
+    def test_change_list_replace_items_with_item(self):
+        """Replace items with a different item."""
+        changed = self._change_list_test_helper('alice, bob', 'carol')
+        self.assertEqual(changed, 'carol')
+        
+    def test_change_list_replace_items(self):
+        """Replace items with different items."""
+        changed = self._change_list_test_helper('alice, bob', 'carol, dave')
+        self.assertEqual(changed, 'carol, dave')
+        
+    def test_change_list_replace_items_partial(self):
+        """Replace items with different (or not) items."""
+        changed = self._change_list_test_helper('alice, bob', 'bob, dave')
+        self.assertEqual(changed, 'bob, dave')
+        
+    def test_change_list_clear(self):
+        """Clear field."""
+        changed = self._change_list_test_helper('alice bob', '')
+        self.assertEqual(changed, '')
+
+    # Add / remove list items
     
-    def test_merge_keywords_append(self):
-        """New keywords are appended to a non-empty list."""
-        combined = self._merge_keywords_test_helper('foo bar', 'baz')
-        self.assertEqual(combined, 'foo bar baz')
+    def test_change_list_add_item(self):
+        """Append additional item."""
+        changed = self._change_list_test_helper('alice', '+bob')
+        self.assertEqual(changed, 'alice, bob')
+        
+    def test_change_list_add_items(self):
+        """Append additional items."""
+        changed = self._change_list_test_helper('alice, bob', '+carol, +dave')
+        self.assertEqual(changed, 'alice, bob, carol, dave')
+        
+    def test_change_list_remove_item(self):
+        """Remove existing item."""
+        changed = self._change_list_test_helper('alice, bob', '-bob')
+        self.assertEqual(changed, 'alice')
+        
+    def test_change_list_remove_items(self):
+        """Remove existing items."""
+        changed = self._change_list_test_helper('alice, bob, carol',
+                                                '-alice, -carol')
+        self.assertEqual(changed, 'bob')
+        
+    def test_change_list_remove_idempotent(self):
+        """Ignore missing item to be removed."""
+        changed = self._change_list_test_helper('alice', '-bob')
+        self.assertEqual(changed, 'alice')
+        
+    def test_change_list_remove_mixed(self):
+        """Ignore only missing item to be removed."""
+        changed = self._change_list_test_helper('alice, bob', '-bob, -carol')
+        self.assertEqual(changed, 'alice')
+        
+    def test_change_list_add_remove(self):
+        """Remove existing item and append additional item."""
+        changed = self._change_list_test_helper('alice, bob',
+                                                '-alice, +carol')
+        self.assertEqual(changed, 'bob, carol')
+        
+    def test_change_list_add_no_duplicates(self):
+        """Existing items are not duplicated."""
+        changed = self._change_list_test_helper('alice, bob', '+bob, carol')
+        self.assertEqual(changed, 'alice, bob, carol')
+        
+    def test_change_list_remove_all_duplicates(self):
+        """Remove all duplicates."""
+        changed = self._change_list_test_helper('alice, bob, alice', '-alice')
+        self.assertEqual(changed, 'bob')
+        
+    # Add / remove multiple list items with one operator
+
+    def test_change_list_multiadd(self):
+        """Also add following items without oeprator."""
+        changed = self._change_list_test_helper('alice', '+bob, carol, dave')
+        self.assertEqual(changed, 'alice, bob, carol, dave')
+        
+    def test_change_list_multiremove(self):
+        """Also remove following items without operator."""
+        changed = self._change_list_test_helper('alice, bob, carol',
+                                                '-alice, carol')
+        self.assertEqual(changed, 'bob')
+        
+    def test_change_list_multiadd_multiremove(self):
+        """Also add or remove following items without operator."""
+        changed = self._change_list_test_helper('alice, bob',
+                                                '+carol, dave, -alice, bob')
+        self.assertEqual(changed, 'carol, dave')
+        
+    # Replace and add / remove list items at the same time (not very useful)
     
-    def test_merge_keywords_no_duplicates(self):
-        """Existing keywords are not duplicated."""
-        combined = self._merge_keywords_test_helper('foo bar', 'bar baz')
-        self.assertEqual(combined, 'foo bar baz')
+    def test_change_list_replace_then_add(self):
+        """Replace item with different item, then add additional item."""
+        changed = self._change_list_test_helper('alice', 'bob, +carol')
+        self.assertEqual(changed, 'bob, carol')
         
-    def test_merge_keywords_remove(self):
-        """Remove existing keywords by beginning with a dash."""
-        combined = self._merge_keywords_test_helper('foo bar', '-bar')
-        self.assertEqual(combined, 'foo')
+    def test_change_list_replace_then_remove(self):
+        """Replace item with different item, then ignore item to be removed,
+        as it was already replaced."""
+        changed = self._change_list_test_helper('alice', 'bob, -alice')
+        self.assertEqual(changed, 'bob')
     
-    def test_merge_keywords_remove_idempotent(self):
-        """Keywords beginning with a dash are ignored."""
-        combined = self._merge_keywords_test_helper('foo bar', '-baz')
-        self.assertEqual(combined, 'foo bar')
+    # Save
     
-    def test_custom_separator_and_connector(self):
-        """Custom separator and connector strings can be used."""
-        self.env.config.set('batchmod', 'list_separator_regex', '|')
-        self.env.config.set('batchmod', 'list_connector_string', '|')
-        combined = self._merge_keywords_test_helper('foo|bar', 'baz')
-        self.assertEqual(combined, 'foo|bar|baz')
-        
     def test_save_comment(self):
         """Comments are saved to all selected tickets."""
         first_ticket_id = self._insert_ticket('Test 1', reporter='joe')
         self.assertFieldChanged(first_ticket_id, 'component', 'bar')
         self.assertFieldChanged(second_ticket_id, 'component', 'bar')
     
-    def test_multiple_fields_as_list(self):
-        """Multiple fields can be lists."""
-        self.env.config.set('batchmod', 'fields_as_list', 
-                            'keywords,stakeholders')
-        self.env.config.set('ticket-custom', 'stakeholders', 'text')
-        first_ticket_id = self._insert_ticket('Test 1', reporter='joe', 
-                                              keywords='foo bar',
-                                              stakeholders='shirley')
-        second_ticket_id = self._insert_ticket('Test 2', reporter='joe',
-                                               keywords='foo', 
-                                               stakeholders='jim')
-        selected_tickets = [first_ticket_id, second_ticket_id]
-        values = { 'keywords' : 'bar baz', 'stakeholders' : 'joe' }
-        
-        batch = BatchModifyModule(self.env)
-        batch._save_ticket_changes(self.req, selected_tickets, values, '',
-                                   'leave')
-        
-        self.assertFieldChanged(first_ticket_id, 'stakeholders',
-                                'shirley joe')
-        self.assertFieldChanged(second_ticket_id, 'keywords', 'foo bar baz')
-        self.assertFieldChanged(second_ticket_id, 'stakeholders', 'jim joe')
-    
     def test_action_with_state_change(self):
         """Actions can have change status."""
         self.env.config.set('ticket-workflow', 'embiggen', '* -> big')

File trac/wiki/default-pages/TracBatchModify

 
 To perform a batch modification select the tickets you wish to modify and set the new field values using the section underneath the query results. 
 
-Values prepended with a '+' or '-' will be appended or removed from the field.
+== List fields
 
-For example, given a field with the value `apples, berries, cherries`. Adding the value `+dates` will result in `apples, berries, cherries, dates`.
-Writing `-berries` in the field instead would have removed that from the list, leaving only `apples, cherries`.
+The `Keywords` and `Cc` fields offer some special list management features. The new field value can consist of multiple items (i.e. multiple keywords or cc addresses).
+
+If all new items are prepended with a '+' or '-', they will get merged with the old items:
+* Items prepended with a '+' are appended to the old list. (No duplicates are added.)
+* Items prepended with a '-' are removed from the old llist. (All occurrences are removed.)
+
+Otherwise the old list gets replaced by the new item(s).
+
+=== Examples
+
+First a few examples where the entire list is replaced (because no '+' or '-' appears anywhere):
+
+||=Old Cc's          ||=Batch Cc field   ||=New Cc's                ||= Summary ||
+||                   || alice            || alice                   || Replace emtpy field with single item. ||
+||                   || alice, bob       || alice, bob              || Replace emtpy field with two items. ||
+|| alice             || bob              || bob                     || Replace item with a different item. ||
+|| alice             || bob, carol       || bob, carol              || Replace item with different items. ||
+|| alice, bob        || carol            || carol                   || Replace items with a different item. ||
+|| alice, bob        || carol, dave      || carol, dave             || Replace items with different items. ||
+|| alice, bob        || bob, dave        || bob, dave               || Replace items with different (or not) items. ||
+|| alice, bob        ||                  ||                         || Replace items with emtpy field. ||
+
+Now a few simple examples with '+' and '-' only:
+
+||=Old Cc's          ||=Batch Cc field   ||=New Cc's                ||= Summary ||
+|| alice             || +bob             || alice, bob              || Append additional item. ||
+|| alice, bob        || +carol, +dave    || alice, bob, carol, dave || Append additional items. ||
+|| alice, bob        || -bob             || alice                   || Remove existing item. ||
+|| alice, bob, carol || -alice, -carol   || bob                     || Remove existing items. ||
+|| alice             || -bob             || alice                   || Ignore missing item to be removed. ||
+|| alice, bob        || -bob, -carol     || alice                   || Ignore only missing item to be removed. ||
+|| alice, bob        || -alice, +carol   || bob, carol              || Remove existing item and append additional item. ||
+
+'+' and '-' carry over to items with no operator:
+
+||=Old Cc's          ||=Batch Cc field   ||=New Cc's                ||= Summary ||
+|| alice             || +carol, bob      || alice, carol, bob       || Add both items. ||
+|| alice, bob        || -alice, bob      ||                         || Remove both items. ||
+|| alice, bob        || +carol, dave, -alice, bob || carol, dave    || Add and remove items. ||
+
+Mixing replacement with '+' and '-' is possible, but not very useful:
+||=Old Cc's          ||=Batch Cc field   ||=New Cc's                ||= Summary ||
+|| alice             || bob, +carol      || bob, carol              || Replace item with different item, then add additional item. ||
+|| alice             || bob, -alice      || bob                     || Replace item with different item, then ignore item to be removed, as it was already replaced. ||