Quality of life fix for rbt post when updating request.

The issue: you post a review request with a different set of target people or
groups than are in your .reviewboardrc. When you then update this existing
review request, if you don't re-specify your custom people/groups, then rbt
will overwrite your custom choices with the ones specified in your
.reviewboardrc.

With this change, rbt post will use the options in the .reviewboardrc only when
creating a new review request. If you are updating an existing request, then it
will ignore the settings in the .reviewboardrc. You can still manually change
the groups/people in your update if you would like. Unit tests show this
functionality.

Testing Done:
Been using this at work successfully.
Manually walked through all the posting options.
Unit tests should cover each case.

Reviewed at https://reviews.reviewboard.org/r/9442/

Fixes bug 4650.

rbtools/commands/post.py

@@ -228,15 +228,13 @@ class Post(Command):
                        help='The comma-separated list of bug IDs closed.'),
                 Option('--target-groups',
                        dest='target_groups',
-                       config_key='TARGET_GROUPS',
                        metavar='NAME[,...]',
                        default=None,
                        help='The names of the groups that should perform the '
                             'review.'),
                 Option('--target-people',
                        dest='target_people',
                        metavar='USERNAME[,...]',
-                       config_key='TARGET_PEOPLE',
                        default=None,
                        help='The usernames of the people who should perform '
                             'the review.'),
@@ -304,6 +302,17 @@ def post_process_options(self):
 
         self.options.extra_fields = extra_fields
 
+        # Only use default target-users / groups when creating a new review
+        # request. Otherwise we'll overwrite any user changes.
+        if not self.options.update and self.options.rid is None:
+            if (self.options.target_groups is None and
+               'TARGET_GROUPS' in self.config):
+                self.options.target_groups = self.config['TARGET_GROUPS']
+
+            if (self.options.target_people is None and
+               'TARGET_PEOPLE' in self.config):
+                self.options.target_people = self.config['TARGET_PEOPLE']
+
         # -g implies --guess-summary and --guess-description
         self.options.guess_fields = self.normalize_guess_value(
             self.options.guess_fields, '--guess-fields')

rbtools/commands/tests/test_post.py

@@ -10,27 +10,6 @@
 class PostCommandTests(RBTestBase):
     """Tests for rbt post command."""
 
-    def _create_post_command(self, fields):
-        """Create an argument parser with the given extra fields.
-
-        Args:
-            fields (list of unicode):
-                A list of key-value pairs for the field argument.
-
-                Each pair should be of the form key=value.
-
-        Returns:
-            argparse.ArgumentParser:
-            Argument parser for commandline arguments
-        """
-        post = Post()
-        argv = ['rbt', 'post']
-        parser = post.create_arg_parser(argv)
-        post.options = parser.parse_args(argv[2:])
-        post.options.fields = fields
-
-        return post
-
     def test_post_one_extra_fields(self):
         """Testing one extra field argument with rbt post --field foo=bar"""
         post = self._create_post_command(['foo=bar'])
@@ -96,3 +75,112 @@ def test_arg_field_set_again_by_custom_fields(self):
         post.options.description = 'test'
 
         self.assertRaises(CommandError, post.post_process_options)
+
+    def test_post_setting_target_users(self):
+        """Testing setting the target person with rbt post
+        --target-people=test_person
+        """
+        post = self._create_post_command(args=['--target-people',
+                                               'test_person'])
+        post.post_process_options()
+        self.assertEqual(post.options.target_people, 'test_person')
+
+    def test_post_setting_target_groups(self):
+        """Testing setting the target group with rbt post
+        --target-groups=test_group
+        """
+        post = self._create_post_command(args=['--target-groups',
+                                               'test_group'])
+        post.post_process_options()
+        self.assertEqual(post.options.target_groups, 'test_group')
+
+    def test_post_setting_target_users_on_update(self):
+        """Testing setting the target person on an update with rbt post
+        --target-people=test_person --review-request-id=12345
+        """
+        post = self._create_post_command(args=['--target-people',
+                                               'test_person',
+                                               '--review-request-id', '12345'])
+        post.post_process_options()
+        self.assertEqual(post.options.target_people, 'test_person')
+
+    def test_post_setting_target_groups_on_update(self):
+        """Testing setting the target group on an update with rbt post
+        --target-groups=test_group --review-request-id=12345
+        """
+        post = self._create_post_command(args=['--target-groups', 'test_group',
+                                               '--review-request-id', '12345'])
+        post.post_process_options()
+        self.assertEqual(post.options.target_groups, 'test_group')
+
+    def test_post_default_target_users(self):
+        """Testing setting target person via config file with rbt post
+        """
+        with self.reviewboardrc({'TARGET_PEOPLE': 'test_person'},
+                                use_temp_dir=True):
+            post = self._create_post_command()
+            post.post_process_options()
+            self.assertEqual(post.options.target_people, 'test_person')
+
+    def test_post_default_target_groups(self):
+        """Testing setting target group via config file with rbt post
+        """
+        with self.reviewboardrc({'TARGET_GROUPS': 'test_group'},
+                                use_temp_dir=True):
+            post = self._create_post_command()
+            post.post_process_options()
+            self.assertEqual(post.options.target_groups, 'test_group')
+
+    def test_post_no_default_target_users_update(self):
+        """Testing setting target person on update via config file
+        with rbt post --review-request-id=12345
+        """
+        with self.reviewboardrc({'TARGET_PEOPLE': 'test_person'},
+                                use_temp_dir=True):
+            post = self._create_post_command(args=['--review-request-id',
+                                                   '12345'])
+            post.post_process_options()
+            self.assertEqual(post.options.target_people, None)
+
+    def test_post_no_default_target_groups_update(self):
+        """Testing setting target group on update via config file
+        with rbt post --review-request-id=12345
+        """
+        with self.reviewboardrc({'TARGET_GROUPS': 'test_group'},
+                                use_temp_dir=True):
+            post = self._create_post_command(args=['--review-request-id',
+                                                   '12345'])
+            post.post_process_options()
+            self.assertEqual(post.options.target_groups, None)
+
+    def _create_post_command(self, fields=None, args=None):
+        """Create an argument parser with the given extra fields.
+
+        Args:
+            fields (list of unicode):
+                A list of key-value pairs for the field argument.
+
+                Each pair should be of the form key=value.
+
+            args (list of unicode):
+                A list of command line arguments to be passed to the parser.
+
+                The command line will receive each item in the list.
+
+        Returns:
+            rbtools.commands.post.POST:
+            A POST instance for communicating with the rbt server
+        """
+        post = Post()
+        argv = ['rbt', 'post']
+
+        if args is not None:
+            argv.extend(args)
+
+        parser = post.create_arg_parser(argv)
+        post.options = parser.parse_args(argv[2:])
+
+        if fields is not None:
+            post.options.fields = fields
+
+        return post

rbtools/utils/testbase.py

@@ -1,10 +1,14 @@
 from __future__ import unicode_literals
 
+import contextlib
 import os
+import shutil
 import sys
+import tempfile
 import uuid
 
 from six.moves import cStringIO as StringIO
+from six import iteritems
 
 from rbtools.utils.filesystem import cleanup_tempfiles, make_tempdir
 from rbtools.testing import TestCase
@@ -65,3 +69,35 @@ def catch_output(self, func):
         func()
         sys.stdout = stdout
         return outbuf.getvalue()
+
+    @contextlib.contextmanager
+    def reviewboardrc(self, data, use_temp_dir=False):
+        """Manage a temporary .reviewboardrc file.
+
+        Args:
+            data (dict)
+                A dictionary of key-value pairs to write into the
+                .reviewboardrc file.
+
+                A best effort attempt will be made to convert the value into
+                an appropriate string.
+
+            use_temp_dir (boolean)
+                A boolean that indicates if a temporary directory should be
+                created and used as the working directory for the context.
+        """
+        if use_temp_dir:
+            temp_dir = tempfile.mkdtemp()
+            cwd = os.getcwd()
+            os.chdir(temp_dir)
+
+        with open('.reviewboardrc', 'w') as fp:
+            for key, value in iteritems(data):
+                fp.write('%s = %r\n' % (key, value))
+
+        try:
+            yield
+        finally:
+            if use_temp_dir:
+                os.chdir(cwd)
+                shutil.rmtree(temp_dir)