Add a max attachment size for diff file attachments.

Review Request #13579 — Created Feb. 27, 2024 and submitted

Information

Review Board
release-7.x

Reviewers

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.

Ran unit tests.

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
Description From Last Updated

Extra comma added (will turn this into a tuple).

chipx86chipx86

Same here.

chipx86chipx86

Same here.

chipx86chipx86

Missing a "Version Added". Also make sure this ends up in the coderef index.

chipx86chipx86

Missing docs.

chipx86chipx86

Can we make this a keyword-only argument?

chipx86chipx86

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

chipx86chipx86

Let's put these in alphabetical order.

chipx86chipx86

Let's put these in alphabetical order.

chipx86chipx86

Let's put these in alphabetical order. Mayb worth using .update()? Easier for maintenance.

chipx86chipx86

I'm trying to move more of these tests to do a full-on equality check of the response, to avoid missing …

chipx86chipx86
maubin
  1. Ship It!
  2. 
      
chipx86
  1. 
      
  2. Show all issues

    Extra comma added (will turn this into a tuple).

  3. Show all issues

    Same here.

  4. Show all issues

    Same here.

  5. reviewboard/attachments/errors.py (Diff revision 1)
     
     
    Show all issues

    Missing a "Version Added".

    Also make sure this ends up in the coderef index.

  6. reviewboard/attachments/errors.py (Diff revision 1)
     
     
    Show all issues

    Missing docs.

  7. reviewboard/attachments/errors.py (Diff revision 1)
     
     
    Show all issues

    Can we make this a keyword-only argument?

  8. reviewboard/attachments/forms.py (Diff revision 1)
     
     
    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.

  9. Show all issues

    Let's put these in alphabetical order.

  10. Show all issues

    Let's put these in alphabetical order.

  11. reviewboard/webapi/server_info.py (Diff revision 1)
     
     
     
     
     
    Show all issues

    Let's put these in alphabetical order.

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

  12. reviewboard/webapi/tests/test_draft_filediff.py (Diff revision 1)
     
     
     
     
     
     
    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?

  13. 
      
david
chipx86
  1. Ship It!
  2. 
      
david
Review request changed
Status:
Completed
Change Summary:
Pushed to release-7.x (21ac319)