From e7696f8b3063c6fc4ef7129e714e8ea6f9e02439 Mon Sep 17 00:00:00 2001 From: Tonye Jack Date: Wed, 24 Mar 2021 19:45:38 -0400 Subject: [PATCH 1/3] Increased test coverage. --- model_clone/__init__.py | 6 ++- model_clone/tests/__init__.py | 0 .../{tests.py => tests/test_clone_mixin.py} | 0 .../tests/test_create_copy_of_instance.py | 40 ++++++++++++++ model_clone/utils.py | 54 ++++++++++--------- 5 files changed, 75 insertions(+), 25 deletions(-) create mode 100644 model_clone/tests/__init__.py rename model_clone/{tests.py => tests/test_clone_mixin.py} (100%) create mode 100644 model_clone/tests/test_create_copy_of_instance.py diff --git a/model_clone/__init__.py b/model_clone/__init__.py index a8dee106..250e3db5 100644 --- a/model_clone/__init__.py +++ b/model_clone/__init__.py @@ -4,5 +4,9 @@ from model_clone.admin import CloneModelAdmin, CloneModelAdminMixin from model_clone.mixins.clone import CloneMixin +from model_clone.utils import create_copy_of_instance -__all__ = ["CloneMixin", "CloneModelAdmin", "CloneModelAdminMixin"] +__all__ = [ + "CloneMixin", "CloneModelAdmin", "CloneModelAdminMixin", + "create_copy_of_instance", +] diff --git a/model_clone/tests/__init__.py b/model_clone/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/model_clone/tests.py b/model_clone/tests/test_clone_mixin.py similarity index 100% rename from model_clone/tests.py rename to model_clone/tests/test_clone_mixin.py diff --git a/model_clone/tests/test_create_copy_of_instance.py b/model_clone/tests/test_create_copy_of_instance.py new file mode 100644 index 00000000..69261c05 --- /dev/null +++ b/model_clone/tests/test_create_copy_of_instance.py @@ -0,0 +1,40 @@ +from django.contrib.auth import get_user_model +from django.core.exceptions import ValidationError +from django.db import IntegrityError +from django.test import TestCase +from django.utils.text import slugify + +from model_clone import create_copy_of_instance +from sample.models import Library, Book + +User = get_user_model() + + +class CreateCopyOfInstanceTestCase(TestCase): + @classmethod + def setUpTestData(cls): + cls.user1 = User.objects.create(username="user 1") + cls.user2 = User.objects.create(username="user 2") + + def test_cloning_model_with_custom_id(self): + instance = Library.objects.create(name="First library", user=self.user1) + clone = create_copy_of_instance(instance, attrs={"user": self.user2}) + + self.assertNotEqual(instance.pk, clone.pk) + self.assertEqual(clone.user, self.user2) + + def test_cloning_unique_fk_field_without_a_fallback_value_is_invalid(self): + name = "New Library" + instance = Library.objects.create(name=name, user=self.user1) + + with self.assertRaises(ValidationError): + create_copy_of_instance(instance) + + def test_cloning_excluded_field_without_a_fallback_value_is_invalid(self): + name = "New Library" + instance = Book.objects.create( + name=name, created_by=self.user1, slug=slugify(name) + ) + + with self.assertRaises(IntegrityError): + create_copy_of_instance(instance, exclude={'slug'}, attrs={"created_by": self.user2}) \ No newline at end of file diff --git a/model_clone/utils.py b/model_clone/utils.py index 1713f0d9..a48078cd 100644 --- a/model_clone/utils.py +++ b/model_clone/utils.py @@ -11,28 +11,30 @@ def create_copy_of_instance(instance, exclude=(), save_new=True, attrs=None): """ Clone an instance of `django.db.models.Model`. - Args: - instance(django.db.models.Model): The model instance to clone. - exclude(list|set): List or set of fields to exclude from unique validation. - save_new(bool): Save the model instance after duplication calling .save(). - attrs(dict): Kwargs of field and value to set on the duplicated instance. - - Returns: - (django.db.models.Model): The new duplicated instance. - - Examples: - >>> from django.contrib.auth import get_user_model - >>> from sample.models import Book - >>> instance = Book.objects.create(name='The Beautiful Life') - >>> instance.pk - 1 - >>> instance.name - "The Beautiful Life" - >>> duplicate = instance.make_clone(attrs={'name': 'Duplicate Book 2'}) - >>> duplicate.pk - 2 - >>> duplicate.name - "Duplicate Book 2" + :param instance: The model instance to clone. + :type instance: django.db.models.Model + :param exclude: List or set of fields to exclude from unique validation. + :type exclude: list|set + :param save_new: Save the model instance after duplication calling .save(). + :type save_new: bool + :param attrs: Kwargs of field and value to set on the duplicated instance. + :type attrs: dict + :return: The new duplicated instance. + :rtype: django.db.models.Model + + :example: + >>> from django.contrib.auth import get_user_model + >>> from sample.models import Book + >>> instance = Book.objects.create(name='The Beautiful Life') + >>> instance.pk + 1 + >>> instance.name + "The Beautiful Life" + >>> duplicate = instance.make_clone(attrs={'name': 'Duplicate Book 2'}) + >>> duplicate.pk + 2 + >>> duplicate.name + "Duplicate Book 2" """ defaults = {} @@ -52,13 +54,17 @@ def create_copy_of_instance(instance, exclude=(), save_new=True, attrs=None): if all( [ not f.auto_created, + not f.primary_key, f.concrete, f.editable, f not in instance.__class__._meta.related_objects, f not in instance.__class__._meta.many_to_many, ] ): - defaults[f.attname] = getattr(instance, f.attname, f.get_default()) + # Prevent duplicates + if f.name not in attrs: + defaults[f.attname] = getattr(instance, f.attname, f.get_default()) + defaults.update(attrs) new_obj = instance.__class__(**defaults) @@ -66,7 +72,7 @@ def create_copy_of_instance(instance, exclude=(), save_new=True, attrs=None): exclude = exclude or [ f.name for f in instance._meta.fields - if any([f.name not in defaults, f.has_default(), f.null]) + if any([all([f.name not in defaults, f.attname not in defaults]), f.has_default(), f.null]) ] try: From 82869511299afbca5e44bd79bafb9b9b154d05f7 Mon Sep 17 00:00:00 2001 From: Tonye Jack Date: Wed, 24 Mar 2021 19:46:05 -0400 Subject: [PATCH 2/3] Fixed lint errors. --- model_clone/__init__.py | 4 +++- model_clone/admin.py | 4 +--- model_clone/mixins/clone.py | 20 +++++-------------- model_clone/tests/test_clone_mixin.py | 8 ++------ .../tests/test_create_copy_of_instance.py | 16 +++++++-------- model_clone/utils.py | 8 ++++---- 6 files changed, 23 insertions(+), 37 deletions(-) diff --git a/model_clone/__init__.py b/model_clone/__init__.py index 250e3db5..cdcbbb49 100644 --- a/model_clone/__init__.py +++ b/model_clone/__init__.py @@ -7,6 +7,8 @@ from model_clone.utils import create_copy_of_instance __all__ = [ - "CloneMixin", "CloneModelAdmin", "CloneModelAdminMixin", + "CloneMixin", + "CloneModelAdmin", + "CloneModelAdminMixin", "create_copy_of_instance", ] diff --git a/model_clone/admin.py b/model_clone/admin.py index 1505cc25..c2d912e1 100644 --- a/model_clone/admin.py +++ b/model_clone/admin.py @@ -17,9 +17,7 @@ class CloneModelAdminMixin(object): def change_view(self, request, object_id, form_url="", extra_context=None): extra_context = extra_context or {} - extra_context[ - "include_duplicate_object_link" - ] = self.include_duplicate_object_link + extra_context["include_duplicate_object_link"] = self.include_duplicate_object_link if self.include_duplicate_object_link: to_field = request.POST.get(TO_FIELD_VAR, request.GET.get(TO_FIELD_VAR)) if to_field and not self.to_field_allowed(request, to_field): diff --git a/model_clone/mixins/clone.py b/model_clone/mixins/clone.py index 520e92e7..80ba697e 100644 --- a/model_clone/mixins/clone.py +++ b/model_clone/mixins/clone.py @@ -206,9 +206,7 @@ def check(cls, **kwargs): "Conflicting configuration.", hint=( 'Please provide either "_clone_fields"' - + ' or "_clone_excluded_fields" for model {}'.format( - cls.__name__ - ) + + ' or "_clone_excluded_fields" for model {}'.format(cls.__name__) ), obj=cls, id="{}.E002".format(ModelCloneConfig.name), @@ -221,9 +219,7 @@ def check(cls, **kwargs): "Conflicting configuration.", hint=( 'Please provide either "_clone_m2m_fields"' - + ' or "_clone_excluded_m2m_fields" for model {}'.format( - cls.__name__ - ) + + ' or "_clone_excluded_m2m_fields" for model {}'.format(cls.__name__) ), obj=cls, id="{}.E002".format(ModelCloneConfig.name), @@ -243,9 +239,7 @@ def check(cls, **kwargs): "Please provide either " + '"_clone_m2o_or_o2m_fields"' + " or " - + '"_clone_excluded_m2o_or_o2m_fields" for {}'.format( - cls.__name__ - ) + + '"_clone_excluded_m2o_or_o2m_fields" for {}'.format(cls.__name__) ), obj=cls, id="{}.E002".format(ModelCloneConfig.name), @@ -370,9 +364,7 @@ def make_clone(self, attrs=None, sub_clone=False): for field in one_to_one_fields: rel_object = getattr(self, field.related_name, None) if rel_object: - if hasattr(rel_object, "make_clone") and callable( - rel_object.make_clone - ): + if hasattr(rel_object, "make_clone") and callable(rel_object.make_clone): rel_object.make_clone( attrs={field.remote_field.name: duplicate}, sub_clone=True ) @@ -388,9 +380,7 @@ def make_clone(self, attrs=None, sub_clone=False): items = [] for item in getattr(self, field.related_name).all(): try: - item_clone = item.make_clone( - attrs={field.remote_field.name: duplicate} - ) + item_clone = item.make_clone(attrs={field.remote_field.name: duplicate}) except IntegrityError: item_clone = item.make_clone( attrs={field.remote_field.name: duplicate}, sub_clone=True diff --git a/model_clone/tests/test_clone_mixin.py b/model_clone/tests/test_clone_mixin.py index 7f087dac..f9b56a9e 100644 --- a/model_clone/tests/test_clone_mixin.py +++ b/model_clone/tests/test_clone_mixin.py @@ -60,9 +60,7 @@ def test_cloning_with_field_overridden(self): def test_cloning_using_auto_now_field_is_updated(self): name = "New Book" - instance = Book.objects.create( - name=name, created_by=self.user1, slug=slugify(name) - ) + instance = Book.objects.create(name=name, created_by=self.user1, slug=slugify(name)) new_name = "My New Book" clone = instance.make_clone(attrs={"name": new_name}) @@ -187,9 +185,7 @@ def test_cloning_unique_fields_is_valid(self): "{} {} {}".format(first_name, Author.UNIQUE_DUPLICATE_SUFFIX, 1), ) - @patch( - "sample.models.Author.USE_UNIQUE_DUPLICATE_SUFFIX", new_callable=PropertyMock - ) + @patch("sample.models.Author.USE_UNIQUE_DUPLICATE_SUFFIX", new_callable=PropertyMock) def test_cloning_unique_field_with_use_unique_duplicate_suffix_set_to_False( self, use_unique_duplicate_suffix_mock, diff --git a/model_clone/tests/test_create_copy_of_instance.py b/model_clone/tests/test_create_copy_of_instance.py index 69261c05..6d866b6e 100644 --- a/model_clone/tests/test_create_copy_of_instance.py +++ b/model_clone/tests/test_create_copy_of_instance.py @@ -19,22 +19,22 @@ def setUpTestData(cls): def test_cloning_model_with_custom_id(self): instance = Library.objects.create(name="First library", user=self.user1) clone = create_copy_of_instance(instance, attrs={"user": self.user2}) - + self.assertNotEqual(instance.pk, clone.pk) self.assertEqual(clone.user, self.user2) - + def test_cloning_unique_fk_field_without_a_fallback_value_is_invalid(self): name = "New Library" instance = Library.objects.create(name=name, user=self.user1) - + with self.assertRaises(ValidationError): create_copy_of_instance(instance) def test_cloning_excluded_field_without_a_fallback_value_is_invalid(self): name = "New Library" - instance = Book.objects.create( - name=name, created_by=self.user1, slug=slugify(name) - ) - + instance = Book.objects.create(name=name, created_by=self.user1, slug=slugify(name)) + with self.assertRaises(IntegrityError): - create_copy_of_instance(instance, exclude={'slug'}, attrs={"created_by": self.user2}) \ No newline at end of file + create_copy_of_instance( + instance, exclude={"slug"}, attrs={"created_by": self.user2} + ) diff --git a/model_clone/utils.py b/model_clone/utils.py index a48078cd..000a13fa 100644 --- a/model_clone/utils.py +++ b/model_clone/utils.py @@ -72,7 +72,9 @@ def create_copy_of_instance(instance, exclude=(), save_new=True, attrs=None): exclude = exclude or [ f.name for f in instance._meta.fields - if any([all([f.name not in defaults, f.attname not in defaults]), f.has_default(), f.null]) + if any( + [all([f.name not in defaults, f.attname not in defaults]), f.has_default(), f.null] + ) ] try: @@ -135,9 +137,7 @@ def generate_value(value, suffix, transform, max_length, max_attempts): yield get_value(value, suffix, transform, max_length, i) raise StopIteration( - "CloneError: max unique attempts for {} exceeded ({})".format( - value, max_attempts - ) + "CloneError: max unique attempts for {} exceeded ({})".format(value, max_attempts) ) From 08f6a3c8e3e733392eabec6693ad6b3a6969394f Mon Sep 17 00:00:00 2001 From: Lint Action Date: Wed, 24 Mar 2021 23:55:10 +0000 Subject: [PATCH 3/3] Fix code style issues with Black --- model_clone/admin.py | 4 +++- model_clone/mixins/clone.py | 20 ++++++++++++++----- model_clone/tests/test_clone_mixin.py | 8 ++++++-- .../tests/test_create_copy_of_instance.py | 4 +++- model_clone/utils.py | 10 ++++++++-- 5 files changed, 35 insertions(+), 11 deletions(-) diff --git a/model_clone/admin.py b/model_clone/admin.py index c2d912e1..1505cc25 100644 --- a/model_clone/admin.py +++ b/model_clone/admin.py @@ -17,7 +17,9 @@ class CloneModelAdminMixin(object): def change_view(self, request, object_id, form_url="", extra_context=None): extra_context = extra_context or {} - extra_context["include_duplicate_object_link"] = self.include_duplicate_object_link + extra_context[ + "include_duplicate_object_link" + ] = self.include_duplicate_object_link if self.include_duplicate_object_link: to_field = request.POST.get(TO_FIELD_VAR, request.GET.get(TO_FIELD_VAR)) if to_field and not self.to_field_allowed(request, to_field): diff --git a/model_clone/mixins/clone.py b/model_clone/mixins/clone.py index 80ba697e..520e92e7 100644 --- a/model_clone/mixins/clone.py +++ b/model_clone/mixins/clone.py @@ -206,7 +206,9 @@ def check(cls, **kwargs): "Conflicting configuration.", hint=( 'Please provide either "_clone_fields"' - + ' or "_clone_excluded_fields" for model {}'.format(cls.__name__) + + ' or "_clone_excluded_fields" for model {}'.format( + cls.__name__ + ) ), obj=cls, id="{}.E002".format(ModelCloneConfig.name), @@ -219,7 +221,9 @@ def check(cls, **kwargs): "Conflicting configuration.", hint=( 'Please provide either "_clone_m2m_fields"' - + ' or "_clone_excluded_m2m_fields" for model {}'.format(cls.__name__) + + ' or "_clone_excluded_m2m_fields" for model {}'.format( + cls.__name__ + ) ), obj=cls, id="{}.E002".format(ModelCloneConfig.name), @@ -239,7 +243,9 @@ def check(cls, **kwargs): "Please provide either " + '"_clone_m2o_or_o2m_fields"' + " or " - + '"_clone_excluded_m2o_or_o2m_fields" for {}'.format(cls.__name__) + + '"_clone_excluded_m2o_or_o2m_fields" for {}'.format( + cls.__name__ + ) ), obj=cls, id="{}.E002".format(ModelCloneConfig.name), @@ -364,7 +370,9 @@ def make_clone(self, attrs=None, sub_clone=False): for field in one_to_one_fields: rel_object = getattr(self, field.related_name, None) if rel_object: - if hasattr(rel_object, "make_clone") and callable(rel_object.make_clone): + if hasattr(rel_object, "make_clone") and callable( + rel_object.make_clone + ): rel_object.make_clone( attrs={field.remote_field.name: duplicate}, sub_clone=True ) @@ -380,7 +388,9 @@ def make_clone(self, attrs=None, sub_clone=False): items = [] for item in getattr(self, field.related_name).all(): try: - item_clone = item.make_clone(attrs={field.remote_field.name: duplicate}) + item_clone = item.make_clone( + attrs={field.remote_field.name: duplicate} + ) except IntegrityError: item_clone = item.make_clone( attrs={field.remote_field.name: duplicate}, sub_clone=True diff --git a/model_clone/tests/test_clone_mixin.py b/model_clone/tests/test_clone_mixin.py index f9b56a9e..7f087dac 100644 --- a/model_clone/tests/test_clone_mixin.py +++ b/model_clone/tests/test_clone_mixin.py @@ -60,7 +60,9 @@ def test_cloning_with_field_overridden(self): def test_cloning_using_auto_now_field_is_updated(self): name = "New Book" - instance = Book.objects.create(name=name, created_by=self.user1, slug=slugify(name)) + instance = Book.objects.create( + name=name, created_by=self.user1, slug=slugify(name) + ) new_name = "My New Book" clone = instance.make_clone(attrs={"name": new_name}) @@ -185,7 +187,9 @@ def test_cloning_unique_fields_is_valid(self): "{} {} {}".format(first_name, Author.UNIQUE_DUPLICATE_SUFFIX, 1), ) - @patch("sample.models.Author.USE_UNIQUE_DUPLICATE_SUFFIX", new_callable=PropertyMock) + @patch( + "sample.models.Author.USE_UNIQUE_DUPLICATE_SUFFIX", new_callable=PropertyMock + ) def test_cloning_unique_field_with_use_unique_duplicate_suffix_set_to_False( self, use_unique_duplicate_suffix_mock, diff --git a/model_clone/tests/test_create_copy_of_instance.py b/model_clone/tests/test_create_copy_of_instance.py index 6d866b6e..df98ccbc 100644 --- a/model_clone/tests/test_create_copy_of_instance.py +++ b/model_clone/tests/test_create_copy_of_instance.py @@ -32,7 +32,9 @@ def test_cloning_unique_fk_field_without_a_fallback_value_is_invalid(self): def test_cloning_excluded_field_without_a_fallback_value_is_invalid(self): name = "New Library" - instance = Book.objects.create(name=name, created_by=self.user1, slug=slugify(name)) + instance = Book.objects.create( + name=name, created_by=self.user1, slug=slugify(name) + ) with self.assertRaises(IntegrityError): create_copy_of_instance( diff --git a/model_clone/utils.py b/model_clone/utils.py index 000a13fa..5500c985 100644 --- a/model_clone/utils.py +++ b/model_clone/utils.py @@ -73,7 +73,11 @@ def create_copy_of_instance(instance, exclude=(), save_new=True, attrs=None): f.name for f in instance._meta.fields if any( - [all([f.name not in defaults, f.attname not in defaults]), f.has_default(), f.null] + [ + all([f.name not in defaults, f.attname not in defaults]), + f.has_default(), + f.null, + ] ) ] @@ -137,7 +141,9 @@ def generate_value(value, suffix, transform, max_length, max_attempts): yield get_value(value, suffix, transform, max_length, i) raise StopIteration( - "CloneError: max unique attempts for {} exceeded ({})".format(value, max_attempts) + "CloneError: max unique attempts for {} exceeded ({})".format( + value, max_attempts + ) )