1234161 - Don't render filediff review button on drafts

Status: RESOLVED FIXED

(MozReview Graveyard :: General, defect)

Description

[Mauro Doglio [:mdoglio]]

Drafts don't have a get_diffsets method, which is used to retrieve the data for the filediff buttons. The diff view is currently broken.

Comment 1

[Mauro Doglio [:mdoglio]]

Attachment: MozReview Request: mozreview: don't render filediff buttons on drafts (bug 1234161); r?smacleod

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

Comment 2

[Steven MacLeod [:smacleod]]

Comment on attachment 8700576 [details]
MozReview Request: mozreview: don't render filediff buttons on drafts (bug 1234161); r?smacleod

https://reviewboard.mozilla.org/r/28767/#review26217

::: pylib/mozreview/mozreview/fields.py:311
(Diff revision 1)
> +                type(self.review_request_details) == ReviewRequest):

The case where the review request is not published yet will still get past this (there is no draft I believe, just `.public=False`)

Comment 3

[Mauro Doglio [:mdoglio]]

https://reviewboard.mozilla.org/r/28767/#review26217

> The case where the review request is not published yet will still get past this (there is no draft I believe, just `.public=False`)

That shouldn't be a big deal. As long as we get a ReviewRequest object we can call .get_diffsets() on it and that's all we need, right?

Comment 5

[Steven MacLeod [:smacleod]]

https://reviewboard.mozilla.org/r/28767/#review26217

> That shouldn't be a big deal. As long as we get a ReviewRequest object we can call .get_diffsets() on it and that's all we need, right?

Yup, you're absolutely correct. Lets use isinstance instead of type here though and it should be good.

Comment 6

[Mauro Doglio [:mdoglio]]

https://reviewboard.mozilla.org/r/28767/#review26711

::: pylib/mozreview/mozreview/fields.py:311
(Diff revision 1)
> +                type(self.review_request_details) == ReviewRequest):

use isinstance instead of type

Comment 7

[Mauro Doglio [:mdoglio]]

Comment on attachment 8700576 [details]
MozReview Request: mozreview: don't render filediff buttons on drafts (bug 1234161); r?smacleod

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

Comment 8

[Steven MacLeod [:smacleod]]

Comment on attachment 8700576 [details]
MozReview Request: mozreview: don't render filediff buttons on drafts (bug 1234161); r?smacleod

https://reviewboard.mozilla.org/r/28767/#review27313