From c6f776dba51b8f2620380d0875f42740839800ee Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Mon, 29 Apr 2024 14:00:20 +0200 Subject: [PATCH 1/6] Add tests to ensure history update_time gets updated We need this after a bulk operation to ensure the client will catch-up with the results (even if no-op) so it knows the operation ended. --- lib/galaxy_test/api/test_history_contents.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/lib/galaxy_test/api/test_history_contents.py b/lib/galaxy_test/api/test_history_contents.py index f894d206891a..18d8a93c3f80 100644 --- a/lib/galaxy_test/api/test_history_contents.py +++ b/lib/galaxy_test/api/test_history_contents.py @@ -1625,6 +1625,7 @@ def _get_item_with_id_from_history_contents( return None def _apply_bulk_operation(self, history_id: str, payload, query: str = "", expected_status_code: int = 200): + original_history_update_time = self._get_history_update_time(history_id) if query: query = f"?{query}" response = self._put( @@ -1633,8 +1634,22 @@ def _apply_bulk_operation(self, history_id: str, payload, query: str = "", expec json=True, ) self._assert_status_code_is(response, expected_status_code) - return response.json() + result = response.json() + + if "err_msg" in result or result.get("success_count", 0) == 0: + # We don't need to check the history update time if there was an error or no items were updated + return result + + # After a successful operation, history update time should be updated so the changes can be detected by the frontend + after_bulk_operation_history_update_time = self._get_history_update_time(history_id) + assert after_bulk_operation_history_update_time > original_history_update_time + + return result def _assert_bulk_success(self, bulk_operation_result, expected_success_count: int): assert bulk_operation_result["success_count"] == expected_success_count, bulk_operation_result assert not bulk_operation_result["errors"] + + def _get_history_update_time(self, history_id: str): + history = self._get(f"histories/{history_id}").json() + return history.get("update_time") From 7329257d99e2934ec1d10822c4817999f346311a Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Mon, 29 Apr 2024 14:00:52 +0200 Subject: [PATCH 2/6] Mark the item as updated after altering the associated tags --- lib/galaxy/webapps/galaxy/services/history_contents.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/galaxy/webapps/galaxy/services/history_contents.py b/lib/galaxy/webapps/galaxy/services/history_contents.py index 962f3f2b9d38..86b09ac29b11 100644 --- a/lib/galaxy/webapps/galaxy/services/history_contents.py +++ b/lib/galaxy/webapps/galaxy/services/history_contents.py @@ -1510,6 +1510,10 @@ def _change_dbkey(self, item: HistoryItemModel, params: ChangeDbkeyOperationPara def _add_tags(self, trans: ProvidesUserContext, item: HistoryItemModel, params: TagOperationParams): trans.tag_handler.add_tags_from_list(trans.user, item, params.tags, flush=self.flush) + # Changing tags does not change the item, but we need to update the history to trigger the completion of the operation + item.update() def _remove_tags(self, trans: ProvidesUserContext, item: HistoryItemModel, params: TagOperationParams): trans.tag_handler.remove_tags_from_list(trans.user, item, params.tags, flush=self.flush) + # Changing tags does not change the item, but we need to update the history to trigger the completion of the operation + item.update() From a1fc936c4d43f960e5c3286d613da934f6f94d47 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Mon, 29 Apr 2024 14:26:45 +0200 Subject: [PATCH 3/6] Mark item as updated when undelete had no effect Usually if the dataset was not deleted or if it was purged and un-deleting does not do anything --- lib/galaxy/webapps/galaxy/services/history_contents.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/galaxy/webapps/galaxy/services/history_contents.py b/lib/galaxy/webapps/galaxy/services/history_contents.py index 86b09ac29b11..1743bf3c453f 100644 --- a/lib/galaxy/webapps/galaxy/services/history_contents.py +++ b/lib/galaxy/webapps/galaxy/services/history_contents.py @@ -1445,6 +1445,9 @@ def _undelete(self, item: HistoryItemModel): raise exceptions.ItemDeletionException("This item has been permanently deleted and cannot be recovered.") manager = self._get_item_manager(item) manager.undelete(item, flush=self.flush) + # Again, we need to force an update in the edge case where all selected items are already undeleted + # or when the item was purged as undelete will not trigger an update + item.update() def _purge(self, item: HistoryItemModel, trans: ProvidesHistoryContext): if getattr(item, "purged", False): From 407708f9462d9216ba7388de7b2a409214922047 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Mon, 29 Apr 2024 16:17:25 +0200 Subject: [PATCH 4/6] Move update to tag handler --- lib/galaxy/model/tags.py | 1 + lib/galaxy/webapps/galaxy/services/history_contents.py | 4 ---- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/galaxy/model/tags.py b/lib/galaxy/model/tags.py index 22e7ae63ef75..6e9ca0592660 100644 --- a/lib/galaxy/model/tags.py +++ b/lib/galaxy/model/tags.py @@ -95,6 +95,7 @@ def set_tags_from_list( if flush: with transaction(self.sa_session): self.sa_session.commit() + item.update() return item.tags def get_tag_assoc_class(self, item_class): diff --git a/lib/galaxy/webapps/galaxy/services/history_contents.py b/lib/galaxy/webapps/galaxy/services/history_contents.py index 1743bf3c453f..79968b13b5d6 100644 --- a/lib/galaxy/webapps/galaxy/services/history_contents.py +++ b/lib/galaxy/webapps/galaxy/services/history_contents.py @@ -1513,10 +1513,6 @@ def _change_dbkey(self, item: HistoryItemModel, params: ChangeDbkeyOperationPara def _add_tags(self, trans: ProvidesUserContext, item: HistoryItemModel, params: TagOperationParams): trans.tag_handler.add_tags_from_list(trans.user, item, params.tags, flush=self.flush) - # Changing tags does not change the item, but we need to update the history to trigger the completion of the operation - item.update() def _remove_tags(self, trans: ProvidesUserContext, item: HistoryItemModel, params: TagOperationParams): trans.tag_handler.remove_tags_from_list(trans.user, item, params.tags, flush=self.flush) - # Changing tags does not change the item, but we need to update the history to trigger the completion of the operation - item.update() From dbdfcb4f2228addfa5d2cf039de935a7174d8e18 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Mon, 29 Apr 2024 18:56:31 +0200 Subject: [PATCH 5/6] Only call update if it exists --- lib/galaxy/model/tags.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/model/tags.py b/lib/galaxy/model/tags.py index 6e9ca0592660..604efa650719 100644 --- a/lib/galaxy/model/tags.py +++ b/lib/galaxy/model/tags.py @@ -95,7 +95,8 @@ def set_tags_from_list( if flush: with transaction(self.sa_session): self.sa_session.commit() - item.update() + if hasattr(item, "update"): + item.update() return item.tags def get_tag_assoc_class(self, item_class): From 51d8743b5f174abb41bffe3259c50a0b19212aee Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Tue, 30 Apr 2024 10:41:58 +0200 Subject: [PATCH 6/6] Update models to include UsesCreateAndUpdateTime mixin For the sake of consistency, those models that have `create_time`, `update_time` and `tags` should share the same interface to update the `update_time` when tags are added or removed. --- lib/galaxy/model/__init__.py | 13 ++++++++----- lib/galaxy/model/tags.py | 3 +-- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index d78689534474..2519eaec39af 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -2927,7 +2927,7 @@ def prune(cls, sa_session): session.execute(q) -class History(Base, HasTags, Dictifiable, UsesAnnotations, HasName, Serializable): +class History(Base, HasTags, Dictifiable, UsesAnnotations, HasName, Serializable, UsesCreateAndUpdateTime): __tablename__ = "history" __table_args__ = (Index("ix_history_slug", "slug", mysql_length=200),) @@ -3094,6 +3094,9 @@ def username(self): def count(self): return self.hid_counter - 1 + def update(self): + self._update_time = now() + def add_pending_items(self, set_output_hid=True): # These are assumed to be either copies of existing datasets or new, empty datasets, # so we don't need to set the quota. @@ -7362,7 +7365,7 @@ def __init__(self): self.user = None -class StoredWorkflow(Base, HasTags, Dictifiable, RepresentById): +class StoredWorkflow(Base, HasTags, Dictifiable, RepresentById, UsesCreateAndUpdateTime): """ StoredWorkflow represents the root node of a tree of objects that compose a workflow, including workflow revisions, steps, and subworkflows. It is responsible for the metadata associated with a workflow including owner, name, published, and create/update time. @@ -7740,7 +7743,7 @@ def log_str(self): InputConnDictType = Dict[str, Union[Dict[str, Any], List[Dict[str, Any]]]] -class WorkflowStep(Base, RepresentById): +class WorkflowStep(Base, RepresentById, UsesCreateAndUpdateTime): """ WorkflowStep represents a tool or subworkflow, its inputs, annotations, and any outputs that are flagged as workflow outputs. @@ -10061,7 +10064,7 @@ def equals(self, user_id, provider, authn_id, config): ) -class Page(Base, HasTags, Dictifiable, RepresentById): +class Page(Base, HasTags, Dictifiable, RepresentById, UsesCreateAndUpdateTime): __tablename__ = "page" __table_args__ = (Index("ix_page_slug", "slug", mysql_length=200),) @@ -10175,7 +10178,7 @@ class PageUserShareAssociation(Base, UserShareAssociation): page = relationship("Page", back_populates="users_shared_with") -class Visualization(Base, HasTags, Dictifiable, RepresentById): +class Visualization(Base, HasTags, Dictifiable, RepresentById, UsesCreateAndUpdateTime): __tablename__ = "visualization" __table_args__ = ( Index("ix_visualization_dbkey", "dbkey", mysql_length=200), diff --git a/lib/galaxy/model/tags.py b/lib/galaxy/model/tags.py index 604efa650719..6e9ca0592660 100644 --- a/lib/galaxy/model/tags.py +++ b/lib/galaxy/model/tags.py @@ -95,8 +95,7 @@ def set_tags_from_list( if flush: with transaction(self.sa_session): self.sa_session.commit() - if hasattr(item, "update"): - item.update() + item.update() return item.tags def get_tag_assoc_class(self, item_class):