[damned-lies] Refactored cherry-pick stuff to allow displaying a proper success/failure message

[Claude Paroz <claudep src gnome org>]

commit ce470aff0cbe0060ac2ca475004aba68ab22b56a
Author: Claude Paroz <claude 2xlibre net>
Date:   Sat Oct 17 17:47:12 2015 +0200

    Refactored cherry-pick stuff to allow displaying a proper success/failure message

 stats/models.py         |   13 ++++++++-----
 stats/tests/tests.py    |   16 +++++++---------
 vertimus/forms.py       |    2 +-
 vertimus/models.py      |   17 +++++++++++++----
 vertimus/tests/tests.py |    7 ++++---
 vertimus/views.py       |    5 ++++-
 6 files changed, 37 insertions(+), 23 deletions(-)
---
diff --git a/stats/models.py b/stats/models.py
index 93fdaa5..35c5b06 100644
--- a/stats/models.py
+++ b/stats/models.py
@@ -654,7 +654,7 @@ class Branch(models.Model):
         else:
             return command_list
 
-    def commit_po(self, po_file, domain, language, author, sync_master=False):
+    def commit_po(self, po_file, domain, language, author):
         """ Commit the file 'po_file' in the branch VCS repository """
         if self.is_vcs_readonly():
             raise Exception("This branch is in read-only mode. Unable to commit")
@@ -716,12 +716,13 @@ class Branch(models.Model):
             stat.update_stats(dest_path)
         else:
             self.update_stats(force=False, checkout=False, domain=domain)
-
-        if sync_master and not self.is_head():
-            # Cherry-pick the commit on the master branch
-            self.module.get_head_branch().cherrypick_commit(commit_hash, domain)
+        return commit_hash
 
     def cherrypick_commit(self, commit_hash, domain):
+        """
+        Try to cherry-pick a branch commit `commit_hash` into master.
+        Return True if the cherry-pick succeeds, False otherwise.
+        """
         if self.module.vcs_type != "git":
             raise NotImplementedError("Commit cherry-pick is not implemented for '%s'" % 
self.module.vcs_type)
         with ModuleLock(self.module):
@@ -732,9 +733,11 @@ class Branch(models.Model):
             if result[0] == utils.STATUS_OK:
                 utils.run_shell_command(
                     ['git', 'push', 'origin', self.name], raise_on_error=True, cwd=commit_dir)
+                return True
             else:
                 # Revert
                 utils.run_shell_command(['git', 'cherry-pick', '--abort'], cwd=commit_dir)
+                return False
 
 
 DOMAIN_TYPE_CHOICES = (
diff --git a/stats/tests/tests.py b/stats/tests/tests.py
index fb45b46..0d0add2 100644
--- a/stats/tests/tests.py
+++ b/stats/tests/tests.py
@@ -312,7 +312,8 @@ class ModuleTestCase(TestCase):
             'git add fr.po',
             # Quoting is done at the Popen level
             'git commit -m Updated French translation --author Author <someone example org>',
-            'git push origin master'
+            'git push origin master', 'git log -n1 --format=oneline',
+            'msgfmt --statistics -o /dev/null',
         )
         with patch_shell_command() as cmds:
             branch.commit_po(po_file, domain, fr_lang, 'Author <someone example org>')
@@ -368,21 +369,18 @@ class ModuleTestCase(TestCase):
         self.mod.vcs_root = self.mod.vcs_root.replace('git://', 'ssh://')
         self.mod.save()
 
+        with patch_shell_command(only=['git push', 'git fetch', 'git reset']) as cmds:
+            commit_hash = branch.commit_po(po_file, domain, fr_lang, 'Author <someone example org>')
         update_repo_sequence = (
-            'git checkout -f gnome-3-18', 'git fetch', 'git reset --hard origin/gnome-3-18',
+            'git checkout -f master', 'git fetch', 'git reset --hard origin/master',
             'git clean -dfq', 'if [ -e .gitmodules ]; then git submodule update --init; fi',
         )
-        update_repo_sequence_master = tuple(cmd.replace('gnome-3-18', 'master') for cmd in 
update_repo_sequence)
         git_ops = update_repo_sequence + (
-            'git add fr.po',
-            'git commit -m Updated French translation --author Author <someone example org>',
-            'git push origin gnome-3-18', 'git log -n1 --format=oneline', 'msgfmt --statistics -o /dev/null',
-        ) + update_repo_sequence_master + (
             'git cherry-pick -x',
             'git push origin master',
         )
-        with patch_shell_command(only=['git pull', 'git push', 'git fetch', 'git reset']) as cmds:
-            branch.commit_po(po_file, domain, fr_lang, 'Author <someone example org>', sync_master=True)
+        with patch_shell_command(only=['git push', 'git fetch', 'git reset']) as cmds:
+            self.branch.cherrypick_commit(commit_hash, domain)
             for idx, cmd in enumerate(git_ops):
                 self.assertIn(cmd, cmds[idx])
 
diff --git a/vertimus/forms.py b/vertimus/forms.py
index f031573..70b36f3 100644
--- a/vertimus/forms.py
+++ b/vertimus/forms.py
@@ -91,7 +91,7 @@ class ActionForm(forms.Form):
             self.fields['author'].initial = state.get_latest_po_file_action().person
         if not has_mailing_list:
             del self.fields['send_to_ml']
-        if state.branch.is_head():
+        if state and state.branch.is_head():
             del self.fields['sync_master']
 
     def clean_file(self):
diff --git a/vertimus/models.py b/vertimus/models.py
index 924c5fb..840e667 100644
--- a/vertimus/models.py
+++ b/vertimus/models.py
@@ -638,16 +638,25 @@ class ActionCI(Action):
     def apply_on(self, state, form_data):
         self.state_db = state
         action_with_po = self.get_previous_action_with_po()
+        author = form_data['author'].as_author() if form_data['author'] else None
         try:
-            author = form_data['author'].as_author() if form_data['author'] else None
-            sync = form_data.get('sync_master', False)
             state.branch.commit_po(
-                action_with_po.file.path, state.domain, state.language, author, sync_master=sync)
+                action_with_po.file.path, state.domain, state.language, author)
         except Exception:
             # Commit failed, state unchanged
             raise Exception(_("The commit failed. The error was: '%s'") % sys.exc_info()[1])
 
-        super(ActionCI, self).apply_on(state, form_data) # Mail sent in super
+        msg = ugettext("The file has been successfully committed to the repository.")
+        if form_data.get('sync_master', False) and not state.branch.is_head():
+            # Cherry-pick the commit on the master branch
+            success = state.branch.module.get_head_branch().cherrypick_commit(commit_hash, state.domain)
+            if success:
+                msg += ugettext(" Additionally, the synchronization with the master branch succeeded.")
+            else:
+                msg += ugettext(" However, the synchronization with the master branch failed.")
+
+        super(ActionCI, self).apply_on(state, form_data)  # Mail sent in super
+        return msg
 
 class ActionRC(Action):
     name = 'RC'
diff --git a/vertimus/tests/tests.py b/vertimus/tests/tests.py
index aab6e1f..80a00ad 100644
--- a/vertimus/tests/tests.py
+++ b/vertimus/tests/tests.py
@@ -391,9 +391,10 @@ class VertimusTest(TeamsAndRolesTests):
         self.assertTrue(form.is_valid())
         with patch_shell_command() as cmds:
             action = Action.new_by_name('CI', person=self.pcoo)
-            action.apply_on(state, form.cleaned_data)
-            self.assertIn("git commit", cmds[-2])
-            self.assertNotIn("--author", cmds[-2])
+            msg = action.apply_on(state, form.cleaned_data)
+        self.assertIn("git commit", cmds[-3])
+        self.assertNotIn("--author", cmds[-3])
+        self.assertEqual(msg, 'The file has been successfully committed to the repository.')
 
 
     def test_action_ic(self):
diff --git a/vertimus/views.py b/vertimus/views.py
index dfb2463..bf2ac4e 100644
--- a/vertimus/views.py
+++ b/vertimus/views.py
@@ -119,13 +119,16 @@ def vertimus(request, branch, domain, language, stats=None, level="0"):
                 action = Action.new_by_name(action, person=person,
                     file=request.FILES.get('file', None))
                 try:
-                    action.apply_on(state, action_form.cleaned_data)
+                    msg = action.apply_on(state, action_form.cleaned_data)
                 except SendMailFailed:
                     messages.error(request,
                         _("A problem occurred while sending mail, no mail have been sent"))
                 except Exception as e:
                     messages.error(request,
                         _("An error occurred during applying your action: %s") % e)
+                else:
+                    if msg:
+                        messages.success(request, msg)
 
                 return HttpResponseRedirect(
                     urlresolvers.reverse('vertimus_by_names',