Merge pull request #206 from editorsnotes/fix-editing-bugs

Fix editing bugs

Author: ptgolden

editorsnotes/admin_custom/forms/topics.py

@@ -16,15 +16,10 @@ class Media:
         )
     class Meta:
         model = Topic
-        fields = ('preferred_name', 'topic_type',)
+        fields = ('preferred_name', 'topic_type', 'summary',)
         widgets = {
             'topic': forms.HiddenInput()
         }
-    def __init__(self, *args, **kwargs):
-        super(TopicForm, self).__init__(*args, **kwargs)
-        instance = kwargs.get('instance', None)
-        if instance is not None:
-            self.fields['topic_type'].initial = instance.topic.type
 
 AlternateNameFormset = models.inlineformset_factory(
     Topic, AlternateName, fields=('name',), extra=1)

editorsnotes/admin_custom/static/function/admin-bootstrap-topic.js

@@ -1,7 +1,8 @@
 $(document).ready(function() {
-  var editor = new wysihtml5.Editor('id_topicsummary-0-content', {
+  var editor = new wysihtml5.Editor('id_summary', {
     toolbar: 'summary-toolbar',
     parserRules: wysihtml5ParserRules,
+    useLineBreaks: false,
     stylesheets: ['/static/function/wysihtml5/stylesheet.css']
   });
 });

editorsnotes/admin_custom/templates/topic_admin.html

@@ -1,5 +1,7 @@
 {% extends "admin_base.html" %}
 
+{% load display %}
+
 {% block page_title %}
 <h3>{% if form.instance.id %}Change {% else %}Add {% endif %}topic</h3>
 {% endblock %}
@@ -9,12 +11,11 @@ <h3>{% if form.instance.id %}Change {% else %}Add {% endif %}topic</h3>
 
     {% include "includes/bootstrap_errors.html" with errors=form.non_field_errors %}
 
-    <fieldset>
-      <legend>Project</legend>
-      <div class="fieldset-content">
-        {{ project }}
-      </div>
-    </fieldset>
+    {% if not form.instance.pk and form.instance.topic_node_id %}
+    <div class="well">
+      Connecting to topic node {{ form.instance.topic_node|as_link }}
+    </div>
+    {% endif %}
 
     <fieldset name="preferred_name_fields">
       <legend>Name</legend>
@@ -51,7 +52,7 @@ <h3>{% if form.instance.id %}Change {% else %}Add {% endif %}topic</h3>
       {% include "includes/bootstrap_errors.html" with errors=form.summary.errors %}
       <div class="fieldset-content article">
         {% include "includes/wysihtml5_full_toolbar.html" with toolbar_id="summary-toolbar" %}
-        {{ formsets.topicsummary.0.content }}
+        {{ form.summary }}
       </div>
 
       <div class="article-citations" style="margin: 1em .5em;">

editorsnotes/admin_custom/tests.py

@@ -0,0 +1,75 @@
+# -*- coding: utf-8 -*-
+
+from django.core.urlresolvers import reverse
+
+from django_webtest import WebTest
+
+from editorsnotes.main.tests import create_test_user
+from editorsnotes.main import models as main_models
+
+class TopicAdminTestCase(WebTest):
+    fixtures = ['projects.json']
+    def test_create_topic(self):
+        "A user should be able to create a topic with a unique name."
+        topic_name = u'Emma Goldman'
+        add_url = reverse('admin:main_topic_add', kwargs={'project_slug': 'emma'})
+
+        form = self.app.get(add_url, user='barry').forms[1]
+        form['preferred_name'] = topic_name
+
+        resp = form.submit()
+        self.assertEqual(resp.status_code, 302)
+        resp = resp.follow()
+        self.assertEqual(resp.context['topic'].preferred_name, topic_name)
+
+        # Duplicate should raise an error
+        duplicate_form = self.app.get(add_url, user='barry').forms[1]
+        form['preferred_name'] = topic_name
+        resp = form.submit()
+        self.assertEqual(resp.status_code, 200)
+
+        error_messages = resp.forms[1].html.select('.error-message li')
+        self.assertEqual(1, len(error_messages))
+        self.assertEqual(error_messages[0].text.strip(),
+                         u'Topic with this preferred name already exists.')
+
+    def test_create_empty_topic_error(self):
+        "A user should not be able to create a topic with an empty name."
+        add_url = reverse('admin:main_topic_add', kwargs={'project_slug': 'emma'})
+
+        # Submit a blank form
+        resp = self.app.get(add_url, user='barry').forms[1].submit()
+        self.assertEqual(resp.status_code, 200)
+
+        error_messages = resp.forms[1].html.select('.error-message li')
+        self.assertEqual(1, len(error_messages))
+        self.assertEqual(error_messages[0].text.strip(), u'This field is required.')
+
+    def test_connect_to_topic_node(self):
+        "A user should create a new topic connecting to an existing topic node."
+        node_name = u'Емма Голдман'
+        topic_node = main_models.TopicNode.objects.create(
+            _preferred_name=node_name, creator_id=2, last_updater_id=2)
+
+        add_url = reverse('admin:main_topic_add', kwargs={'project_slug': 'emma'})
+        add_url += '?topic_node={}'.format(topic_node.id)
+
+        resp = self.app.get(add_url, user='barry')
+        form = resp.forms[1]
+        self.assertEqual(form['preferred_name'].value, node_name)
+
+        form['preferred_name'] = 'Emma Goldman'
+        resp = form.submit().follow()
+        self.assertEqual(resp.context['topic'].topic_node_id, topic_node.id)
+
+        # doing this again should fail, since the project is already connected
+        # to the node
+        form = self.app.get(add_url, user='barry').forms[1]
+        form['preferred_name'] = u'shouldn\'t matter'
+        resp = form.submit()
+        self.assertEqual(resp.status_code, 200)
+        error_messages = resp.forms[1].html.select('.error-message li')
+        self.assertEqual(1, len(error_messages))
+        self.assertEqual(error_messages[0].text.strip(),
+                         u'This project is already connected with topic node '
+                         '{}.'.format(topic_node))

editorsnotes/admin_custom/tests.py.old

@@ -94,8 +94,8 @@ def post(self, request, *args, **kwargs):
         return super(BaseAdminView, self).post(request, *args, **kwargs)
     def get_default_perms(self):
         opts = self.model._meta
-        perm = opts.get_add_permission if self.object is None \
-                else opts.get_change_permission()
+        add_action = self.object is None or not self.object.id
+        perm = opts.get_add_permission() if add_action else opts.get_change_permission()
         return ['{}.{}'.format(opts.app_label, perm)]
     def check_perms(self):
         perms = getattr(self, 'permissions', self.get_default_perms())
@@ -109,9 +109,27 @@ def get_form_kwargs(self):
         kwargs = super(ModelFormMixin, self).get_form_kwargs()
         if hasattr(self, 'object') and self.object:
             kwargs.update({'instance': self.object})
+        else:
+            has_project_field = all([
+                hasattr(self.model, 'project'),
+                hasattr(self.model.project, 'field'),
+                hasattr(self.model.project.field, 'related'),
+                self.model.project.field.related.parent_model == Project
+            ])
+            if has_project_field:
+                # Then create an instance with the project already set
+                instance = self.model(project=self.project)
+                kwargs.update({ 'instance': instance })
+
         return kwargs
 
-    def set_additional_object_properties(self, obj):
+    def set_additional_object_properties(self, obj, form):
+        """
+        This must be called for additional object properties which must be set
+        but are not edited by the user. A common example is an item's last
+        updater or creator. It is called *after* the object instance is already
+        cleaned.
+        """
         return obj
 
     def save_object(self, form, formsets):
@@ -135,7 +153,6 @@ def form_valid(self, form, formsets):
 
             self.save_formsets(formsets)
             form.save_m2m()
-
         return redirect(self.get_success_url())
     def form_invalid(self, form, formsets):
         return self.render_to_response(

editorsnotes/admin_custom/views/common.py

@@ -106,7 +106,7 @@ def get_object(self, document_id):
         self.document = get_object_or_404(
             Document, id=document_id, project_id=self.project.id)
         return self.document.transcript if self.document.has_transcript() else None
-    def set_additional_object_properties(self, obj):
+    def set_additional_object_properties(self, obj, form):
         obj.document = self.document
         return obj
     def save_object(self, form, formsets):

editorsnotes/admin_custom/views/documents.py

@@ -40,9 +40,6 @@ def get_breadcrumb(self):
                 ('Edit', None)
             )
         return breadcrumbs
-    def set_additional_object_properties(self, obj, form):
-        obj.project = self.project
-        return obj
     def get_form(self, form_class):
         form = form_class(**self.get_form_kwargs())
         form.fields['assigned_users'].queryset = self.project.members.all()

editorsnotes/admin_custom/views/notes.py

@@ -13,7 +13,6 @@ class TopicAdminView(BaseAdminView):
     form_class = forms.TopicForm
     formset_classes = (
         forms.topics.AlternateNameFormset,
-        #forms.topics.TopicSummaryFormset,
         # forms.TopicAssignmentFormset,
         forms.topics.CitationFormset,
     )
@@ -28,28 +27,37 @@ def get_breadcrumb(self):
                                kwargs={'project_slug': self.project.slug})),
         )
         if self.object is None:
-            breadcrumbs += (
-                ('Add', None),
-            )
+            breadcrumbs += ( ('Add', None),)
         else:
             breadcrumbs += (
                 (self.object.as_text(), self.object.get_absolute_url()),
                 ('Edit', None)
             )
         return breadcrumbs
+    def get_form_kwargs(self):
+        """
+        Set topic node if it's been included in the request. Needs to be set now
+        so that it can be validated unique within a project.
+        """
+        kwargs = super(TopicAdminView, self).get_form_kwargs()
+        instance = kwargs.get('instance', None)
+        if instance and not instance.id and 'topic_node' in self.request.GET:
+            topic_node_id = self.request.GET.get('topic_node')
+            topic_node = get_object_or_404(TopicNode, id=topic_node_id)
+            instance.topic_node = topic_node
+            if 'initial' not in kwargs:
+                kwargs['initial'] = {}
+            kwargs['initial']['preferred_name'] = topic_node.preferred_name
+            kwargs['initial']['topic_type'] = topic_node.type
+        return kwargs
     def set_additional_object_properties(self, obj, form):
-        if not obj.id:
-            existing_node_id = self.request.GET.get('topic_node')
-            if existing_node_id:
-                topic_node = get_object_or_404(TopicNode, id=existing_node_id)
-            else:
-                topic_node = TopicNode.objects.create(
-                    _preferred_name=obj.preferred_name,
-                    type=form.cleaned_data['topic_type'],
-                    creator=self.request.user,
-                    last_updater=self.request.user)
-            obj.topic_node_id = topic_node.id
-            obj.project_id = self.project.id
+        if not obj.id and not obj.topic_node_id:
+            topic_node = TopicNode.objects.create(
+                _preferred_name=obj.preferred_name,
+                type=form.cleaned_data['topic_type'],
+                creator=self.request.user,
+                last_updater=self.request.user)
+            obj.topic_node = topic_node
         return obj
     def save_citation_formset_form(self, form):
         obj = form.save(commit=False)