Add ability to disposition comparison patches

Add the ability to mark each patch with a disposition indicating whether
the patch is interesting or not.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>

layerindex/admin.py

@@ -178,6 +178,10 @@ class PatchAdmin(admin.ModelAdmin):
     def has_delete_permission(self, request, obj=None):
         return False
 
+class PatchDispositionAdmin(admin.ModelAdmin):
+    search_fields = ['patch__path']
+    list_filter = ['patch__recipe__layerbranch__layer__name', 'patch__recipe__layerbranch__branch__name']
+
 class IncFileAdmin(admin.ModelAdmin):
     search_fields = ['path']
     list_filter = ['layerbranch__layer__name', 'layerbranch__branch__name']
@@ -230,6 +234,7 @@ admin.site.register(BBAppend, BBAppendAdmin)
 admin.site.register(BBClass, BBClassAdmin)
 admin.site.register(IncFile, IncFileAdmin)
 admin.site.register(Patch, PatchAdmin)
+admin.site.register(PatchDisposition, PatchDispositionAdmin)
 admin.site.register(LayerRecipeExtraURL)
 admin.site.register(RecipeChangeset, RecipeChangesetAdmin)
 admin.site.register(ClassicRecipe, ClassicRecipeAdmin)

layerindex/forms.py

@@ -22,7 +22,7 @@ import settings
 from layerindex.models import (Branch, ClassicRecipe,
                                LayerBranch, LayerItem, LayerMaintainer,
                                LayerNote, RecipeChange, RecipeChangeset,
-                               SecurityQuestion, UserProfile)
+                               SecurityQuestion, UserProfile, PatchDisposition)
 
 
 class StyledForm(forms.Form):
@@ -344,3 +344,13 @@ class ComparisonRecipeSelectForm(StyledForm):
     q = forms.CharField(label='Keyword', max_length=255, required=False)
     oe_layer = forms.ModelChoiceField(label='OE Layer', queryset=LayerItem.objects.filter(comparison=False).filter(status__in=['P', 'X']).order_by('name'), empty_label="(any)", required=False)
 
+
+class PatchDispositionForm(StyledModelForm):
+    class Meta:
+        model = PatchDisposition
+        fields = ('patch', 'disposition', 'comment')
+        widgets = {
+            'patch': forms.HiddenInput(),
+        }
+
+PatchDispositionFormSet = modelformset_factory(PatchDisposition, form=PatchDispositionForm, extra=0)

layerindex/migrations/0032_patchdisposition.py

@@ -0,0 +1,31 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.16 on 2019-03-21 20:37
+from __future__ import unicode_literals
+
+from django.conf import settings
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+        ('layerindex', '0031_securityquestion_populate'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='PatchDisposition',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('disposition', models.CharField(choices=[('A', 'Apply'), ('R', 'Further review'), ('E', 'Existing'), ('N', 'Not needed'), ('V', 'Different version'), ('I', 'Invalid')], default='A', max_length=1)),
+                ('comment', models.TextField(blank=True)),
+                ('patch', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, to='layerindex.Patch')),
+                ('user', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to=settings.AUTH_USER_MODEL)),
+            ],
+            options={
+                'permissions': (('patch_disposition', 'Can disposition patches'),),
+            },
+        ),
+    ]

layerindex/models.py

@@ -882,3 +882,26 @@ class SecurityQuestionAnswer(models.Model):
     user = models.ForeignKey(UserProfile, on_delete=models.CASCADE)
     security_question = models.ForeignKey(SecurityQuestion)
     answer = models.CharField(max_length = 250, null=False)
+
+
+class PatchDisposition(models.Model):
+    PATCH_DISPOSITION_CHOICES = (
+        ('A', 'Apply'),
+        ('R', 'Further review'),
+        ('E', 'Existing'),
+        ('N', 'Not needed'),
+        ('V', 'Different version'),
+        ('I', 'Invalid'),
+    )
+    patch = models.OneToOneField(Patch, on_delete=models.CASCADE)
+    user = models.ForeignKey(User, null=True, on_delete=models.SET_NULL)
+    disposition = models.CharField(max_length=1, choices=PATCH_DISPOSITION_CHOICES, default='A')
+    comment = models.TextField(blank=True)
+
+    class Meta:
+        permissions = (
+            ("patch_disposition", "Can disposition patches"),
+        )
+
+    def __str__(self):
+        return '%s - %s' % (self.patch, self.get_disposition_display())

layerindex/views.py

@@ -46,14 +46,15 @@ from layerindex.forms import (AdvancedRecipeSearchForm, BulkChangeEditFormSet,
                               ClassicRecipeForm, ClassicRecipeSearchForm,
                               ComparisonRecipeSelectForm, EditLayerForm,
                               EditNoteForm, EditProfileForm,
-                              LayerMaintainerFormSet, RecipeChangesetForm)
+                              LayerMaintainerFormSet, RecipeChangesetForm,
+                              PatchDispositionForm, PatchDispositionFormSet)
 from layerindex.models import (BBAppend, BBClass, Branch, ClassicRecipe,
                                Distro, DynamicBuildDep, IncFile, LayerBranch,
                                LayerDependency, LayerItem, LayerMaintainer,
                                LayerNote, LayerUpdate, Machine, Patch, Recipe,
                                RecipeChange, RecipeChangeset, Source, StaticBuildDep,
                                Update, SecurityQuestion, SecurityQuestionAnswer,
-                               UserProfile)
+                               UserProfile, PatchDisposition)
 
 
 from . import simplesearch, tasks, utils
@@ -1331,6 +1332,14 @@ class ClassicRecipeDetailView(SuccessMessageMixin, DetailView):
             return False
         return True
 
+    def _can_disposition_patches(self):
+        if self.request.user.is_authenticated():
+            if not self.request.user.has_perm('layerindex.patch_disposition'):
+                return False
+        else:
+            return False
+        return True
+
     def get_context_data(self, **kwargs):
         context = super(ClassicRecipeDetailView, self).get_context_data(**kwargs)
         context['can_edit'] = self._can_edit()
@@ -1346,8 +1355,46 @@ class ClassicRecipeDetailView(SuccessMessageMixin, DetailView):
         context['layerbranch_desc'] = str(recipe.layerbranch.branch)
         context['to_desc'] = 'OpenEmbedded'
         context['recipes'] = [recipe, cover_recipe]
+
+        context['can_disposition_patches'] = self._can_disposition_patches()
+        if context['can_disposition_patches']:
+            nodisposition_ids = list(recipe.patch_set.filter(patchdisposition__isnull=True).values_list('id', flat=True))
+            patch_initial = [{'patch': p} for p in nodisposition_ids]
+            patch_formset = PatchDispositionFormSet(queryset=PatchDisposition.objects.filter(patch__recipe=recipe), initial=patch_initial, prefix='patchdispositiondialog')
+            patch_formset.extra = len(patch_initial)
+            context['patch_formset'] = patch_formset
         return context
 
+    def post(self, request, *args, **kwargs):
+        if not self._can_disposition_patches():
+            raise PermissionDenied
+
+        recipe = get_object_or_404(ClassicRecipe, pk=self.kwargs['pk'])
+        # What follows is a bit hacky, because we are receiving the form fields
+        # for just one of the forms in the formset which isn't really supported
+        # by Django
+        for field in request.POST:
+            if field.startswith('patchdispositiondialog'):
+                prefix = '-'.join(field.split('-')[:2])
+                instance = None
+                patchdisposition_id = request.POST.get('%s-id' % prefix, '')
+                if patchdisposition_id != '':
+                    instance = get_object_or_404(PatchDisposition, pk=int(patchdisposition_id))
+
+                form = PatchDispositionForm(request.POST, prefix=prefix, instance=instance)
+                if form.is_valid():
+                    instance = form.save(commit=False)
+                    instance.user = request.user
+                    instance.save()
+                    messages.success(request, 'Changes to patch %s saved successfully.' % instance.patch.src_path)
+                    return HttpResponseRedirect(reverse('comparison_recipe', args=(recipe.id,)))
+                else:
+                    # FIXME this is ugly because HTML gets escaped
+                    messages.error(request, 'Failed to save changes: %s' % form.errors)
+                break
+
+        return self.get(request, *args, **kwargs)
+
 
 class ClassicRecipeStatsView(TemplateView):
     def get_context_data(self, **kwargs):

templates/layerindex/classicrecipedetail.html

@@ -69,3 +69,68 @@
                         </tr>
                         {% endif %}
                         {% endblock %}
+
+                                    {% block patch_status_heading %}
+                                    {% if rcp.layerbranch.branch.comparison %}
+                                        {% if can_disposition_patches %}
+                                    <th class="col-md-3">Disposition</th>
+                                    <th></th>
+                                        {% endif %}
+                                    {% else %}
+                                    <th class="col-md-3">Status</th>
+                                    {% endif %}
+                                    {% endblock %}
+
+                                    {% block patch_status %}
+                                    {% if rcp.layerbranch.branch.comparison %}
+                                        {% if can_disposition_patches %}
+                                        <td>{{ patch.patchdisposition.get_disposition_display }}</td>
+                                        <td><a href="#patchDialog{{ patch.id }}" role="button" data-toggle="modal" class="btn btn-default pull-right patch_disposition_button" patch-id="{{ patch.id }}" patch-name="{{ patch.src_path }}">...</a></td>
+                                        {% endif %}
+                                    {% else %}
+                                    <td>{{ patch.get_status_display }} {{ patch.status_extra | urlize }}</td>
+                                    {% endif %}
+                                    {% endblock %}
+
+{% block content_extra %}
+{% if can_disposition_patches %}
+{% for form in patch_formset %}
+    {% with patch_id=form.patch.initial %}
+    <form id="patch_form_{{ patch_id }}" method="post">
+        <div id="patchDialog{{ patch_id }}" class="modal fade patchdialog" tabindex="-1" role="dialog" aria-labelledby="patchDialogLabel{{ patch_id }}">
+            <div class="modal-dialog" role="document">
+                <div class="modal-content">
+                    <div class="modal-header">
+                        <button type="button" class="close" data-dismiss="modal" aria-label="Close"><span aria-hidden="true">&times;</span></button>
+                        <h3 id="patchDialogLabel{{ patch_id }}">Dialog title</h3>
+                    </div>
+                    <div class="modal-body">
+                        {% csrf_token %}
+                        {{ form }}
+                    </div>
+                    <div class="modal-footer">
+                        <button class="btn btn-primary patchdialog-save" data-dismiss="modal" patch-id="{{ patch_id }}">Save</button>
+                        <button class="btn btn-default" data-dismiss="modal">Cancel</button>
+                    </div>
+                </div><!-- /.modal-content -->
+            </div><!-- /.modal-dialog -->
+        </div>
+    </form>
+    {% endwith %}
+{% endfor %}
+{% endif %}
+{% endblock %}
+
+{% block scripts_extra %}
+    $('.patch_disposition_button').click(function (e) {
+        patch_id = $(this).attr('patch-id');
+        patch_name = $(this).attr('patch-name');
+        $('#patchDialogLabel' + patch_id).html('Disposition patch ' + patch_name);
+    });
+    $('.patchdialog-save').click(function (e) {
+        patch_id = $(this).attr('patch-id');
+        $('#patch_form_' + patch_id).submit()
+        $('#patchDialog' + patch_id).modal('hide')
+    });
+{% endblock %}
+

templates/layerindex/comparisonrecipebase.html

@@ -179,14 +179,22 @@
                             <thead>
                                 <tr>
                                     <th>Patch</th>
+                                    {% block patch_status_heading %}
+                                    {% if not rcp.layerbranch.branch.comparison %}
                                     <th class="col-md-3">Status</th>
+                                    {% endif %}
+                                    {% endblock %}
                                 </tr>
                             </thead>
                             <tbody>
                                 {% for patch in rcp.patch_set.all %}
                                 <tr>
                                     <td><a href="{{ patch.vcs_web_url }}">{{ patch.src_path }}</a></td>
+                                    {% block patch_status %}
+                                    {% if not rcp.layerbranch.branch.comparison %}
                                     <td>{{ patch.get_status_display }} {{ patch.status_extra | urlize }}</td>
+                                    {% endif %}
+                                    {% endblock %}
                                 </tr>
                                 {% endfor %}
                             </tbody>
@@ -201,6 +209,9 @@
             </div>
         </div>
 
+{% block content_extra %}
+{% endblock %}
+
 {% endautoescape %}
 
 {% endblock %}
@@ -225,5 +236,7 @@
                 $('#id_span_cover_opts').removeClass('text-muted');
             }
         }
+{% block scripts_extra %}
+{% endblock %}
     </script>
 {% endblock %}