Story #12519

Multi-cluster project search

Added by Tom Morris over 3 years ago. Updated over 3 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Target version:
Start date:
10/31/2017
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
2.0

Description

In the same way that we can search collections across cluster we would also like to be able to search for projects as well. Project names and descriptions will searched. Each result will be labeled with a C or P and a tooltip on giving the full Collection or Project object type.

No filtering by type will be included. No types other than Project will be added.

Multisite search.png (585 KB) Multisite search.png Lucas Di Pentima, 11/21/2017 02:40 PM
Object type icons.png (299 KB) Object type icons.png Lucas Di Pentima, 11/21/2017 03:45 PM

Subtasks

Task #12581: Review 12519-multisite-project-searchResolvedTom Clegg

Associated revisions

Revision 3737e056
Added by Lucas Di Pentima over 3 years ago

Merge branch '12519-multisite-project-search'
Closes #12519

Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <>

History

#1 Updated by Tom Morris over 3 years ago

  • Target version changed from To Be Groomed to 2017-11-22 Sprint

#2 Updated by Tom Morris over 3 years ago

  • Description updated (diff)
  • Story points set to 2.0

#3 Updated by Lucas Di Pentima over 3 years ago

  • Assigned To set to Lucas Di Pentima

#4 Updated by Lucas Di Pentima over 3 years ago

  • Status changed from New to In Progress

#5 Updated by Lucas Di Pentima over 3 years ago

Updates at 336973bf2 - branch 12519-multisite-project-search

Took advantage of MergingLoader and added a way to add multiple queries for every session.
This is a WIP, if the approach is correct I should rename components like CollectionsTable & CollectionsSearch to something more generic.
Also, this should be moved to another URL, now it's below collections/ and if it's going to be multi-object, I think it makes more sense to place it on its own controller.

#6 Updated by Tom Clegg over 3 years ago

I wonder if it would be easier to do a deeper tree: a MergingLoader with one child per session, where each child has one grandchild per object type being searched. I'm thinking that would avoid the session grouping. MergingLoader offers the same interface it expects from its children, so this should work out of the box.

#7 Updated by Lucas Di Pentima over 3 years ago

You were right, it allowed to simplify the code: 357cbabe1
Any ideas about component organization and search url placement?

#8 Updated by Tom Clegg over 3 years ago

Seems like this page is turning into "/search", and /collections/multisite should redirect there.

Maybe CollectionsSearch→Search and CollectionsTable→SearchResultsTable?

#9 Updated by Lucas Di Pentima over 3 years ago

Updates at d4de94839

Couldn't make /search to work as it's already being used by the normal search feature, so I picked /multisite

#10 Updated by Tom Clegg over 3 years ago

found a way to move to /search: 12519-multisite-project-search @ 9ea1f795a9c4050d8f01cd2f130a3c6c3ea1fd69

#11 Updated by Lucas Di Pentima over 3 years ago

Thanks! Adding a screenshot.

#13 Updated by Lucas Di Pentima over 3 years ago

Some tests are failing because of Javascript errors. While tracking them down, here's a new screenshot with object type icons instead of text labels.

#14 Updated by Lucas Di Pentima over 3 years ago

Updates at fda471556
Test run: https://ci.curoverse.com/job/developer-run-tests/508/

Fixed workbench integration test failures due to a for loop iteration not supported by Capybara.
Replaced text labels with an icon on every search result.
Replaced classic style tooltips with bootstrap ones.

#15 Updated by Tom Clegg over 3 years ago

I don't think onupdate is the right way to hook up bootstrap-jquery stuff: it does N^2 work on dom updates. We should be able to run that on each tooltipped element just once. I think we want https://mithril.js.org/lifecycle-methods.html#oncreate, something like this.

                                    'data-toggle': 'tooltip',
                                    oncreate: function(vnode) { $(vnode.dom).tooltip() },
                                    href: item.workbenchBaseURL()+'/'+item.objectType.wb_path+'/'+item.uuid,

With the icon change, it looks like objectKind isn't needed any more, and "label" is an extra level of indirection. We could lose both, and say something like iconsMap[item.objectType.wb_path] in SearchResultsTable.

#16 Updated by Lucas Di Pentima over 3 years ago

Updates at cf6ebed6c
Test run: https://ci.curoverse.com/job/developer-run-tests/509/

Removed superfluous code & moved tooltip calls as requested.

#17 Updated by Lucas Di Pentima over 3 years ago

  • Target version changed from 2017-11-22 Sprint to 2017-12-06 Sprint

#18 Updated by Lucas Di Pentima over 3 years ago

Updates at 2f83263d2
Test run: https://ci.curoverse.com/job/developer-run-tests/510/

On sprint review demo we noticed that qr1hi's urls had a double slash like this:

https://workbench.qr1hi.arvadosapi.com//collections/qr1hi-4zz18-zi49o54dbcc666h

That was because qr1hi does not publish workbenchUrl on its discovery document so the javascript code tries to guess it. Fixed that guessing code to remove the workbenchUrl trailing slash, if it exists.

#19 Updated by Tom Clegg over 3 years ago

LGTM

one more optimization/simplification though:

diff --git a/apps/workbench/app/assets/javascripts/components/search.js b/apps/workbench/app/assets/javascripts/components/search.js
index c593ee11e..4d0cb9d3b 100644
--- a/apps/workbench/app/assets/javascripts/components/search.js
+++ b/apps/workbench/app/assets/javascripts/components/search.js
@@ -127,15 +127,11 @@ window.Search = {
                             return new MultipageLoader({
                                 sessionKey: key,
                                 loadFunc: function(filters) {
+                                    filters = filters.concat(obj_type.filters)
                                     var tsquery = to_tsquery(q)
                                     if (tsquery) {
-                                        filters = filters.slice(0)
                                         filters.push(['any', '@@', tsquery])
                                     }
-                                    // Apply additional type dependant filters, if any.
-                                    for (i = 0; i < obj_type.filters.length; i++) {
-                                        filters.push(obj_type.filters[i])
-                                    }
                                     return vnode.state.sessionDB.request(session, obj_type.api_path, {
                                         data: {
                                             filters: JSON.stringify(filters),

#20 Updated by Lucas Di Pentima over 3 years ago

Updated as suggested at 90a8c9cd8

#21 Updated by Anonymous over 3 years ago

  • Status changed from In Progress to Resolved
  • % Done changed from 0 to 100

Applied in changeset arvados|commit:3737e05681b6cfb22ea0af0da08598e458da16f0.

Also available in: Atom PDF