Additional actions for pinboard submit should also be disabled if no pinned jobs or related bugs

RESOLVED FIXED

Status

Tree Management
Treeherder
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: wlach, Assigned: crosscent)

Tracking

Details

Attachments

(2 attachments)

In bug 1198978 we disabled the submit button on the pinboard if no item was selected. We should disable the side-menu (this thing: https://github.com/mozilla/treeherder/commit/590d688eadebaa0a6c5726fcbffdc6634c9b140d#diff-686a25b367215fd8175fec553976c640R75) similarly. This is actually a little more complicated as there are a bunch of options, and which ones make sense to enable depend on context. Specifically I'd do the following:

* Change it to a button from a span
* Change the title to "No pinned jobs" if there are no pinned jobs
* Change the ng-disabled to just `!hasPinnedJobs()` (it doesn't make sense to enable this if there are selected bugs, all the actions require at least one selected bug)
* In the case that no bugs are selected (`!hasRelatedBugs()`) Add a "disabled" class to the li for the "save bugs only" option, and set the title to "No bugs selected", and use ng-disabled to disable the ng-click action for this case.

Comment 1

2 years ago
Created attachment 8736995 [details] [review]
[treeherder] crosscent:bug1261082 > mozilla:master
(Assignee)

Updated

2 years ago
Attachment #8736995 - Flags: review?(wlachance)
(Assignee)

Comment 2

2 years ago
(In reply to William Lachance (:wlach) from comment #0)
> In bug 1198978 we disabled the submit button on the pinboard if no item was
> selected. We should disable the side-menu (this thing:
> https://github.com/mozilla/treeherder/commit/
> 590d688eadebaa0a6c5726fcbffdc6634c9b140d#diff-
> 686a25b367215fd8175fec553976c640R75) similarly. This is actually a little
> more complicated as there are a bunch of options, and which ones make sense
> to enable depend on context. Specifically I'd do the following:
> 
> * Change it to a button from a span
> * Change the title to "No pinned jobs" if there are no pinned jobs
> * Change the ng-disabled to just `!hasPinnedJobs()` (it doesn't make sense
> to enable this if there are selected bugs, all the actions require at least
> one selected bug)
> * In the case that no bugs are selected (`!hasRelatedBugs()`) Add a
> "disabled" class to the li for the "save bugs only" option, and set the
> title to "No bugs selected", and use ng-disabled to disable the ng-click
> action for this case.

I have sent a PR. Hopefully I got the process down this time!
Comment on attachment 8736995 [details] [review]
[treeherder] crosscent:bug1261082 > mozilla:master

I just realized there was one minor additional code change I'd like to see.

Also, could you change the commit message to something a little more precise? Maybe something like:

"Disable additional pinboard actions when they don't make sense"

Otherwise this looks great. :)
Attachment #8736995 - Flags: review?(wlachance) → review-
(Assignee)

Comment 4

2 years ago
(In reply to William Lachance (:wlach) from comment #3)
> Comment on attachment 8736995 [details] [review]
> [treeherder] crosscent:bug1261082 > mozilla:master
> 
> I just realized there was one minor additional code change I'd like to see.
> 
> Also, could you change the commit message to something a little more
> precise? Maybe something like:
> 
> "Disable additional pinboard actions when they don't make sense"
> 
> Otherwise this looks great. :)

Thank you for the input Will! I will admit my commit messages are pretty awful as I don't really know what level of detail to go into. Hopefully it will improve soon. Anyway, the new PR is sent.
(Assignee)

Updated

2 years ago
Attachment #8736995 - Flags: review?(wlachance)
Attachment #8736995 - Flags: review-
Comment on attachment 8736995 [details] [review]
[treeherder] crosscent:bug1261082 > mozilla:master

I think you might have forgotten to reset the commit message. Could you set the title of the commit to `Bug 1261082 - Disable additional pinboard actions when they don't make sense`? We want the commit message to follow the format elsewhere in treeherder. 

P.S. A great general guide on writing good commit messages is here -- http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html

... though note that treeherder's commit messages differ from the format in the following ways:

1. We always include the bug number in the header
2. The first line can be longer than 50 characters
Attachment #8736995 - Flags: review?(wlachance)
Attachment #8736995 - Flags: review-
(Assignee)

Comment 6

2 years ago
Thanks Will for catching that!

I hope the new commit message fits the criteria this time.
(Assignee)

Updated

2 years ago
Attachment #8736995 - Flags: review?(wlachance)

Comment 7

2 years ago
Created attachment 8737957 [details] [review]
[treeherder] wlach:master > mozilla:master
Comment on attachment 8736995 [details] [review]
[treeherder] crosscent:bug1261082 > mozilla:master

This looks great! I'll merge
Attachment #8736995 - Flags: review?(wlachance) → review+

Comment 9

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

https://github.com/mozilla/treeherder/commit/8b6ec3c96964ccaa5cd2243d0858c1d00b5d9d18
Bug 1261082 - Disable additional pinboard actions when they don't make sense

* Add boolean check to 'Save bugs only' so that ng-click is fully disabled.
* Disable dropdown if no job was selected, and keep Save Bugs Only disabled until a bug has been selected.
Thanks for the patch!
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.