1059322 - Filters: URL parameter for filtering isn't honoured

Status: RESOLVED FIXED

(Tree Management :: Treeherder, defect, P2)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

In order to sheriff talos results, I need to look at a certain job over a build range, I do this by going to:
https://tbpl.mozilla.org/?fromchange=86afc8441856&tochange=76b55c0850ca&jobname=Windows%20XP%2032-bit%20mozilla-central%20pgo%20pgo%20talos%20other

applying that to treeherder, I did this:
https://treeherder.mozilla.org/ui/#/jobs?fromchange=86afc8441856&tochange=76b55c0850ca&jobname=Windows%20XP%2032-bit%20mozilla-central%20pgo%20talos%20other

the fromchange and tochange worked, but jobname didn't filter.  If there is another way to do this, please let me know!

Comment 1

[Ed Morley [:emorley]]

This is a few issues, some of which have other bugs filed:
1) The url param name has changed. Might be worth changing it back, though there are now more complex filtering modes in addition to quick filter, so need to consider that. However if this were the only issue you could use the other param name or just enter the string in the UI.
2) Filtering by buildername doesn't currently work when entered in the UI (bug 1045857).
3) The query string param isn't honoured

Let's make this about #3 and maybe also at the same time #1, but I'll let Cameron comment on that part first.

Reduced STR:
1) Open treeherder
2) Enter "talos" in the quick filters box top right of the UI
3) Copy the resultant URL from the address bar (https://treeherder.mozilla.org/ui/#/jobs?searchQuery=talos)
4) Paste that URL into a new tab

Expected:
New treeherder page opens, filtered to talos jobs

Actual:
New treeherder page opens, but as though no searchQuery param passed.

Comment 2

[Jonathan Eads ( :jeads )]

#3 in comment 1 was fixed in https://github.com/mozilla/treeherder-ui/commit/5658db9b31b5c072953a22b2f15baedb7c5361d0 and reviewed by camd in a PR. Multiple space delimited words in the "Filter platforms & jobs" input box are now functional in the url params.

You can test with:
https://treeherder.mozilla.org/ui/#/jobs?searchQuery=linux%20talos

We will address:

1) The url param name has changed. Might be worth changing it back, though there are now more complex filtering modes in addition to quick filter, so need to consider that.

In a follow up PR.

Not sure what to do about the jobname parameter, it doesn't exactly refer to what treeherder calls a job name.

Maybe we could keep the jobname parameter but have it filter by buildername specifically instead of the search parameters in the "Filter platforms & jobs" box entry?

Until we do that you can use the "searchQuery" parameter in place of jobname. We will likely not address this until all of the transition blockers have been fixed.

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

somehow the magical link above works:
https://treeherder.mozilla.org/ui/#/jobs?searchQuery=linux%20talos

bug what about:
https://treeherder.mozilla.org/ui/#/jobs?searchQuery=linux%20talos%20tp5o

I get no results.

So I start with the url for linux talos and then I edit the url to add %20tp5o and end up (after spinning my cpu for 20 seconds):
https://treeherder.mozilla.org/ui/#/jobs?searchQuery=linux

it appears we have a good bit of work to do for supporting the searchQuery.  here are some common use cases:
1) click a generated query to view a specific job over time
2) edit it to adjust the platform so I can see additional platforms of the same job
3) edit it to change the jobname to narrow down the scope.

Comment 4

[Ed Morley [:emorley]]

(In reply to Jonathan Eads ( :jeads ) from comment #2)
> #3 in comment 1 was fixed in
> https://github.com/mozilla/treeherder-ui/commit/
> 5658db9b31b5c072953a22b2f15baedb7c5361d0 and reviewed by camd in a PR.
> Multiple space delimited words in the "Filter platforms & jobs" input box
> are now functional in the url params.
> 
> You can test with:
> https://treeherder.mozilla.org/ui/#/jobs?searchQuery=linux%20talos
> 
> We will address:
> 
> 1) The url param name has changed. Might be worth changing it back, though
> there are now more complex filtering modes in addition to quick filter, so
> need to consider that.
> 
> In a follow up PR.
> 
> Not sure what to do about the jobname parameter, it doesn't exactly refer to
> what treeherder calls a job name.

I've filed bug 1060440 for this, but it's lower priority & not a blocker.

> Maybe we could keep the jobname parameter but have it filter by buildername
> specifically instead of the search parameters in the "Filter platforms &
> jobs" box entry?

Hmm that might be an idea - happy to break that out of this bug though.

(In reply to Joel Maher (:jmaher) from comment #3)
> somehow the magical link above works:
> https://treeherder.mozilla.org/ui/#/jobs?searchQuery=linux%20talos
> 
> bug what about:
> https://treeherder.mozilla.org/ui/#/jobs?searchQuery=linux%20talos%20tp5o
> 
> I get no results.

This works:
https://treeherder.mozilla.org/ui/#/jobs?searchQuery=linux%20talos%20tp

So this is bug 1045857.

> So I start with the url for linux talos and then I edit the url to add
> %20tp5o and end up (after spinning my cpu for 20 seconds):
> https://treeherder.mozilla.org/ui/#/jobs?searchQuery=linux

Why hand-edit URLs once the page is open? Similar to TBPL, the search term is available in the UI, once you've clicked the first link you can edit from there :-)

> it appears we have a good bit of work to do for supporting the searchQuery. 
> here are some common use cases:
> 1) click a generated query to view a specific job over time
> 2) edit it to adjust the platform so I can see additional platforms of the
> same job
> 3) edit it to change the jobname to narrow down the scope.

With the bugs mentioned above fixed, all these workflows should be possible if I understand them correctly? You may just need to use different search terms in treeherder compared to TBPL. Could you file new bugs for not anything already covered?

Comment 5

[Ed Morley [:emorley]]

Jeads, thank you for taking a look at this.

The searchQuery param is now observed after using a URL that specifies it, however I now see:

1) After going to https://treeherder.mozilla.org/ui/#/jobs?searchQuery=linux then clearing the quickfilter field and pressing enter - document.location doesn't update.

2) After going to https://treeherder.mozilla.org/ui/#/jobs?searchQuery=linux%20talos I see the quickfilter field has the value "linux,talos" -> are the commas intentional?

Comment 6

[Jonathan Eads ( :jeads )]

edmorley thanks for testing this.

> 1) After going to https://treeherder.mozilla.org/ui/#/jobs?searchQuery=linux
> then clearing the quickfilter field and pressing enter - document.location
> doesn't update.

That's a bug, I will fix.

> 2) After going to
> https://treeherder.mozilla.org/ui/#/jobs?searchQuery=linux%20talos I see the
> quickfilter field has the value "linux,talos" -> are the commas intentional?

The commas are not intentional. It's a bug I ran into in development where an angularjs directive is being invoked by default that forces a join between multiple words on a comma.

Could you try clearing your browser cache and see if it persists? I'm not able to reproduce but I definitely saw that in development, also do you see the same behavior on stage?

https://treeherder.allizom.org/ui/#/jobs?searchQuery=linux%20talos

I will address the buildername issues discussed here in Bug 1045857 as suggested.

Comment 7

[Ed Morley [:emorley]]

Clearing the cache fixed #2 for me, ty :-)

Comment 8

[Jonathan Eads ( :jeads )]

jmaher, I believe I have addressed your concerns for filtering by the buildername in Bug 1045857. The following urls are now functioning as expected:

https://treeherder.mozilla.org/ui/#/jobs?fromchange=86afc8441856&tochange=76b55c0850ca&jobname=Windows%20XP%2032-bit%20mozilla-central%20pgo%20talos%20other

https://treeherder.mozilla.org/ui/#/jobs?searchQuery=linux%20talos%20tp5o

I've add the "jobname" parameter to maintain back compatibility with tbpl. In a treeherder session if you use the quick search it will still use the "searchQuery" parameter to encourage moving along. The quick search searches across the entire buildername along with the visibly displayed information in the UI for platform, job group, and job names, we need to do both so we can support reporting systems that do not have buildernames (jenkins, taskcluster):

https://github.com/mozilla/treeherder-ui/blob/master/webapp/app/js/directives/clonejobs.js#L249

Please try your use cases again and see if this meets your requirements.

Comment 9

[Jonathan Eads ( :jeads )]

edmorley, I believe I have addressed your concerns regarding this bug

> 1) After going to https://treeherder.mozilla.org/ui/#/jobs?searchQuery=linux
> then clearing the quickfilter field and pressing enter - document.location
> doesn't update.

Please confirm that this works as you expect. Also, you don't actually need to press enter when clearing the text in the "Filter platforms & jobs" quick search. As soon as all text is deleted the application should clear the search parameters in document.location, if this is not occurring it's another bug that should be fixed.

Also, edmorley and jmaher, please clear your caches before testing. Unfortunately, a number of the js modifications seem to stick in the cache even on refresh. Thanks

Comment 10

[Joel Maher ( :jmaher ) (UTC -8)]

jeads, this seems to be working great for me.

One problem is when I am filtering on winxp, I get both winxp and windows 7:
query text box value; Windows XP 32-bit mozilla-central talos

url: https://treeherder.mozilla.org/ui/#/jobs?fromchange=86afc8441856&tochange=76b55c0850ca&searchQuery=windows%20xp%2032-bit%20mozilla-central%20talos

also the speed is noticeably slower than tbpl, but I assume that is another issue.

Comment 11

[Ed Morley [:emorley]]

Works for me now, thank you.

The only issue I see is that:
https://treeherder.mozilla.org/ui/#/jobs?searchQuery=windows%20xp%2032-bit%20mozilla-central%20talos

Gets changed to:
https://treeherder.mozilla.org/ui/#/jobs?searchQuery=windows&searchQuery=xp&searchQuery=32-bit&searchQuery=mozilla-central&searchQuery=talos

And removing the param from the URL and pressing enter just redirects back to a page that includes the param in the URL again.

...but I'll file a separate bugs for these, since unlike they don't block.

Comment 12

[Ed Morley [:emorley]]

(In reply to Joel Maher (:jmaher) from comment #10)
> also the speed is noticeably slower than tbpl, but I assume that is another
> issue.

Might this be because you'd just cleared the cache?

I tried:
1) Clearing cache
2) Loading homepage of both TBPL and treeherder
3) Opening webconsole to network tab
4) Navigating to each of these separately:

https://treeherder.mozilla.org/ui/#/jobs?fromchange=86afc8441856&tochange=76b55c0850ca&searchQuery=windows%20xp%2032-bit%20mozilla-central%20talos
-> 5.1MB, 5s

https://tbpl.mozilla.org/?fromchange=86afc8441856&tochange=76b55c0850ca&jobname=windows%20xp%2032-bit%20mozilla-central%20talos
-> 3.9MB, 5.1s

So they seem pretty comparable? (There's still bloat we can cut out of the treeherder payload, such as bug 1053231 & bug 1061283. Also treeherder treats fromchange as inclusive, unlike pushlog/TBPL, so treeherder is retrieving one more push than TBPL - I'll file another bug for this).

Comment 13

[Ed Morley [:emorley]]

(In reply to Ed Morley [:edmorley] from comment #12)
> 2) Loading homepage of both TBPL and treeherder

(So that the static assets were cached, but not the json blobs for the pushes in question)

> Also treeherder
> treats fromchange as inclusive, unlike pushlog/TBPL, so treeherder is
> retrieving one more push than TBPL - I'll file another bug for this).

Bug 1061681