From c1f2ad16c809c5892a1a2760228ae805e7ef59cf Mon Sep 17 00:00:00 2001 From: orzionpour Date: Thu, 25 Nov 2021 11:33:46 +0200 Subject: [PATCH 1/5] Add create_task function - Add a function "create_task" to add task to DB - Add tests for the above function Co-Authored-By: Kobi Hakimi --- tasks/models.py | 25 +++++- tasks/tests/test_create_task.py | 154 ++++++++++++++++++++++++++++++++ 2 files changed, 178 insertions(+), 1 deletion(-) create mode 100644 tasks/tests/test_create_task.py diff --git a/tasks/models.py b/tasks/models.py index 6bdb404..8f22a6b 100644 --- a/tasks/models.py +++ b/tasks/models.py @@ -1,6 +1,6 @@ from django.db import models from enumchoicefield import ChoiceEnum, EnumChoiceField -from users.models import User +from users.models import Role, User class Status(ChoiceEnum): @@ -53,6 +53,29 @@ def filter_by_symbol(cls, priority_filter): print(priority_filter) return cls.objects.filter(priority=priority_filter) + @classmethod + def create_task(cls, title, assignee, assigner, priority, status, description): + if not title or title == "": + raise ValueError("Must add title") + if not isinstance(priority, Priority): + raise ValueError("Must use Priority enum") + if not isinstance(status, Status): + raise ValueError("Must use Status enum") + assigner_role = assigner.role + assigner_team = assigner.team_id + assigne_team = assignee.team_id + if assigne_team != assigner_team: + raise ValueError("Manager can assign tasks only for his own employees") + if assigner_role != Role.MANAGER: + raise ValueError("User must be a manager to assign tasks") + task = Task.objects.create(title=title, + assignee=assignee, + created_by=assigner, + priority=priority, + status=status, + description=description) + return task + class Comment(models.Model): appUser = models.ForeignKey( diff --git a/tasks/tests/test_create_task.py b/tasks/tests/test_create_task.py new file mode 100644 index 0000000..5e34c5e --- /dev/null +++ b/tasks/tests/test_create_task.py @@ -0,0 +1,154 @@ +import pytest +from tasks.models import Priority, Status, Task +from users.models import User, Team, Role +from django.contrib.auth.models import User as DjangoUser + + +@pytest.mark.django_db +class TestCreateTask: + """ + Add test team to DB + """ + @pytest.fixture + def team(self): + team = Team(name="TestTeam", + description="This is a test team") + team.save() + return team + + """ + Add another test team to DB + """ + @pytest.fixture + def other_team(self): + team = Team(name="TestOtherTeam", + description="This is a test team") + team.save() + return team + + """ + Add manager user to the DB + """ + @pytest.fixture + def manager(self, team): + django_user = DjangoUser.objects.create_user(username="TestManager", + email="example@gmail.com", + password='xsdDS23', + first_name="Test", + last_name="Test") + manager = User.objects.create(user=django_user, + role=Role.MANAGER, + team=team) + manager.save() + return manager + + """ + Add an employee to the team in "team" fixture + """ + @pytest.fixture + def employee(self, team): + django_user = DjangoUser.objects.create_user(username="TestEmployee", + email="example@gmail.com", + password='xsdDS23', + first_name="Test", + last_name="Test") + employee = User.objects.create(user=django_user, + role=Role.EMPLOYEE, + team=team) + employee.save() + return employee + + """ + Add an employee to the team in "other_team" fixture + """ + @pytest.fixture + def employee_other_team(self, other_team): + django_user = DjangoUser.objects.create_user(username="TestEmployee", + email="example@gmail.com", + password='xsdDS23', + first_name="Test", + last_name="Test") + employee = User.objects.create(user=django_user, + role=Role.EMPLOYEE, + team=other_team) + employee.save() + return employee + + """ + Test add new task + """ + def test_add_task_to_db(self, manager, employee): + Task.create_task(title="TestTask", + assignee=employee, + assigner=manager, + priority=Priority.CRITICAL, + status=Status.BACKLOG, + description="This is a test task") + assert len(Task.objects.all()) == 1 + + """ + Test that a user who is not manager cannot assign task to other + teams employees. + """ + def test_assigned_by_non_manager(self, manager, employee): + assert employee.role != Role.MANAGER + with pytest.raises(ValueError): + Task.create_task(title="TestTask", + assignee=manager, + assigner=employee, + priority=Priority.CRITICAL, + status=Status.BACKLOG, + description="This is a test task") + assert len(Task.objects.all()) == 0 + + """ + Test that title is required when creating a task. + """ + def test_no_title(self, manager, employee): + with pytest.raises(ValueError): + Task.create_task(title="", + assignee=employee, + assigner=manager, + priority=Priority.CRITICAL, + status=Status.BACKLOG, + description="This is a test task") + assert len(Task.objects.all()) == 0 + + """ + Test that priority must be a valid enum value + """ + def test_invalid_priority(self, manager, employee): + with pytest.raises(ValueError): + Task.create_task(title="TestTask", + assignee=employee, + assigner=manager, + priority='INVALID', + status=Status.BACKLOG, + description="This is a test task") + assert len(Task.objects.all()) == 0 + + """ + Test that status must be a valid enum value + """ + def test_invalid_status(self, manager, employee): + with pytest.raises(ValueError): + Task.create_task(title="TestTask", + assignee=employee, + assigner=manager, + priority=Priority.CRITICAL, + status='INVALID', + description="This is a test task") + assert len(Task.objects.all()) == 0 + + """ + Test that manager cannot assign task to other teams employees + """ + def test_assign_other_team(self, manager, employee_other_team): + with pytest.raises(ValueError): + Task.create_task(title="TestTask", + assignee=employee_other_team, + assigner=manager, + priority=Priority.CRITICAL, + status=Status.BACKLOG, + description="This is a test task") + assert len(Task.objects.all()) == 0 From 0f29b2b19a8d7a3848426d114f96938a73429259 Mon Sep 17 00:00:00 2001 From: orzionpour Date: Sat, 27 Nov 2021 12:50:44 +0200 Subject: [PATCH 2/5] Refactor code - Removed all save() operations and replaced with Django's default object creation. - Changed user creation in tests to use create_user funciton. - Removed raise of ValueError in cases where Django raises its own exception. --- tasks/models.py | 21 ++++----- tasks/tests/test_create_task.py | 82 +++++++++++++++------------------ 2 files changed, 46 insertions(+), 57 deletions(-) diff --git a/tasks/models.py b/tasks/models.py index 8f22a6b..5cf15bc 100644 --- a/tasks/models.py +++ b/tasks/models.py @@ -1,4 +1,4 @@ -from django.db import models +from django.db import models, transaction from enumchoicefield import ChoiceEnum, EnumChoiceField from users.models import Role, User @@ -54,23 +54,20 @@ def filter_by_symbol(cls, priority_filter): return cls.objects.filter(priority=priority_filter) @classmethod - def create_task(cls, title, assignee, assigner, priority, status, description): - if not title or title == "": - raise ValueError("Must add title") - if not isinstance(priority, Priority): - raise ValueError("Must use Priority enum") - if not isinstance(status, Status): - raise ValueError("Must use Status enum") - assigner_role = assigner.role - assigner_team = assigner.team_id - assigne_team = assignee.team_id + @transaction.atomic + def create_task(cls, title, assignee, created_by, priority, status, description): + if title == "": + raise ValueError("Title must contain at lease one character") + assigner_role = created_by.role + assigner_team = created_by.team + assigne_team = assignee.team if assigne_team != assigner_team: raise ValueError("Manager can assign tasks only for his own employees") if assigner_role != Role.MANAGER: raise ValueError("User must be a manager to assign tasks") task = Task.objects.create(title=title, assignee=assignee, - created_by=assigner, + created_by=created_by, priority=priority, status=status, description=description) diff --git a/tasks/tests/test_create_task.py b/tasks/tests/test_create_task.py index 5e34c5e..ed8877a 100644 --- a/tasks/tests/test_create_task.py +++ b/tasks/tests/test_create_task.py @@ -2,7 +2,7 @@ from tasks.models import Priority, Status, Task from users.models import User, Team, Role from django.contrib.auth.models import User as DjangoUser - +from django.db.transaction import TransactionManagementError @pytest.mark.django_db class TestCreateTask: @@ -11,9 +11,8 @@ class TestCreateTask: """ @pytest.fixture def team(self): - team = Team(name="TestTeam", - description="This is a test team") - team.save() + team = Team.objects.create(name="TestTeam", + description="This is a test team") return team """ @@ -21,9 +20,8 @@ def team(self): """ @pytest.fixture def other_team(self): - team = Team(name="TestOtherTeam", - description="This is a test team") - team.save() + team = Team.objects.create(name="TestOtherTeam", + description="This is a test team") return team """ @@ -31,15 +29,13 @@ def other_team(self): """ @pytest.fixture def manager(self, team): - django_user = DjangoUser.objects.create_user(username="TestManager", - email="example@gmail.com", - password='xsdDS23', - first_name="Test", - last_name="Test") - manager = User.objects.create(user=django_user, - role=Role.MANAGER, - team=team) - manager.save() + manager = User.create_user(username="TestManager", + email="example@gmail.com", + password='xsdDS23', + first_name='Test', + last_name='Test', + role=Role.MANAGER, + team=team) return manager """ @@ -47,15 +43,13 @@ def manager(self, team): """ @pytest.fixture def employee(self, team): - django_user = DjangoUser.objects.create_user(username="TestEmployee", - email="example@gmail.com", - password='xsdDS23', - first_name="Test", - last_name="Test") - employee = User.objects.create(user=django_user, - role=Role.EMPLOYEE, - team=team) - employee.save() + employee = User.create_user(username="TestEmployee", + email="example@gmail.com", + password='xsdDS23', + first_name='Test', + last_name='Test', + role=Role.EMPLOYEE, + team=team) return employee """ @@ -63,15 +57,13 @@ def employee(self, team): """ @pytest.fixture def employee_other_team(self, other_team): - django_user = DjangoUser.objects.create_user(username="TestEmployee", - email="example@gmail.com", - password='xsdDS23', - first_name="Test", - last_name="Test") - employee = User.objects.create(user=django_user, - role=Role.EMPLOYEE, - team=other_team) - employee.save() + employee = User.create_user(username="TestEmployee", + email="example@gmail.com", + password='xsdDS23', + first_name='Test', + last_name='Test', + role=Role.EMPLOYEE, + team=other_team) return employee """ @@ -80,7 +72,7 @@ def employee_other_team(self, other_team): def test_add_task_to_db(self, manager, employee): Task.create_task(title="TestTask", assignee=employee, - assigner=manager, + created_by=manager, priority=Priority.CRITICAL, status=Status.BACKLOG, description="This is a test task") @@ -92,10 +84,10 @@ def test_add_task_to_db(self, manager, employee): """ def test_assigned_by_non_manager(self, manager, employee): assert employee.role != Role.MANAGER - with pytest.raises(ValueError): + with pytest.raises(Exception): Task.create_task(title="TestTask", assignee=manager, - assigner=employee, + created_by=employee, priority=Priority.CRITICAL, status=Status.BACKLOG, description="This is a test task") @@ -105,10 +97,10 @@ def test_assigned_by_non_manager(self, manager, employee): Test that title is required when creating a task. """ def test_no_title(self, manager, employee): - with pytest.raises(ValueError): + with pytest.raises(Exception): Task.create_task(title="", assignee=employee, - assigner=manager, + created_by=manager, priority=Priority.CRITICAL, status=Status.BACKLOG, description="This is a test task") @@ -118,10 +110,10 @@ def test_no_title(self, manager, employee): Test that priority must be a valid enum value """ def test_invalid_priority(self, manager, employee): - with pytest.raises(ValueError): + with pytest.raises(Exception): Task.create_task(title="TestTask", assignee=employee, - assigner=manager, + created_by=manager, priority='INVALID', status=Status.BACKLOG, description="This is a test task") @@ -131,10 +123,10 @@ def test_invalid_priority(self, manager, employee): Test that status must be a valid enum value """ def test_invalid_status(self, manager, employee): - with pytest.raises(ValueError): + with pytest.raises(Exception): Task.create_task(title="TestTask", assignee=employee, - assigner=manager, + created_by=manager, priority=Priority.CRITICAL, status='INVALID', description="This is a test task") @@ -144,10 +136,10 @@ def test_invalid_status(self, manager, employee): Test that manager cannot assign task to other teams employees """ def test_assign_other_team(self, manager, employee_other_team): - with pytest.raises(ValueError): + with pytest.raises(Exception): Task.create_task(title="TestTask", assignee=employee_other_team, - assigner=manager, + created_by=manager, priority=Priority.CRITICAL, status=Status.BACKLOG, description="This is a test task") From fb5163cc27510da27356e46f7b63552fcbd31e51 Mon Sep 17 00:00:00 2001 From: orzionpour Date: Sat, 27 Nov 2021 12:57:32 +0200 Subject: [PATCH 3/5] Update test_create_task.py Fixed flake8 errors --- tasks/tests/test_create_task.py | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/tasks/tests/test_create_task.py b/tasks/tests/test_create_task.py index ed8877a..3e8110e 100644 --- a/tasks/tests/test_create_task.py +++ b/tasks/tests/test_create_task.py @@ -1,8 +1,7 @@ import pytest from tasks.models import Priority, Status, Task from users.models import User, Team, Role -from django.contrib.auth.models import User as DjangoUser -from django.db.transaction import TransactionManagementError + @pytest.mark.django_db class TestCreateTask: @@ -44,12 +43,12 @@ def manager(self, team): @pytest.fixture def employee(self, team): employee = User.create_user(username="TestEmployee", - email="example@gmail.com", - password='xsdDS23', - first_name='Test', - last_name='Test', - role=Role.EMPLOYEE, - team=team) + email="example@gmail.com", + password='xsdDS23', + first_name='Test', + last_name='Test', + role=Role.EMPLOYEE, + team=team) return employee """ @@ -58,12 +57,12 @@ def employee(self, team): @pytest.fixture def employee_other_team(self, other_team): employee = User.create_user(username="TestEmployee", - email="example@gmail.com", - password='xsdDS23', - first_name='Test', - last_name='Test', - role=Role.EMPLOYEE, - team=other_team) + email="example@gmail.com", + password='xsdDS23', + first_name='Test', + last_name='Test', + role=Role.EMPLOYEE, + team=other_team) return employee """ From 4934882d1a40e12ec942163d47223e59ecff8ed9 Mon Sep 17 00:00:00 2001 From: orzionpour Date: Sun, 28 Nov 2021 21:00:57 +0200 Subject: [PATCH 4/5] Refactor create_task tests Change the negative tests of create_task to be one parameterized test. --- tasks/tests/test_create_task.py | 114 ++++++++++---------------------- 1 file changed, 36 insertions(+), 78 deletions(-) diff --git a/tasks/tests/test_create_task.py b/tasks/tests/test_create_task.py index 3e8110e..aa11482 100644 --- a/tasks/tests/test_create_task.py +++ b/tasks/tests/test_create_task.py @@ -65,81 +65,39 @@ def employee_other_team(self, other_team): team=other_team) return employee - """ - Test add new task - """ - def test_add_task_to_db(self, manager, employee): - Task.create_task(title="TestTask", - assignee=employee, - created_by=manager, - priority=Priority.CRITICAL, - status=Status.BACKLOG, - description="This is a test task") - assert len(Task.objects.all()) == 1 - - """ - Test that a user who is not manager cannot assign task to other - teams employees. - """ - def test_assigned_by_non_manager(self, manager, employee): - assert employee.role != Role.MANAGER - with pytest.raises(Exception): - Task.create_task(title="TestTask", - assignee=manager, - created_by=employee, - priority=Priority.CRITICAL, - status=Status.BACKLOG, - description="This is a test task") - assert len(Task.objects.all()) == 0 - - """ - Test that title is required when creating a task. - """ - def test_no_title(self, manager, employee): - with pytest.raises(Exception): - Task.create_task(title="", - assignee=employee, - created_by=manager, - priority=Priority.CRITICAL, - status=Status.BACKLOG, - description="This is a test task") - assert len(Task.objects.all()) == 0 - - """ - Test that priority must be a valid enum value - """ - def test_invalid_priority(self, manager, employee): - with pytest.raises(Exception): - Task.create_task(title="TestTask", - assignee=employee, - created_by=manager, - priority='INVALID', - status=Status.BACKLOG, - description="This is a test task") - assert len(Task.objects.all()) == 0 - - """ - Test that status must be a valid enum value - """ - def test_invalid_status(self, manager, employee): - with pytest.raises(Exception): - Task.create_task(title="TestTask", - assignee=employee, - created_by=manager, - priority=Priority.CRITICAL, - status='INVALID', - description="This is a test task") - assert len(Task.objects.all()) == 0 - - """ - Test that manager cannot assign task to other teams employees - """ - def test_assign_other_team(self, manager, employee_other_team): - with pytest.raises(Exception): - Task.create_task(title="TestTask", - assignee=employee_other_team, - created_by=manager, - priority=Priority.CRITICAL, - status=Status.BACKLOG, - description="This is a test task") - assert len(Task.objects.all()) == 0 + @pytest.mark.parametrize('title, assignee, assigner, priority, status, description, length', [ + ('TestTask', 'employee', 'manager', Priority.CRITICAL, Status.BACKLOG, + 'This is description', 1), + ('TestTask', 'manager', 'employee', Priority.CRITICAL, Status.BACKLOG, + 'This is description', 0), + ('', 'employee', 'manager', Priority.CRITICAL, Status.BACKLOG, + 'This is description', 0), + ('TestTask', 'employee', 'manager', 'INVALID', Status.BACKLOG, + 'This is description', 0), + ('TestTask', 'employee', 'manager', Priority.CRITICAL, 'INVALID', + 'This is description', 0), + ('TestTask', 'employee_other_team', 'manager', Priority.CRITICAL, Status.BACKLOG, + 'This is description', 0), + ], + ids=[ + "test_add_task_to_db", + "test_assigned_by_non_manager", + "test_no_title", + "test_invalid_priority", + "test_invalid_status", + "test_assign_other_team", + ] + ) + def test_invalid_input(self, request, title, assignee, assigner, priority, status, description, length): + assignee = request.getfixturevalue(assignee) + created_by = request.getfixturevalue(assigner) + try: + Task.create_task(title=title, + assignee=assignee, + created_by=created_by, + priority=priority, + status=status, + description=description) + except ValueError: + pass + assert len(Task.objects.all()) == length From 3242965e475ab89a9c7ee774182fdebba04a669c Mon Sep 17 00:00:00 2001 From: orzionpour Date: Sun, 28 Nov 2021 21:04:47 +0200 Subject: [PATCH 5/5] Refactor create_tasks tests --- tasks/tests/test_create_task.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tasks/tests/test_create_task.py b/tasks/tests/test_create_task.py index aa11482..278e57d 100644 --- a/tasks/tests/test_create_task.py +++ b/tasks/tests/test_create_task.py @@ -88,7 +88,7 @@ def employee_other_team(self, other_team): "test_assign_other_team", ] ) - def test_invalid_input(self, request, title, assignee, assigner, priority, status, description, length): + def test_create_task(self, request, title, assignee, assigner, priority, status, description, length): assignee = request.getfixturevalue(assignee) created_by = request.getfixturevalue(assigner) try: