Page MenuHomePhabricator

Differential revision list "draft notification" bubble should ignore deleted drafts
Closed, DuplicatePublic


Steps to reproduce:

  1. open another user commit
  2. add 2 inline comments:
    • 1st comment covers one line
    • 2nd comment covers several lines including line of 1st comment
  3. press Delete button on 1st comment

At this point the deleted inline comment draft is still in database and if I submit the form below (e.g. accept commit, add comment) it would still be in database as draft, but invisible. An invisible draft would cause Unsubmitted comments bubble in the list.

I think, that the Delete button should delete draft comment directly in database and if Undo link is used, then we can create it again there.

Event Timeline

aik099 created this task.Apr 9 2015, 8:04 AM
aik099 renamed this task from Deleted commit inline comments stay as drafts forever to Unable to delete draft inline comment (still stays in db).
aik099 raised the priority of this task from to Needs Triage.
aik099 updated the task description. (Show Details)
aik099 added a project: Audit.
aik099 added a subscriber: aik099.
aik099 updated the task description. (Show Details)Apr 9 2015, 8:07 AM
epriestley renamed this task from Unable to delete draft inline comment (still stays in db) to Differential revision list "draft notification" bubble should ignore deleted drafts.Apr 9 2015, 1:44 PM
epriestley added a subscriber: epriestley.

We'll fix the bubble behavior. The persistence behavior is intentional.

As it turns out, this is fairly messy.

T5031 doesn't really need to happen first but we'd end up in a cleaner place if it did.

aik099 added a comment.EditedApr 9 2015, 3:34 PM

If we add a code in place where we create transaction (audit action takes place) and all draft inlines become real inlines we can drop (from db) draft inlines that had Undo link. No side effects and small code amount.

Not sure though how do we distinguish drafts that should be live VS ones, that were deleted but are still in db.

chad moved this task from Backlog to v2 (UI/Mobile) on the Differential board.Apr 21 2015, 12:25 AM
chad added a subscriber: chad.Aug 6 2015, 6:53 PM

I'm not able to still reproduce this, I think it was similar to the bug we recently resolved around draft states, can't find that diff offhand though.

This comment was removed by epriestley.