git cl issue 0: Remove Change-Id from message, not description

Using self.GetDescription() uses the description from the web.
Given that the user is purposefully disassociating their current
branch from the review on the web, they may have already changed
the commit message. We shouldn't set it back.

Bug: 742730
Change-Id: I0545cb6288c332fd475d1de7fb302f71ee41a415
Reviewed-on: https://chromium-review.googlesource.com/578229
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>

git_cl.py

@@ -1471,12 +1471,6 @@ class Changelist(object):
             self._codereview_impl.CodereviewServerConfigKey(),
             codereview_server)
     else:
-      desc = self.GetDescription()
-      if desc and git_footers.get_footer_change_id(desc):
-        print('WARNING: The change patched into this branch has a Change-Id. '
-              'Removing it.')
-        RunGit(['commit', '--amend', '-m',
-                git_footers.remove_footer(desc, 'Change-Id')])
       # Reset all of these just to be clean.
       reset_suffixes = [
           'last-upload-hash',
@@ -1486,6 +1480,12 @@ class Changelist(object):
       ] + self._PostUnsetIssueProperties()
       for prop in reset_suffixes:
         self._GitSetBranchConfigValue(prop, None, error_ok=True)
+      msg = RunGit(['log', '-1', '--format=%B']).strip()
+      if msg and git_footers.get_footer_change_id(msg):
+        print('WARNING: The change patched into this branch has a Change-Id. '
+              'Removing it.')
+        RunGit(['commit', '--amend', '-m',
+                git_footers.remove_footer(msg, 'Change-Id')])
       self.issue = None
       self.patchset = None
 

tests/git_cl_test.py

@@ -2648,8 +2648,6 @@ class TestGitCl(TestCase):
   def test_cmd_issue_erase_existing(self):
     out = StringIO.StringIO()
     self.mock(git_cl.sys, 'stdout', out)
-    self.mock(git_cl.Changelist, 'GetDescription',
-              lambda _: 'This is a description')
     self.calls = [
         ((['git', 'symbolic-ref', 'HEAD'],), 'feature'),
         ((['git', 'config', 'branch.feature.rietveldissue'],), CERR1),
@@ -2662,6 +2660,7 @@ class TestGitCl(TestCase):
         ((['git', 'config', '--unset', 'branch.feature.gerritserver'],), ''),
         ((['git', 'config', '--unset', 'branch.feature.gerritsquashhash'],),
          ''),
+        ((['git', 'log', '-1', '--format=%B'],), 'This is a description'),
     ]
     self.assertEqual(0, git_cl.main(['issue', '0']))
 
@@ -2674,7 +2673,6 @@ class TestGitCl(TestCase):
         ((['git', 'symbolic-ref', 'HEAD'],), 'feature'),
         ((['git', 'config', 'branch.feature.rietveldissue'],), CERR1),
         ((['git', 'config', 'branch.feature.gerritissue'],), '123'),
-        ((['git', 'commit', '--amend', '-m', 'This is a description\n'],), ''),
         # Let this command raise exception (retcode=1) - it should be ignored.
         ((['git', 'config', '--unset', 'branch.feature.last-upload-hash'],),
          CERR1),
@@ -2683,6 +2681,9 @@ class TestGitCl(TestCase):
         ((['git', 'config', '--unset', 'branch.feature.gerritserver'],), ''),
         ((['git', 'config', '--unset', 'branch.feature.gerritsquashhash'],),
          ''),
+        ((['git', 'log', '-1', '--format=%B'],),
+         'This is a description\n\nChange-Id: Ideadbeef'),
+        ((['git', 'commit', '--amend', '-m', 'This is a description\n'],), ''),
     ]
     self.assertEqual(0, git_cl.main(['issue', '0']))