From fac44bc8123c0e2482fdc4a6ec4dc11b25ad9ca7 Mon Sep 17 00:00:00 2001
From: Paul Marillonnet <pmarillonnet@entrouvert.com>
Date: Fri, 5 Apr 2019 13:10:17 +0200
Subject: [PATCH 4/4] delete old values when set new multiple attribute values
 (#32025)

---
 src/authentic2/models.py      | 40 +++++++++++++++++++++++------------
 tests/test_attribute_kinds.py | 13 ++++++++++++
 tests/test_concurrency.py     |  2 +-
 3 files changed, 40 insertions(+), 15 deletions(-)

diff --git a/src/authentic2/models.py b/src/authentic2/models.py
index 2621c17e..4a523b1e 100644
--- a/src/authentic2/models.py
+++ b/src/authentic2/models.py
@@ -2,7 +2,7 @@ import time
 import uuid
 from django.utils.http import urlquote
 from django.conf import settings
-from django.db import models
+from django.db import models, transaction
 from django.db.models.query import Q
 from django.utils import six
 from django.utils.translation import ugettext_lazy as _
@@ -214,19 +214,31 @@ class Attribute(models.Model):
             assert isinstance(value, (list, set, tuple))
             values = value
             avs = []
-            for value in values:
-                content = serialize(value)
-                av, created = AttributeValue.objects.get_or_create(
-                        content_type=ContentType.objects.get_for_model(owner),
-                        object_id=owner.pk,
-                        attribute=self,
-                        multiple=True,
-                        content=content,
-                        defaults={'verified': verified})
-                if not created:
-                    av.verified = verified
-                    av.save()
-                avs.append(av)
+            content_list = []
+
+            with transaction.atomic():
+                for value in values:
+                    content = serialize(value)
+                    av, created = AttributeValue.objects.get_or_create(
+                            content_type=ContentType.objects.get_for_model(owner),
+                            object_id=owner.pk,
+                            attribute=self,
+                            multiple=True,
+                            content=content,
+                            defaults={'verified': verified})
+                    if not created:
+                        av.verified = verified
+                        av.save()
+                    avs.append(av)
+                    content_list.append(content)
+
+                AttributeValue.objects.filter(
+                    attribute=self,
+                    content_type=ContentType.objects.get_for_model(owner),
+                    object_id=owner.pk,
+                    multiple=True
+                ).exclude(content__in=content_list).delete()
+
             return avs
         else:
             content = serialize(value)
diff --git a/tests/test_attribute_kinds.py b/tests/test_attribute_kinds.py
index 58238d08..9102cf13 100644
--- a/tests/test_attribute_kinds.py
+++ b/tests/test_attribute_kinds.py
@@ -447,3 +447,16 @@ def test_profile_image(db, app, admin, mailoutbox, media):
     response = app.get('/accounts/edit/')
     form = response.form
     assert form['edit-profile-cityscape_image'].attrs['accept'] == 'image/*'
+
+
+def test_multiple_attribute_setter(db, app, simple_user):
+    nicks = Attribute.objects.create(name='nicknames', label='Nicknames', kind='string',
+                             required=False, multiple=True, user_visible=True, user_editable=True)
+
+    simple_user.attributes.nicknames = ['Roger', 'Tony', 'Robie']
+    simple_user.save()
+    assert 'Tony' in [atv.content for atv in simple_user.attribute_values.filter(attribute=nicks)]
+
+    simple_user.attributes.nicknames = ['Roger', 'Timmy', 'Robie']
+    simple_user.save()
+    assert not 'Tony' in [atv.content for atv in simple_user.attribute_values.filter(attribute=nicks)]
diff --git a/tests/test_concurrency.py b/tests/test_concurrency.py
index fe16dcfd..a8e278dd 100644
--- a/tests/test_concurrency.py
+++ b/tests/test_concurrency.py
@@ -41,7 +41,7 @@ def test_attribute_value_uniqueness(migrations, transactional_db, simple_user, c
         connection.close()
     map_threads(f, range(concurrency))
     map_threads(f, range(concurrency))
-    assert AttributeValue.objects.filter(attribute=multiple_at).count() == concurrency
+    assert AttributeValue.objects.filter(attribute=multiple_at).count() == 1
 
     def f(i):
         simple_user.attributes.single = str(i)
-- 
2.20.1

