From: Magnus Hagander <magnus@hagander.net>
Date: Tue, 15 Jul 2014 09:50:10 +0000 (+0200)
Subject: Add proper support for committing patches
X-Git-Url: http://git.postgresql.org/gitweb/static/gitweb.js?a=commitdiff_plain;h=0eba14c4159a492edfa9e66fab0403440645154f;p=pgcommitfest2.git

Add proper support for committing patches

This will prompt for who was the committer, and default to either the user
listed as a committer if there is one, or to the current user if the current
user happens to be a committer.
---

diff --git a/pgcommitfest/commitfest/static/commitfest/js/commitfest.js b/pgcommitfest/commitfest/static/commitfest/js/commitfest.js
index 1fd5d04..579ad18 100644
--- a/pgcommitfest/commitfest/static/commitfest/js/commitfest.js
+++ b/pgcommitfest/commitfest/static/commitfest/js/commitfest.js
@@ -4,14 +4,6 @@ function verify_reject() {
 function verify_returned() {
    return confirm('Are you sure you want to close this patch as Returned with Feedback?\n\nThis means the patch will be marked as closed in this commitfest, but will automatically be moved to the next one. If no further work is expected on this patch, it should be closed with "Rejected" istead.\n\nSo - are you sure?');
 }
-function verify_committed(is_committer) {
-   if (is_committer) 
-      return confirm('Are you sure you want to close this patch as Committed?');
-   else {
-      alert('Currently, only the committer who actually made the commit can do this. We should make a little prompt for the committer field otherwise..');
-      return false;
-   }
-}
 
 function findLatestThreads() {
    $('#attachThreadListWrap').addClass('loading');
@@ -110,6 +102,22 @@ function doAttachThread(cfid, patchid, msgid) {
    });
 }
 
+function flagCommitted(committer) {
+   $('#commitModal').modal();
+   $('#committerSelect').val(committer);
+   $('#doCommitButton').unbind('click');
+   $('#doCommitButton').click(function() {
+       var c = $('#committerSelect').val();
+       if (!c) {
+	   alert('You need to select a committer before you can mark a patch as committed!');
+	   return;
+       }
+       document.location.href='close/committed/?c=' + c;
+   });
+   return false;
+}
+
+
 function sortpatches(sortby) {
    $('#id_sortkey').val(sortby);
    $('#filterform').submit();
diff --git a/pgcommitfest/commitfest/templates/patch.html b/pgcommitfest/commitfest/templates/patch.html
index df3659a..428c0a5 100644
--- a/pgcommitfest/commitfest/templates/patch.html
+++ b/pgcommitfest/commitfest/templates/patch.html
@@ -111,6 +111,34 @@
 
 {%include "patch_commands.inc"%}
 
+{%comment%}commit dialog{%endcomment%}
+<div class="modal fade" id="commitModal" role="dialog">
+ <div class="modal-dialog modal-lg">
+   <div class="modal-content">
+     <div class="modal-header">
+       <button type="button" class="close" data-dismiss="modal" aria-hidden="true">&times;</button>
+       <h3>Flag as committed</h3>
+     </div>
+     <div class="modal-body">
+       <form class="form" style="margin-bottom: 5px;">
+	 <div class="form-group">
+	   <label for="committerlist">Committer</label>
+	   <select id="committerSelect" class="form-control">
+	     <option value="" style="display:none;"></option>
+	     {%for c in committers%}
+	     <option value="{{c.user.username}}">{{c.user.first_name}} {{c.user.last_name}}</option>
+	     {%endfor%}
+	   </select>
+	 </div>
+     </div>
+     <div class="modal-footer">
+       <a href="#" class="btn btn-default" data-dismiss="modal">Close</a>
+       <a href="#" class="btn btn-default btn-primary" id="doCommitButton">Flag as committed</a>
+     </div>
+   </div>
+ </div>
+</div>
+
 {%include "thread_attach.inc"%}
 {%endblock%}
 
diff --git a/pgcommitfest/commitfest/templates/patch_commands.inc b/pgcommitfest/commitfest/templates/patch_commands.inc
index c78a1c0..71959ff 100644
--- a/pgcommitfest/commitfest/templates/patch_commands.inc
+++ b/pgcommitfest/commitfest/templates/patch_commands.inc
@@ -20,7 +20,7 @@
   <li role="presentation" class="dropdown-header">Closed statuses</li>
   <li role="presentation"><a href="close/reject/" onclick="return verify_reject()">Rejected</a></li>
   <li role="presentation"><a href="close/feedback/" onclick="return verify_returned()">Returned with feedback</a></li>
-  <li role="presentation"><a href="close/committed/" onclick="return verify_committed({{is_committer|yesno:"true,false"}}))">Committed</a></li>
+  <li role="presentation"><a href="close/committed/" onclick="return flagCommitted({%if patch.committer%}'{{patch.committer}}'{%elif is_committer%}'{{user.username}}'{%else%}null{%endif%})">Committed</a></li>
  </ul>
 </div>
 
diff --git a/pgcommitfest/commitfest/views.py b/pgcommitfest/commitfest/views.py
index a7217a0..0230c41 100644
--- a/pgcommitfest/commitfest/views.py
+++ b/pgcommitfest/commitfest/views.py
@@ -123,11 +123,12 @@ def patch(request, cfid, patchid):
 	cf = get_object_or_404(CommitFest, pk=cfid)
 	patch = get_object_or_404(Patch.objects.select_related(), pk=patchid, commitfests=cf)
 	patch_commitfests = PatchOnCommitFest.objects.select_related('commitfest').filter(patch=patch).order_by('-commitfest__startdate')
+	committers = Committer.objects.filter(active=True).order_by('user__last_name', 'user__first_name')
 
 	#XXX: this creates a session, so find a smarter way. Probably handle
 	#it in the callback and just ask the user then?
 	if request.user.is_authenticated():
-		committer = list(Committer.objects.filter(user=request.user, active=True))
+		committer = [c for c in committers if c.user==request.user]
 		if len(committer) > 0:
 			is_committer=  True
 			is_this_committer = committer[0] == patch.committer
@@ -135,7 +136,6 @@ def patch(request, cfid, patchid):
 			is_committer = is_this_committer = False
 
 		is_reviewer = request.user in patch.reviewers.all()
-#		is_reviewer = len([x for x in patch.reviewers.all() if x==request.user]) > 0
 	else:
 		is_committer = False
 		is_this_committer = False
@@ -148,6 +148,7 @@ def patch(request, cfid, patchid):
 		'is_committer': is_committer,
 		'is_this_committer': is_this_committer,
 		'is_reviewer': is_reviewer,
+		'committers': committers,
 		'title': patch.name,
 		'breadcrumbs': [{'title': cf.title, 'href': '/%s/' % cf.pk},],
 		}, context_instance=RequestContext(request))
@@ -383,10 +384,12 @@ def close(request, cfid, patchid, status):
 			newpoc = PatchOnCommitFest(patch=poc.patch, commitfest=newcf[0], enterdate=datetime.now())
 			newpoc.save()
 		elif status == 'committed':
+			committer = get_object_or_404(Committer, user__username=request.GET['c'])
+			if committer != poc.patch.committer:
+				# Committer changed!
+				poc.patch.committer = committer
+				PatchHistory(patch=poc.patch, by=request.user, what='Changed committer to %s' % committer).save()
 			poc.status = PatchOnCommitFest.STATUS_COMMITTED
-			#XXX: need to prompt for a committer here!
-			raise Exception("Need to prompt for committed if the user who just committed isn't one!")
-			poc.patch.committer = Committer.objects.get(user=request.user)
 		else:
 			raise Exception("Can't happen")