Implement editing of layers

Allow users with publish permission to edit any layer, and users with
the same email address as one of the maintainers of a layer to edit that
layer.

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

TODO

@@ -15,6 +15,7 @@ TODO:
  * Show unpublished layers in a different style in the dependency list?
 
 Later:
+* Allow adding/editing notes
 * Usage links in list page?
 * Avoid page content changing size depending on whether scrollbar is there or not?
 * Style/extend about page?
@@ -22,7 +23,6 @@ Later:
 * Style machine list on detail
 * Provide a delete function for unpublished layers?
 * Show count of layers to be reviewed next to review button
-* Ability for users to edit existing layers
 * Something to help with compatibility (although maybe this should just be handled using the existing versioned layer dependencies in layer.conf)
 * Query backend service? i.e. special URL to query information for external apps/scripts
 * Tool for finding/comparing duplicate recipes?

layerindex/detail.html

@@ -26,10 +26,15 @@
             <div class="row-fluid">
                 <div class="page-header">
                     <h1>{{ layeritem.name }}
-                        {% if user.is_authenticated and perms.layeritem.publish_layer %}
+                        {% if user.is_authenticated %}
-                            {% if layeritem.status = "N" %}
+                            <span class="pull-right">
-                                <a href="{% url publish layeritem.name %}" class="btn btn-primary pull-right">Publish layer</a>
+                                {% if perms.layeritem.publish_layer or useredit %}
-                            {% endif %}
+                                    <a href="{% url edit_layer layeritem.name %}" class="btn">Edit layer</a>
+                                {% endif %}
+                                {% if layeritem.status = "N" and perms.layeritem.publish_layer %}
+                                    <a href="{% url publish layeritem.name %}" class="btn btn-primary">Publish layer</a>
+                                {% endif %}
+                            </span>
                         {% endif %}
                     </h1>
                 </div>

layerindex/forms.py

@@ -40,12 +40,20 @@ LayerMaintainerFormSet = inlineformset_factory(LayerItem, LayerMaintainer, form=
 
 class SubmitLayerForm(forms.ModelForm):
     # Additional form fields
-    deps = forms.ModelMultipleChoiceField(label='Other layers this layer depends upon', queryset=LayerItem.objects.all(), required=False, initial=[l.pk for l in LayerItem.objects.filter(name='openembedded-core')])
+    deps = forms.ModelMultipleChoiceField(label='Other layers this layer depends upon', queryset=LayerItem.objects.all(), required=False)
 
     class Meta:
         model = LayerItem
         fields = ('name', 'layer_type', 'summary', 'description', 'vcs_url', 'vcs_subdir', 'vcs_web_url', 'vcs_web_tree_base_url', 'vcs_web_file_base_url', 'usage_url', 'mailing_list_url')
 
+    def __init__(self, *args, **kwargs):
+        super(self.__class__, self).__init__(*args, **kwargs)
+        if self.instance.pk:
+            self.fields['deps'].initial = [d.dependency.pk for d in self.instance.dependencies_set.all()]
+        else:
+            self.fields['deps'].initial = [l.pk for l in LayerItem.objects.filter(name='openembedded-core')]
+        self.was_saved = False
+
     def checked_deps(self):
         val = [int(v) for v in self['deps'].value()]
         return val

layerindex/models.py

@@ -75,6 +75,13 @@ class LayerItem(models.Model):
     def active_maintainers(self):
         return self.layermaintainer_set.filter(status='A')
 
+    def user_can_edit(self, user):
+        if user.is_authenticated():
+            for maintainer in self.active_maintainers():
+                if maintainer.email == user.email:
+                    return True
+        return False
+
     def __unicode__(self):
         return self.name
 

layerindex/submitlayer.html

@@ -1,5 +1,4 @@
-{% extends "base.html" %}
+{% extends "layerindex/editlayer.html" %}
-{% load i18n %}
 
 {% comment %}
 
@@ -20,157 +19,11 @@
 <li><a href="#">Submit layer</a></li>
 {% endblock %}
 
-{% block content %}
-{% autoescape on %}
 
+{% block formtag %}
 <form action="{% url submit_layer %}" method="post">
-{% csrf_token %}
+{% endblock %}
-    {% for hidden in form.hidden_fields %}
-        {{ hidden }}
-    {% endfor %}
 
-    {% for field in form.visible_fields %}
+{% block submitbuttons %}
-        <div class="fieldWrapper">
-            {{ field.errors }}
-            {{ field.label_tag }}
-            {% if field.name = 'deps' %}
-                <div class="scrolling">
-                    <table><tbody>
-                        {% for deplayer in deplistlayers %}
-                            {% if deplayer.id in form.checked_deps %}
-                                <tr>
-                                    <td><input type="checkbox" name="deps" value="{{ deplayer.id }}" id="id_deps_{{forloop.counter}}" checked="checked" /></td>
-                                    {% if deplayer.status = 'N' %}
-                                    <td><label class="muted" for="id_deps_{{forloop.counter}}">{{ deplayer.name }} (new)</label></td>
-                                    {% else %}
-                                    <td><label for="id_deps_{{forloop.counter}}">{{ deplayer.name }}</label></td>
-                                    {% endif %}
-                                </tr>
-                            {% endif %}
-                        {% endfor %}
-                        {% for deplayer in deplistlayers %}
-                            {% if not deplayer.id in form.checked_deps %}
-                                <tr>
-                                    <td><input type="checkbox" name="deps" value="{{ deplayer.id }}" id="id_deps_{{forloop.counter}}" /></td>
-                                    {% if deplayer.status = 'N' %}
-                                    <td><label class="muted" for="id_deps_{{forloop.counter}}">{{ deplayer.name }} (new)</label></td>
-                                    {% else %}
-                                    <td><label for="id_deps_{{forloop.counter}}">{{ deplayer.name }}</label></td>
-                                    {% endif %}
-                                </tr>
-                            {% endif %}
-                        {% endfor %}
-                    </tbody></table>
-                </div>
-            {% else %}
-                {{ field }}
-            {% endif %}
-            {{ field.help_text }}
-        </div>
-    {% endfor %}
-    <h3>Maintainers</h3>
-    {{ maintainerformset.non_form_errors }}
-    {{ maintainerformset.management_form }}
-    {% for maintainerform in maintainerformset %}
-        <h4>Maintainer {{forloop.counter}}</h4>
-        {% for hidden in maintainerform.hidden_fields %}
-            {{ hidden }}
-        {% endfor %}
-
-        {% for field in maintainerform.visible_fields %}
-            <div class="fieldWrapper">
-                {{ field.errors }}
-                {{ field.label_tag }}
-                {{ field }}
-                {{ field.help_text }}
-            </div>
-        {% endfor %}
-    {% endfor %}
 <input type="submit" value="Submit" class='btn' />
-</form>
-
-{% endautoescape %}
-
-{% endblock %}
-
-{% block scripts %}
-<script>
-    if (typeof String.prototype.startsWith != 'function') {
-        String.prototype.startsWith = function (str){
-            return this.slice(0, str.length) == str;
-        };
-    }
-
-    auto_web_fields = function (e) {
-        repoval = $('#id_vcs_url').val()
-        if( repoval[repoval.length-1] == '/' )
-            repoval = repoval.slice(0, repoval.length-1)
-
-        if( repoval.startsWith('git://git.openembedded.org/') ) {
-            reponame = repoval.replace(/^.*\//, '')
-            $('#id_vcs_web_url').val('http://cgit.openembedded.org/cgit.cgi/' + reponame)
-            $('#id_vcs_web_tree_base_url').val('http://cgit.openembedded.org/cgit.cgi/' + reponame + '/tree/')
-            $('#id_vcs_web_file_base_url').val('http://cgit.openembedded.org/cgit.cgi/' + reponame + '/tree/')
-        }
-        else if( repoval.indexOf('git.yoctoproject.org/') > -1 ) {
-            reponame = repoval.replace(/^.*\//, '')
-            $('#id_vcs_web_url').val('http://git.yoctoproject.org/cgit/cgit.cgi/' + reponame)
-            $('#id_vcs_web_tree_base_url').val('http://git.yoctoproject.org/cgit/cgit.cgi/' + reponame + '/tree/')
-            $('#id_vcs_web_file_base_url').val('http://git.yoctoproject.org/cgit/cgit.cgi/' + reponame + '/tree/')
-        }
-        else if( repoval.indexOf('github.com/') > -1 ) {
-            reponame = repoval.replace(/^.*github.com\//, '')
-            reponame = reponame.replace(/.git$/, '')
-            $('#id_vcs_web_url').val('http://github.com/' + reponame)
-            $('#id_vcs_web_tree_base_url').val('http://github.com/' + reponame + '/tree/master/')
-            $('#id_vcs_web_file_base_url').val('http://github.com/' + reponame + '/blob/master/')
-        }
-        else if( repoval.indexOf('gitorious.org/') > -1 ) {
-            reponame = repoval.replace(/^.*gitorious.org\//, '')
-            reponame = reponame.replace(/.git$/, '')
-            $('#id_vcs_web_url').val('http://gitorious.org/' + reponame)
-            $('#id_vcs_web_tree_base_url').val('http://gitorious.org/' + reponame + '/trees/master/')
-            $('#id_vcs_web_file_base_url').val('http://gitorious.org/' + reponame + '/blobs/master/')
-        }
-        else if( repoval.indexOf('bitbucket.org/') > -1 ) {
-            reponame = repoval.replace(/^.*bitbucket.org\//, '')
-            reponame = reponame.replace(/.git$/, '')
-            $('#id_vcs_web_url').val('http://bitbucket.org/' + reponame)
-            $('#id_vcs_web_tree_base_url').val('http://bitbucket.org/' + reponame + '/src/master/%path%?at=master')
-            $('#id_vcs_web_file_base_url').val('http://bitbucket.org/' + reponame + '/src/master/%path%?at=master')
-        }
-    };
-
-    split_email = function() {
-        // Split email name/email address pairs
-        name_input = $(this)
-        split_regex = /^"?([^"@$<>]+)"? *<([^<> ]+)>[ -]*(.*)?$/
-        matches = split_regex.exec(name_input.val())
-        if( matches ){
-            name_input.val($.trim(matches[1]))
-            email_id = name_input.attr('id').replace('-name', '-email')
-            $('#' + email_id).val($.trim(matches[2]))
-            resp_id = email_id.replace('-email', '-responsibility')
-            currval = $('#' + resp_id).val()
-            // Set the responsibility with the remainder of the value unless the user has entered a value for
-            // responsibility already
-            if( currval == window['last_' + resp_id] || currval == "" ) {
-                newval = $.trim(matches[3])
-                $('#' + resp_id).val(newval)
-                window['last_' + resp_id] = newval
-            }
-        }
-    }
-
-    for(i=0;i<{{ maintainerformset.total_form_count }};i++) {
-        name_input = $('#id_layermaintainer_set-' + i + '-name')
-        name_input.change(split_email)
-        resp_id = 'id_layermaintainer_set-' + i + '-responsibility'
-        window['last_' + resp_id] = ""
-    }
-
-    $(document).ready(function() {
-        $('#id_vcs_url').change(auto_web_fields)
-    });
-</script>
 {% endblock %}

layerindex/urls.py

@@ -7,14 +7,15 @@
 from django.conf.urls.defaults import *
 from django.views.generic import DetailView, ListView
 from layerindex.models import LayerItem, Recipe
-from layerindex.views import LayerListView, RecipeSearchView, MachineSearchView, PlainTextListView, LayerDetailView
+from layerindex.views import LayerListView, RecipeSearchView, MachineSearchView, PlainTextListView, LayerDetailView, edit_layer_view
 
 urlpatterns = patterns('',
     url(r'^$',
         LayerListView.as_view(
             template_name='layerindex/index.html'),
             name='layer_list'),
-    url(r'^submit/$', 'layerindex.views.submit_layer', name="submit_layer"),
+    url(r'^submit/$', edit_layer_view, {'template_name': 'layerindex/submitlayer.html'}, name="submit_layer"),
+    url(r'^edit/(?P<slug>[-\w]+)/$', edit_layer_view, {'template_name': 'layerindex/editlayer.html'}, name="edit_layer"),
     url(r'^submit/thanks$', 'layerindex.views.submit_layer_thanks', name="submit_layer_thanks"),
     url(r'^recipes/$',
         RecipeSearchView.as_view(

layerindex/views.py

@@ -23,46 +23,70 @@ import simplesearch
 import settings
 
 
-def submit_layer(request):
+def edit_layer_view(request, template_name, slug=None):
-    if request.method == 'POST':
+    useredit = False
+    if slug:
+        # Edit mode
+        layeritem = get_object_or_404(LayerItem, name=slug)
+        if not (request.user.is_authenticated() and (request.user.has_perm('layerindex.publish_layer') or layeritem.user_can_edit(request.user))):
+            raise PermissionDenied
+    else:
+        # Submit mode
         layeritem = LayerItem()
+
+    if request.method == 'POST':
         form = SubmitLayerForm(request.POST, instance=layeritem)
         maintainerformset = LayerMaintainerFormSet(request.POST, instance=layeritem)
         if form.is_valid() and maintainerformset.is_valid():
             with transaction.commit_on_success():
                 form.save()
                 maintainerformset.save()
-                # Save dependencies
+                if slug:
-                for dep in form.cleaned_data['deps']:
+                    new_deps = form.cleaned_data['deps']
-                    deprec = LayerDependency()
+                    existing_deps = [deprec.dependency for deprec in layeritem.dependencies_set.all()]
-                    deprec.layer = layeritem
+                    for dep in new_deps:
-                    deprec.dependency = dep
+                        if dep not in existing_deps:
-                    deprec.save()
+                            deprec = LayerDependency()
-                # Send email
+                            deprec.layer = layeritem
-                plaintext = get_template('layerindex/submitemail.txt')
+                            deprec.dependency = dep
-                perm = Permission.objects.get(codename='publish_layer')
+                            deprec.save()
-                users = User.objects.filter(Q(groups__permissions=perm) | Q(user_permissions=perm) ).distinct()
+                    for dep in existing_deps:
-                for user in users:
+                        if dep not in new_deps:
-                    d = Context({
+                            layeritem.dependencies_set.filter(dependency=dep).delete()
-                        'user_name': user.get_full_name(),
+                else:
-                        'layer_name': layeritem.name,
+                    # Save dependencies
-                        'layer_url': request.build_absolute_uri(reverse('layer_item', args=(layeritem.name,))),
+                    for dep in form.cleaned_data['deps']:
-                    })
+                        deprec = LayerDependency()
-                    subject = '%s - %s' % (settings.SUBMIT_EMAIL_SUBJECT, layeritem.name)
+                        deprec.layer = layeritem
-                    from_email = settings.SUBMIT_EMAIL_FROM
+                        deprec.dependency = dep
-                    to_email = user.email
+                        deprec.save()
-                    text_content = plaintext.render(d)
+                    # Send email
-                    msg = EmailMessage(subject, text_content, from_email, [to_email])
+                    plaintext = get_template('layerindex/submitemail.txt')
-                    msg.send()
+                    perm = Permission.objects.get(codename='publish_layer')
-                return HttpResponseRedirect(reverse('submit_layer_thanks'))
+                    users = User.objects.filter(Q(groups__permissions=perm) | Q(user_permissions=perm) ).distinct()
+                    for user in users:
+                        d = Context({
+                            'user_name': user.get_full_name(),
+                            'layer_name': layeritem.name,
+                            'layer_url': request.build_absolute_uri(reverse('layer_item', args=(layeritem.name,))),
+                        })
+                        subject = '%s - %s' % (settings.SUBMIT_EMAIL_SUBJECT, layeritem.name)
+                        from_email = settings.SUBMIT_EMAIL_FROM
+                        to_email = user.email
+                        text_content = plaintext.render(d)
+                        msg = EmailMessage(subject, text_content, from_email, [to_email])
+                        msg.send()
+                    return HttpResponseRedirect(reverse('submit_layer_thanks'))
+            form.was_saved = True
     else:
-        form = SubmitLayerForm()
+        form = SubmitLayerForm(instance=layeritem)
-        maintainerformset = LayerMaintainerFormSet()
+        maintainerformset = LayerMaintainerFormSet(instance=layeritem)
 
-    return render(request, 'layerindex/submitlayer.html', {
+    return render(request, template_name, {
         'form': form,
         'maintainerformset': maintainerformset,
-        'deplistlayers': LayerItem.objects.all().order_by('name')
+        'deplistlayers': LayerItem.objects.all().order_by('name'),
+        'useredit': useredit
     })
 
 def submit_layer_thanks(request):
@@ -98,7 +122,9 @@ class LayerDetailView(DetailView):
     model = LayerItem
     slug_field = 'name'
 
+    # This is a bit of a mess. Surely there has to be a better way to handle this...
     def dispatch(self, request, *args, **kwargs):
+        self.user = request.user
         res = super(LayerDetailView, self).dispatch(request, *args, **kwargs)
         l = self.get_object()
         if l:
@@ -107,6 +133,12 @@ class LayerDetailView(DetailView):
                     raise PermissionDenied
         return res
 
+    def get_context_data(self, **kwargs):
+        context = super(LayerDetailView, self).get_context_data(**kwargs)
+        layer = context['layeritem']
+        context['useredit'] = layer.user_can_edit(self.user)
+        return context
+
 class RecipeSearchView(ListView):
     context_object_name = 'recipe_list'
     paginate_by = 50