From 6fb3629779e08d242d9029ae1a71bba7fcd6d4ff Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 29 Apr 2022 10:45:09 -0400 Subject: [PATCH 01/17] Switch job index filtering to backend. Enables tagged search and will enable pagination in the future. --- client/src/components/admin/Jobs.vue | 61 +++++++++++++++++++---- client/src/components/admin/JobsTable.vue | 17 +++++-- 2 files changed, 63 insertions(+), 15 deletions(-) diff --git a/client/src/components/admin/Jobs.vue b/client/src/components/admin/Jobs.vue index 2ff17dee7925..f198bf93b1d2 100644 --- a/client/src/components/admin/Jobs.vue +++ b/client/src/components/admin/Jobs.vue @@ -38,13 +38,9 @@ - - - - + + + @@ -66,7 +62,6 @@ v-model="jobsItemsModel" :fields="unfinishedJobFields" :items="unfinishedJobs" - :filter="filter" :table-caption="runningTableCaption" :no-items-message="runningNoJobsMessage" :loading="loading" @@ -92,9 +87,12 @@ :table-caption="finishedTableCaption" :fields="finishedJobFields" :items="finishedJobs" - :filter="filter" :no-items-message="finishedNoJobsMessage" :loading="loading" + @tool-clicked="(toolId) => appendTagFilter('tool', toolId)" + @runner-clicked="(runner) => appendTagFilter('runner', runner)" + @handler-clicked="(handler) => appendTagFilter('handler', handler)" + @user-clicked="(user) => appendTagFilter('user', user)" :busy="busy"> @@ -110,14 +108,41 @@ import JOB_STATES_MODEL from "mvc/history/job-states-model"; import { commonJobFields } from "./JobFields"; import { errorMessageAsString } from "utils/simple-error"; import { jobsProvider } from "components/providers/JobProvider"; +import IndexFilter from "components/Indices/IndexFilter"; function cancelJob(jobId, message) { const url = `${getAppRoot()}api/jobs/${jobId}`; return axios.delete(url, { data: { message: message } }); } +const helpHtml = `
+

This textbox box can be used to filter the jobs displayed. + +

Text entered here will be searched against job user, tool ID, job runner, and handler. Additionally, +advanced filtering tags can be used to refine the search more precisely. Tags are of the form +<tag_name>:<tag_value> or <tag_name>:'<tag_value>'. +For instance to search just for jobs with cat1 in the tool name, tool_id:cat1 can be used. +Notice by default the search is not case-sensitive. + +If the quoted version of tag is used, the search is not case sensitive and only full matches will be +returned. So tool_id:'cat1' would show only jobs from the cat1 tool exactly. + +

The available tags are: +

+
user
+
This filters the job index to contain only jobs executed by matching user(s). You may also just click on a user in the list of jobs to filter on that exact user using this directly.
+
handler
+
This filters the job index to contain only jobs executed on matching handler(s). You may also just click on a handler in the list of jobs to filter on that exact user using this directly.
+
runner
+
This filters the job index to contain only jobs executed on matching job runner(s). You may also just click on a runner in the list of jobs to filter on that exact user using this directly.
+
tool
+
This filters the job index to contain only jobs from the matching tool(s). You may also just click on a tool in the list of jobs to filter on that exact user using this directly.
+
+
+`; + export default { - components: { JobLock, JobsTable }, + components: { JobLock, JobsTable, IndexFilter }, data() { return { jobs: [], @@ -142,6 +167,8 @@ export default { busy: true, cutoffMin: 5, showAllRunning: false, + titleSearchJobs: `Search Jobs`, + helpHtml: helpHtml, }; }, computed: { @@ -210,6 +237,9 @@ export default { const dateRangeMin = new Date(Date.now() - cutoff * 60 * 1000).toISOString(); params.date_range_min = `${dateRangeMin}`; } + if (this.filter) { + params.search = this.filter; + } const ctx = { root: getAppRoot(), ...params, @@ -239,6 +269,17 @@ export default { toggleAll(checked) { this.selectedStopJobIds = checked ? this.jobsItemsModel.reduce((acc, j) => [...acc, j["id"]], []) : []; }, + appendTagFilter(tag, text) { + this.appendFilter(`${tag}:'${text}'`); + }, + appendFilter(text) { + const initialFilter = this.filter; + if (initialFilter.length === 0) { + this.filter = text; + } else if (initialFilter.indexOf(text) < 0) { + this.filter = `${text} ${initialFilter}`; + } + }, }, }; diff --git a/client/src/components/admin/JobsTable.vue b/client/src/components/admin/JobsTable.vue index 34d1a80023b2..28504504a9f1 100644 --- a/client/src/components/admin/JobsTable.vue +++ b/client/src/components/admin/JobsTable.vue @@ -4,7 +4,6 @@ v-model="innerValue" :fields="fields" :items="items" - :filter="filter" hover responsive striped @@ -27,6 +26,18 @@ + + + + @@ -55,10 +66,6 @@ export default { items: { required: true, }, - filter: { - type: String, - required: true, - }, busy: { type: Boolean, required: true, From 8452ae932b49f1dd7dd2ded0386ac4c56f94142b Mon Sep 17 00:00:00 2001 From: John Chilton Date: Thu, 5 May 2022 19:28:59 -0400 Subject: [PATCH 02/17] Add effectiveFilter to filtersMixin... --- client/src/components/Indices/filtersMixin.js | 11 +++++++++++ client/src/components/Indices/filtersMixin.test.js | 12 ++++++++++++ 2 files changed, 23 insertions(+) diff --git a/client/src/components/Indices/filtersMixin.js b/client/src/components/Indices/filtersMixin.js index 719297f4a4a3..8a384d5a007e 100644 --- a/client/src/components/Indices/filtersMixin.js +++ b/client/src/components/Indices/filtersMixin.js @@ -7,12 +7,23 @@ export default { data() { return { filter: "", + implicitFilter: null, }; }, computed: { isFiltered() { return !!this.filter; }, + effectiveFilter() { + let filter = this.filter; + const implicitFilter = this.implicitFilter; + if (implicitFilter && filter) { + filter = `${implicitFilter} ${filter}`; + } else if (implicitFilter) { + filter = implicitFilter; + } + return filter; + }, }, methods: { appendTagFilter(tag, text) { diff --git a/client/src/components/Indices/filtersMixin.test.js b/client/src/components/Indices/filtersMixin.test.js index f0ae0f650ba8..8a5caab0825c 100644 --- a/client/src/components/Indices/filtersMixin.test.js +++ b/client/src/components/Indices/filtersMixin.test.js @@ -43,4 +43,16 @@ describe("filtersMixin.js", () => { wrapper.vm.appendTagFilter("name", "foobar"); expect(wrapper.vm.filter).toBe("name:'foobar'"); }); + + it("should have an effective filter that combines implicit and explicit filter", async () => { + wrapper.vm.implicitFilter = "tag:cowdog"; + wrapper.vm.appendTagFilter("name", "foobar"); + expect(wrapper.vm.filter).toBe("name:'foobar'"); + expect(wrapper.vm.effectiveFilter).toBe("tag:cowdog name:'foobar'"); + }); + + it("should just use implicit filter as effective if filter is empty", async () => { + wrapper.vm.implicitFilter = "tag:cowdog"; + expect(wrapper.vm.effectiveFilter).toBe("tag:cowdog"); + }); }); From cefe1e9ee0e0fe60cbf85c2b126d8df65ffb5f25 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Mon, 2 May 2022 14:52:01 -0400 Subject: [PATCH 03/17] Allow viewing invocations for a history. --- .../CurrentHistory/HistoryNavigation.vue | 7 ++++ .../Workflow/HistoryInvocations.vue | 33 +++++++++++++++++++ .../components/Workflow/Invocations.test.js | 33 ++++++++++++++++++- .../src/components/Workflow/Invocations.vue | 27 +++++++++++---- client/src/components/plugins/icons.js | 2 ++ client/src/entry/analysis/router.js | 6 ++++ lib/galaxy/webapps/galaxy/buildapp.py | 1 + 7 files changed, 102 insertions(+), 7 deletions(-) create mode 100644 client/src/components/Workflow/HistoryInvocations.vue diff --git a/client/src/components/History/CurrentHistory/HistoryNavigation.vue b/client/src/components/History/CurrentHistory/HistoryNavigation.vue index 63cc5d368928..baaccd354664 100644 --- a/client/src/components/History/CurrentHistory/HistoryNavigation.vue +++ b/client/src/components/History/CurrentHistory/HistoryNavigation.vue @@ -102,6 +102,13 @@ Extract Workflow + + + Show Invocations + + + + + + + diff --git a/client/src/components/Workflow/Invocations.test.js b/client/src/components/Workflow/Invocations.test.js index 8b1271e2bfec..5b79f7837f6c 100644 --- a/client/src/components/Workflow/Invocations.test.js +++ b/client/src/components/Workflow/Invocations.test.js @@ -62,7 +62,11 @@ describe("Invocations.vue", () => { describe("for a workflow with an empty invocation list", () => { beforeEach(async () => { - axiosMock.onAny().reply(200, [], { total_matches: "0" }); + axiosMock + .onGet("/api/invocations", { + params: { limit: 50, offset: 0, include_terminal: false, workflow_id: "abcde145678" }, + }) + .reply(200, [], { total_matches: "0" }); const propsData = { ownerGrid: false, storedWorkflowName: "My Workflow", @@ -83,6 +87,33 @@ describe("Invocations.vue", () => { }); }); + describe("for a history with an empty invocation list", () => { + beforeEach(async () => { + axiosMock + .onGet("/api/invocations", { + params: { limit: 50, offset: 0, include_terminal: false, history_id: "abcde145678" }, + }) + .reply(200, [], { total_matches: "0" }); + const propsData = { + ownerGrid: false, + historyName: "My History", + historyId: "abcde145678", + }; + wrapper = mount(Invocations, { + propsData, + localVue, + }); + }); + + it("title should be shown", async () => { + expect(wrapper.find("#invocations-title").text()).toBe("Workflow Invocations for My History"); + }); + + it("no invocations message should be shown when not loading", async () => { + expect(wrapper.find("#no-invocations").exists()).toBe(true); + }); + }); + describe("with invocation", () => { beforeEach(async () => { axiosMock diff --git a/client/src/components/Workflow/Invocations.vue b/client/src/components/Workflow/Invocations.vue index 44b517ab491f..6577e1d6f7e3 100644 --- a/client/src/components/Workflow/Invocations.vue +++ b/client/src/components/Workflow/Invocations.vue @@ -102,19 +102,25 @@ export default { headerMessage: { type: String, default: "" }, ownerGrid: { type: Boolean, default: true }, userId: { type: String, default: null }, + historyId: { type: String, default: null }, + historyName: { type: String, default: null }, storedWorkflowId: { type: String, default: null }, storedWorkflowName: { type: String, default: null }, }, data() { - const fields = [ - { key: "expand", label: "", class: "col-button" }, - { key: "workflow_id", label: "Workflow", class: "col-name" }, - { key: "history_id", label: "History", class: "col-history" }, + const fields = [{ key: "expand", label: "", class: "col-button" }]; + if (!this.storedWorkflowId) { + fields.push({ key: "workflow_id", label: "Workflow", class: "col-name" }); + } + if (!this.historyId) { + fields.push({ key: "history_id", label: "History", class: "col-history" }); + } + fields.push( { key: "create_time", label: "Invoked", class: "col-small", sortable: true }, { key: "update_time", label: "Updated", class: "col-small", sortable: true }, { key: "state", class: "col-small" }, - { key: "execute", label: "", class: "col-button" }, - ]; + { key: "execute", label: "", class: "col-button" } + ); return { tableId: "invocation-list-table", invocationItems: [], @@ -132,6 +138,9 @@ export default { if (this.storedWorkflowName) { title += ` for ${this.storedWorkflowName}`; } + if (this.historyName) { + title += ` for ${this.historyName}`; + } return title; }, effectiveNoInvocationsMessage() { @@ -139,6 +148,9 @@ export default { if (this.storedWorkflowName) { message += ` for ${this.storedWorkflowName}`; } + if (this.historyName) { + message += ` for ${this.historyName}`; + } return message; }, }, @@ -168,6 +180,9 @@ export default { if (this.storedWorkflowId) { extraParams["workflow_id"] = this.storedWorkflowId; } + if (this.historyId) { + extraParams["history_id"] = this.historyId; + } if (this.userId) { extraParams["user_id"] = this.userId; } diff --git a/client/src/components/plugins/icons.js b/client/src/components/plugins/icons.js index b6b6c7f6661b..2c4bcba52a5c 100644 --- a/client/src/components/plugins/icons.js +++ b/client/src/components/plugins/icons.js @@ -31,6 +31,7 @@ import { faPlus, faQuestion, faShareAlt, + faSitemap, faStream, faTags, faTrash, @@ -72,6 +73,7 @@ library.add( faPlus, faQuestion, faShareAlt, + faSitemap, faStream, faTags, faTrash, diff --git a/client/src/entry/analysis/router.js b/client/src/entry/analysis/router.js index 78f8d417c930..58f59777b9f0 100644 --- a/client/src/entry/analysis/router.js +++ b/client/src/entry/analysis/router.js @@ -29,6 +29,7 @@ import GridShared from "components/Grid/GridShared"; import GridHistory from "components/Grid/GridHistory"; import HistoryImport from "components/HistoryImport"; import HistoryView from "components/HistoryView"; +import HistoryInvocations from "components/Workflow/HistoryInvocations"; import HistoryMultipleView from "components/History/Multiple/MultipleView"; import InteractiveTools from "components/InteractiveTools/InteractiveTools"; import InvocationReport from "components/Workflow/InvocationReport"; @@ -200,6 +201,11 @@ export function getRouter(Galaxy) { component: HistoryExport, props: true, }, + { + path: "histories/:historyId/invocations", + component: HistoryInvocations, + props: true, + }, { path: "histories/:actionId", component: GridHistory, diff --git a/lib/galaxy/webapps/galaxy/buildapp.py b/lib/galaxy/webapps/galaxy/buildapp.py index 43c92c51fd8a..9f5490b606f4 100644 --- a/lib/galaxy/webapps/galaxy/buildapp.py +++ b/lib/galaxy/webapps/galaxy/buildapp.py @@ -233,6 +233,7 @@ def app_pair(global_conf, load_app_kwds=None, wsgi_preflight=True, **kwargs): webapp.add_client_route("/histories/list") webapp.add_client_route("/histories/import") webapp.add_client_route("/histories/{history_id}/export") + webapp.add_client_route("/histories/{history_id}/invocations") webapp.add_client_route("/histories/list_published") webapp.add_client_route("/histories/list_shared") webapp.add_client_route("/histories/rename") From 113aadfa64e8a9a03965054c461eb079d5a37dd1 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Tue, 3 May 2022 14:04:48 -0400 Subject: [PATCH 04/17] Implement search tag to pages index API endpoint. --- lib/galaxy/managers/pages.py | 72 ++++++++++++++++- lib/galaxy/schema/schema.py | 1 + lib/galaxy/webapps/galaxy/api/pages.py | 17 ++++ lib/galaxy_test/api/test_pages.py | 108 ++++++++++++++++++++++--- lib/galaxy_test/base/populators.py | 2 +- 5 files changed, 187 insertions(+), 13 deletions(-) diff --git a/lib/galaxy/managers/pages.py b/lib/galaxy/managers/pages.py index 88078ceedf01..aa0563674620 100644 --- a/lib/galaxy/managers/pages.py +++ b/lib/galaxy/managers/pages.py @@ -20,6 +20,7 @@ or_, true, ) +from sqlalchemy.orm import aliased from galaxy import ( exceptions, @@ -37,6 +38,12 @@ ready_galaxy_markdown_for_export, ready_galaxy_markdown_for_import, ) +from galaxy.model.index_filter_util import ( + append_user_filter, + raw_text_column_filter, + tag_filter, + text_column_filter, +) from galaxy.model.item_attrs import UsesAnnotations from galaxy.schema.schema import ( CreatePagePayload, @@ -46,6 +53,11 @@ from galaxy.structured_app import MinimalManagerApp from galaxy.util import unicodify from galaxy.util.sanitize_html import sanitize_html +from galaxy.util.search import ( + FilteredTerm, + parse_filters_structured, + RawTextTerm, +) log = logging.getLogger(__name__) @@ -80,6 +92,17 @@ 159: "\u0178", # latin capital letter y with diaeresis } +INDEX_SEARCH_FILTERS = { + "title": "title", + "slug": "slug", + "tag": "tag", + "user": "user", + "u": "user", + "s": "slug", + "t": "tag", + "is": "is", +} + class PageManager(sharable.SharableModelManager, UsesAnnotations): """Provides operations for managing a Page.""" @@ -103,12 +126,13 @@ def index_query(self, trans: ProvidesUserContext, payload: PageIndexQueryPayload query = trans.sa_session.query(model.Page) is_admin = trans.user_is_admin user = trans.user + show_shared = payload.show_shared if not is_admin: filters = [model.Page.user == trans.user] if payload.show_published: filters.append(model.Page.published == true()) - if user and payload.show_shared: + if user and show_shared: filters.append(model.PageUserShareAssociation.user == user) query = query.outerjoin(model.Page.users_shared_with) @@ -124,6 +148,52 @@ def index_query(self, trans: ProvidesUserContext, payload: PageIndexQueryPayload if payload.user_id: query = query.filter(model.Page.user_id == payload.user_id) + if payload.search: + search_query = payload.search + parsed_search = parse_filters_structured(search_query, INDEX_SEARCH_FILTERS) + + def p_tag_filter(term_text: str, quoted: bool): + nonlocal query + alias = aliased(model.PageTagAssociation) + query = query.outerjoin(model.Page.tags.of_type(alias)) + return tag_filter(alias, term_text, quoted) + + for term in parsed_search.terms: + if isinstance(term, FilteredTerm): + key = term.filter + q = term.text + if key == "tag": + pg = p_tag_filter(term.text, term.quoted) + query = query.filter(pg) + elif key == "title": + query = query.filter(text_column_filter(model.Page.title, term)) + elif key == "slug": + query = query.filter(text_column_filter(model.Page.slug, term)) + elif key == "user": + query = append_user_filter(query, model.Page, term) + elif key == "is": + if q == "published": + query = query.filter(model.Page.published == true()) + elif q == "shared_with_me": + if not show_shared: + message = "Can only use tag is:shared_with_me if show_shared parameter also true." + raise exceptions.RequestParameterInvalidException(message) + query = query.filter(model.PageUserShareAssociation.user == user) + elif isinstance(term, RawTextTerm): + tf = p_tag_filter(term.text, False) + alias = aliased(model.User) + query = query.outerjoin(model.Page.user.of_type(alias)) + query = query.filter( + raw_text_column_filter( + [ + model.Page.title, + model.Page.slug, + tf, + alias.username, + ], + term, + ) + ) total_matches = query.count() sort_column = getattr(model.Page, payload.sort_by) if payload.sort_desc: diff --git a/lib/galaxy/schema/schema.py b/lib/galaxy/schema/schema.py index 1baae04d2389..fb01c431d510 100644 --- a/lib/galaxy/schema/schema.py +++ b/lib/galaxy/schema/schema.py @@ -1180,6 +1180,7 @@ class PageIndexQueryPayload(Model): sort_desc: bool = Field(default=True, descritpion="Sort in descending order?") show_published: bool = True show_shared: bool = False + search: Optional[str] = None limit: int = 500 offset: int = 0 diff --git a/lib/galaxy/webapps/galaxy/api/pages.py b/lib/galaxy/webapps/galaxy/api/pages.py index 7837b30a8489..68775c9a1040 100644 --- a/lib/galaxy/webapps/galaxy/api/pages.py +++ b/lib/galaxy/webapps/galaxy/api/pages.py @@ -32,7 +32,9 @@ from galaxy.webapps.galaxy.api import ( depends, DependsOnTrans, + IndexQueryTag, Router, + search_query_param, ) from galaxy.webapps.galaxy.services.pages import PagesService @@ -78,6 +80,19 @@ title="Number of pages to skip in sorted query (to enable pagination).", ) +query_tags = [ + IndexQueryTag("title", "The pages's title."), + IndexQueryTag("slug", "The pages's slug.", "s"), + IndexQueryTag("tag", "The pages's tags.", "t"), + IndexQueryTag("user", "The pages's owner's username.", "u"), +] + +SearchQueryParam: Optional[str] = search_query_param( + model_name="Page", + tags=query_tags, + free_text_fields=["title", "slug", "tag", "user"], +) + @router.cbv class FastAPIPages: @@ -99,6 +114,7 @@ async def index( sort_desc: bool = SortDescQueryParam, limit: int = LimitQueryParam, offset: int = OffsetQueryParam, + search: Optional[str] = SearchQueryParam, ) -> PageSummaryList: """Get a list with summary information of all Pages available to the user.""" payload = PageIndexQueryPayload.construct( @@ -110,6 +126,7 @@ async def index( sort_desc=sort_desc, limit=limit, offset=offset, + search=search, ) return self.service.index(trans, payload) diff --git a/lib/galaxy_test/api/test_pages.py b/lib/galaxy_test/api/test_pages.py index 2df2f28bd2c9..99cb33a537f7 100644 --- a/lib/galaxy_test/api/test_pages.py +++ b/lib/galaxy_test/api/test_pages.py @@ -6,6 +6,7 @@ Union, ) from unittest import SkipTest +from uuid import uuid4 from requests import delete from requests.models import Response @@ -27,8 +28,8 @@ def setUp(self): self.dataset_populator = DatasetPopulator(self.galaxy_interactor) self.workflow_populator = WorkflowPopulator(self.galaxy_interactor) - def _create_valid_page_with_slug(self, slug): - return self.dataset_populator.new_page(slug=slug) + def _create_valid_page_with_slug(self, slug, **kwd) -> Dict[str, Any]: + return self.dataset_populator.new_page(slug=slug, **kwd) def _create_valid_page_as(self, other_email, slug): run_as_user = self._setup_user(other_email) @@ -135,8 +136,7 @@ def test_index_user_published(self): user_id = self.dataset_populator.user_id() response1 = self._create_valid_page_with_slug("indexuseridpublish1") with self._different_user(): - response2 = self._create_valid_page_with_slug("indexuseridpublish2") - self._make_public(response2["id"]) + response2 = self._create_published_page_with_slug("indexuseridpublish2") assert self._users_index_has_page_with_id(response1) assert self._users_index_has_page_with_id(response2) @@ -145,8 +145,7 @@ def test_index_user_published(self): def test_index_show_published(self): with self._different_user(): - response = self._create_valid_page_with_slug("indexshowpublish2") - self._make_public(response["id"]) + response = self._create_published_page_with_slug("indexshowpublish2") assert self._users_index_has_page_with_id(response) assert self._users_index_has_page_with_id(response, dict(show_published=True)) @@ -155,8 +154,7 @@ def test_index_show_published(self): def test_index_show_shared_with_me(self): user_id = self.dataset_populator.user_id() with self._different_user(): - response_published = self._create_valid_page_with_slug("indexshowsharedpublished") - self._make_public(response_published["id"]) + response_published = self._create_published_page_with_slug("indexshowsharedpublished") response_shared = self._create_valid_page_with_slug("indexshowsharedshared") self._share_with_user(response_shared["id"], user_id) @@ -169,8 +167,7 @@ def test_index_show_shared_with_me(self): def test_index_show_shared_with_me_deleted(self): user_id = self.dataset_populator.user_id() with self._different_user(): - response_published = self._create_valid_page_with_slug("indexshowsharedpublisheddeleted") - self._make_public(response_published["id"]) + response_published = self._create_published_page_with_slug("indexshowsharedpublisheddeleted") response_shared = self._create_valid_page_with_slug("indexshowsharedshareddeleted") self._share_with_user(response_shared["id"], user_id) self._delete(f"pages/{response_published['id']}").raise_for_status() @@ -185,6 +182,37 @@ def test_index_show_shared_with_me_deleted(self): response_published, dict(show_shared=True, show_published=True, deleted=True) ) + def test_index_owner(self): + my_workflow_id_1 = self._create_valid_page_with_slug("ownertags-m-1") + email_1 = f"{uuid4()}@test.com" + with self._different_user(email=email_1): + published_page_id_1 = self._create_published_page_with_slug("ownertags-p-1")["id"] + owner_1 = self._get("users").json()[0]["username"] + + email_2 = f"{uuid4()}@test.com" + with self._different_user(email=email_2): + published_page_id_2 = self._create_published_page_with_slug("ownertags-p-2")["id"] + + index_ids = self._index_ids(dict(search="is:published", show_published=True)) + assert published_page_id_1 in index_ids + assert published_page_id_2 in index_ids + assert my_workflow_id_1 not in index_ids + + index_ids = self._index_ids(dict(search=f"is:published u:{owner_1}", show_published=True)) + assert published_page_id_1 in index_ids + assert published_page_id_2 not in index_ids + assert my_workflow_id_1 not in index_ids + + index_ids = self._index_ids(dict(search=f"is:published u:'{owner_1}'", show_published=True)) + assert published_page_id_1 in index_ids + assert published_page_id_2 not in index_ids + assert my_workflow_id_1 not in index_ids + + index_ids = self._index_ids(dict(search=f"is:published {owner_1}", show_published=True)) + assert published_page_id_1 in index_ids + assert published_page_id_2 not in index_ids + assert my_workflow_id_1 not in index_ids + def test_index_ordering(self): older1 = self._create_valid_page_with_slug("indexorderingcreatedfirst")["id"] newer1 = self._create_valid_page_with_slug("indexorderingcreatedsecond")["id"] @@ -207,7 +235,7 @@ def test_index_ordering(self): index_ids = self._index_ids(dict(sort_by="create_time")) assert index_ids.index(older1) > index_ids.index(newer1) - def test_limit_offset(self): + def test_index_limit_offset(self): older1 = self._create_valid_page_with_slug("indexlimitoffsetcreatedfirst")["id"] newer1 = self._create_valid_page_with_slug("indexlimitoffsetcreatedsecond")["id"] index_ids = self._index_ids(dict(limit=1)) @@ -218,6 +246,59 @@ def test_limit_offset(self): assert newer1 not in index_ids assert older1 in index_ids + def test_index_search_slug(self): + response = self._create_valid_page_with_slug("indexsearchstringfoo") + print(response) + older1 = response["id"] + newer1 = self._create_valid_page_with_slug("indexsearchstringbar")["id"] + + index_ids = self._index_ids(dict(search="slug:indexsearchstringfoo")) + assert newer1 not in index_ids + assert older1 in index_ids + + index_ids = self._index_ids(dict(search="slug:'indexsearchstringfoo'")) + assert newer1 not in index_ids + assert older1 in index_ids + + index_ids = self._index_ids(dict(search="slug:foo")) + assert newer1 not in index_ids + assert older1 in index_ids + + index_ids = self._index_ids(dict(search="foo")) + assert newer1 not in index_ids + assert older1 in index_ids + + def test_index_search_title(self): + page_id = self._create_valid_page_with_slug("indexsearchbytitle", title="mycooltitle")["id"] + assert page_id in self._index_ids(dict(search="mycooltitle")) + assert page_id not in self._index_ids(dict(search="mycoolwrongtitle")) + assert page_id in self._index_ids(dict(search="title:mycoolti")) + assert page_id in self._index_ids(dict(search="title:'mycooltitle'")) + assert page_id not in self._index_ids(dict(search="title:'mycoolti'")) + + def test_index_search_sharing_tags(self): + user_id = self.dataset_populator.user_id() + with self._different_user(): + response_published = self._create_valid_page_with_slug("indexshowsharedpublished")["id"] + self._make_public(response_published) + response_shared = self._create_valid_page_with_slug("indexshowsharedshared")["id"] + self._share_with_user(response_shared, user_id) + + assert response_published in self._index_ids(dict(show_published=True, show_shared=True)) + assert response_shared in self._index_ids(dict(show_published=True, show_shared=True)) + + assert response_published in self._index_ids(dict(show_published=True, show_shared=True, search="is:published")) + assert response_shared not in self._index_ids( + dict(show_published=True, show_shared=True, search="is:published") + ) + + assert response_published not in self._index_ids( + dict(show_published=True, show_shared=True, search="is:shared_with_me") + ) + assert response_shared in self._index_ids( + dict(show_published=True, show_shared=True, search="is:shared_with_me") + ) + def test_index_does_not_show_unavailable_pages(self): create_response_json = self._create_valid_page_as("others_page_index@bx.psu.edu", "otherspageindex") assert not self._users_index_has_page_with_id(create_response_json) @@ -355,6 +436,11 @@ def test_400_on_download_pdf_when_unsupported_content_format(self): pdf_response = self._get(f"pages/{page_id}.pdf") self._assert_status_code_is(pdf_response, 400) + def _create_published_page_with_slug(self, slug, **kwd) -> Dict[str, Any]: + response = self.dataset_populator.new_page(slug=slug, **kwd) + self._make_public(response["id"]) + return response + def _make_public(self, page_id: str) -> dict: sharing_response = self._put(f"pages/{page_id}/publish") assert sharing_response.status_code == 200 diff --git a/lib/galaxy_test/base/populators.py b/lib/galaxy_test/base/populators.py index 558fe6cc88a1..214e04014f87 100644 --- a/lib/galaxy_test/base/populators.py +++ b/lib/galaxy_test/base/populators.py @@ -1327,7 +1327,7 @@ def new_page( self, slug: str = "mypage", title: str = "MY PAGE", content_format: str = "html", content: Optional[str] = None ) -> Dict[str, Any]: page_response = self.new_page_raw(slug=slug, title=title, content_format=content_format, content=content) - page_response.raise_for_status() + api_asserts.assert_status_code_is(page_response, 200) return page_response.json() def new_page_raw( From fef9312d47b919e3f598ade43a542de0e4de8a47 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 4 May 2022 14:37:43 -0400 Subject: [PATCH 05/17] Use filtersMixin to improve Jobs index. --- client/src/components/admin/Jobs.vue | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/client/src/components/admin/Jobs.vue b/client/src/components/admin/Jobs.vue index f198bf93b1d2..8e0c6e4e05c8 100644 --- a/client/src/components/admin/Jobs.vue +++ b/client/src/components/admin/Jobs.vue @@ -108,7 +108,7 @@ import JOB_STATES_MODEL from "mvc/history/job-states-model"; import { commonJobFields } from "./JobFields"; import { errorMessageAsString } from "utils/simple-error"; import { jobsProvider } from "components/providers/JobProvider"; -import IndexFilter from "components/Indices/IndexFilter"; +import filtersMixin from "components/Indices/filtersMixin"; function cancelJob(jobId, message) { const url = `${getAppRoot()}api/jobs/${jobId}`; @@ -142,7 +142,8 @@ returned. So tool_id:'cat1' would show only jobs from the cat `; export default { - components: { JobLock, JobsTable, IndexFilter }, + components: { JobLock, JobsTable }, + mixins: [filtersMixin], data() { return { jobs: [], @@ -160,7 +161,6 @@ export default { allSelected: false, indeterminate: false, stopMessage: "", - filter: "", message: "", status: "info", loading: true, @@ -269,17 +269,6 @@ export default { toggleAll(checked) { this.selectedStopJobIds = checked ? this.jobsItemsModel.reduce((acc, j) => [...acc, j["id"]], []) : []; }, - appendTagFilter(tag, text) { - this.appendFilter(`${tag}:'${text}'`); - }, - appendFilter(text) { - const initialFilter = this.filter; - if (initialFilter.length === 0) { - this.filter = text; - } else if (initialFilter.indexOf(text) < 0) { - this.filter = `${text} ${initialFilter}`; - } - }, }, }; From 38df261943d9f7a7bb301fdd14a327f75533083f Mon Sep 17 00:00:00 2001 From: John Chilton Date: Mon, 9 May 2022 09:28:09 -0400 Subject: [PATCH 06/17] Create tableProvider abstraction for paginationMixin.js. --- .../src/components/Workflow/Invocations.vue | 37 ++++++++----------- .../src/components/Workflow/WorkflowList.vue | 27 ++++++++------ .../components/Workflow/paginationMixin.js | 14 +++++++ 3 files changed, 45 insertions(+), 33 deletions(-) diff --git a/client/src/components/Workflow/Invocations.vue b/client/src/components/Workflow/Invocations.vue index 6577e1d6f7e3..a434df6129cb 100644 --- a/client/src/components/Workflow/Invocations.vue +++ b/client/src/components/Workflow/Invocations.vue @@ -11,7 +11,6 @@ v-bind="indexTableAttrs" v-model="invocationItemsModel" :fields="invocationFields" - :items="provider" class="invocations-table"> diff --git a/client/src/components/Page/PageIndexActions.test.js b/client/src/components/Page/PageIndexActions.test.js new file mode 100644 index 000000000000..7d494b29d917 --- /dev/null +++ b/client/src/components/Page/PageIndexActions.test.js @@ -0,0 +1,28 @@ +import PageIndexActions from "./PageIndexActions"; +import { shallowMount } from "@vue/test-utils"; +import { getLocalVue } from "jest/helpers"; + +import "jest-location-mock"; + +const localVue = getLocalVue(); + +describe("PageIndexActions.vue", () => { + let wrapper; + + beforeEach(async () => { + const propsData = { + root: "/rootprefix/", + }; + wrapper = shallowMount(PageIndexActions, { + propsData, + localVue, + }); + }); + + describe("naviation", () => { + it("should create a page when create is clicked", async () => { + await wrapper.find("#page-create").trigger("click"); + expect(window.location).toBeAt("/rootprefix/pages/create"); + }); + }); +}); diff --git a/client/src/components/Page/PageIndexActions.vue b/client/src/components/Page/PageIndexActions.vue new file mode 100644 index 000000000000..717ce2f9adb1 --- /dev/null +++ b/client/src/components/Page/PageIndexActions.vue @@ -0,0 +1,35 @@ + + + diff --git a/client/src/components/Page/PageUrl.test.js b/client/src/components/Page/PageUrl.test.js new file mode 100644 index 000000000000..39421807aee6 --- /dev/null +++ b/client/src/components/Page/PageUrl.test.js @@ -0,0 +1,36 @@ +import PageUrl from "./PageUrl"; +import { mount } from "@vue/test-utils"; +import { getLocalVue } from "jest/helpers"; + +const localVue = getLocalVue(true); + +describe("PageUrl.vue", () => { + let wrapper; + + beforeEach(async () => { + const propsData = { + root: "/rootprefix/", + owner: "jmchilton", + slug: "my-cool-slug", + }; + wrapper = mount(PageUrl, { + propsData, + localVue, + }); + }); + + describe("component", () => { + it("should localize title text", async () => { + expect(wrapper.find("svg title").text()).toBeLocalized(); + }); + + it("should emit an event when owner is clicked on", async () => { + const ownerEl = wrapper.find("a.page-url-owner"); + expect(ownerEl.text()).toBe("jmchilton"); + await ownerEl.trigger("click"); + + const emitted = wrapper.emitted(); + expect(emitted["click-owner"][0][0]).toEqual("jmchilton"); + }); + }); +}); diff --git a/client/src/components/Page/PageUrl.vue b/client/src/components/Page/PageUrl.vue new file mode 100644 index 000000000000..78b51744dc55 --- /dev/null +++ b/client/src/components/Page/PageUrl.vue @@ -0,0 +1,55 @@ + + + diff --git a/client/src/components/Page/services.js b/client/src/components/Page/services.js new file mode 100644 index 000000000000..1265b95c24bd --- /dev/null +++ b/client/src/components/Page/services.js @@ -0,0 +1,20 @@ +import axios from "axios"; +import { rethrowSimple } from "utils/simple-error"; +import { getAppRoot } from "onload/loadConfig"; + +/** Page request helper **/ +export class Services { + constructor(options = {}) { + this.root = options.root || getAppRoot(); + } + + async deletePage(id) { + const url = `${this.root}api/pages/${id}`; + try { + const response = await axios.delete(url); + return response.data; + } catch (e) { + rethrowSimple(e); + } + } +} diff --git a/client/src/components/providers/PageProvider.js b/client/src/components/providers/PageProvider.js new file mode 100644 index 000000000000..71853a9ec5cf --- /dev/null +++ b/client/src/components/providers/PageProvider.js @@ -0,0 +1,18 @@ +import axios from "axios"; +import { cleanPaginationParameters } from "./utils"; + +export function pagesProvider(ctx, callback, extraParams = {}) { + const { root, ...requestParams } = ctx; + const apiUrl = `${root}api/pages`; + const cleanParams = cleanPaginationParameters(requestParams); + const promise = axios.get(apiUrl, { params: { ...cleanParams, ...extraParams } }); + + // Must return a promise that resolves to an array of items + return promise.then((data) => { + // Pluck the array of items off our axios response + const items = data.data; + callback && callback(data); + // Must return an array of items or an empty array if an error occurred + return items || []; + }); +} diff --git a/client/src/components/providers/PageProvider.test.js b/client/src/components/providers/PageProvider.test.js new file mode 100644 index 000000000000..472f94f9fe87 --- /dev/null +++ b/client/src/components/providers/PageProvider.test.js @@ -0,0 +1,43 @@ +import axios from "axios"; +import MockAdapter from "axios-mock-adapter"; + +import { pagesProvider } from "./PageProvider"; + +describe("PageProvider", () => { + let axiosMock; + + beforeEach(async () => { + axiosMock = new MockAdapter(axios); + }); + + afterEach(() => { + axiosMock.restore(); + }); + + describe("fetching workflows without an error", () => { + it("should make an API call and fire callback", async () => { + axiosMock + .onGet("/prefix/api/pages", { + params: { limit: 50, offset: 0, search: "rna tutorial" }, + }) + .reply(200, [{ model_class: "Page" }], { total_matches: "1" }); + + const ctx = { + root: "/prefix/", + perPage: 50, + currentPage: 1, + }; + const extras = { + search: "rna tutorial", + }; + + let called = false; + const callback = function () { + called = true; + }; + const promise = pagesProvider(ctx, callback, extras); + await promise; + expect(called).toBeTruthy(); + }); + }); +}); From b84b983f36accefc5c7bfa6c0e17a7aebd8b2584 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Tue, 10 May 2022 07:25:58 -0400 Subject: [PATCH 08/17] More de-duplication around grids... --- client/src/components/Indices/filtersMixin.js | 14 ++++++ .../components/Workflow/Invocations.test.js | 48 +++++++++++++++++++ .../src/components/Workflow/Invocations.vue | 6 +-- .../src/components/Workflow/WorkflowList.vue | 16 ++----- .../components/Workflow/paginationMixin.js | 16 ++++++- 5 files changed, 83 insertions(+), 17 deletions(-) diff --git a/client/src/components/Indices/filtersMixin.js b/client/src/components/Indices/filtersMixin.js index 8a384d5a007e..d2c3870c1bb5 100644 --- a/client/src/components/Indices/filtersMixin.js +++ b/client/src/components/Indices/filtersMixin.js @@ -4,10 +4,17 @@ export default { components: { IndexFilter, }, + props: { + inputDebounceDelay: { + type: Number, + default: 500, + }, + }, data() { return { filter: "", implicitFilter: null, + helpHtml: null, }; }, computed: { @@ -24,6 +31,13 @@ export default { } return filter; }, + filterAttrs() { + return { + "debounce-delay": this.inputDebounceDelay, + placeholder: this.titleSearch, + "help-html": this.helpHtml, + }; + }, }, methods: { appendTagFilter(tag, text) { diff --git a/client/src/components/Workflow/Invocations.test.js b/client/src/components/Workflow/Invocations.test.js index 5b79f7837f6c..a41e3465b9c2 100644 --- a/client/src/components/Workflow/Invocations.test.js +++ b/client/src/components/Workflow/Invocations.test.js @@ -85,6 +85,10 @@ describe("Invocations.vue", () => { it("no invocations message should be shown when not loading", async () => { expect(wrapper.find("#no-invocations").exists()).toBe(true); }); + + it("should not render pager", async () => { + expect(wrapper.find(".gx-invocations-grid-pager").exists()).toBeFalsy(); + }); }); describe("for a history with an empty invocation list", () => { @@ -112,6 +116,10 @@ describe("Invocations.vue", () => { it("no invocations message should be shown when not loading", async () => { expect(wrapper.find("#no-invocations").exists()).toBe(true); }); + + it("should not render pager", async () => { + expect(wrapper.find(".gx-invocations-grid-pager").exists()).toBeFalsy(); + }); }); describe("with invocation", () => { @@ -186,5 +194,45 @@ describe("Invocations.vue", () => { await wrapper.find(".workflow-run").trigger("click"); expect(window.location).toBeAt("workflows/run?id=workflowId"); }); + + it("should not render pager", async () => { + expect(wrapper.find(".gx-invocations-grid-pager").exists()).toBeFalsy(); + }); + }); + + describe("pagiantions", () => { + beforeEach(async () => { + axiosMock + .onGet("/api/invocations", { params: { limit: 1, offset: 0, include_terminal: false } }) + .reply(200, [mockInvocationData], { total_matches: "3" }); + const propsData = { + ownerGrid: false, + loading: false, + defaultPerPage: 1, + }; + wrapper = mount(Invocations, { + propsData, + computed: { + getWorkflowNameByInstanceId: (state) => (id) => "workflow name", + getWorkflowByInstanceId: (state) => (id) => { + return { id: "workflowId" }; + }, + getHistoryById: (state) => (id) => { + return { id: "historyId" }; + }, + getHistoryNameById: () => () => "history name", + }, + stubs: { + "workflow-invocation-state": { + template: "", + }, + }, + localVue, + }); + }); + + it("title should render pager", async () => { + expect(wrapper.find(".gx-invocations-grid-pager").exists()).toBeTruthy(); + }); }); }); diff --git a/client/src/components/Workflow/Invocations.vue b/client/src/components/Workflow/Invocations.vue index a434df6129cb..9247252fb713 100644 --- a/client/src/components/Workflow/Invocations.vue +++ b/client/src/components/Workflow/Invocations.vue @@ -6,7 +6,7 @@ {{ headerMessage }} - {{ message }} + {{ message }} @@ -124,7 +124,7 @@ export default { dataProvider: invocationsProvider, invocationItemsModel: [], invocationFields: fields, - perPage: this.rowsPerPage(50), + perPage: this.rowsPerPage(this.defaultPerPage || 50), }; }, computed: { diff --git a/client/src/components/Workflow/WorkflowList.vue b/client/src/components/Workflow/WorkflowList.vue index 17294ba73ff5..4b4c631d1281 100644 --- a/client/src/components/Workflow/WorkflowList.vue +++ b/client/src/components/Workflow/WorkflowList.vue @@ -1,15 +1,9 @@