From 6084ad4fb22d949cc106e1e132eab4d5a4810198 Mon Sep 17 00:00:00 2001 From: abhiabhi94 <13880786+abhiabhi94@users.noreply.github.com> Date: Sat, 15 May 2021 19:51:07 +0530 Subject: [PATCH] test: Add tests for permissions - almost but takes the test coverage to 100%. Remove unnecessary check for previous urlhash from migrations - since the attribute was created and filled in this migration itself, it wasn't necessary. --- comment/migrations/0008_comment_urlhash.py | 5 +- comment/signals/post_migrate.py | 7 +- comment/tests/base.py | 26 +++--- comment/tests/test_migrations.py | 104 ++++++++++++++++++++- 4 files changed, 122 insertions(+), 20 deletions(-) diff --git a/comment/migrations/0008_comment_urlhash.py b/comment/migrations/0008_comment_urlhash.py index 9bdf4b9..37de87c 100644 --- a/comment/migrations/0008_comment_urlhash.py +++ b/comment/migrations/0008_comment_urlhash.py @@ -12,10 +12,9 @@ def generate_urlhash(): def set_unique_urlhash(model, instance): - if not instance.urlhash: + instance.urlhash = generate_urlhash() + while model.objects.filter(urlhash=instance.urlhash).exists(): instance.urlhash = generate_urlhash() - while model.objects.filter(urlhash=instance.urlhash).exists(): - instance.urlhash = generate_urlhash() def set_default_urlhash(apps, schema_editor): diff --git a/comment/signals/post_migrate.py b/comment/signals/post_migrate.py index ca179ea..376535b 100644 --- a/comment/signals/post_migrate.py +++ b/comment/signals/post_migrate.py @@ -26,6 +26,7 @@ def create_permission_groups(sender, **kwargs): def adjust_flagged_comments(sender, **kwargs): - if settings.COMMENT_FLAGS_ALLOWED: - for comment in Comment.objects.all(): - comment.flag.toggle_flagged_state() + [ + comment.flag.toggle_flagged_state() + for comment in Comment.objects.all() + ] diff --git a/comment/tests/base.py b/comment/tests/base.py index bb280bb..aecc5c5 100644 --- a/comment/tests/base.py +++ b/comment/tests/base.py @@ -364,32 +364,24 @@ def app(self): def setUp(self): super().setUp() assert self.migrate_to and self.migrate_from, \ - f'TestCase {type(self).__name} must define migrate_to and migrate_from properties' + f'TestCase {type(self).__name__} must define migrate_to and migrate_from properties' self.migrate_from = [(self.app, self.migrate_from)] self.migrate_to = [(self.app, self.migrate_to)] self.executor = MigrationExecutor(connection) self.old_apps = self.executor.loader.project_state(self.migrate_from).apps - self.user = User.objects.create_user(username="tester-1") - self.post = Post.objects.create( - author=self.user, - title="post 3", - body="third post body" - ) - content_type = ContentType.objects.get(model='post') - self.ct_object = content_type.get_object_for_this_type(id=self.post.id) - + self._create_data() # revert to the original migration self.executor.migrate(self.migrate_from) # ensure return to the latest migration, even if the test fails self.addCleanup(self.force_migrate) - self.setUpBeforeMigration(self.old_apps) + self.setUpBeforeMigration() self.executor.loader.build_graph() self.executor.migrate(self.migrate_to) self.new_apps = self.executor.loader.project_state(self.migrate_to).apps - def setUpBeforeMigration(self, apps): + def setUpBeforeMigration(self): pass @property @@ -407,6 +399,16 @@ def force_migrate(self, migrate_to=None): migrate_to = [key for key in self.executor.loader.graph.leaf_nodes() if key[0] == self.app] self.executor.migrate(migrate_to) + def _create_data(self): + self.user = User.objects.create_user(username="tester-1") + self.post = Post.objects.create( + author=self.user, + title="post 3", + body="third post body" + ) + content_type = ContentType.objects.get(model='post') + self.ct_object = content_type.get_object_for_this_type(id=self.post.id) + class BaseCommentMixinTest(BaseCommentTest): base_url = None diff --git a/comment/tests/test_migrations.py b/comment/tests/test_migrations.py index d52a450..06f8091 100644 --- a/comment/tests/test_migrations.py +++ b/comment/tests/test_migrations.py @@ -1,3 +1,10 @@ +from unittest.mock import patch + +from django.test import TestCase +from django.core.management import call_command +from django.contrib.auth.models import ContentType, Group, Permission + +from comment.conf import settings from comment.tests.base import BaseCommentMigrationTest @@ -7,6 +14,7 @@ class CommentMigrationTest(BaseCommentMigrationTest): def create_comment(self): self.instance += 1 + return self.old_model.objects.create( content_type_id=self.ct_object.id, object_id=self.instance, @@ -14,12 +22,104 @@ def create_comment(self): user_id=self.user.id, ) - def setUpBeforeMigration(self, apps): + def setUpBeforeMigration(self): self.instance = 0 self.comment = self.create_comment() def test_email_and_urlhash_migrated(self): comment = self.new_model.objects.get(id=self.comment.id) - self.assertEqual(hasattr(comment, 'urlhash'), True) + self.assertIs(hasattr(comment, 'urlhash'), True) self.assertEqual(comment.email, comment.user.email) + + +class GroupsAndPermissionsTest(TestCase): + groups = [ + 'comment_admin', + 'comment_moderator', + ] + permissions = [ + 'delete_comment', + 'delete_flagged_comment', + ] + + def setUp(self): + super().setUp() + Group.objects.filter(name__in=self.groups).delete() + Permission.objects.filter(codename__in=self.permissions).delete() + + @staticmethod + def _migrate(): + call_command('migrate', verbosity=0) + + def _get_groups_not_created(self): + return ( + set(self.groups) -\ + set( + Group.objects + .filter(name__in=self.groups) + .values_list('name', flat=True) + ) + ) + + def _get_permissions_not_created(self): + return ( + set(self.permissions) -\ + set( + Permission.objects + .filter(codename__in=self.permissions) + .values_list("codename", flat=True) + ) + ) + + def permissions_and_groups_created_test(self): + self._migrate() + + self.assertEqual( + Group.objects.filter(name__in=self.groups).count(), + len(self.groups), + msg=f'Group(s) {self._get_groups_not_created()} has/have not been created', + ) + self.assertEqual( + Permission.objects.filter(codename__in=self.permissions).count(), + len(self.permissions), + msg=f'Permission(s) {self._get_permissions_not_created()} has/have not been created', + ) + + @patch.object(settings, 'COMMENT_ALLOW_BLOCKING_USERS', True) + @patch.object(settings, 'COMMENT_FLAGS_ALLOWED', 0) + def test_when_blocking_is_allowed_and_flagging_is_not(self): + self.permissions_and_groups_created_test() + + @patch.object(settings, 'COMMENT_ALLOW_BLOCKING_USERS', False) + @patch.object(settings, 'COMMENT_FLAGS_ALLOWED', 1) + def test_when_flagging_is_allowed_and_blocking_is_not(self): + self.permissions_and_groups_created_test() + + @patch.object(settings, 'COMMENT_ALLOW_BLOCKING_USERS', False) + @patch.object(settings, 'COMMENT_FLAGS_ALLOWED', 0) + def test_when_both_flagging_and_blocking_is_disabled(self): + self._migrate() + + self.assertIs( + Group.objects.filter(name__in=self.groups).exists(), + False, + msg=( + 'Group(s) ' + f'{Group.objects.filter(name__in=self.groups).values("name")} ' + 'has/have not been deleted', + ), + ) + + # delete permission is re-created because it is one of the default ones generated by django + delete_permission = 'delete_comment' + permissions = set(self.permissions) - {delete_permission} + self.assertIs( + Permission.objects.filter(codename__in=permissions).exists(), + False, + msg=( + 'Permission(s) ' + f'{Permission.objects.filter(codename__in=permissions).values("codename")} ' + 'has/have not been deleted', + ), + )