diff --git a/alembic/versions/a2007ad7f535_added_review_version_id_column.py b/alembic/versions/a2007ad7f535_added_review_version_id_column.py new file mode 100644 index 00000000..dca4c5fd --- /dev/null +++ b/alembic/versions/a2007ad7f535_added_review_version_id_column.py @@ -0,0 +1,33 @@ +"""Added Review.version_id column + +Revision ID: a2007ad7f535 +Revises: 91ed52b72b82 +Create Date: 2024-11-26 11:36:07.776169 +""" + +from alembic import op + +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = "a2007ad7f535" +down_revision = "91ed52b72b82" + + +def upgrade(): + """Upgrade the tables.""" + op.add_column("Reviews", sa.Column("version_id", sa.Integer(), nullable=True)) + op.create_foreign_key( + "Reviews_version_id_fkey", + "Reviews", + "Versions", + ["version_id"], + ["id"], + ) + + +def downgrade(): + """Downgrade the tables.""" + op.drop_constraint("Reviews_version_id_fkey", "Reviews", type_="foreignkey") + op.drop_column("Reviews", "version_id") diff --git a/src/stalker/db/setup.py b/src/stalker/db/setup.py index 70570680..878be40c 100644 --- a/src/stalker/db/setup.py +++ b/src/stalker/db/setup.py @@ -36,7 +36,7 @@ logger: logging.Logger = log.get_logger(__name__) # TODO: Try to get it from the API (it was not working inside a package before) -alembic_version: str = "91ed52b72b82" +alembic_version: str = "a2007ad7f535" def setup(settings: Optional[Dict[str, Any]] = None) -> None: diff --git a/src/stalker/models/review.py b/src/stalker/models/review.py index 7cde09e1..355e957b 100644 --- a/src/stalker/models/review.py +++ b/src/stalker/models/review.py @@ -40,16 +40,30 @@ class Review(SimpleEntity, ScheduleMixin, StatusMixin): Each Review instance with the same :attr:`.review_number` for a :class:`.Task` represents a set of reviews. - Creating a review will automatically cap the schedule timing value of the - related task to the total logged time logs for that task and then extend - the timing values according to the review schedule values. + .. version-added:: 1.0.0 + + Review -> Version relation + + Versions can now be attached to reviews. + + Review instances, alongside the :class:`.Task` can also optionally hold a + :class:`.Version` instance. This allows the information of which + :class:`.Version` instance has been reviewed as a part of the review + process to be much cleaner, and when the Review history is investigated, + it will be much easier to identify which :class:`.Version` the review was + about. Args: task (Task): A :class:`.Task` instance that this review is related to. - It cannot be skipped. + It can be skipped if a :class:`.Version` instance has been given. + + version (Version): A :class:`.Version` instance that this review + instance is related to. The :class:`.Version` and the + :class:`.Task` should be related, a ``ValueError`` will be raised + if they are not. - review_number (int): This number represents the revision set id that this - Review instance belongs to. + review_number (int): This number represents the revision set id that + this Review instance belongs to. reviewer (User): One of the responsible of the related Task. There will be only one Review instances with the same review_number for every @@ -89,6 +103,16 @@ class Review(SimpleEntity, ScheduleMixin, StatusMixin): doc="The :class:`.Task` instance that this Review is created for", ) + version_id: Mapped[Optional[int]] = mapped_column( + "version_id", ForeignKey("Versions.id") + ) + + version: Mapped[Optional["Version"]] = relationship( + primaryjoin="Reviews.c.version_id==Versions.c.id", + uselist=False, + back_populates="reviews", + ) + reviewer_id: Mapped[int] = mapped_column( ForeignKey("Users.id"), nullable=False, @@ -105,6 +129,7 @@ class Review(SimpleEntity, ScheduleMixin, StatusMixin): def __init__( self, task: Optional["Task"] = None, + version: Optional["Version"] = None, reviewer: Optional["User"] = None, description: str = "", **kwargs: Dict[str, Any], @@ -115,6 +140,7 @@ def __init__( StatusMixin.__init__(self, **kwargs) self.task = task + self.version = version self.reviewer = reviewer # set the status to NEW @@ -132,15 +158,14 @@ def _validate_task( """Validate the given task value. Args: - key (str): The name of the validated column. - task (Task): The task value to be validated. + task (Union[None, Task]): The task value to be validated. Raises: TypeError: If the given task value is not a Task instance. ValueError: If the given task is not a leaf task. Returns: - Task: The validated Task instance. + Union[None, Task]: The validated Task instance. """ if task is None: return task @@ -165,6 +190,38 @@ def _validate_task( return task + @validates("version") + def _validate_version( + self, key: str, version: Union[None, "Version"] + ) -> Union[None, "Version"]: + """Validate the given version value. + + Args: + key (str): The name of the validated column. + version (Union[None, Version]): The version value to be validated. + """ + if version is None: + return version + + from stalker.models.version import Version + + if not isinstance(version, Version): + raise TypeError( + f"{self.__class__.__name__}.version should be a Version " + f"instance, not {version.__class__.__name__}: '{version}'" + ) + + if self.task is not None: + if version.task != self.task: + raise ValueError( + f"{self.__class__.__name__}.version should be a Version " + f"instance related to this Task: {version}" + ) + else: + self.task = version.task + + return version + @validates("reviewer") def _validate_reviewer(self, key: str, reviewer: "User") -> "User": """Validate the given reviewer value. diff --git a/src/stalker/models/studio.py b/src/stalker/models/studio.py index 7a8c3469..04c3abd8 100644 --- a/src/stalker/models/studio.py +++ b/src/stalker/models/studio.py @@ -871,7 +871,9 @@ def _validate_working_hours_value(self, value: List) -> List: err = ( "{}.working_hours value should be a list of lists of two " "integers and the range of integers should be between 0-1440, " - "not {}: '{}'".format(self.__class__.__name__, value.__class__.__name__, value) + "not {}: '{}'".format( + self.__class__.__name__, value.__class__.__name__, value + ) ) if not isinstance(value, list): diff --git a/src/stalker/models/task.py b/src/stalker/models/task.py index 8c72a6e3..5032138b 100644 --- a/src/stalker/models/task.py +++ b/src/stalker/models/task.py @@ -2578,8 +2578,8 @@ def create_time_log( This will ease creating TimeLog instances for task. Args: - resource (User): The :class:`stalker.models.auth.User` instance as the - resource for the TimeLog. + resource (User): The :class:`stalker.models.auth.User` instance as + the resource for the TimeLog. start (datetime.datetime): The start date and time. end (datetime.datetime): The end date and time. @@ -2591,7 +2591,7 @@ def create_time_log( return TimeLog(task=self, resource=resource, start=start, end=end) # also updating parent statuses are done in TimeLog._validate_task - def request_review(self) -> List[Review]: + def request_review(self, version: Optional["Version"] = None) -> List[Review]: """Create and return Review instances for each of the responsible of this task. Also set the task status to PREV. @@ -2601,15 +2601,20 @@ def request_review(self) -> List[Review]: Only applicable to leaf tasks. + Args: + version (Optional[Version]): An optional :class:`.Version` instance + can also be passed. The :class:`.Version` should be related to + this :class:`.Task`. + Raises: - StatusError: If the current task status is not WIP a StatusError will be - raised as the task has either not been started on being worked yet, - it is already on review, a dependency might be under review or this is - stopped, hold or completed. + StatusError: If the current task status is not WIP a StatusError + will be raised as the task has either not been started on being + worked yet, it is already on review, a dependency might be + under review or this is stopped, hold or completed. Returns: - List[Review]: The list of :class:`stalker.models.review.Review` instances - created. + List[Review]: The list of :class:`stalker.models.review.Review` + instances created. """ # check task status with DBSession.no_autoflush: @@ -2627,7 +2632,7 @@ def request_review(self) -> List[Review]: # create Review instances for each Responsible of this task reviews = [] for responsible in self.responsible: - reviews.append(Review(task=self, reviewer=responsible)) + reviews.append(Review(task=self, version=version, reviewer=responsible)) # update the status to PREV self.status = prev diff --git a/src/stalker/models/variant.py b/src/stalker/models/variant.py index 38c87f8f..c562738c 100644 --- a/src/stalker/models/variant.py +++ b/src/stalker/models/variant.py @@ -7,7 +7,6 @@ from stalker.models.task import Task - class Variant(Task): """A Task derivative to keep track of Variants in a Task hierarchy. @@ -36,4 +35,4 @@ class Variant(Task): "id", ForeignKey("Tasks.id"), primary_key=True, - ) \ No newline at end of file + ) diff --git a/src/stalker/models/version.py b/src/stalker/models/version.py index a3d5e757..13f8c861 100644 --- a/src/stalker/models/version.py +++ b/src/stalker/models/version.py @@ -16,6 +16,7 @@ from stalker.log import get_logger from stalker.models.link import Link from stalker.models.mixins import DAGMixin +from stalker.models.review import Review from stalker.models.task import Task from stalker.utils import walk_hierarchy @@ -138,6 +139,10 @@ class Version(Link, DAGMixin): """, ) + reviews: Mapped[Optional[List[Review]]] = relationship( + primaryjoin="Reviews.c.version_id==Versions.c.id" + ) + is_published: Mapped[Optional[bool]] = mapped_column(default=False) created_with: Mapped[Optional[str]] = mapped_column(String(256)) @@ -619,6 +624,14 @@ def walk_inputs(self, method: int = 0) -> Generator[None, "Version", None]: for v in walk_hierarchy(self, "inputs", method=method): yield v + def request_review(self): + """Call the self.task.request_review(). + + This is a shortcut to the Task.request_review() method of the related + task. + """ + return self.task.request_review(version=self) + # VERSION INPUTS Version_Inputs = Table( diff --git a/tests/db/test_db.py b/tests/db/test_db.py index deb158c0..93e7a684 100644 --- a/tests/db/test_db.py +++ b/tests/db/test_db.py @@ -271,11 +271,9 @@ def test_daily_status_list_initialization(setup_postgresql_db): def test_variant_status_list_initialization(setup_postgresql_db): """Variant statuses are correctly created.""" - variant_status_list = ( - StatusList.query - .filter(StatusList.target_entity_type == "Variant") - .first() - ) + variant_status_list = StatusList.query.filter( + StatusList.target_entity_type == "Variant" + ).first() assert isinstance(variant_status_list, StatusList) assert variant_status_list.name == "Variant Statuses" expected_status_names = [ @@ -4271,8 +4269,7 @@ def test_persistence_of_review(setup_postgresql_db): start=datetime.datetime.now(pytz.utc) + datetime.timedelta(2), end=datetime.datetime.now(pytz.utc) + datetime.timedelta(3), ) - rev1 = Review(task=task2, reviewer=user1, schedule_timing=1, schedule_unit="h") - DBSession.add_all( + DBSession.save( [ task1, child_task1, @@ -4283,10 +4280,20 @@ def test_persistence_of_review(setup_postgresql_db): time_log3, user1, user2, - rev1, ] ) - DBSession.commit() + + version1 = Version(task=task2) + DBSession.save(version1) + + rev1 = Review( + task=task2, + reviewer=user1, + version=version1, + schedule_timing=1, + schedule_unit="h", + ) + DBSession.save(rev1) created_by = rev1.created_by date_created = rev1.date_created @@ -4296,6 +4303,7 @@ def test_persistence_of_review(setup_postgresql_db): schedule_unit = rev1.schedule_unit task = rev1.task updated_by = rev1.updated_by + version = rev1.version del rev1 @@ -4312,6 +4320,7 @@ def test_persistence_of_review(setup_postgresql_db): assert rev1_db.updated_by == updated_by assert rev1_db.schedule_timing == schedule_timing assert rev1_db.schedule_unit == schedule_unit + assert rev1_db.version == version # delete tests diff --git a/tests/models/test_project.py b/tests/models/test_project.py index 64a6447b..1760b622 100644 --- a/tests/models/test_project.py +++ b/tests/models/test_project.py @@ -1357,7 +1357,55 @@ def test_tjp_id_is_working_as_expected(setup_project_db_test): assert data["test_project"].tjp_id == "Project_654654" -def test_to_tjp_is_working_as_expected(setup_project_db_test): +@pytest.mark.parametrize( + "entity_name", + [ + "test_task1", + "test_task2", + "test_task3", + "test_task4", + "test_task5", + "test_task6", + "test_task7", + "test_task8", + "test_task9", + "test_task10", + "test_task11", + "test_task12", + "test_task13", + "test_task14", + "test_task15", + "test_task16", + "test_task17", + "test_task18", + "test_task19", + "test_task20", + "test_task21", + "test_task22", + "test_task23", + "test_task24", + "test_task25", + "test_task26", + "test_task27", + "test_asset1", + "test_asset2", + "test_asset3", + "test_asset4", + "test_asset5", + "test_shot1", + "test_shot2", + "test_shot3", + "test_shot4", + "test_seq1", + "test_seq2", + "test_seq3", + "test_seq4", + "test_seq5", + "test_seq6", + "test_seq7", + ], +) +def test_to_tjp_is_working_as_expected(setup_project_db_test, entity_name): """to_tjp attribute is working as expected.""" data = setup_project_db_test # because of the randomness in the order of the test data being created, @@ -1370,56 +1418,8 @@ def test_to_tjp_is_working_as_expected(setup_project_db_test): assert result.startswith( 'task Project_{id} "Project_{id}" {{'.format(id=data["test_project"].id) ) - - entities_to_test = [ - data["test_project"], - data["test_task1"], - data["test_task2"], - data["test_task3"], - data["test_task4"], - data["test_task5"], - data["test_task6"], - data["test_task7"], - data["test_task8"], - data["test_task9"], - data["test_task10"], - data["test_task11"], - data["test_task12"], - data["test_task13"], - data["test_task14"], - data["test_task15"], - data["test_task16"], - data["test_task17"], - data["test_task18"], - data["test_task19"], - data["test_task20"], - data["test_task21"], - data["test_task22"], - data["test_task23"], - data["test_task24"], - data["test_task25"], - data["test_task26"], - data["test_task27"], - data["test_asset1"], - data["test_asset2"], - data["test_asset3"], - data["test_asset4"], - data["test_asset5"], - data["test_shot1"], - data["test_shot2"], - data["test_shot3"], - data["test_shot4"], - data["test_seq1"], - data["test_seq2"], - data["test_seq3"], - data["test_seq4"], - data["test_seq5"], - data["test_seq6"], - data["test_seq7"], - ] - assert all( - condition_tjp_output(entity.to_tjp) in result for entity in entities_to_test - ) + entity = data[entity_name] + assert condition_tjp_output(entity.to_tjp) in result def test_project_instance_does_not_have_active_attribute(setup_project_db_test): diff --git a/tests/models/test_review.py b/tests/models/test_review.py index 6901a00a..0d11d81e 100644 --- a/tests/models/test_review.py +++ b/tests/models/test_review.py @@ -8,7 +8,7 @@ import pytz -from stalker import Project, Repository, Review, Status, Structure, Task, User +from stalker import Project, Repository, Review, Status, Structure, Task, User, Version from stalker.db.session import DBSession @@ -155,6 +155,19 @@ def test_task_argument_is_not_a_leaf_task(setup_review_db_test): ) +def test_task_argument_can_be_skipped_if_version_is_given(setup_review_db_test): + """task argument can be skipped if the version arg is given.""" + data = setup_review_db_test + task1 = Task(name="Task1", project=data["project"]) + DBSession.save(task1) + version = Version(task=task1) + DBSession.save(version) + data["kwargs"]["version"] = version + data["kwargs"].pop("task") + review = Review(**data["kwargs"]) + assert review.task == task1 + + def test_task_argument_is_working_as_expected(setup_review_db_test): """task argument value is passed to the task argument.""" data = setup_review_db_test @@ -841,3 +854,108 @@ def test_review_set_property_return_all_the_revision_instances_with_same_review_ assert review5.review_number == 3 assert review6.review_number == 3 assert review7.review_number == 3 + + +def test_review__init__version_arg_is_skipped(setup_review_db_test): + """Review.__init__() version arg can be skipped.""" + data = setup_review_db_test + data["task1"].responsible = [data["user1"], data["user2"], data["user3"]] + now = datetime.datetime.now(pytz.utc) + data["task1"].create_time_log( + resource=data["user1"], start=now, end=now + datetime.timedelta(hours=1) + ) + data["task1"].status = data["status_wip"] + review = Review(task=data["task1"], reviewer=data["task1"].responsible[0]) + assert isinstance(review, Review) + + +def test_review__init__version_arg_is_none(setup_review_db_test): + """Review.__init__() version arg can be None.""" + data = setup_review_db_test + data["task1"].responsible = [data["user1"], data["user2"], data["user3"]] + now = datetime.datetime.now(pytz.utc) + data["task1"].create_time_log( + resource=data["user1"], start=now, end=now + datetime.timedelta(hours=1) + ) + data["task1"].status = data["status_wip"] + review = Review( + task=data["task1"], version=None, reviewer=data["task1"].responsible[0] + ) + assert isinstance(review, Review) + + +def test_review__init__version_arg_is_not_a_version_instance(setup_review_db_test): + """Review.__init__() version arg is not a Version instance.""" + data = setup_review_db_test + data["task1"].responsible = [data["user1"], data["user2"], data["user3"]] + now = datetime.datetime.now(pytz.utc) + data["task1"].create_time_log( + resource=data["user1"], start=now, end=now + datetime.timedelta(hours=1) + ) + data["task1"].status = data["status_wip"] + with pytest.raises(TypeError) as cm: + _ = Review( + task=data["task1"], + version="not a version", + reviewer=data["task1"].responsible[0], + ) + assert str(cm.value) == ( + "Review.version should be a Version instance, " "not str: 'not a version'" + ) + + +def test_review__init__version_arg_is_not_related_to_the_given_task( + setup_review_db_test, +): + """Review.__init__() raises ValueError if the version is not matching the task.""" + data = setup_review_db_test + data["task1"].responsible = [data["user1"], data["user2"], data["user3"]] + now = datetime.datetime.now(pytz.utc) + data["task1"].create_time_log( + resource=data["user1"], start=now, end=now + datetime.timedelta(hours=1) + ) + data["task1"].status = data["status_wip"] + version = Version(task=data["task2"]) + + with pytest.raises(ValueError) as cm: + _ = Review( + task=data["task1"], version=version, reviewer=data["task1"].responsible[0] + ) + assert str(cm.value) == ( + "Review.version should be a Version instance " + f"related to this Task: {version}" + ) + + +def test_review___init__accepts_a_version_with_version_argument(setup_review_db_test): + """Review.__init__() accepts a Version instance with version argument.""" + data = setup_review_db_test + data["task1"].responsible = [data["user1"], data["user2"], data["user3"]] + now = datetime.datetime.now(pytz.utc) + data["task1"].create_time_log( + resource=data["user1"], start=now, end=now + datetime.timedelta(hours=1) + ) + data["task1"].status = data["status_wip"] + version = Version(task=data["task1"]) + + review = Review( + task=data["task1"], version=version, reviewer=data["task1"].responsible[0] + ) + assert isinstance(review, Review) + + +def test_review___init__version_arg_value_passed_to_version_attr(setup_review_db_test): + """Review.__init__() version arg value is passed to the version attr.""" + data = setup_review_db_test + data["task1"].responsible = [data["user1"], data["user2"], data["user3"]] + now = datetime.datetime.now(pytz.utc) + data["task1"].create_time_log( + resource=data["user1"], start=now, end=now + datetime.timedelta(hours=1) + ) + data["task1"].status = data["status_wip"] + version = Version(task=data["task1"]) + + review = Review( + task=data["task1"], version=version, reviewer=data["task1"].responsible[0] + ) + assert review.version == version diff --git a/tests/models/test_task_status_workflow.py b/tests/models/test_task_status_workflow.py index 76385b41..3836b1f2 100644 --- a/tests/models/test_task_status_workflow.py +++ b/tests/models/test_task_status_workflow.py @@ -17,6 +17,7 @@ TimeLog, Type, User, + Version, ) from stalker.db.session import DBSession from stalker.exceptions import StatusError @@ -28,10 +29,16 @@ def setup_task_status_workflow_tests(): data = dict() # test users data["test_user1"] = User( - name="Test User 1", login="tuser1", email="tuser1@test.com", password="secret" + name="Test User 1", + login="tuser1", + email="tuser1@test.com", + password="secret", ) data["test_user2"] = User( - name="Test User 2", login="tuser2", email="tuser2@test.com", password="secret" + name="Test User 2", + login="tuser2", + email="tuser2@test.com", + password="secret", ) # create a couple of tasks data["status_new"] = Status(name="New", code="NEW") @@ -4171,3 +4178,69 @@ def test_review_set_review_number_is_skipped(setup_task_status_workflow_db_tests # now check if review_set() will return reviews2 assert data["test_task3"].review_set() == reviews2 + + +def test_request_review_version_arg_is_skipped(setup_task_status_workflow_db_tests): + """request_review() version arg can be skipped.""" + data = setup_task_status_workflow_db_tests + data["test_task3"].status = data["status_wip"] + + # request a review + reviews = data["test_task3"].request_review() # Version arg is skipped + assert len(reviews) == 2 + + +def test_request_review_version_arg_is_none(setup_task_status_workflow_db_tests): + """request_review() version arg can be None.""" + data = setup_task_status_workflow_db_tests + data["test_task3"].status = data["status_wip"] + + # request a review + reviews = data["test_task3"].request_review(version=None) + assert len(reviews) == 2 + + +def test_request_review_version_arg_is_not_a_version_instance( + setup_task_status_workflow_db_tests, +): + """request_review() version arg is not a Version instance raises TypeError.""" + data = setup_task_status_workflow_db_tests + data["test_task3"].status = data["status_wip"] + + # request a review + with pytest.raises(TypeError) as cm: + _ = data["test_task3"].request_review(version="Not a version instance") + + assert str(cm.value) == ( + "Review.version should be a Version instance, " + "not str: 'Not a version instance'" + ) + + +def test_request_review_version_arg_is_not_related_to_the_task( + setup_task_status_workflow_db_tests, +): + """request_review() version arg is not related to the Task raises ValueError.""" + data = setup_task_status_workflow_db_tests + data["test_task3"].status = data["status_wip"] + version = Version(task=data["test_task2"]) + + # request a review + with pytest.raises(ValueError) as cm: + _ = data["test_task3"].request_review(version=version) + + assert str(cm.value) == ( + f"Review.version should be a Version instance related to this Task: {version}" + ) + + +def test_request_review_accepts_version_instance(setup_task_status_workflow_db_tests): + """request_review() a Version instance can be passed to it.""" + data = setup_task_status_workflow_db_tests + data["test_task3"].status = data["status_wip"] + version = Version(task=data["test_task3"]) + + # request a review + reviews = data["test_task3"].request_review(version=version) + assert reviews[0].version == version + assert reviews[1].version == version diff --git a/tests/models/test_variant.py b/tests/models/test_variant.py index 52179115..267799a4 100644 --- a/tests/models/test_variant.py +++ b/tests/models/test_variant.py @@ -91,7 +91,6 @@ def setup_variant_tests(): macos_path="/Volumes/T/", ) - data["test_user1"] = User( name="User1", login="user1", email="user1@user1.com", password="1234" ) diff --git a/tests/models/test_version.py b/tests/models/test_version.py index 6946eed3..3b8fe551 100644 --- a/tests/models/test_version.py +++ b/tests/models/test_version.py @@ -19,6 +19,7 @@ Structure, Task, Type, + User, Version, defaults, log, @@ -50,22 +51,23 @@ def setup_version_db_tests(setup_postgresql_db): """Set up the tests for the Version class with a DB.""" data = dict() data["patcher"] = PlatformPatcher() - # statuses - data["test_status1"] = Status(name="Status1", code="STS1") - data["test_status2"] = Status(name="Status2", code="STS2") - data["test_status3"] = Status(name="Status3", code="STS3") - data["test_status4"] = Status(name="Status4", code="STS4") - data["test_status5"] = Status(name="Status5", code="STS5") - DBSession.add_all( - [ - data["test_status1"], - data["test_status2"], - data["test_status3"], - data["test_status4"], - data["test_status5"], - ] + + # Users + data["test_user1"] = User( + name="Test User 1", + login="tuser1", + email="tuser1@test.com", + password="secret", + ) + data["test_user2"] = User( + name="Test User 2", + login="tuser2", + email="tuser2@test.com", + password="secret", ) - DBSession.commit() + + # statuses + data["status_wip"] = Status.query.filter_by(code="WIP").first() # repository data["test_repo"] = Repository( @@ -1649,12 +1651,43 @@ def test_walk_inputs_is_working_as_expected_in_dfs_mode(setup_version_db_tests): # data["fail"]() +def test_reviews_attribute_is_a_list_of_reviews(setup_version_db_tests): + """Version.reviews attribute is filled with Review instances.""" + data = setup_version_db_tests + data["test_task1"].status = data["status_wip"] + data["test_task1"].responsible = [data["test_user1"], data["test_user2"]] + version = Version(task=data["test_task1"]) + + # request a review + reviews = data["test_task1"].request_review(version=version) + assert reviews[0].version == version + assert reviews[1].version == version + assert isinstance(version.reviews, list) + assert len(version.reviews) == 2 + assert version.reviews == reviews + + @pytest.fixture(scope="function") def setup_version_tests(): """Set up non-DB related tests of Version class.""" data = dict() data["patcher"] = PlatformPatcher() + # users + # test users + data["test_user1"] = User( + name="Test User 1", + login="tuser1", + email="tuser1@test.com", + password="secret", + ) + data["test_user2"] = User( + name="Test User 2", + login="tuser2", + email="tuser2@test.com", + password="secret", + ) + # statuses data["test_status1"] = Status(name="Status1", code="STS1") data["test_status2"] = Status(name="Status2", code="STS2") @@ -1910,3 +1943,48 @@ def test__hash__is_working_as_expected(setup_version_tests): result = hash(v) assert isinstance(result, int) assert result == v.__hash__() + + +def test_request_review_method_calls_task_request_review_method( + setup_version_tests, monkeypatch +): + """request_review() calls Task.request_review() method.""" + data = setup_version_tests + called = [] + + def patched_request_review(self, version=None): + """Patch the request review method.""" + called.append(version) + + data["test_task2"].responsible = [ + data["test_user1"], + data["test_user2"], + ] + + monkeypatch.setattr( + "stalker.models.version.Task.request_review", patched_request_review + ) + v = Version(task=data["test_task2"]) + + assert len(called) == 0 + _ = v.request_review() + assert len(called) == 1 + assert called[0] == v + + +def test_request_review_method_returns_reviews(setup_version_db_tests): + """request_review() returns Reviews.""" + data = setup_version_db_tests + task = data["test_task1"] + task.responsible = [ + data["test_user1"], + data["test_user2"], + ] + task.status = data["status_wip"] + v = Version(task=task) + reviews = v.request_review() + assert len(reviews) == 2 + from stalker.models.review import Review + + assert isinstance(reviews[0], Review) + assert isinstance(reviews[1], Review) diff --git a/tests/models/test_working_hours.py b/tests/models/test_working_hours.py index cd7d13b8..61de5976 100644 --- a/tests/models/test_working_hours.py +++ b/tests/models/test_working_hours.py @@ -78,11 +78,13 @@ def test_working_hours_attribute_is_set_to_a_dictionary_of_other_formatted_data( "not str: 'properly valued'" ) + @pytest.mark.parametrize( - "test_key, test_value", [ + "test_key, test_value", + [ ["sun", [[-10, 1000]]], ["sat", [[900, 1080], [1090, 1500]]], - ] + ], ) def test_working_hours_argument_data_is_not_in_correct_range1(test_key, test_value): """ValueError raised if the time values are not correct in the working_hours arg.""" @@ -99,10 +101,11 @@ def test_working_hours_argument_data_is_not_in_correct_range1(test_key, test_val @pytest.mark.parametrize( - "test_key, test_value", [ + "test_key, test_value", + [ ["sun", [[-10, 1000]]], ["sat", [[900, 1080], [1090, 1500]]], - ] + ], ) def test_working_hours_attribute_data_is_not_in_correct_range1(test_key, test_value): """ValueError raised if the times are not correct in the working_hours attr.""" @@ -167,35 +170,30 @@ def test_working_hours_can_be_string_indexed_with_the_date_short_name(): @pytest.mark.parametrize( - "test_key, test_value, error_type", [ + "test_key, test_value, error_type", + [ [0, "not a proper data", TypeError], ["sun", "not a proper data", TypeError], - [0, ["no proper data"], TypeError], ["sun", ["no proper data"], TypeError], - [0, [["no proper data"]], ValueError], ["sun", [["no proper data"]], ValueError], - [0, [[3]], ValueError], [2, [[2, "a"]], TypeError], [1, [[20, 10], ["a", 300]], TypeError], [5, [[323, 1344], [2, "d"]], TypeError], [0, [[4, 100, 3]], ValueError], - ["mon", [[3]], ValueError], ["mon", [[2, "a"]], TypeError], ["tue", [[20, 10], ["a", 300]], TypeError], ["fri", [[323, 1344], [2, "d"]], TypeError], ["sat", [[4, 100, 3]], ValueError], - ["sun", [[-10, 100]], ValueError], ["sat", [[0, 1800]], ValueError], - [7, [[32, 23], [233, 324]], IndexError], [7, [[32, 23], [233, 324]], IndexError], ["zon", [[32, 23], [233, 324]], KeyError], - ] + ], ) def test___setitem__checks_the_given_data(test_key, test_value, error_type): """__setitem__ checks the given data format.""" @@ -214,7 +212,7 @@ def test___setitem__checks_the_given_data(test_key, test_value, error_type): "two integers and the range of integers should be between 0-1440, " f"not {test_value.__class__.__name__}: '{test_value}'" ), - IndexError: "list index out of range", + IndexError: "list index out of range", KeyError: ( "\"WorkingHours accepts only ['mon', 'tue', 'wed', 'thu', " "'fri', 'sat', 'sun'] as key, not 'zon'\"" @@ -426,7 +424,8 @@ def test_weekly_working_days_is_a_read_only_attribute(): @pytest.mark.parametrize( - "test_data, expected_result", [ + "test_data, expected_result", + [ [ { "mon": [[1, 2]], @@ -437,7 +436,7 @@ def test_weekly_working_days_is_a_read_only_attribute(): "sat": [], "sun": [], }, - 5 + 5, ], [ { @@ -449,7 +448,7 @@ def test_weekly_working_days_is_a_read_only_attribute(): "sat": [[11, 12]], "sun": [], }, - 6 + 6, ], [ { @@ -461,9 +460,9 @@ def test_weekly_working_days_is_a_read_only_attribute(): "sat": [[11, 12]], "sun": [[13, 14]], }, - 7 + 7, ], - ] + ], ) def test_weekly_working_days_is_calculated_correctly(test_data, expected_result): """weekly working days are calculated correctly.""" @@ -491,9 +490,9 @@ def test_yearly_working_days_is_a_read_only_attribute(): assert str(cm.value) == error_message - @pytest.mark.parametrize( - "test_data, expected_result", [ + "test_data, expected_result", + [ [ { "mon": [[1, 2]], @@ -504,7 +503,7 @@ def test_yearly_working_days_is_a_read_only_attribute(): "sat": [], "sun": [], }, - 261 + 261, ], [ { @@ -516,7 +515,7 @@ def test_yearly_working_days_is_a_read_only_attribute(): "sat": [[11, 12]], "sun": [], }, - 313 + 313, ], [ { @@ -528,9 +527,9 @@ def test_yearly_working_days_is_a_read_only_attribute(): "sat": [[11, 12]], "sun": [[13, 14]], }, - 365 - ] - ] + 365, + ], + ], ) def test_yearly_working_days_is_calculated_correctly(test_data, expected_result): """yearly_working_days is calculated correctly."""