bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

Commit table truncated in public view when there are unpublished drafts

RESOLVED FIXED

Status

MozReview
General
P1
normal
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: gps, Assigned: mdoglio)

Tracking

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
/r/19391 had 4 published children review requests. mcomella pushed a single changeset to it and didn't publish. The parent review request in his browser was showing a commit table with one entry and the draft banner.

However, the review requests in my, non-submitter, non-admin view had commit tables with only a single commit! The 4 old commits were published and the new single commit was draft. Somehow draft state was impacting what the commit table displays.

I have a dump of the database at reviewboard1.webapp.scl3:/root/db-20151009.sql if anyone wants to poke around.

Here is extra_data from the parent (19391):

{"p2rb.first_public_ancestor": "ed8188590f14b1aae2e4f44c8196994f375a99f4", "p2rb.is_squashed": true, "p2rb.base_commit": "a2ad0230946a4acc5c5cf9f6fa76d93aa7b2b861", "p2rb.identifier": "bz://1201206/mcomella", "p2rb.reviewer_map": "{\"19515\": [101835], \"19511\": [101835], \"19399\": [186753], \"19513\": [101835], \"19397\": [186753], \"19395\": [186753], \"19393\": [186753]}", "p2rb.discard_on_publish_rids": "[19395, 19397, 19399]", "p2rb.unpublished_rids": "[]", "p2rb.commits": "[[\"8447895866926612dfee3cedbe1a2c5370c76b64\", 19393], [\"b91041a0e06937b22920759b5b829ed9f102849e\", 19395], [\"60b3297b1f07e98f0189d055a4254cfca6851f49\", 19397], [\"5ff23b0e802006a2ef15f3344f21e0bf63a1fcd2\", 19399]]", "p2rb": true}

This is a P1 because we don't want draft state impacting the public view.
(Reporter)

Comment 1

3 years ago
Looks like the commits table is populated indirectly from the output of mozreview.extra_data.gen_child_rrs().

    commit_tuples = json.loads(review_request.extra_data[COMMITS_KEY])
    for commit_tuple in commit_tuples:
        child = get_rr_for_id(commit_tuple[1])

        # TODO: We should fail if we can't find a child; it indicates
        # something very bad has happened.  Unfortunately this call is
        # used in several different contexts, so we need to make changes
        # there as well.
        if not child:
            continue
        elif user is None:
            yield child
        elif child.is_accessible_by(user):
            yield child.get_draft(user) or child

I'm guessing the bug is somewhere in the last 2 lines.
(Assignee)

Updated

3 years ago
Assignee: nobody → mdoglio
Status: NEW → ASSIGNED
(Assignee)

Comment 2

3 years ago
I can reproduce it locally, working on a fix.
(Assignee)

Comment 3

3 years ago
Created attachment 8673250 [details]
MozReview Request: mozreview: always pass the current user when getting a draft (bug 1213456); r?smacleod

mozreview: always pass the current user when getting a draft (bug 1213456); r?smacleod
Attachment #8673250 - Flags: review?(smacleod)
Attachment #8673250 - Flags: review?(smacleod) → review+
Comment on attachment 8673250 [details]
MozReview Request: mozreview: always pass the current user when getting a draft (bug 1213456); r?smacleod

https://reviewboard.mozilla.org/r/21877/#review19703
(Assignee)

Updated

3 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Product: Developer Services → MozReview
You need to log in before you can comment on or make changes to this bug.