Perform extra cleanup when review request drafts get discarded.

Review Request #13135 — Created July 13, 2023 and submitted

Information

Review Board
release-6.x

Reviewers

This adds a handler for performing extra cleanup when review request drafts
get deleted. Right now the only thing that we do in the handler is delete all
draft file attachments that are being discarded alongside the draft. That
fixes a bug where discarding a review request draft that contained an
update to a file attachment would erase the entire file attachment from the
review request, rather than reverting the file attachment to its previously
published version. It also makes sense to not keep the draft file attachment
around, since it will never be published/accessed again after being discarded.

  • Ran unit tests
  • Made sure that the bug was fixed.
Summary
Perform extra cleanup when review request drafts get discarded.
Description From Last Updated

I left a similar comment on /r/13142/, but I think we should do the deletion in the model, to keep …

chipx86chipx86

These should just be parens for a tuple. A set has a bit more overhead that won't result in anything …

chipx86chipx86

continuation line over-indented for hanging indent Column: 14 Error code: E126

reviewbotreviewbot
david
  1. Ship It!
  2. 
      
maubin
david
  1. Ship It!
  2. 
      
chipx86
  1. 
      
  2. reviewboard/reviews/signal_handlers.py (Diff revision 2)
     
     
     
     
     
     

    I left a similar comment on /r/13142/, but I think we should do the deletion in the model, to keep the logic in one place. Signals are more useful if we're dealing with code we don't control or performing operations unrelated to the purpose of that model. In this case, ReviewRequestDraft manages its file attachments, and as such should also manage its deletion.

  3. 
      
maubin
Review request changed

Commits:

Summary
-
Perform extra cleanup when review request drafts get discarded.
+
Perform extra cleanup when review request drafts get discarded.

Diff:

Revision 3 (+400)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

maubin
david
  1. Ship It!
  2. 
      
chipx86
  1. 
      
  2. reviewboard/reviews/signal_handlers.py (Diff revisions 2 - 4)
     
     
     

    These should just be parens for a tuple. A set has a bit more overhead that won't result in anything faster than just scanning a tuple in this case.

  3. 
      
maubin
david
  1. Ship It!
  2. 
      
maubin
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-6.x (a310ab6)
Loading...