1205018 - People besides review requesters should be able autoland

Status: RESOLVED FIXED

(MozReview Graveyard :: General, defect)

Description

[Dan Minor [:dminor]]

At the moment one of our conditions for enabling the 'try' button on the Automation menu is currentIsMutableByUser [1] which seems to prevent reviewers from autolanding to try.

It would be nice if reviewers could send changesets to try if the review requester has not done so themselves.

[1] https://dxr.mozilla.org/hgcustom_version-control-tools/source/pylib/mozreview/mozreview/static/mozreview/js/try.js#89

Comment 1

[Dan Minor [:dminor]]

We also have this restriction on autolanding to inbound. I believe the main concern is that we are not tracking the user who triggered request for auditing purposes. That should be easy to fix.

Comment 2

[Dan Minor [:dminor]]

If we fix the user recorded in the push log then we will have the appropriate auditing to remove this restriction.

Comment 3

[Dan Minor [:dminor]]

Attachment: MozReview Request: testing: make it possible to specify repo name in create_basic_repo r?mdoglio

Review commit: https://reviewboard.mozilla.org/r/30187/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/30187/

Comment 4

[Dan Minor [:dminor]]

Attachment: MozReview Request: mozreview: people besides review requesters should be able autoland (bug 1205018) r?mdoglio

This removes the check for whether or not the review is mutable by the
current user, which allows people other than the review requester to
autoland provided they have sufficient privileges.

Review commit: https://reviewboard.mozilla.org/r/30189/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/30189/

Comment 5

[Mauro Doglio [:mdoglio]]

Comment on attachment 8705775 [details]
MozReview Request: testing: make it possible to specify repo name in create_basic_repo r?mdoglio

https://reviewboard.mozilla.org/r/30187/#review26999

Comment 6

[Mauro Doglio [:mdoglio]]

Comment on attachment 8705776 [details]
MozReview Request: mozreview: people besides review requesters should be able autoland (bug 1205018) r?mdoglio

https://reviewboard.mozilla.org/r/30189/#review27001

::: pylib/mozreview/mozreview/tests/test-autoland-try.py:56
(Diff revision 1)
>          # 2. The review must be mutable by the current user

The second condition here should be removed

Comment 7

[Steven MacLeod [:smacleod]]

https://reviewboard.mozilla.org/r/30189/#review27035

::: pylib/mozreview/mozreview/static/mozreview/js/autoland.js
(Diff revision 1)
> -  } else if (!MozReview.currentIsMutableByUser) {

I was under the impression we were going to add UI to show who triggered something (in MozReview, to the results stuff under the commit table?) before making this change?

::: pylib/mozreview/mozreview/static/mozreview/js/autoland.js
(Diff revision 1)
> -    try_trigger.attr('title', 'Only the author may trigger a try build at this time');

We need to consider how the results table intereacts with others triggering things on your request. If someone triggers a try build on my review request it will bump my try push from the page (and I might not even realize it right now because we don't show who made the push).

Comment 8

[Dan Minor [:dminor]]

Comment on attachment 8705775 [details]
MozReview Request: testing: make it possible to specify repo name in create_basic_repo r?mdoglio

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30187/diff/1-2/

Comment 9

[Dan Minor [:dminor]]

Comment on attachment 8705776 [details]
MozReview Request: mozreview: people besides review requesters should be able autoland (bug 1205018) r?mdoglio

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30189/diff/1-2/

Comment 10

[Dan Minor [:dminor]]

https://reviewboard.mozilla.org/r/30189/#review27035

> I was under the impression we were going to add UI to show who triggered something (in MozReview, to the results stuff under the commit table?) before making this change?

I'm planning to fix this as part of Bug 1238166.

> We need to consider how the results table intereacts with others triggering things on your request. If someone triggers a try build on my review request it will bump my try push from the page (and I might not even realize it right now because we don't show who made the push).

It will still show up as a change description on the parent review - not ideal, I know. Bug 1238166 will fix showing who made the push. Now that the pushlog records the requester, the original try results will still be accessible through email or searching on treeherder. I've filed Bug 1238600 as a follow on to this to improve the UI.