[damned-lies] Always add current user to commit action author select

[Claude Paroz <claudep src gnome org>]

commit deb2ddb3da85d7952bdc322fe7c56cbb2a03e818
Author: Claude Paroz <claude 2xlibre net>
Date:   Sat Mar 3 23:21:08 2018 +0100

    Always add current user to commit action author select
    
    Fixes #794032.

 vertimus/forms.py       |    7 +++++--
 vertimus/models.py      |    6 ++++--
 vertimus/tests/tests.py |   40 ++++++++++++++++++++++++++--------------
 vertimus/views.py       |    5 +++--
 4 files changed, 38 insertions(+), 20 deletions(-)
---
diff --git a/vertimus/forms.py b/vertimus/forms.py
index 9b5d855..7f25984 100644
--- a/vertimus/forms.py
+++ b/vertimus/forms.py
@@ -57,7 +57,7 @@ class ActionForm(forms.Form):
                            help_text=_("Upload a .po, .gz, .bz2 or .png file"))
     send_to_ml = forms.BooleanField(label=_("Send message to the team mailing list"), required=False)
 
-    def __init__(self, state, actions, has_mailing_list, *args, **kwargs):
+    def __init__(self, current_user, state, actions, has_mailing_list, *args, **kwargs):
         super().__init__(*args, **kwargs)
         self.actions = actions
         self.fields['action'].choices = [(
@@ -66,7 +66,8 @@ class ActionForm(forms.Form):
         ) for act in actions]
         self.fields['action'].help_link = reverse('help', args=['vertimus_workflow', 1])
         if state and ActionCI in map(type, self.actions):
-            self.fields['author'].queryset = state.involved_persons()
+            self.fields['author'].queryset = state.involved_persons(extra_user=current_user
+                ).order_by('last_name', 'username')
             self.fields['author'].initial = state.get_latest_po_file_action().person
         if not has_mailing_list:
             del self.fields['send_to_ml']
@@ -91,6 +92,8 @@ class ActionForm(forms.Form):
         action_code = cleaned_data.get('action')
         if action_code is None:
             raise ValidationError(_("Invalid action. Someone probably posted another action just before 
you."))
+        elif action_code == 'CI' and not cleaned_data['author']:
+            raise ValidationError(_("Committing a file requires a commit author."))
         comment = cleaned_data.get('comment')
         file = cleaned_data.get('file')
         action = Action.new_by_name(action_code, comment=comment, file=file)
diff --git a/vertimus/models.py b/vertimus/models.py
index 106ed94..d1bb3ae 100644
--- a/vertimus/models.py
+++ b/vertimus/models.py
@@ -4,7 +4,7 @@ from datetime import datetime, timedelta
 
 from django.conf import settings
 from django.db import models
-from django.db.models import Max
+from django.db.models import Max, Q
 from django.db.models.signals import post_save, pre_delete
 from django.dispatch import receiver
 from django.urls import reverse
@@ -107,10 +107,12 @@ class State(models.Model):
             sequence = query[0]['sequence']
         return sequence
 
-    def involved_persons(self):
+    def involved_persons(self, extra_user=None):
         """
         Return all persons having posted any action on the current state.
         """
+        if extra_user is not None:
+            return Person.objects.filter(Q(action__state_db=self) | Q(pk=extra_user.pk))
         return Person.objects.filter(action__state_db=self).distinct()
 
     def get_latest_po_file_action(self):
diff --git a/vertimus/tests/tests.py b/vertimus/tests/tests.py
index 98287c3..8bb7c08 100644
--- a/vertimus/tests/tests.py
+++ b/vertimus/tests/tests.py
@@ -217,7 +217,7 @@ class VertimusTest(TeamsAndRolesTests):
     def test_action_menu(self):
         state = StateNone(branch=self.b, domain=self.d, language=self.l)
         state.save()
-        form = ActionForm(state, state.get_available_actions(self.pt), False)
+        form = ActionForm(self.pt, state, state.get_available_actions(self.pt), False)
         self.assertHTMLEqual(
             str(form['action']),
             '<select id="id_action" name="action">'
@@ -225,7 +225,7 @@ class VertimusTest(TeamsAndRolesTests):
             '<option value="WC">Write a comment</option>'
             '</select>'
         )
-        form = ActionForm(state, state.get_available_actions(self.pcoo), False)
+        form = ActionForm(self.pcoo, state, state.get_available_actions(self.pcoo), False)
         self.assertHTMLEqual(
             str(form['action']),
             '<select id="id_action" name="action">'
@@ -253,7 +253,7 @@ class VertimusTest(TeamsAndRolesTests):
         self.assertEqual(len(mail.outbox), 2)
         self.assertEqual(mail.outbox[1].recipients(), [self.pt.email])
         # Test that submitting a comment without text generates a validation error
-        form = ActionForm(state, [ActionWC()], True, QueryDict('action=WC&comment='))
+        form = ActionForm(self.pt, state, [ActionWC()], True, QueryDict('action=WC&comment='))
         self.assertTrue("A comment is needed" in str(form.errors))
         self.assertNotEqual(state.updated, prev_updated)
 
@@ -395,18 +395,21 @@ class VertimusTest(TeamsAndRolesTests):
 
         self.assertIn(ActionCI, map(type, state.get_available_actions(self.pcoo)))
 
-        form = ActionForm(state, state.get_available_actions(self.pcoo), True)
-        self.assertEqual(len(form.fields['author'].choices), 5)
+        form = ActionForm(self.pcoo, state, state.get_available_actions(self.pcoo), True)
+        self.assertEqual(len(form.fields['author'].choices), 6)
         self.assertEqual(form.fields['author'].initial, self.pr)
         self.assertHTMLEqual(
             str(form['author']),
             '<select id="id_author" name="author">'
             '<option value="">---------</option>'
+            '<option disabled value="%d">ûsername (full name missing)</option>'
+            '<option disabled value="%d">Sven Brkc (email missing)</option>'
+            '<option value="%d">John Coordinator</option>'
             '<option selected value="%d">John Reviewer</option>'
             '<option value="%d">John Translator</option>'
-            '<option disabled value="%d">Sven Brkc (email missing)</option>'
-            '<option disabled value="%d">ûsername (full name missing)</option>'
-            '</select>' % (self.pr.pk, self.pt.pk, pers_no_email.pk, pers_no_full_name.pk)
+            '</select>' % (
+                pers_no_full_name.pk, pers_no_email.pk, self.pcoo.pk, self.pr.pk, self.pt.pk,
+            )
         )
 
     def test_action_ci_no_fullname(self):
@@ -420,14 +423,23 @@ class VertimusTest(TeamsAndRolesTests):
         state = StateProofreading(branch=self.b, domain=self.d, language=self.l, person=pers)
         state.save()
         self.upload_file(state, 'UP', pers=pers)
-        form = ActionForm(state, state.get_available_actions(self.pcoo), True, {
-            'action': 'CI', 'author': '', 'comment': '', 'send_to_ml': True})
+        post_data = {
+            'action': 'CI',
+            'author': '',
+            'comment': '',
+            'send_to_ml': True
+        }
+        form = ActionForm(self.pcoo, state, state.get_available_actions(self.pcoo), True, post_data)
+        # Missing author
+        self.assertFalse(form.is_valid())
+        post_data['author'] = str(self.pcoo.pk)
+        form = ActionForm(self.pcoo, state, state.get_available_actions(self.pcoo), True, post_data)
         self.assertTrue(form.is_valid())
         with patch_shell_command() as cmds:
             action = Action.new_by_name('CI', person=self.pcoo)
             msg = action.apply_on(state, form.cleaned_data)
         self.assertIn("git commit", cmds[-3])
-        self.assertNotIn("--author", cmds[-3])
+        self.assertIn('--author John Coordinator <jcoo imthebigboss fr>', cmds[-3])
         self.assertEqual(msg, 'The file has been successfully committed to the repository.')
         state.refresh_from_db()
         # All actions should have been archived
@@ -619,18 +631,18 @@ class VertimusTest(TeamsAndRolesTests):
         # Test a non valid po file
         post_content = QueryDict('action=WC&comment=Test1')
         post_file = MultiValueDict({'file': [SimpleUploadedFile('filename.po', b'Not valid po file 
content')]})
-        form = ActionForm(None, [ActionWC()], True, post_content, post_file)
+        form = ActionForm(self.pt, None, [ActionWC()], True, post_content, post_file)
 
         self.assertTrue('file' in form.errors)
 
         # Test a valid po file
         with open(os.path.join(os.path.dirname(os.path.abspath(__file__)), "valid_po.po"), 'rb') as fh:
             post_file = MultiValueDict({'file': [File(fh)]})
-            form = ActionForm(None, [ActionWC()], True, post_content, post_file)
+            form = ActionForm(self.pt, None, [ActionWC()], True, post_content, post_file)
             self.assertTrue(form.is_valid())
 
         # Test form without file
-        form = ActionForm(None, [ActionWC()], True, post_content)
+        form = ActionForm(self.pt, None, [ActionWC()], True, post_content)
         self.assertTrue(form.is_valid())
 
     def test_feeds(self):
diff --git a/vertimus/views.py b/vertimus/views.py
index 08e9a06..160e548 100644
--- a/vertimus/views.py
+++ b/vertimus/views.py
@@ -93,7 +93,8 @@ def vertimus(request, branch, domain, language, stats=None, level="0"):
         has_ml = language.team and bool(language.team.mailing_list) or False
         if request.method == 'POST':
             action_form = ActionForm(
-                state, available_actions, has_ml, request.POST, request.FILES)
+                request.user, state, available_actions, has_ml, request.POST, request.FILES
+            )
 
             if action_form.is_valid():
                 # Process the data in form.cleaned_data
@@ -118,7 +119,7 @@ def vertimus(request, branch, domain, language, stats=None, level="0"):
                         args=(branch.module.name, branch.name, domain.name,
                               language.locale)))
         elif available_actions:
-            action_form = ActionForm(state, available_actions, has_ml)
+            action_form = ActionForm(request.user, state, available_actions, has_ml)
 
     context = {
         'pageSection': 'module',