Unable to load prior pushes using "get next N" buttons when only one result set/revision visible

RESOLVED FIXED

Status

P2
normal
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: emorley, Assigned: camd)

Tracking

({regression})

Details

Attachments

(1 attachment)

46 bytes, text/x-github-pull-request
emorley
: review+
emorley
: feedback+
Details | Review | Splinter Review
(Reporter)

Description

4 years ago
Bug 1056308 spotted that pressing the "get next N" buttons didn't work when in single push mode (ie: &rev=FOO). The fix there was to hide the buttons in the single push case.

However unfortunately we sometimes need to load the result sets leading up to that revision, so we need to revert the PR in bug 1056308 (to unhide the buttons in the single result set case) and instead make the buttons work like they do on TBPL.

Even better, we could improve functionality over TBPL, and make the URL params work with this seamlessly.

ie:
1) Be on &rev=SHA1 (ie this doesn't apply when viewing the repo tip)
2) Press "get next 10"
3) URL becomes "&fromchange=(SHA-of-10-pushes-ago)&tochange=SHA1
4) Press "get next 10" again
5) URL becomes "&fromchange=(SHA-of-20-pushes-ago)&tochange=SHA1

...that way state can be preserved by copying and pasting links (TBPL is unable to do this).
(Reporter)

Updated

4 years ago
Duplicate of this bug: 1081557
(Reporter)

Updated

4 years ago
Summary: Unable to load prior pushes when only one result set visible → Unable to load prior pushes using "get next N" buttons when only one result set/revision visible
(Reporter)

Updated

4 years ago
Assignee: nobody → emorley
Status: NEW → ASSIGNED

Updated

4 years ago
Duplicate of this bug: 1095214
Might also be nice to have other buttons to load pushes in the other direction in the pushlog.
(Reporter)

Updated

4 years ago
Keywords: regression
This is pretty essential for bisecting the results of intermittent oranges that started more than a day or so ago.  I have to use TBPL to do that.
(Reporter)

Updated

4 years ago
Priority: P2 → P1
(Reporter)

Comment 5

4 years ago
(In reply to Wes Kocher (:KWierso) from comment #3)
> Might also be nice to have other buttons to load pushes in the other
> direction in the pushlog.

Good idea, but please file a new bug for this :-)
(Reporter)

Updated

4 years ago
Priority: P1 → P2
(Reporter)

Updated

4 years ago
Assignee: emorley → nobody
Status: ASSIGNED → NEW
(Reporter)

Updated

4 years ago
No longer blocks: 1059400
(Assignee)

Updated

3 years ago
Assignee: nobody → cdawson
(Assignee)

Updated

3 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 6

3 years ago
After playing with this a bit, these are the workflows I added.  This is close to what Ed proposed, but one difference with ``tochange``:

1. after loading default of latest 10, click next 10 will load them and set only the ``fromchange`` value to the latest revision loaded.  Hit next N again and it will update the ``fromchange`` to the latest again.  
Note: ``tochange`` is not updated in this scenario, so that new resultsets will continue to be loaded by default

2. The "action" menu (at the right end of each resultset) has two new menu items:

a. "Set as tochange" will set the ``tochange`` param to that revision and will therefore stop loading any new resultsets.

b. "Set as fromchange`` will set the ``fromchange`` param to that revision.  Hitting "Next N" will update that value as you go.

3. If you have the ``revision`` value set (so you're viewing a single revision) hitting "Next N" will change the param from ``revision`` to ``tochange`` and set the ``fromchange`` to the latest loaded (earliest) resultset.
(Assignee)

Comment 7

3 years ago
Created attachment 8611388 [details] [review]
fix get next PR
Attachment #8611388 - Flags: review?(emorley)
(Reporter)

Comment 8

3 years ago
Comment on attachment 8611388 [details] [review]
fix get next PR

Almost ready to r+, just had the query about the 100 result sets being opened in the tochange case:
https://github.com/mozilla/treeherder/pull/560#issuecomment-106384345
Attachment #8611388 - Flags: review?(emorley) → feedback+
(Assignee)

Comment 9

3 years ago
Ed: Cool, fixed that to be only 10 in that case.  Now, it sets it to 100 if the ``fromchange`` param is set.  But if not (which would be ``tochange`` only, or none) then it uses the default of 10.
(Assignee)

Comment 10

3 years ago
Comment on attachment 8611388 [details] [review]
fix get next PR

Hey Ed: ok, I made that fix.  Thanks for the suggestion.  I also re-worded the menu items to be a little more human-ish.

Ready for your re-review.  Thanks!!
Attachment #8611388 - Flags: review?(emorley)
(Reporter)

Updated

3 years ago
Attachment #8611388 - Flags: review?(emorley) → review+

Comment 11

3 years ago
Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/7ed4137124831c383127bbd560f32a808c856072
Bug 1078209 - Fix "get next N" when revision set.

Here are the workflows this change facilitates:

1. If you have the ``revision`` value set (so you're viewing a single
revision) hitting "Next N" will change the param from ``revision`` to
``tochange`` and set the ``fromchange`` to the latest loaded (earliest)
resultset.

2. after loading default of latest 10, click next 10 will load them and
set only the ``fromchange`` value to the latest revision loaded.  Hit
next N again and it will update the ``fromchange`` to the latest again.

Note: ``tochange`` is not updated in this scenario, so that new
resultsets will continue to be loaded by default

3. The "action" menu (at the right end of each resultset) has two new
menu items:

    i. "Set as tochange" will set the ``tochange`` param to that
revision and will therefore stop loading any new resultsets.

    ii. "Set as fromchange`` will set the ``fromchange`` param to that
revision.  Hitting "Next N" will update that value as you go.
(Assignee)

Updated

3 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Reporter)

Updated

3 years ago
Depends on: 1169758
(Reporter)

Updated

3 years ago
Duplicate of this bug: 1056010

Updated

3 years ago
Depends on: 1187366
You need to log in before you can comment on or make changes to this bug.