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 1fd5d04ad2e005306eae8d69fc9c802856b96a1c..579ad1840cb1bdbdc131f32924adb1f5fb45ac5a 100644 (file)
--- 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 df3659a1ae30772e3f3cabd10bc42aaa4ff80294..428c0a5d8e43f2996212ca9000fc89d95b739292 100644 (file)
--- 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 c78a1c0c7c259f0c8c9f5f809b6a05cb6ac6a706..71959ffabd98a45a5c0d8378cf51d90d97aac307 100644 (file)
--- 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 a7217a0645ada1ff6762176bf5effb4dbfef86bc..0230c41e907b973981818172005f4591bf7d685c 100644 (file)
--- 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")