From baaff9a724bb0274e276162ac43e72aedab314ee Mon Sep 17 00:00:00 2001 From: Aadesh-Baral Date: Fri, 1 Sep 2023 11:53:57 +0545 Subject: [PATCH 1/5] Record task extension duration in action_text. ------------------ In the previous implementation, there was an issue where the duration of a task extension was not being recorded when a task was extended and then unlocked/stopped. Prior to the implementation of the task lock extension feature, the last recorded action for a task was always either "LOCKED_FOR_MAPPING" or "LOCKED_FOR_VALIDATION," which matched the task status. To update the duration, we used the current task status as the last action to filter the task history and update the last entry with the lock duration. However, after the task extension feature was introduced, two additional last actions were possible: "EXTENDED_FOR_MAPPING" and "EXTENDED_FOR_VALIDATION." This introduced a discrepancy between the last recorded action and the task status. For example, the task status could still be "LOCKED_FOR_MAPPING" even when a user had extended the task. As a result, the entry with the action "EXTENDED_FOR_MAPPING" in the task history was not being updated with the correct duration. To address this issue, this commit fixed the problem by retrieving the last task action and using it as the filter criteria for updating the task history, rather than relying on the task status as the last action. --- backend/models/postgis/task.py | 96 +++++++++++++++++++++++++++++ backend/services/mapping_service.py | 6 +- 2 files changed, 101 insertions(+), 1 deletion(-) diff --git a/backend/models/postgis/task.py b/backend/models/postgis/task.py index 5bb7352ad5..8652083013 100644 --- a/backend/models/postgis/task.py +++ b/backend/models/postgis/task.py @@ -1103,6 +1103,102 @@ async def reset_task(task_id: int, project_id: int, user_id: int, db: Database): new_state=TaskStatus.READY, ) + @staticmethod + async def unlock_task( + task_id: int, + project_id: int, + user_id: int, + new_state: TaskStatus, + db: Database, + comment: Optional[str] = None, + undo: bool = False, + issues: Optional[List[Dict[str, Any]]] = None, + ): + """Unlock task and ensure duration task locked is saved in History""" + # If not undo, update the duration of the lock + if not undo: + last_history = await TaskHistory.get_last_action(project_id, task_id, db) + # To unlock a task the last action must have been either lock or extension + last_action = TaskAction[last_history["result"][0]["action"]] + TaskHistory.update_task_locked_with_duration( + task_id, project_id, last_action, user_id + ) + + # Only create new history after updating the duration since we need the last action to update the duration. + if comment: + await Task.set_task_history( + task_id, + project_id, + user_id, + TaskAction.COMMENT, + db, + comment=comment, + mapping_issues=issues, + ) + + # Record state change in history + history = await Task.set_task_history( + task_id, + project_id, + user_id, + TaskAction.STATE_CHANGE, + db, + comment=comment, + new_state=new_state, + mapping_issues=issues, + ) + # If undo, clear the mapped_by and validated_by fields + if undo: + if new_state == TaskStatus.MAPPED: + self.validated_by = None + elif new_state == TaskStatus.READY: + self.mapped_by = None + elif ( + new_state in [TaskStatus.MAPPED, TaskStatus.BADIMAGERY] + and TaskStatus(self.task_status) != TaskStatus.LOCKED_FOR_VALIDATION + ): + # Don't set mapped if state being set back to mapped after validation + self.mapped_by = user_id + elif new_state == TaskStatus.VALIDATED: + TaskInvalidationHistory.record_validation( + self.project_id, self.id, user_id, history + ) + self.validated_by = user_id + elif new_state == TaskStatus.INVALIDATED: + TaskInvalidationHistory.record_invalidation( + self.project_id, self.id, user_id, history + ) + self.mapped_by = None + self.validated_by = None + + self.task_status = new_state.value + self.locked_by = None + self.update() + + def reset_lock(self, user_id, comment=None): + """Removes a current lock from a task, resets to last status and + updates history with duration of lock""" + last_history = TaskHistory.get_last_action(self.project_id, self.id) + # To reset a lock the last action must have been either lock or extension + last_action = TaskAction[last_history.action] + TaskHistory.update_task_locked_with_duration( + self.id, self.project_id, last_action, user_id + ) + + # Only set task history after updating the duration since we need the last action to update the duration. + if comment: + self.set_task_history( + action=TaskAction.COMMENT, comment=comment, user_id=user_id + ) + + self.clear_lock() + + def clear_lock(self): + """Resets to last status and removes current lock from a task""" + self.task_status = TaskHistory.get_last_status(self.project_id, self.id).value + self.locked_by = None + self.update() + @staticmethod async def clear_task_lock(task_id: int, project_id: int, db: Database): """Unlocks task in scope, clears the lock as though it never happened.""" diff --git a/backend/services/mapping_service.py b/backend/services/mapping_service.py index c0ad17a914..ee1505d5e3 100644 --- a/backend/services/mapping_service.py +++ b/backend/services/mapping_service.py @@ -578,10 +578,14 @@ async def extend_task_lock_time(extend_dto: ExtendLockTimeDTO, db: Database): else TaskAction.EXTENDED_FOR_VALIDATION ) + # Update the duration of the lock/extension before creating new history + last_history = TaskHistory.get_last_action(task.project_id, task.id) + # To reset a lock the last action must have been either lock or extension + last_action = TaskAction[last_history["result"][0]["action"]] await TaskHistory.update_task_locked_with_duration( task_id, extend_dto.project_id, - TaskStatus(task["task_status"]), + last_action, extend_dto.user_id, db, ) From a68cbc3b7297a997d086fde7632ef99d28644952 Mon Sep 17 00:00:00 2001 From: Aadesh-Baral Date: Fri, 1 Sep 2023 12:10:47 +0545 Subject: [PATCH 2/5] Fix failing test cases due to missing task lock history. --- .../integration/api/tasks/test_actions.py | 33 +++++++++---------- .../unit/services/test_mapping_service.py | 12 +++++++ 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/tests/backend/integration/api/tasks/test_actions.py b/tests/backend/integration/api/tasks/test_actions.py index d27621a08a..ee0e162319 100644 --- a/tests/backend/integration/api/tasks/test_actions.py +++ b/tests/backend/integration/api/tasks/test_actions.py @@ -560,9 +560,8 @@ def test_mapping_unlock_returns_200_on_success(self): """Test returns 200 on success.""" # Arrange task = Task.get(1, self.test_project.id) - task.task_status = TaskStatus.LOCKED_FOR_MAPPING.value - task.locked_by = self.test_user.id - task.update() + task.status = TaskStatus.READY.value + task.lock_task_for_mapping(self.test_user.id) # Act response = self.client.post( self.url, @@ -583,9 +582,8 @@ def test_mapping_unlock_returns_200_on_success_with_comment(self): """Test returns 200 on success.""" # Arrange task = Task.get(1, self.test_project.id) - task.task_status = TaskStatus.LOCKED_FOR_MAPPING.value - task.locked_by = self.test_user.id - task.update() + task.status = TaskStatus.READY.value + task.lock_task_for_mapping(self.test_user.id) # Act response = self.client.post( self.url, @@ -684,9 +682,8 @@ def test_mapping_stop_returns_200_on_success(self): """Test returns 200 on success.""" # Arrange task = Task.get(1, self.test_project.id) - task.task_status = TaskStatus.LOCKED_FOR_MAPPING.value - task.locked_by = self.test_user.id - task.update() + task.status = TaskStatus.READY.value + task.lock_task_for_mapping(self.test_user.id) # Act response = self.client.post( self.url, @@ -702,9 +699,8 @@ def test_mapping_stop_returns_200_on_success_with_comment(self): """Test returns 200 on success.""" # Arrange task = Task.get(1, self.test_project.id) - task.task_status = TaskStatus.LOCKED_FOR_MAPPING.value - task.locked_by = self.test_user.id - task.update() + task.status = TaskStatus.READY.value + task.lock_task_for_mapping(self.test_user.id) # Act response = self.client.post( self.url, @@ -991,11 +987,14 @@ def test_validation_unlock_returns_403_if_task_not_locked_for_validation(self): def lock_task_for_validation(task_id, project_id, user_id, mapped_by=None): """Lock task for validation.""" task = Task.get(task_id, project_id) - task.task_status = TaskStatus.LOCKED_FOR_VALIDATION.value - task.locked_by = user_id + if mapped_by: - task.mapped_by = mapped_by - task.update() + task.status = TaskStatus.READY.value + task.lock_task_for_mapping(mapped_by) + task.unlock_task(mapped_by, TaskStatus.MAPPED) + + task.status = TaskStatus.MAPPED.value + task.lock_task_for_validating(user_id) def test_validation_unlock_returns_403_if_task_locked_by_other_user(self): """Test returns 403 if task locked by other user.""" @@ -1206,7 +1205,6 @@ def test_validation_stop_returns_200_if_task_locked_by_user(self): """Test returns 200 if task locked by user.""" # Arrange task = Task.get(1, self.test_project.id) - task.unlock_task(self.test_user.id, TaskStatus.MAPPED) last_task_status = TaskStatus(task.task_status).name TestTasksActionsValidationUnlockAPI.lock_task_for_validation( 1, self.test_project.id, self.test_user.id, self.test_user.id @@ -1227,7 +1225,6 @@ def test_validation_stop_returns_200_if_task_locked_by_user_with_comment(self): """Test returns 200 if task locked by user with comment.""" # Arrange task = Task.get(1, self.test_project.id) - task.unlock_task(self.test_user.id, TaskStatus.MAPPED) last_task_status = TaskStatus(task.task_status).name TestTasksActionsValidationUnlockAPI.lock_task_for_validation( 1, self.test_project.id, self.test_user.id, self.test_user.id diff --git a/tests/backend/unit/services/test_mapping_service.py b/tests/backend/unit/services/test_mapping_service.py index 731de0c30c..0f7b01ee5f 100644 --- a/tests/backend/unit/services/test_mapping_service.py +++ b/tests/backend/unit/services/test_mapping_service.py @@ -3,6 +3,7 @@ from backend.models.dtos.mapping_dto import LockTaskDTO, MappedTaskDTO from backend.models.postgis.project_info import ProjectInfo from backend.models.postgis.task import TaskAction, TaskHistory, User + from backend.services.mapping_service import ( MappingNotAllowed, MappingService, @@ -128,6 +129,7 @@ def test_if_new_state_not_acceptable_raise_error(self, mock_task): with self.assertRaises(MappingServiceError): MappingService.unlock_task_after_mapping(self.mapped_task_dto) + @patch.object(TaskHistory, "get_last_action") @patch.object(ProjectService, "send_email_on_project_progress") @patch.object(ProjectInfo, "get_dto_for_locale") @patch.object(Task, "get_per_task_instructions") @@ -148,6 +150,7 @@ def test_unlock_with_comment_sets_history( mock_state, mock_project_name, mock_send_email, + mock_last_action, ): # Arrange self.task_stub.task_status = TaskStatus.LOCKED_FOR_MAPPING.value @@ -155,6 +158,10 @@ def test_unlock_with_comment_sets_history( mock_task.return_value = self.task_stub mock_state.return_value = TaskStatus.LOCKED_FOR_MAPPING mock_project_name.name.return_value = "Test project" + history = TaskHistory(1, 1, 1) + history.action = TaskAction.LOCKED_FOR_MAPPING.name + mock_last_action.return_value = history + # Act test_task = MappingService.unlock_task_after_mapping(self.mapped_task_dto) @@ -164,6 +171,7 @@ def test_unlock_with_comment_sets_history( self.assertEqual(TaskAction.COMMENT.name, test_task.task_history[0].action) self.assertEqual(test_task.task_history[0].action_text, "Test comment") + @patch.object(TaskHistory, "get_last_action") @patch.object(ProjectService, "send_email_on_project_progress") @patch.object(Task, "get_per_task_instructions") @patch.object(StatsService, "update_stats_after_task_state_change") @@ -180,11 +188,15 @@ def test_unlock_with_status_change_sets_history( mock_instructions, mock_state, mock_send_email, + mock_last_action, ): # Arrange self.task_stub.task_status = TaskStatus.LOCKED_FOR_MAPPING.value mock_task.return_value = self.task_stub mock_state.return_value = TaskStatus.LOCKED_FOR_MAPPING + history = TaskHistory(1, 1, 1) + history.action = TaskAction.LOCKED_FOR_MAPPING.name + mock_last_action.return_value = history # Act test_task = MappingService.unlock_task_after_mapping(self.mapped_task_dto) From e45474d7dc8c04944d06c17575d34a2bde187ebe Mon Sep 17 00:00:00 2001 From: Aadesh-Baral Date: Mon, 4 Sep 2023 16:37:49 +0545 Subject: [PATCH 3/5] Add test case to check if extension duration is recorded --- .../integration/api/tasks/test_actions.py | 14 +++++++ .../services/test_mapping_service.py | 42 ++++++++++++++++++- 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/tests/backend/integration/api/tasks/test_actions.py b/tests/backend/integration/api/tasks/test_actions.py index ee0e162319..f5008d2452 100644 --- a/tests/backend/integration/api/tasks/test_actions.py +++ b/tests/backend/integration/api/tasks/test_actions.py @@ -577,6 +577,10 @@ def test_mapping_unlock_returns_200_on_success(self): self.assertEqual(last_task_history["action"], TaskAction.STATE_CHANGE.name) self.assertEqual(last_task_history["actionText"], TaskStatus.MAPPED.name) self.assertEqual(last_task_history["actionBy"], self.test_user.username) + # Check if lock duration is saved + # Since locked duration is saved in entry with action as "LOCKED_FOR_MAPPING" + # we need to look for second entry in task history + self.assertIsNotNone(response.json["taskHistory"][1]["actionText"]) def test_mapping_unlock_returns_200_on_success_with_comment(self): """Test returns 200 on success.""" @@ -609,6 +613,11 @@ def test_mapping_unlock_returns_200_on_success_with_comment(self): self.assertEqual(last_comment_history["actionText"], "cannot map") self.assertEqual(last_comment_history["actionBy"], self.test_user.username) + # Check if lock duration is saved + # Since locked duration is saved in entry with action as "LOCKED_FOR_MAPPING" + # we need to look for third entry in task history as second entry is comment + self.assertIsNotNone(response.json["taskHistory"][2]["actionText"]) + class TestTasksActionsMappingStopAPI(BaseTestCase): def setUp(self): @@ -1252,6 +1261,11 @@ def test_validation_stop_returns_200_if_task_locked_by_user_with_comment(self): self.assertEqual(task_history_comment["actionText"], "Test comment") self.assertEqual(task_history_comment["actionBy"], self.test_user.username) + # Check if lock duration is saved + # Since locked duration is saved in entry with action as "LOCKED_FOR_MAPPING" + # we need to look for third entry in task history as second entry is comment + self.assertIsNotNone(response.json["tasks"][0]["taskHistory"][2]["actionText"]) + class TestTasksActionsSplitAPI(BaseTestCase): def setUp(self): diff --git a/tests/backend/integration/services/test_mapping_service.py b/tests/backend/integration/services/test_mapping_service.py index e8158d433d..7c4a0f339c 100644 --- a/tests/backend/integration/services/test_mapping_service.py +++ b/tests/backend/integration/services/test_mapping_service.py @@ -2,8 +2,13 @@ import xml.etree.ElementTree as ET from unittest.mock import patch +from backend.services.mapping_service import ( + MappingService, + Task, + TaskHistory, + ExtendLockTimeDTO, +) from backend.models.postgis.task import TaskStatus -from backend.services.mapping_service import MappingService, Task from tests.backend.base import BaseTestCase from tests.backend.helpers.test_helpers import create_canned_project @@ -165,3 +170,38 @@ def test_reset_all_bad_imagery( # Assert for task in self.test_project.tasks: self.assertNotEqual(task.task_status, TaskStatus.BADIMAGERY.value) + + def test_task_extend_duration_is_recorded(self): + if self.skip_tests: + return + + # Arrange + task = Task.get(1, self.test_project.id) + task.task_status = TaskStatus.READY.value + task.update() + task.lock_task_for_mapping(self.test_user.id) + extend_lock_dto = ExtendLockTimeDTO() + extend_lock_dto.task_ids = [task.id] + extend_lock_dto.project_id = self.test_project.id + extend_lock_dto.user_id = self.test_user.id + # Act + # Extend the task lock time twice and check the task history + MappingService.extend_task_lock_time(extend_lock_dto) + MappingService.extend_task_lock_time(extend_lock_dto) + task.reset_lock(self.test_user.id) + + # Assert + # Check that the task history has 2 entries for EXTENDED_FOR_MAPPING and that the action_text is not None + extended_task_history = ( + TaskHistory.query.filter_by( + task_id=task.id, + project_id=self.test_project.id, + ) + .order_by(TaskHistory.action_date.desc()) + .limit(5) + .all() + ) + self.assertEqual(extended_task_history[0].action, "EXTENDED_FOR_MAPPING") + self.assertEqual(extended_task_history[1].action, "EXTENDED_FOR_MAPPING") + self.assertIsNotNone(extended_task_history[0].action_text) + self.assertIsNotNone(extended_task_history[1].action_text) From 4a884240d381bdef3db3f13a782ac59cbe753ac0 Mon Sep 17 00:00:00 2001 From: Aadesh-Baral Date: Mon, 6 Oct 2025 19:49:59 +0545 Subject: [PATCH 4/5] #6014: Rebased with develop to incorporate changes --- backend/models/postgis/task.py | 241 +++++++++------------------- backend/services/mapping_service.py | 2 +- 2 files changed, 77 insertions(+), 166 deletions(-) diff --git a/backend/models/postgis/task.py b/backend/models/postgis/task.py index 8652083013..6918e99c5c 100644 --- a/backend/models/postgis/task.py +++ b/backend/models/postgis/task.py @@ -1120,8 +1120,8 @@ async def unlock_task( last_history = await TaskHistory.get_last_action(project_id, task_id, db) # To unlock a task the last action must have been either lock or extension last_action = TaskAction[last_history["result"][0]["action"]] - TaskHistory.update_task_locked_with_duration( - task_id, project_id, last_action, user_id + await TaskHistory.update_task_locked_with_duration( + task_id, project_id, last_action, user_id, db ) # Only create new history after updating the duration since we need the last action to update the duration. @@ -1148,156 +1148,6 @@ async def unlock_task( mapping_issues=issues, ) # If undo, clear the mapped_by and validated_by fields - if undo: - if new_state == TaskStatus.MAPPED: - self.validated_by = None - elif new_state == TaskStatus.READY: - self.mapped_by = None - elif ( - new_state in [TaskStatus.MAPPED, TaskStatus.BADIMAGERY] - and TaskStatus(self.task_status) != TaskStatus.LOCKED_FOR_VALIDATION - ): - # Don't set mapped if state being set back to mapped after validation - self.mapped_by = user_id - elif new_state == TaskStatus.VALIDATED: - TaskInvalidationHistory.record_validation( - self.project_id, self.id, user_id, history - ) - self.validated_by = user_id - elif new_state == TaskStatus.INVALIDATED: - TaskInvalidationHistory.record_invalidation( - self.project_id, self.id, user_id, history - ) - self.mapped_by = None - self.validated_by = None - - self.task_status = new_state.value - self.locked_by = None - self.update() - - def reset_lock(self, user_id, comment=None): - """Removes a current lock from a task, resets to last status and - updates history with duration of lock""" - last_history = TaskHistory.get_last_action(self.project_id, self.id) - # To reset a lock the last action must have been either lock or extension - last_action = TaskAction[last_history.action] - TaskHistory.update_task_locked_with_duration( - self.id, self.project_id, last_action, user_id - ) - - # Only set task history after updating the duration since we need the last action to update the duration. - if comment: - self.set_task_history( - action=TaskAction.COMMENT, comment=comment, user_id=user_id - ) - - self.clear_lock() - - def clear_lock(self): - """Resets to last status and removes current lock from a task""" - self.task_status = TaskHistory.get_last_status(self.project_id, self.id).value - self.locked_by = None - self.update() - - @staticmethod - async def clear_task_lock(task_id: int, project_id: int, db: Database): - """Unlocks task in scope, clears the lock as though it never happened.""" - - # Get the last locked action and delete it from the task history - last_action = await TaskHistory.get_last_locked_action(project_id, task_id, db) - if last_action: - delete_action_query = """ - DELETE FROM task_history - WHERE id = :history_id - """ - await db.execute( - query=delete_action_query, values={"history_id": last_action["id"]} - ) - - # Clear the lock from the task itself - await Task.clear_lock(task_id=task_id, project_id=project_id, db=db) - - @staticmethod - async def record_auto_unlock( - task_id: int, project_id: int, lock_duration: str, db: Database - ): - """Automatically unlocks the task and records the auto-unlock action in task history""" - - # Fetch the locked user and last locked action for the task - locked_user_query = """ - SELECT locked_by - FROM tasks - WHERE id = :task_id AND project_id = :project_id - """ - locked_user = await db.fetch_one( - query=locked_user_query, - values={"task_id": task_id, "project_id": project_id}, - ) - - last_action = await TaskHistory.get_last_locked_action(project_id, task_id, db) - - if last_action and last_action["action"] == "LOCKED_FOR_MAPPING": - next_action = TaskAction.AUTO_UNLOCKED_FOR_MAPPING - else: - next_action = TaskAction.AUTO_UNLOCKED_FOR_VALIDATION - - # Clear the task lock (clear the lock and delete the last locked action) - await Task.clear_task_lock(task_id, project_id, db) - - # Add AUTO_UNLOCKED action in the task history - auto_unlocked = await Task.set_task_history( - task_id=task_id, - project_id=project_id, - user_id=locked_user["locked_by"], - action=next_action, - db=db, - ) - - # Update the action_text with the lock duration - update_history_query = """ - UPDATE task_history - SET action_text = :lock_duration - WHERE id = :history_id - """ - await db.execute( - query=update_history_query, - values={"lock_duration": lock_duration, "history_id": auto_unlocked["id"]}, - ) - - @staticmethod - async def unlock_task( - task_id: int, - project_id: int, - user_id: int, - new_state: TaskStatus, - db: Database, - comment: Optional[str] = None, - undo: bool = False, - issues: Optional[List[Dict[str, Any]]] = None, - ): - """Unlock the task and change its state.""" - # Add task comment history if provided - if comment: - await Task.set_task_history( - task_id, - project_id, - user_id, - TaskAction.COMMENT, - db, - comment=comment, - mapping_issues=issues, - ) - # Record state change in history - history = await Task.set_task_history( - task_id, - project_id, - user_id, - TaskAction.STATE_CHANGE, - db, - comment=comment, - new_state=new_state, - mapping_issues=issues, - ) if undo: if new_state == TaskStatus.MAPPED: update_query = """ @@ -1378,11 +1228,6 @@ async def unlock_task( }, ) - # Update task locked duration in the history when `undo` is False - # Using a slightly evil side effect of Actions and Statuses having the same name here :) - await TaskHistory.update_task_locked_with_duration( - task_id, project_id, TaskStatus(current_status), user_id, db - ) # Final query for updating task status final_update_query = """ UPDATE tasks @@ -1398,6 +1243,71 @@ async def unlock_task( }, ) + @staticmethod + async def clear_task_lock(task_id: int, project_id: int, db: Database): + """Unlocks task in scope, clears the lock as though it never happened.""" + + # Get the last locked action and delete it from the task history + last_action = await TaskHistory.get_last_locked_action(project_id, task_id, db) + if last_action: + delete_action_query = """ + DELETE FROM task_history + WHERE id = :history_id + """ + await db.execute( + query=delete_action_query, values={"history_id": last_action["id"]} + ) + + # Clear the lock from the task itself + await Task.clear_lock(task_id=task_id, project_id=project_id, db=db) + + @staticmethod + async def record_auto_unlock( + task_id: int, project_id: int, lock_duration: str, db: Database + ): + """Automatically unlocks the task and records the auto-unlock action in task history""" + + # Fetch the locked user and last locked action for the task + locked_user_query = """ + SELECT locked_by + FROM tasks + WHERE id = :task_id AND project_id = :project_id + """ + locked_user = await db.fetch_one( + query=locked_user_query, + values={"task_id": task_id, "project_id": project_id}, + ) + + last_action = await TaskHistory.get_last_locked_action(project_id, task_id, db) + + if last_action and last_action["action"] == "LOCKED_FOR_MAPPING": + next_action = TaskAction.AUTO_UNLOCKED_FOR_MAPPING + else: + next_action = TaskAction.AUTO_UNLOCKED_FOR_VALIDATION + + # Clear the task lock (clear the lock and delete the last locked action) + await Task.clear_task_lock(task_id, project_id, db) + + # Add AUTO_UNLOCKED action in the task history + auto_unlocked = await Task.set_task_history( + task_id=task_id, + project_id=project_id, + user_id=locked_user["locked_by"], + action=next_action, + db=db, + ) + + # Update the action_text with the lock duration + update_history_query = """ + UPDATE task_history + SET action_text = :lock_duration + WHERE id = :history_id + """ + await db.execute( + query=update_history_query, + values={"lock_duration": lock_duration, "history_id": auto_unlocked["id"]}, + ) + @staticmethod async def reset_lock( task_id: int, @@ -1416,6 +1326,15 @@ async def reset_lock( :param comment: Optional comment provided during the reset. :param db: The database connection. """ + + last_history = TaskHistory.get_last_action(project_id, task_id, db) + # To reset a lock the last action must have been either lock or extension + last_action = TaskAction[last_history["result"][0]["action"]] + await TaskHistory.update_task_locked_with_duration( + task_id, project_id, last_action, user_id, db + ) + + # Only set task history after updating the duration since we need the last action to update the duration. # If a comment is provided, set the task history with a comment action if comment: await Task.set_task_history( @@ -1426,14 +1345,6 @@ async def reset_lock( comment=comment, db=db, ) - # Update task lock history with duration - await TaskHistory.update_task_locked_with_duration( - task_id=task_id, - project_id=project_id, - lock_action=TaskStatus(task_status), - user_id=user_id, - db=db, - ) # Clear the lock on the task await Task.clear_lock(task_id=task_id, project_id=project_id, db=db) diff --git a/backend/services/mapping_service.py b/backend/services/mapping_service.py index ee1505d5e3..65cca590e8 100644 --- a/backend/services/mapping_service.py +++ b/backend/services/mapping_service.py @@ -579,7 +579,7 @@ async def extend_task_lock_time(extend_dto: ExtendLockTimeDTO, db: Database): ) # Update the duration of the lock/extension before creating new history - last_history = TaskHistory.get_last_action(task.project_id, task.id) + last_history = await TaskHistory.get_last_action(extend_dto.project_id, task_id) # To reset a lock the last action must have been either lock or extension last_action = TaskAction[last_history["result"][0]["action"]] await TaskHistory.update_task_locked_with_duration( From ec504037aea1a9f3bed15e28d9bee7bdbfa8c0a2 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 6 Oct 2025 14:07:55 +0000 Subject: [PATCH 5/5] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- backend/services/mapping_service.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/backend/services/mapping_service.py b/backend/services/mapping_service.py index 65cca590e8..c494b78236 100644 --- a/backend/services/mapping_service.py +++ b/backend/services/mapping_service.py @@ -579,7 +579,9 @@ async def extend_task_lock_time(extend_dto: ExtendLockTimeDTO, db: Database): ) # Update the duration of the lock/extension before creating new history - last_history = await TaskHistory.get_last_action(extend_dto.project_id, task_id) + last_history = await TaskHistory.get_last_action( + extend_dto.project_id, task_id + ) # To reset a lock the last action must have been either lock or extension last_action = TaskAction[last_history["result"][0]["action"]] await TaskHistory.update_task_locked_with_duration(