Page MenuHomePhabricator

Able to accept differential revision twice in a row
Closed, InvalidPublic


Not totally sure how I got into this state. Didn't try to repro yet.

  • I had an old session open with the unaccepted code
  • The first accept was from a !accept in an email
  • The second accept was from the old open session
  • jing requested review somewhere in between.

Event Timeline

nipunn created this task.Sep 14 2015, 7:05 PM
nipunn renamed this task from Able to accept twice in a row to Able to accept differential revision twice in a row.
nipunn updated the task description. (Show Details)
nipunn added a project: Restricted Project.
nipunn added subscribers: jhurwitz, angie, nipunn.

This doesn't seem problematic to me. It accurately represents the actions you took (submitting multiple accepts from the web UI and via email) and doesn't create any inconsistencies, problems with revision state, etc. It's a little odd, but the workflow was a little odd.

If you do something silly and processing that silly thing doesn't actually create problems, we try to just do the silly thing literally rather than second guess you. This is usually more clear.

For example, if you'd swapped the order of these actions and sent an "!accept" email and we ignored the action if it didn't prompt a state change, we would seem to ignore the email. A user might reasonably be confused that their email was ignored if their web-UI accept was 50 comments ago and they'd forgotten about it.

Am I missing anything, like a real problem or inconsistency this creates?

nipunn closed this task as Invalid.Sep 14 2015, 8:39 PM
nipunn claimed this task.

Nope totally sounds reasonable.
I think the thing that was weird to me was that I was able to accept from the UI after it was already in an accepted state, but since I had an old session, sure do it twice.

angie moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.Sep 14 2015, 8:40 PM

If you're running the notification server, the old session should have had a "Page has been updated, click to reload." badge on it. Not sure if you guys have that configured, but that can provide a cue that you're out of date.