4610: Issue Verification cannot drop or verify

Misery
david
david

What version are you running?

3.0 Final

What's the URL of the page containing the problem?

http://localhost:8080/r/7/

What steps will reproduce the problem?

  1. Create user A and B
  2. Let A create a post-review in /r/new
  3. Let B add an issue (verify) to review request
  4. Let B click fix or drop his own issue
  5. Issue is NOT marked fixed or dropped!
  6. Still waiting for verification.... but how?

Another:
4. Let A click fix or drop issue of user B
5. User A see "waiting for...."
6. User B see "waiting for....", too. But has no button "Verify".

Only an administrator has a "Verify" button.

What is the expected output? What do you see instead?

  • Reviewer can click OWN fix/drop without verification
  • Reviewer should see a "Verify" button without beeing an administrator

What operating system are you using? What browser?

Linux, Firefox 57

Please provide any additional information below.

I'm using a fresh test installation with ./contrib/internal/devserver.py

theBoss
#1 theBoss

@RB team, any possibility to fix it? This is major feature in this release and sadly is not working :/

david
#2 david

I'm not sure I'd classify it as "not working", since your use case is somewhat unusual. The general intent of the feature is:

  1. A creates and publishes the review request
  2. B adds an issue in their review
  3. A adds a new diff and marks the issue as "Fixed"
  4. B verifies that the issue is actually fixed

If, in your workflow, the filer of the issue (B) is the one who's marking things as fixed, you already have verification.

That said, we should handle this case more gracefully and if B marks as fixed, it should probably go straight from open -> fixed without waiting to verify. We'll look to get that fixed for 3.0.2

  • +david
Misery
#3 Misery

It is unusual that B can drop his own issue without a verification? Who should verify his own drop?

But I understand that B cannot verify a "fixed" from A without another diffset?

david
#4 david

The author of the issue has full permission to close or reopen their own issues.

If B drops their own issue, I don't think it makes sense to ask anyone to verify--the only cases I've ever seen were ones where B realized that the issue was inappropriate, either on their own or after some discussion.

Similarly, there's nothing enforcing anyone to actually wait for a new diffset to mark an issue as fixed. If A marks as fixed, B gets the chance to verify. If B marks as fixed, we assume it's because they know best (maybe it was fixed via a separate change, or even fixed in a third-party dependency). With verification, if A marks the issue as fixed, B will get to verify.

david
#5 david
  • -New
    +PendingReview
#6 mariusz

@david, the general use case described in your previous comment doesn't work if B does not have administrator rights, so he can't mark the issues fixed by A as verified (eventhough B was the original person to mark 'verification required' for that issue.

david
#7 david

I've located the cause and landed a fix in release-3.0.x (9526405). This will ship very soon in 3.0.2. Thanks!

  • -PendingReview
    +Fixed