Page MenuHomePhabricator

Incorrect "Unsubmitted" state detection for Audit inline comments
Closed, ResolvedPublic

Description

  • UserCommitter - user who made a commit
  • UserAuditor - user who is doing audit

Steps to reproduce:

  1. open 2 browsers:
    • in 1st login as UserCommitter
    • in 2nd login as UserAuditor
  2. as UserAuditor
    • open commit (differential revision) made by UserCommiter
    • add an inline comment (see that "Unsubmitted" tag is shown and comment has grey header)
    • submit changes
    • see that "Unsubmitted" tag is gone and comment has yellow header
  3. as UserCommiter
    • check the "Done" checkbox on that inline comment (see that "Unsubmitted" tag is added and comment header changes from yellow to grey)

The UserAudtor now also sees same unsubmitted state of inline comment, but he shouldn't because it's UserCommiter's Done state that isn't submitted not an inline comment itself, that was created by UserAuditor.

Event Timeline

aik099 created this task.Apr 1 2015, 12:23 PM
aik099 raised the priority of this task from to Needs Triage.
aik099 updated the task description. (Show Details)
aik099 added projects: Audit, Differential.
aik099 added a subscriber: aik099.

Some possible evidence that this is confusing in practice rather than just in theory.

In the second case, "Unsubmitted" refers to the fact that you clicked the checkbox not being published yet. So this behavior is intentional, but not necessarily as clear as it could be.

refers to the fact that you clicked the checkbox not being published yet

Er, maybe more clearly; it refers to the fact that your state change to the checkbox is still only visible to you, and has not been published to other users yet. So they can't see that you've marked the inline "done".

aik099 added a comment.EditedApr 1 2015, 12:41 PM

In the second case, "Unsubmitted" refers to the fact that you clicked the checkbox not being published yet. So this behavior is intentional, but not necessarily as clear as it could be.

That is correct. But I'm describing the case, when somebody else (the inline comment author, that isn't same user as commit author for that inline comment) also sees that unsubmitted state created because Done checkbox click wasn't submitted by commit author.

Oh, I just didn't read this correctly at all. Ignore me.

chad renamed this task from Incorrect "Unsubmitted" state detection for inline comments to Incorrect "Unsubmitted" state detection for Audit inline comments.Apr 1 2015, 4:01 PM