Closed Bug 1144085 Opened 8 years ago Closed 8 years ago

Mercurial client incorrectly uses review status field rather than public field to indicate drafts

Categories

(MozReview Graveyard :: General, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: dminor, Assigned: dminor)

References

Details

Attachments

(1 file, 1 obsolete file)

The client is currently using the review status field to determine whether or not the user needs to publish the review, but the status field is one of "pending", "submitted", "discarded" [1]. We should be using the public field instead [2].

[1] https://github.com/reviewboard/reviewboard/blob/master/reviewboard/webapi/resources/review_request.py#L172
[2] https://github.com/reviewboard/reviewboard/blob/master/reviewboard/webapi/resources/review_request.py#L176
Assignee: nobody → dminor
Attached file MozReview Request: bz://1144085/dminor (obsolete) —
/r/5531 - hgext: use public field rather than status field to indicate drafts (bug 1144085) r=gps

Pull down this commit:

hg pull review -r 0b0024e3efef21dda85d5ac624746f94807ac450
Attachment #8578652 - Flags: review?(gps)
Attachment #8578652 - Flags: review?(gps) → review+
Comment on attachment 8578652 [details]
MozReview Request: bz://1144085/dminor

https://reviewboard.mozilla.org/r/5529/#review4775

This patch is good. But we need to consider how we land and deploy this.

If clients pick up these changes before the server-side changes are fixed, they aren't going to see the new field and they won't show the review request as being in the draft state. That could be a bit confusing.

I'd be in favor of splitting this into 2 patches: 1 that adds the new field to the response payload and another that changes the client to utilize it. We can deploy the first change on the server then land the second patch.

Or we can just not care and deal with the minor UX setback in the window between when this lands and when it is deployed. Which I assume could be made very small if we coordinate...
I've pushed the server-side portion of this patch here [1]. I'll follow up with the client side once that is deployed.

[1] https://hg.mozilla.org/hgcustom/version-control-tools/rev/12df0c1d8184
Status: NEW → ASSIGNED
Pushed the client-side portion of the patch to here: https://hg.mozilla.org/hgcustom/version-control-tools/rev/d526e69430e8
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Attachment #8578652 - Attachment is obsolete: true
Attachment #8619782 - Flags: review+
Blocks: 1179552
Product: Developer Services → MozReview
You need to log in before you can comment on or make changes to this bug.