Update the API to allow undoing the pending deletion of file attachments.
Review Request #13251 — Created Aug. 31, 2023 and submitted
This updates the file attachments API update endpoints to allow undoing the
pending deletion of file attachments. We add apending_deletion
query
parameter that can only be set toFalse
to do this.This also reduces the number of queries performed when deleting multiple
revisions of file attachments. Instead of removing revisions one by one,
we do a batch remove which reduces this to just one query instead of
one query per revision.
- Ran unit tests.
- Used in an upcoming change.
Summary |
---|
Description | From | Last Updated |
---|---|---|
"false" for the API. (We normalize case, but document lowercase.) |
|
|
In the case of the API, "parameter" isn't the right word. Instead, we can say something like "Set pending_deletion=false in … |
|
|
I keep reading this final sentence incorrectly, and realized I was missing how this connected. Maybe we can reword it … |
|
|
This should have a trailing comma. |
|
|
This should have a trailing comma. |
|
|
When wrapping a multi-line string in parens, leave the ) aligned with the line containing the (, so that it's … |
|
|
This should have a trailing comma. |
|
|
This should have a trailing comma. |
|
|
Missing a type. |
|
|
Since we just need the IDs, you can do: all_revs = list( FileAttachment.objects .filter(...) .values_list('pk', flat=True) ) (The list() resolves … |
|
|
.save() isn't needed to update M2M relations, like file_attachments, so let's avoid this if we're not otherwise modifying state on … |
|
|
Missing a type. |
|
|
Same notes as above. |
|
|
Going forward (starting with recent API error work), I want to start comparing the entirety of rsp for errors, so … |
|
-
-
reviewboard/webapi/resources/base_review_request_file_attachment.py (Diff revision 1) "false" for the API. (We normalize case, but document lowercase.)
-
reviewboard/webapi/resources/base_review_request_file_attachment.py (Diff revision 1) In the case of the API, "parameter" isn't the right word. Instead, we can say something like "Set
pending_deletion=false
in the request to ..." -
reviewboard/webapi/resources/base_review_request_file_attachment.py (Diff revision 1) I keep reading this final sentence incorrectly, and realized I was missing how this connected. Maybe we can reword it to:
Setting this to ``true`` is unsupported and will not delete the file attachment. To perform a deletion, perform a HTTP DELETE on the resource instead.
-
reviewboard/webapi/resources/base_review_request_file_attachment.py (Diff revision 1) When wrapping a multi-line string in parens, leave the
)
aligned with the line containing the(
, so that it's easier to add additional text/lines (same sort of rule as ensuring list items have a,
at the end). -
reviewboard/webapi/resources/base_review_request_file_attachment.py (Diff revision 1) Missing a type.
-
reviewboard/webapi/resources/base_review_request_file_attachment.py (Diff revision 1) Since we just need the IDs, you can do:
all_revs = list( FileAttachment.objects .filter(...) .values_list('pk', flat=True) )
(The
list()
resolves this, converting from aValuesQuerySet
to a list of IDs.) -
reviewboard/webapi/resources/base_review_request_file_attachment.py (Diff revision 1) .save()
isn't needed to update M2M relations, likefile_attachments
, so let's avoid this if we're not otherwise modifying state on the object. -
reviewboard/webapi/resources/base_review_request_file_attachment.py (Diff revision 1) Missing a type.
-
reviewboard/webapi/resources/base_review_request_file_attachment.py (Diff revision 1) Same notes as above.
-
reviewboard/webapi/tests/test_file_attachment_draft.py (Diff revision 1) Going forward (starting with recent API error work), I want to start comparing the entirety of
rsp
for errors, so we can be thorough. It also eases readability a bit. Let's update that here.

Commits: |
|
||||||
---|---|---|---|---|---|---|---|
Diff: |
Revision 2 (+2900 -58) |
Checks run (2 succeeded)
-
Just a few trivial style nits:
-
reviewboard/webapi/resources/base_review_request_file_attachment.py (Diff revisions 1 - 2) This should have a trailing comma.
-
reviewboard/webapi/resources/base_review_request_file_attachment.py (Diff revisions 1 - 2) This should have a trailing comma.
-
reviewboard/webapi/resources/base_review_request_file_attachment.py (Diff revisions 1 - 2) This should have a trailing comma.
-
reviewboard/webapi/resources/base_review_request_file_attachment.py (Diff revisions 1 - 2) This should have a trailing comma.

Change Summary:
Added some missing commas.
Commits: |
|
||||||
---|---|---|---|---|---|---|---|
Diff: |
Revision 3 (+2900 -58) |