1032013 - Drop the feature of remembering what repos were open in a previous instance

Status: RESOLVED FIXED

(Tree Management :: Treeherder, defect, P1)

Description

[Phil Ringnalda (:philor)]

While the way that the repos you last had open in internal-tabs are reopened in a new treeherder instance seems like a good idea, in practice it isn't. Some typical scenarios:

Open a typical sheriff's load of repos in a treeherder instance, at least mozilla-inbound/mozilla-central/b2g-inbound/fx-team/mozilla-aurora/mozilla-beta.

Now click a link to https://treeherder.mozilla.org/ui/#/jobs?repo=mozilla-inbound&revision=b30fb4ddf36d that someone pastes into IRC. You now have two open browser tabs, one with the six trees you are watching, and one that only looks like it has the six trees you are watching, but actually has five plus one revision.

Push to try, and click the link that will eventually be in the email try sends you, to https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=66f4e2177c4f. Now you have three open browser tabs, one with the trees you are watching, two that only look vaguely like it.

If your CPU hasn't yet spun up your fan loud enough to make you close tabs, load an additional 20 pushes on mozilla-inbound, retrigger a job down at the bottom ten times to see if that push was the cause of a new failure, click the linked datetime to open that one push in a new tab (and then try to figure out how to get back to just the tip 10 pushes of mozilla-inbound). Now you have four open browser tabs, all looking quite similar but only one being the one you actually want to have everything it has in it.

Now, get confused about which tab is which, and in the one you opened from the email about your try push, click on the internal-tab for mozilla-inbound, then slap your forehead and click back on the internal-tab for your try push. You've just lost your try push, and opened the tip-most 10 pushes to try, because the &revision= wasn't preserved.

At first, I thought the absolute best case is that every single time you ever open an external link to treeherder, and every time you open a single push, you remember to close all the other tabs that were also opened in it, but that's far far more destructive, since if you open a tab from an external link to look at a try push, close all the other internal-tabs in it, then close that tab and go back to your main treeherder tab and switch to a different internal-tab in it, you suddenly close every other internal-tab in it and open a new internal-tab to the tip of try. I can't see any way to make that feature work with the absolute requirement for sheriffing, to have a half-dozen or dozen trees open at all times and then be bombarded all day long with links to individual pushes, sometimes on those same trees and sometimes on other trees.

Comment 1

[Ed Morley [:emorley]]

Thank you for filing this Phil - I know I'm finding lots of broken (or slow) workflows myself too now - don't think you're alone in that respect - but we'll get there... :-)

Comment 2

[Ed Morley [:emorley]]

This depends on the decision in bug 1032220, but I'm guessing the main cause of comment 0 is that for links that reference a specific repo, we don't just load that repo but all the others previously pinned, plus then permanently change the saved pin repos. If we just showed that repo & no others (and left the pinned repos settings as is) this would be miles better, right?

Comment 3

[Cameron Dawson [:camd]]

I would appreciate some feedback on this latest implementation.  Would one of you give this a try on treeherder-dev and see if it feels about right?  

https://github.com/mozilla/treeherder-ui/commit/e538533d5a1d2f2839f25f5bc4b42738ed711fbb

Comment 4

[Cameron Dawson [:camd]]

Attachment: pull request

Comment 5

[Ed Morley [:emorley]]

(In reply to Cameron Dawson [:camd] from comment #3)
> I would appreciate some feedback on this latest implementation.  Would one
> of you give this a try on treeherder-dev and see if it feels about right?  
> 
> https://github.com/mozilla/treeherder-ui/commit/
> e538533d5a1d2f2839f25f5bc4b42738ed711fbb

Comment 6

[Phil Ringnalda (:philor)]

Sorry, I'm out of pre-internet-vacation time.

Comment 7

[Cameron Dawson [:camd]]

No worries.  I'm going to push it as-is, and mark this fixed.  When you get another chance to look at it if you decide we need further adjustment, then please reopen.  But I think this addresses the requested workflow.

Comment 8

[Ed Morley [:emorley]]

This seems broken for me at the moment?

On dev, open the repos menu, check another repo using the checkbox (eg fx-team) - so you now have mozilla-central and fx-team both checked. Then close the repo menu and click on fx-team to switch to it. At this point mozilla-central disappears from the repo tab bar. If this is intended - then it sounds like we've decided on single repo per tab model from bug 1032220?

Comment 9

[Cameron Dawson [:camd]]

Hrm..  Yep, that was still broken.  Dang, apologies.  I didn't test it enough.  Should be fixed now to do this:

1. check several repos in the ``repos`` menu. 
   -> they're added as watched on the nav bar.  Y
   -> You can click between them on screen.  
   -> They persist for that FF tab only

2. click the link for the repo in the ``repos`` menu
   -> reloads the page with just that repo

3. ctrl-click a repo link in the ``repos`` menu
   -> loads that repo in a new FF tab

I removed the "+" icon.

Comment 10

[Cameron Dawson [:camd]]

Latest fix is here:
https://github.com/mozilla/treeherder-ui/commit/9c3cfc7b6e9fd47e13368cc2704cc9b66be2c39f

Comment 11

[Cameron Dawson [:camd]]

As for bug 1032220, I'm not sure who should make that call.  But I'm sure it shouldn't be me.  :)  I'm attempting to support both workflows with these changes.  Hopefully that'll help inform that decision.  Perhaps after the Sheriffs and devs use it for a bit, we'll be able to more definitively make that call.  But if there's someone (is it you, Ed?  :)) who can make that call now, then that's easy enough.

Comment 12

[Cameron Dawson [:camd]]

Sigh...  #2 above doesn't update the displayed (watched) repo properly.  Working on a fix.

Comment 13

[Cameron Dawson [:camd]]

I did lots of testing and this looks to be working in a sane way now.

https://github.com/mozilla/treeherder-ui/commit/2a842538fd0f70531728fe34a8e1c41bf19c3c6d

The only change from comment 9's description is that if you have several repos checked as watched, then it won't unwatch all those as you click the link.  But if you're looking at just one repo, and click the link for another repo, it will replace the previous repo.

Ed, adding this for your approval upon your return from PTO.

Comment 14

[Ed Morley [:emorley]]

lgtm, thank you :-)

Comment 15

[Treeherder GitHub Bugbot]

Commits pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/fd50ece31ec7fd3f018ca0df2e8a16a2e114d7ec
Fix Bug 1032013 - stop persisting watched repos

https://github.com/mozilla/treeherder/commit/158a6518e034f34728fbb51e2966388fb79839dc
Merge pull request #95 from mozilla/bug-1032013

Fix Bug 1032013 - stop persisting watched repos

https://github.com/mozilla/treeherder/commit/13a8dae5a51570f27aa27c875fd3323dc6025c96
fix for fix of bug 1032013

https://github.com/mozilla/treeherder/commit/013d7681119199e22ed06489703c0322ccbc7f5a
fix bug 1032013: refactor the way watched repos work

I ended up needing a small refactor to handle things properly.  Also
optimized the code a little.