From d34f7582c596e79730cc4d952de1f5dc62c1fd94 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Thu, 28 Mar 2024 19:52:11 +0100 Subject: [PATCH] Add `show_archived` per code review --- client/src/api/schema/schema.ts | 2 ++ .../components/Grid/configs/historiesPublished.ts | 1 + .../src/components/Grid/configs/historiesShared.ts | 1 + lib/galaxy/managers/histories.py | 12 +++--------- lib/galaxy/schema/history.py | 1 + lib/galaxy/webapps/galaxy/api/histories.py | 8 ++++++++ lib/galaxy_test/api/test_histories.py | 8 ++++---- 7 files changed, 20 insertions(+), 13 deletions(-) diff --git a/client/src/api/schema/schema.ts b/client/src/api/schema/schema.ts index cb7b0dc2121..817248b6b43 100644 --- a/client/src/api/schema/schema.ts +++ b/client/src/api/schema/schema.ts @@ -15519,6 +15519,7 @@ export interface operations { parameters?: { /** @description The maximum number of items to return. */ /** @description Starts at the beginning skip the first ( offset - 1 ) items and begin returning at the Nth item */ + /** @description Whether to include archived histories. */ /** @description Sort index by this specified attribute */ /** @description Sort in descending order? */ /** @@ -15570,6 +15571,7 @@ export interface operations { show_own?: boolean; show_published?: boolean; show_shared?: boolean; + show_archived?: boolean | null; sort_by?: "create_time" | "name" | "update_time" | "username"; sort_desc?: boolean; search?: string | null; diff --git a/client/src/components/Grid/configs/historiesPublished.ts b/client/src/components/Grid/configs/historiesPublished.ts index f7480c49445..5abfd9a388d 100644 --- a/client/src/components/Grid/configs/historiesPublished.ts +++ b/client/src/components/Grid/configs/historiesPublished.ts @@ -30,6 +30,7 @@ async function getData(offset: number, limit: number, search: string, sort_by: s show_own: false, show_published: true, show_shared: false, + show_archived: true, }); const totalMatches = parseInt(headers.get("total_matches") ?? "0"); return [data, totalMatches]; diff --git a/client/src/components/Grid/configs/historiesShared.ts b/client/src/components/Grid/configs/historiesShared.ts index 7052c4d3e35..d045f5279bc 100644 --- a/client/src/components/Grid/configs/historiesShared.ts +++ b/client/src/components/Grid/configs/historiesShared.ts @@ -32,6 +32,7 @@ async function getData(offset: number, limit: number, search: string, sort_by: s show_own: false, show_published: false, show_shared: true, + show_archived: true, }); const totalMatches = parseInt(headers.get("total_matches") ?? "0"); return [data, totalMatches]; diff --git a/lib/galaxy/managers/histories.py b/lib/galaxy/managers/histories.py index d3672fb7500..f89687cdc33 100644 --- a/lib/galaxy/managers/histories.py +++ b/lib/galaxy/managers/histories.py @@ -127,6 +127,7 @@ class HistoryManager(sharable.SharableModelManager, deletable.PurgableManagerMix show_published = payload.show_published show_purged = False show_shared = payload.show_shared + show_archived = payload.show_archived is_admin = trans.user_is_admin user = trans.user @@ -207,15 +208,8 @@ class HistoryManager(sharable.SharableModelManager, deletable.PurgableManagerMix self.model_class.deleted == (true() if show_deleted else false()) ) - # Handle archived histories - if show_published: - # Someone published a history and then archived it, it should still be returned - stmt = stmt.where(or_(self.model_class.archived == false(), self.model_class.published == true())) - elif show_shared: - # Someone shared a history with the current user and then archived it, it should still be returned - stmt = stmt.where(or_(self.model_class.archived == false(), self.user_share_model.user == user)) - else: - # By default, only return non-archived histories + # By default, only return non-archived histories + if not show_archived: stmt = stmt.where(self.model_class.archived == false()) stmt = stmt.distinct() diff --git a/lib/galaxy/schema/history.py b/lib/galaxy/schema/history.py index b6e810d7c47..be02370c255 100644 --- a/lib/galaxy/schema/history.py +++ b/lib/galaxy/schema/history.py @@ -26,6 +26,7 @@ class HistoryIndexQueryPayload(Model): show_own: Optional[bool] = None show_published: Optional[bool] = None show_shared: Optional[bool] = None + show_archived: Optional[bool] = None sort_by: HistorySortByEnum = Field("update_time", title="Sort By", description="Sort by this attribute.") sort_desc: Optional[bool] = Field(default=True, title="Sort descending", description="Sort in descending order.") search: Optional[str] = Field(default=None, title="Filter text", description="Freetext to search.") diff --git a/lib/galaxy/webapps/galaxy/api/histories.py b/lib/galaxy/webapps/galaxy/api/histories.py index 32e4c109f1c..067dd425369 100644 --- a/lib/galaxy/webapps/galaxy/api/histories.py +++ b/lib/galaxy/webapps/galaxy/api/histories.py @@ -121,6 +121,12 @@ ShowSharedQueryParam: bool = Query( default=False, title="Include histories shared with authenticated user.", description="" ) +ShowArchivedQueryParam: Optional[bool] = Query( + default=None, + title="Show Archived", + description="Whether to include archived histories.", +) + SortByQueryParam: HistorySortByEnum = Query( default="update_time", title="Sort attribute", @@ -174,6 +180,7 @@ class FastAPIHistories: show_own: bool = ShowOwnQueryParam, show_published: bool = ShowPublishedQueryParam, show_shared: bool = ShowSharedQueryParam, + show_archived: Optional[bool] = ShowArchivedQueryParam, sort_by: HistorySortByEnum = SortByQueryParam, sort_desc: bool = SortDescQueryParam, search: Optional[str] = SearchQueryParam, @@ -196,6 +203,7 @@ class FastAPIHistories: show_own=show_own, show_published=show_published, show_shared=show_shared, + show_archived=show_archived, sort_by=sort_by, sort_desc=sort_desc, limit=limit, diff --git a/lib/galaxy_test/api/test_histories.py b/lib/galaxy_test/api/test_histories.py index 3963404b6af..322dc002584 100644 --- a/lib/galaxy_test/api/test_histories.py +++ b/lib/galaxy_test/api/test_histories.py @@ -310,7 +310,7 @@ class TestHistoriesApi(ApiTestCase, BaseHistories): index_response = self._get("histories", data=data).json() assert len(index_response) == 4 - # Archived histories should not be included in the index + # Archived histories should not be included by default self.dataset_populator.archive_history(archived_history_id) data = dict(search=name_contains, show_published=False) index_response = self._get("histories", data=data).json() @@ -328,13 +328,13 @@ class TestHistoriesApi(ApiTestCase, BaseHistories): index_response = self._get("histories", data=data).json() assert len(index_response) == 0 - # Archived public histories should be included when filtering by published - data = dict(search="is:published") + # Archived public histories should be included when filtering by show_published and show_archived + data = dict(search="is:published", show_archived=True) index_response = self._get("histories", data=data).json() assert len(index_response) == 2 name_contains = "Archived" - data = dict(search=name_contains, show_published=True) + data = dict(search=name_contains, show_published=True, show_archived=True) index_response = self._get("histories", data=data).json() assert len(index_response) == 1