Merge branch '50-la-methode-update-ne-fonctionne-pas' into 'rp2'

Resolve "La méthode update ne fonctionne pas"

See merge request !47

apps/rp/api/serializers.py

-import json
-
+import six
 from rest_framework import serializers
+from taggit_serializer.serializers import (TagListSerializerField,
+                                           TaggitSerializer)
 
-from taggit.models import Tag
 from rp.models import Article
 
 
-class TagListSerializer(serializers.ModelSerializer):
-    name = serializers.CharField(max_length=200)
+class ArticleTagListSerializerField(TagListSerializerField):
+    # We need this to fix a serializer issue:
+    # https://stackoverflow.com/questions/52695298/using-django-taggit-and-django-taggit-serializer-with-issue
+    def to_internal_value(self, value):
+        if isinstance(value, six.string_types):
+            value = value.split(',')
 
-    class Meta:
-        model = Tag
-        fields = ('name', )
+        if not isinstance(value, list):
+            self.fail('Not a list', input_type=(value).__name__)
+
+        for s in value:
+            if not isinstance(s, six.string_types):
+                self.fail('Not a string')
+
+            self.child.run_validation(s)
+        return value
 
-    def to_representation(self, value):
-        return json.dumps({'name': value.name})
+    def to_representation(self, obj):
+        if not isinstance(obj, list):
+            return [tag.name for tag in obj.all()]
+        return obj
 
 
-class ArticleSerializer(serializers.ModelSerializer):
+class ArticleSerializer(TaggitSerializer, serializers.ModelSerializer):
     #: List of short tags to describe the article (eg. "Privacy", "Copyright")
-    tags = TagListSerializer(help_text="""
+    tags = TagListSerializerField(help_text="""
         List of short tags to describe the article (eg."Privacy", "Copyright").
-        It must be a valid JSON list of items with a field named name.
+        It must be a valid JSON list of tags.
 
-        For instance [{"name": "Privacy"}, {"name": "Copyright"}]
-        """, many=True, required=False)
+        For instance ["Privacy", "Copyright"}]
+        """, required=False)
 
     class Meta:
         model = Article
@@ -37,3 +49,17 @@ class ArticleSerializer(serializers.ModelSerializer):
         if article is not None:
             article.save()
         return article
+
+    def update(self, instance, validated_data):
+        tags = validated_data.pop("tags", None)
+
+        # Let's update the classic fields of the
+        # instance first
+        for (k, v) in validated_data.items():
+            setattr(instance, k, v)
+
+        # Let's set the tags to what's provided
+        if tags:
+            instance.tags.set(*tags)
+        instance.save()
+        return instance

apps/rp/api/views.py

@@ -115,5 +115,5 @@ class ArticleTag(viewsets.ModelViewSet, mixins.ListModelMixin):
     serializer_class = ArticleSerializer
 
     def list(self, request, *args, **kwargs):
-        self.queryset = self.queryset.filter(tags__name__in=[kwargs['filter_tag']])
+        self.queryset = self.queryset.filter(tags__name__in=[kwargs['filter_tag']]).distinct()
         return super().list(request, args, kwargs)

apps/rp/factories.py

 import datetime
-from random import choice
+from random import choice, randint
 
 import pytz
 import factory
 from factory.fuzzy import FuzzyDateTime, FuzzyChoice
+from taggit.models import Tag
 
 from .models import Article, STATUS_CHOICES
 
 
+class TagFactory(factory.django.DjangoModelFactory):
+    class Meta:
+        model = Tag
+        django_get_or_create = ('name',)
+
+    name = factory.Faker("word")
+
+
 class ArticleFactory(factory.django.DjangoModelFactory):
     class Meta:
         model = Article

apps/rp/models.py

 from django.db import models
 from django.utils.translation import ugettext_lazy as _
 from django.core import files
-from django.contrib.auth.decorators import permission_required
 
 from taggit.managers import TaggableManager
 from newspaper import Article as ArticleParser, ArticleException
@@ -239,7 +238,8 @@ class Article(models.Model):
 
         # Let's add the tags
         if tags:
-            article.tags.add(','.join([t['name'] for t in tags if len(t) > 0]))
+            article.tags.set(*tags)
+            article.save()
         try:
             r = requests.get(url, timeout=0.5)
             article.original_status = r.status_code

apps/rp/tests/test_article.py

+from random import randint
 import json
 
 from django.test import TestCase, Client
+from django.core import serializers
 from django.contrib.auth.models import User, Permission
 from rest_framework.test import APIClient
 
@@ -13,7 +15,10 @@ from rp.views.articles import ArticleList
 class TestArticle(TestCase):
     def setUp(self):
         self.article = ArticleFactory(archive=False, quote=False, speak=False)
-        self.newarticle = ArticleFactory(status='NEW', archive=False, quote=False, speak=False)
+        self.newarticle = ArticleFactory(status='NEW',
+                                         archive=False,
+                                         quote=False,
+                                         speak=False)
 
     def test_init(self):
         assert RpConfig.name == "rp"
@@ -86,11 +91,18 @@ class TestArticle(TestCase):
         assert self.newarticle.speak
         assert self.newarticle.quote
 
+    def test_update(self):
+        old_title = self.article.title
+        self.article.title = old_title[::-1]
+        self.article.save()
+        self.article.refresh_from_db()
+        assert self.article.title != old_title
+
 
 class TestArticleViews(TestCase):
     def setUp(self):
         self.client = Client()
-        self.articles = [ArticleFactory(tags=['Tag 1', 'Tag2'])
+        self.articles = [ArticleFactory()
                          for i in range(0, 2 * ArticleList.paginate_by)]
         self.user = User.objects.create(username="test",
                                         email="test@example.org",
@@ -112,9 +124,9 @@ class TestArticleViews(TestCase):
             assert len(r.context['object_list']) == 0
 
     def test_filter_tag(self):
-        tag = self.articles[0].tags.all()[1]
+        tag = 'Tag 1'
 
-        r = self.client.get('/rp/by-tag/{}'.format(tag.name))
+        r = self.client.get('/rp/by-tag/{}'.format(tag))
         if r.context['is_paginated']:
             assert len(r.context['object_list']) == ArticleList.paginate_by
         else:
@@ -179,11 +191,28 @@ class TestArticleViews(TestCase):
         r = self.client.get('/rp/article/view/{}'.format(a.pk))
         assert r.context['object'] == a
 
+    def test_article_edit_unauth(self):
+        a = self.articles[0]
+        r = self.client.post('/rp/article/edit/{}'.format(a.pk))
+        assert r.status_code == 403
+
+    def test_article_edit(self):
+        self.user.user_permissions.add(Permission.objects.get(
+            codename='can_edit'))
+        self.client.force_login(user=self.user)
+
+        a = json.loads(serializers.serialize('json', [self.articles[0]]))[0]
+        a['fields']['title'] = 'Zog Zog'
+        r = self.client.post('/rp/article/edit/{}/'.format(a['pk']), a['fields'])
+        a = Article.objects.get(pk=a['pk'])
+        assert r.status_code == 302  # We're redirecting after edit
+        assert a.title == 'Zog Zog'
+
 
 class TestArticleApi(TestCase):
     def setUp(self):
         self.client = APIClient()
-        self.articles = [ArticleFactory(tags=['Tag 1', 'Tag2'])
+        self.articles = [ArticleFactory(tags=["Tag {}".format(n) for n in range(randint(1, 10))])
                          for i in range(0, 2 * ArticleList.paginate_by)]
         self.user = User.objects.create(username="test",
                                         email="test@example.org",
@@ -191,6 +220,7 @@ class TestArticleApi(TestCase):
         self.jedi = User.objects.create(username="obiwan",
                                         email="o.kennoby@example.org",
                                         password="Thisaintthedroidyourelookin")
+
         for a in self.articles:
             a.save()
 
@@ -204,16 +234,49 @@ class TestArticleApi(TestCase):
         assert r.status_code == 200
         assert r.data['count'] == 2 * ArticleList.paginate_by
 
+    def test_api_edit(self):
+        self.user.user_permissions.add(Permission.objects.get(
+            codename='can_edit'))
+        self.client.force_login(user=self.user)
+
+        pk = self.articles[0].pk
+        article = {'title': 'Zog Zog',
+                   'url': 'https://article.org/Zog+Zog'}
+        r = self.client.put('/api/articles/{}/'.format(pk),
+                             article,
+                             format='json')
+
+        assert r.status_code == 200
+        assert Article.objects.get(pk=pk).title == 'Zog Zog'
+
+    def test_api_edit_tags(self):
+        # Checking that we indeed change the tags
+        self.user.user_permissions.add(Permission.objects.get(
+            codename='can_edit'))
+        self.client.force_login(user=self.user)
+
+        pk = self.articles[0].pk
+        article = {'title': 'Zog Zog',
+                   'url': 'https://article.org/Zog+Zog',
+                   'tags': '["New Tag 1", "New Tag 2"]'}
+        r = self.client.put('/api/articles/{}/'.format(pk),
+                            article,
+                            format='json')
+        assert r.status_code == 200
+        a = Article.objects.get(pk=pk)
+        assert list(a.tags.values('name',).order_by('name')) == [{'name': 'New Tag 1'},
+                                                {'name': 'New Tag 2'}]
+
     def test_api_filter_tag(self):
-        tag = self.articles[0].tags.all()[1]
+        tag = 'Tag 1'
+        tagged = Article.objects.filter(tags__name__in=[tag]).count()
 
-        # All articles have Tag2 as a tag
-        r = self.client.get('/api/articles-by-tag/{}/'.format(tag.name))
+        r = self.client.get('/api/articles-by-tag/{}/'.format(tag))
         assert r.status_code == 200
-        assert r.data['count'] == 2 * ArticleList.paginate_by
+        assert r.data['count'] == tagged
 
         # Case sensitivity checking - tags are sensitive to case
-        r = self.client.get('/api/articles-by-tag/{}/'.format(tag.name.lower()))
+        r = self.client.get('/api/articles-by-tag/{}/'.format(tag.upper()))
         assert r.status_code == 200
         assert r.data['count'] == 0
 
@@ -223,7 +286,6 @@ class TestArticleApi(TestCase):
         assert r.data['count'] == 0
 
     def test_api_filter_search(self):
-        # text = ' '.join(self.articles[0].extracts.split(' ')[:10])
         text = self.articles[0].title
         r = self.client.get('/api/articles/', {'q': text})
         assert r.status_code == 200
@@ -236,29 +298,28 @@ class TestArticleApi(TestCase):
         assert r.data['count'] == len(self.articles)
 
     def test_api_tag_push_unauth(self):
-        a = ArticleFactory(tags=['ZogZog'],)
+        a = ArticleFactory()
         r = self.client.post('/api/articles/',
                              {'url': a.url, 'title': a.title,
-                              'tags': a.tags.all().values('name')},
+                              'tags': '["Tag 1", "Tag 2"]'},
                              format='json')
         assert r.status_code == 401
 
     def test_api_tag_push_auth(self):
         self.client.force_login(user=self.user)
-        a = ArticleFactory(tags=['ZogZog', 'Blip Blop'], status='NEW')
+        a = ArticleFactory(status='NEW')
         r = self.client.post('/api/articles/',
                              {'url': a.url, 'title': a.title,
-                              'tags': a.tags.all().values('name')},
+                              'tags': a.tags.all()},
                              format='json')
         assert r.status_code == 201
-        assert list(a.tags.all().values('name')) == [json.loads(t) for t in r.data['tags']]
+        assert list(a.tags.all()) == r.data['tags']
 
         # Need to test if we keep the tags
         r = self.client.post('/api/articles/',
-                             {'url': a.url, 'title': a.title,
-                              },
+                             {'url': a.url, 'title': a.title},
                              format='json')
-        assert list(a.tags.all().values('name')) == [json.loads(t) for t in r.data['tags']]
+        assert list(a.tags.all()) == r.data['tags']
 
     def test_api_recover(self):
         # Can we recover if we're no Jedis
@@ -267,7 +328,7 @@ class TestArticleApi(TestCase):
         r = self.client.post('/api/articles/{}/recover/'.format(a.id))
         assert r.status_code == 403
 
-        # Can we recovr a NEW article and force it to DRAFT?
+        # Can we recover a NEW article and force it to DRAFT?
         self.user.user_permissions.add(Permission.objects.get(
             codename='can_change_status'))
         self.client.force_login(user=self.user)

apps/rp/views/articles.py

@@ -161,7 +161,7 @@ class ArticleEdit(LoginRequiredMixin, PermissionRequiredMixin, UpdateView):
         return self.render_to_response(context)
 
     def form_valid(self, form):
-        self.object = form.save()
+        self.object = form.save(commit=False)
 
         if "preview" in self.request.POST:
             self.success_url = reverse("rp:article-preview", args=[self.object.id])

project/settings/apps.py

@@ -20,6 +20,7 @@ CONTRIB_APPS = [
     "rest_framework.authtoken",
 
     "taggit",
+    "taggit_serializer",
     "crispy_forms",
     "django_markdown2",
     "sorl.thumbnail",

requirements.txt

@@ -3,6 +3,7 @@ djangorestframework==3.6.3
 django-extensions==1.7.9
 django-imagekit==4.0
 django-taggit==0.22.0
+django-taggit-serializer
 Pillow==4.1.0
 selenium
 newspaper3k