1096318 - Restricting a bug's visibility does not delete any associated MozReview review requests

Status: RESOLVED FIXED

(bugzilla.mozilla.org :: Extensions, defect, P1)

Description

[Mark Côté [:mcote]]

I pushed a test review request (https://reviewboard.mozilla.org/r/361) for an invalid bug (bug 1072809), published the review request, and then marked the bug as visible to MoCo employees only.  This should have deleted the review request as per bug 993223, but nothing happened; the review request still exists and is visible.

Several things have changed in MozReview since bug 993223 was fixed, so maybe something broke it.  I'm going to file a separate bug for a version-control-tools test.

Comment 1

[Dylan Hardison [:dylan] (he/him)]

We need to configure the push connector for ReviewBoard. Needs a URL, username and password.

Comment 2

[Dylan Hardison [:dylan] (he/him)]

Let me know if the push connector is successfully deleting reviews now. Thanks!

Comment 3

[Mark Côté [:mcote]]

I made the bug public again and then put it back to MoCo only, but the review request continues to exist.

Comment 4

[Dylan Hardison [:dylan] (he/him)]

Hmm. I think the push connector needs the proxy setting for production. Weirdly, the push log is recording some transient errors:

https://bugzilla.mozilla.org/page.cgi?id=push_log.html

Comment 5

[Dylan Hardison [:dylan] (he/him)]

After working through the misconfiguration, it was discovered that he version of LWP::UserAgent on prod does not support the ->delete() method. We can do one of two things:

1) monkey patch in a delete() method, which is defined in terms of HTTP::Request::Common::DELETE.
2) upgrade LWP::UserAgent.

Comment 6

[Dylan Hardison [:dylan] (he/him)]

Attachment: bug-1096318-v1.patch

Patch to monkey-patch $ua->delete() in. I'm not incredibly fond of this idea, but I'm also not sure how reasonable it is to upgrade LWP::UserAgent on prod. (I'm also a little concerned that -dev/-staging have newer versions, as I remember this code running there.)

Comment 7

[:glob ✱]

Comment on attachment 8520349 [details] [diff] [review]
bug-1096318-v1.patch

Review of attachment 8520349 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to Dylan William Hardison [:dylan] from comment #6)
> Created attachment 8520349 [details] [diff] [review]
> bug-1096318-v1.patch
> 
> Patch to monkey-patch $ua->delete() in. I'm not incredibly fond of this
> idea, but I'm also not sure how reasonable it is to upgrade LWP::UserAgent
> on prod.

i agree, but we're looking at a LWP 5 --> 6 upgrade here, which i've had problems with in the past.  pretty sure bugzilla is ok with lwp6, but given the monkeypatch is trivial it's the safer thing to do here.

> (I'm also a little concerned that -dev/-staging have newer
> versions, as I remember this code running there.)

[[e-mail]3 ~]$ perl -MLWP::UserAgent -e "LWP::UserAgent->delete"
Can't locate object method "delete" via package "LWP::UserAgent" at -e line 1.

[[e-mail]3 ~]$ perl -MLWP::UserAgent -e "LWP::UserAgent->delete"
Can't locate object method "delete" via package "LWP::UserAgent" at -e line 1.

::: extensions/Push/lib/Connector/ReviewBoard/Client.pm
@@ +23,5 @@
> +        *LWP::UserAgent::delete = sub {
> +            require HTTP::Request::Common;
> +            my($self, @parameters) = @_;
> +            my @suff = $self->_process_colonic_headers(\@parameters,1);
> +            return $self->request( HTTP::Request::Common::DELETE( @parameters ), @suff );

nit: this doesn't follow bugzilla's code style; fix on commit

Comment 8

[:glob ✱]

To ssh://[e-mail]/webtools/bmo/bugzilla.git
   2d223dc..5b9d96e  master -> master

Comment 9

[:glob ✱]

now getting:
> garbage after JSON object, at character offset 4 (before "Can't connect to dc-...")

looks like the /etc/hosts entry for dc-proxy doesn't exist everywhere.

Comment 10

[:glob ✱]

that's fixed, now getting:
> The username or password was not correct

Comment 11

[Dylan Hardison [:dylan] (he/him)]

It is really fixed this time.