Summary

Add POST support to the DiffFileAttachmentResource.

Review Request #13544 — Created Feb. 16, 2024 and submitted Feb. 27, 2024, 8:24 a.m.

Information

Owner

david

Repository

Review Board

Branch

release-7.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

This change adds the ability to upload files to attach to diffs via the
DiffFileAttachmentResource. This was previously read-only in the API.

Testing Done


Ran unit tests.
Used this from RBTools.

Commits

Summary	ID
Add POST support to the DiffFileAttachmentResource. This change adds the ability to upload files to attach to diffs via the `DiffFileAttachmentResource`. This was previously read-only in the API. Testing Done: - Ran unit tests. - Used this from RBTools.	e2e06b2801fe4bc071f1f9e6addd7a3773114c5f

Issues

Description	From	Last Updated
continuation line unaligned for hanging indent Column: 33 Error code: E131	reviewbot	Feb. 16, 2024, 12:48 p.m.
'typing.Any' imported but unused Column: 1 Error code: F401	reviewbot	Feb. 16, 2024, 12:48 p.m.
'typing.Optional' imported but unused Column: 1 Error code: F401	reviewbot	Feb. 16, 2024, 12:48 p.m.
'typing.Tuple' imported but unused Column: 1 Error code: F401	reviewbot	Feb. 16, 2024, 12:48 p.m.
Should we put a "Version Changed" saying that we have POST support now?	maubin	Feb. 21, 2024, 3:04 p.m.
Missing the rest of the docs (Version Added, Args, Returns).	maubin	Feb. 21, 2024, 3:05 p.m.
Leftover debug code.	maubin	Feb. 22, 2024, 8:35 a.m.
The code seems structured as: if not filediff: ... if filediff: ... It'd be some code movement, but can this …	chipx86	Feb. 26, 2024, 10:45 p.m.

flake8 failed.

JSHint passed.

flake8

reviewboard/webapi/resources/diff_file_attachment.py (Diff revision 1)
The issue has been dropped. Show all issues
```
continuation line unaligned for hanging indent

Column: 33
Error code: E131
```
reviewboard/webapi/tests/test_diff_file_attachment.py (Diff revision 1)
The issue has been dropped. Show all issues
```
'typing.Any' imported but unused

Column: 1
Error code: F401
```
reviewboard/webapi/tests/test_diff_file_attachment.py (Diff revision 1)
The issue has been dropped. Show all issues
```
'typing.Optional' imported but unused

Column: 1
Error code: F401
```
reviewboard/webapi/tests/test_diff_file_attachment.py (Diff revision 1)
The issue has been dropped. Show all issues
```
'typing.Tuple' imported but unused

Column: 1
Error code: F401
```

Commits:

	Summary	ID
	Add POST support to the DiffFileAttachmentResource. This change adds the ability to upload files to attach to diffs via the `DiffFileAttachmentResource`. This was previously read-only in the API. Testing Done: - Ran unit tests. - Used this from RBTools.	9f20aac4be65cf18462dc95b4721675772563d4b
	Add POST support to the DiffFileAttachmentResource. This change adds the ability to upload files to attach to diffs via the `DiffFileAttachmentResource`. This was previously read-only in the API. Testing Done: - Ran unit tests. - Used this from RBTools.	5c8697b55d73100add926350ef4f02174fb883de

Diff:

Revision 2 (+614 -76)

Show changes

	reviewboard/attachments/forms.py
	reviewboard/testing/testcase.py
	reviewboard/webapi/resources/diff_file_attachment.py
	reviewboard/webapi/tests/test_diff_file_attachment.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/webapi/resources/diff_file_attachment.py (Diff revision 2)

The issue has been resolved. Show all issues

Should we put a "Version Changed" saying that we have POST support now?

reviewboard/webapi/resources/diff_file_attachment.py (Diff revision 2)

The issue has been resolved. Show all issues

Missing the rest of the docs (Version Added, Args, Returns).

david Feb. 21, 2024, 3:09 p.m.

We don't actually use those sections for API method (get/post/etc) docstrings because those get turned into our public-facing docs for the API. That said, I probably should flesh this out a bit :)

reviewboard/webapi/resources/diff_file_attachment.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Leftover debug code.
```

Commits:

	Summary	ID
	Add POST support to the DiffFileAttachmentResource. This change adds the ability to upload files to attach to diffs via the `DiffFileAttachmentResource`. This was previously read-only in the API. Testing Done: - Ran unit tests. - Used this from RBTools.	5c8697b55d73100add926350ef4f02174fb883de
	Add POST support to the DiffFileAttachmentResource. This change adds the ability to upload files to attach to diffs via the `DiffFileAttachmentResource`. This was previously read-only in the API. Testing Done: - Ran unit tests. - Used this from RBTools.	ba62ba51eee78e657d7275f81db7e9c081678489

Branch:

-	master
+	release-7.x

Diff:

Revision 3 (+624 -76)

Show changes

	reviewboard/attachments/forms.py
	reviewboard/testing/testcase.py
	reviewboard/webapi/resources/diff_file_attachment.py
	reviewboard/webapi/tests/test_diff_file_attachment.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

reviewboard/attachments/forms.py (Diff revision 3)

The issue has been resolved. Show all issues

The code seems structured as:

if not filediff:
    ...

if filediff:
    ...

It'd be some code movement, but can this just be an if/else?

Commits:

	Summary	ID
	Add POST support to the DiffFileAttachmentResource. This change adds the ability to upload files to attach to diffs via the `DiffFileAttachmentResource`. This was previously read-only in the API. Testing Done: - Ran unit tests. - Used this from RBTools.	ba62ba51eee78e657d7275f81db7e9c081678489
	Add POST support to the DiffFileAttachmentResource. This change adds the ability to upload files to attach to diffs via the `DiffFileAttachmentResource`. This was previously read-only in the API. Testing Done: - Ran unit tests. - Used this from RBTools.	e2e06b2801fe4bc071f1f9e6addd7a3773114c5f

Diff:

Revision 4 (+622 -76)

Show changes

	reviewboard/attachments/forms.py
	reviewboard/testing/testcase.py
	reviewboard/webapi/resources/diff_file_attachment.py
	reviewboard/webapi/tests/test_diff_file_attachment.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-7.x (406d3b5)