From ed430b40dfe3b173e5721114c8edcecf59011682 Mon Sep 17 00:00:00 2001 From: Sage Abdullah Date: Thu, 29 Aug 2024 13:35:05 +0100 Subject: [PATCH] Fix ordering of workflow history detail view's tasks tab --- .../tests/pages/test_workflow_history.py | 30 ++++++++++++------- wagtail/admin/views/generic/history.py | 2 +- wagtail/snippets/tests/test_workflows.py | 24 +++++++++++---- 3 files changed, 39 insertions(+), 17 deletions(-) diff --git a/wagtail/admin/tests/pages/test_workflow_history.py b/wagtail/admin/tests/pages/test_workflow_history.py index 2c31ed4456..e0449e3e11 100644 --- a/wagtail/admin/tests/pages/test_workflow_history.py +++ b/wagtail/admin/tests/pages/test_workflow_history.py @@ -55,6 +55,7 @@ class TestWorkflowHistoryDetail(AdminTemplateTestUtils, WagtailTestUtils, TestCa self.moderator = self.create_superuser("moderator") self.moderator_name = get_user_display_name(self.moderator) + self.user_name = get_user_display_name(self.user) with freeze_time(self.timestamps[0]): self.christmas_event.save_revision() @@ -177,13 +178,17 @@ class TestWorkflowHistoryDetail(AdminTemplateTestUtils, WagtailTestUtils, TestCa self.assertEqual( cells, [ - # FIXME: This should be rendered by timestamp in ascending order, - # otherwise the "Initial Revision" cell would be incorrect. - ["Initial Revision", "In progress"], + # This is divided into different columns per task, so it makes + # sense to start with the initial revision on the first cell and + # then it should be rendered in ascending order. [ - f"Edited at {self.localized_timestamps[0]}", + "Initial Revision", f"Rejected by {self.moderator_name} at {self.localized_timestamps[2]}", ], + [ + f"Edited by {self.user_name} at {self.localized_timestamps[3]}", + "In progress", + ], ], ) @@ -196,7 +201,8 @@ class TestWorkflowHistoryDetail(AdminTemplateTestUtils, WagtailTestUtils, TestCa self.assertEqual( cells, [ - # Should be rendered in reverse chronological order + # The items are merged into a single column as a timeline, so it + # should be rendered in reverse chronological order. [ self.localized_timestamps[3], "Edited", @@ -304,15 +310,16 @@ class TestWorkflowHistoryDetail(AdminTemplateTestUtils, WagtailTestUtils, TestCa self.assertEqual( cells, [ - # FIXME: This should be rendered by timestamp in ascending order, - # otherwise the "Initial Revision" cell would be incorrect. + # This is divided into different columns per task, so it makes + # sense to start with the initial revision on the first cell and + # then it should be rendered in ascending order. [ "Initial Revision", - f"Approved by {self.moderator_name} at {self.localized_timestamps[4]}", + f"Rejected by {self.moderator_name} at {self.localized_timestamps[2]}", ], [ - f"Edited at {self.localized_timestamps[0]}", - f"Rejected by {self.moderator_name} at {self.localized_timestamps[2]}", + f"Edited by {self.user_name} at {self.localized_timestamps[3]}", + f"Approved by {self.moderator_name} at {self.localized_timestamps[4]}", ], ], ) @@ -326,7 +333,8 @@ class TestWorkflowHistoryDetail(AdminTemplateTestUtils, WagtailTestUtils, TestCa self.assertEqual( cells, [ - # Should be rendered in reverse chronological order + # The items are merged into a single column as a timeline, so it + # should be rendered in reverse chronological order. [ self.localized_timestamps[4], "Workflow completed Approved", diff --git a/wagtail/admin/views/generic/history.py b/wagtail/admin/views/generic/history.py index 310db9a5fb..2944a93359 100644 --- a/wagtail/admin/views/generic/history.py +++ b/wagtail/admin/views/generic/history.py @@ -505,7 +505,7 @@ class WorkflowHistoryDetailView( workflow_state=self.workflow_state ).values_list("revision_id", flat=True), ) - .order_by("-created_at") + .order_by("created_at") ) @cached_property diff --git a/wagtail/snippets/tests/test_workflows.py b/wagtail/snippets/tests/test_workflows.py index 41012262b4..418e97432d 100644 --- a/wagtail/snippets/tests/test_workflows.py +++ b/wagtail/snippets/tests/test_workflows.py @@ -192,6 +192,7 @@ class TestWorkflowHistory(AdminTemplateTestUtils, BaseWorkflowsTestCase): self.moderator = self.create_superuser("moderator") self.moderator_name = get_user_display_name(self.moderator) + self.user_name = get_user_display_name(self.user) self.object.text = "Edited!" with freeze_time(self.timestamps[0]): @@ -279,11 +280,17 @@ class TestWorkflowHistory(AdminTemplateTestUtils, BaseWorkflowsTestCase): self.assertEqual( cells, [ - ["Initial Revision", "In progress"], + # This is divided into different columns per task, so it makes + # sense to start with the initial revision on the first cell and + # then it should be rendered in ascending order. [ - f"Edited at {self.localized_timestamps[0]}", + "Initial Revision", f"Rejected by {self.moderator_name} at {self.localized_timestamps[2]}", ], + [ + f"Edited by {self.user_name} at {self.localized_timestamps[3]}", + "In progress", + ], ], ) @@ -296,6 +303,8 @@ class TestWorkflowHistory(AdminTemplateTestUtils, BaseWorkflowsTestCase): self.assertEqual( cells, [ + # The items are merged into a single column as a timeline, so it + # should be rendered in reverse chronological order. [ self.localized_timestamps[3], "Edited", @@ -394,13 +403,16 @@ class TestWorkflowHistory(AdminTemplateTestUtils, BaseWorkflowsTestCase): self.assertEqual( cells, [ + # This is divided into different columns per task, so it makes + # sense to start with the initial revision on the first cell and + # then it should be rendered in ascending order. [ "Initial Revision", - f"Approved by {self.moderator_name} at {self.localized_timestamps[4]}", + f"Rejected by {self.moderator_name} at {self.localized_timestamps[2]}", ], [ - f"Edited at {self.localized_timestamps[0]}", - f"Rejected by {self.moderator_name} at {self.localized_timestamps[2]}", + f"Edited by {self.user_name} at {self.localized_timestamps[3]}", + f"Approved by {self.moderator_name} at {self.localized_timestamps[4]}", ], ], ) @@ -414,6 +426,8 @@ class TestWorkflowHistory(AdminTemplateTestUtils, BaseWorkflowsTestCase): self.assertEqual( cells, [ + # The items are merged into a single column as a timeline, so it + # should be rendered in reverse chronological order. [ self.localized_timestamps[4], "Workflow completed Approved",