Summary

Add a max attachment size for diff file attachments.

Review Request #13579 — Created Feb. 27, 2024 and submitted March 26, 2024, 10:16 p.m.

Information

Owner

david

Repository

Review Board

Branch

release-7.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

This change adds a configurable maximum attachment size for file
attachments which are connected to diffs. This works very much like the
configured maximum diff size.

Our initial default is 10MB. This should be enough for most images and
documents. The limit can be changed in the diff settings page in the
admin site.

At the moment this is enforced on the server side when attempting to
create the attachment. The limit is also included in the server
capabilities object so clients can make a decision about whether to
attempt to upload. While doing this I also added the max diff size to
the capabilities.

Testing Done

Ran unit tests.

Commits

Summary	ID
Add a max attachment size for diff file attachments. This change adds a configurable maximum attachment size for file attachments which are connected to diffs. This works very much like the configured maximum diff size. Our initial default is 10MB. This should be enough for most images and documents. The limit can be changed in the diff settings page in the admin site. At the moment this is enforced on the server side when attempting to create the attachment. The limit is also included in the server capabilities object so clients can make a decision about whether to attempt to upload. While doing this I also added the max diff size to the capabilities. Testing Done: Ran unit tests.	ed31774a6ad53cf972f2b10d5b0471766a264a51

Summary

Add a max attachment size for diff file attachments.

This change adds a configurable maximum attachment size for file attachments which are connected to diffs. This works very much like the configured maximum diff size. Our initial default is 10MB. This should be enough for most images and documents. The limit can be changed in the diff settings page in the admin site. At the moment this is enforced on the server side when attempting to create the attachment. The limit is also included in the server capabilities object so clients can make a decision about whether to attempt to upload. While doing this I also added the max diff size to the capabilities. Testing Done: Ran unit tests.

ed31774a6ad53cf972f2b10d5b0471766a264a51

Issues

Description	From	Last Updated
Extra comma added (will turn this into a tuple).	chipx86	March 4, 2024, 6:54 p.m.
Same here.	chipx86	March 4, 2024, 6:54 p.m.
Same here.	chipx86	March 4, 2024, 6:54 p.m.
Missing a "Version Added". Also make sure this ends up in the coderef index.	chipx86	March 4, 2024, 6:55 p.m.
Missing docs.	chipx86	March 4, 2024, 6:55 p.m.
Can we make this a keyword-only argument?	chipx86	March 4, 2024, 6:55 p.m.
Missing a return type. I think this (and keyword-only arguments) were covered in another change, but maybe that should be …	chipx86	March 4, 2024, 6:56 p.m.
Let's put these in alphabetical order.	chipx86	March 4, 2024, 6:57 p.m.
Let's put these in alphabetical order.	chipx86	March 4, 2024, 6:57 p.m.
Let's put these in alphabetical order. Mayb worth using .update()? Easier for maintenance.	chipx86	March 4, 2024, 6:58 p.m.
I'm trying to move more of these tests to do a full-on equality check of the response, to avoid missing …	chipx86	March 4, 2024, 7 p.m.

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

reviewboard/admin/forms/diff_settings.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Extra comma added (will turn this into a tuple).
```
reviewboard/admin/forms/diff_settings.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Same here.
```
reviewboard/admin/forms/diff_settings.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Same here.
```
reviewboard/attachments/errors.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Missing a "Version Added".
Also make sure this ends up in the coderef index.
```
reviewboard/attachments/errors.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Missing docs.
```
reviewboard/attachments/errors.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Can we make this a keyword-only argument?
```

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

The issue has been resolved. Show all issues

Missing a return type.
I think this (and keyword-only arguments) were covered in another change, but maybe that should be addressed here and not in that one.

reviewboard/webapi/resources/diff_file_attachment.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Let's put these in alphabetical order.
```
reviewboard/webapi/resources/draft_filediff.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Let's put these in alphabetical order.
```

reviewboard/webapi/server_info.py (Diff revision 1)

The issue has been resolved. Show all issues

Let's put these in alphabetical order.

Mayb worth using .update()? Easier for maintenance.

reviewboard/webapi/tests/test_draft_filediff.py (Diff revision 1)

The issue has been resolved. Show all issues

I'm trying to move more of these tests to do a full-on equality check of the response, to avoid missing things and to ensure things like sub-types of errors are caught. Can we switch that?

Commits:

	Summary	ID
	Add a max attachment size for diff file attachments. This change adds a configurable maximum attachment size for file attachments which are connected to diffs. This works very much like the configured maximum diff size. Our initial default is 10MB. This should be enough for most images and documents. The limit can be changed in the diff settings page in the admin site. At the moment this is enforced on the server side when attempting to create the attachment. The limit is also included in the server capabilities object so clients can make a decision about whether to attempt to upload. While doing this I also added the max diff size to the capabilities. Testing Done: Ran unit tests.	5d06023f4cca96453bb85d5af3b242b7930a32cf
	Add a max attachment size for diff file attachments. This change adds a configurable maximum attachment size for file attachments which are connected to diffs. This works very much like the configured maximum diff size. Our initial default is 10MB. This should be enough for most images and documents. The limit can be changed in the diff settings page in the admin site. At the moment this is enforced on the server side when attempting to create the attachment. The limit is also included in the server capabilities object so clients can make a decision about whether to attempt to upload. While doing this I also added the max diff size to the capabilities. Testing Done: Ran unit tests.	ed31774a6ad53cf972f2b10d5b0471766a264a51

Diff:

Revision 2 (+424 -24)

Show changes

	docs/manual/extending/coderef/index.rst
	reviewboard/admin/siteconfig.py
	reviewboard/admin/forms/diff_settings.py
	reviewboard/attachments/errors.py
	reviewboard/attachments/forms.py
	reviewboard/webapi/server_info.py
	reviewboard/webapi/resources/diff_file_attachment.py
	reviewboard/webapi/resources/draft_filediff.py
	2 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-7.x (21ac319)