Return binary flag when with filediffs and allow querying by the binary flag.

Review Request #10729 — Created Sept. 26, 2019 and updated

Information

Review Board
master

Reviewers

RB Tools required getting the binary status of each filediff so it
could determine which files need an accompanying file upload. This
allows RB Tools to query the backend for the filediffs for a diff. The
filediffs returned will contain the binart flag. An optional flag is
also provide for querying only for filediffs that have the binary flag
set to either true or false.

Included unit test for these changes.

Summary ID
Address David's comments
2cb39d374b5799e821a07e734ed6a614904bf64c
Fix grammar mistake
4b1e111fed1df7c8cd8ae27b89c8f7b1f9dec0b5
Revert "Fix grammar mistake"
This reverts commit 4b1e111fed1df7c8cd8ae27b89c8f7b1f9dec0b5.
9ae87d7ca6a8c5bad33d32df78baae77f8abde4b
Fix grammar
2371a6a041e2ca4c3c9147b85eac56039872fea2
Add binary filter test
46a4cabcc5cf318a5fefe46ebb5b98b5789628b2
replace double quotes with single quotes
9da99cd37e3d89ecd9ea02bd65a4ff646ae5a9c2
Use djblets to map query value to parameter
90b4f57a8285cd9be9d847410144002529972775
Add binary parameter to function description
3047b4be526f39ded08bf118b14930d5252f3b79
drop get_queryset parameters done one line
bd8db61615a5ce70d82438900eb6dff63a7c105d
format get_queryset description to fit line length limit
0cea0271d77078b30ee22e45bbb1356d111b7dbb
Description From Last Updated

Typo in summary: "amd"

brenniebrennie

This is including some strange delete/re-add of some changes. Can you look into what happened, try to revert those?

chipx86chipx86

Can you flesh out the description and testing done? See https://www.notion.so/reviewboard/Writing-Good-Change-Descriptions-10529e7c207743fa8ca90153d4b21fea

daviddavid

Can you add unit tests for both the query parameter and the new field in the output?

daviddavid

Let's keep these in alphabetical order

daviddavid

This needs a period at the end.

daviddavid

Please use single quotes rather than double.

daviddavid

This can be simplified down to: qs = qs.filter(binary=(param == 'true'))

daviddavid

"is binary" or "is a binary file".

brenniebrennie

This should be updated to talk about the new binary query parameter.

daviddavid

Instead of doing this, you can add a binary=False keyword arugment to get_queryset() and the API field will do the …

brenniebrennie

E501 line too long (80 > 79 characters)

reviewbotreviewbot

E501 line too long (80 > 79 characters)

reviewbotreviewbot

E501 line too long (80 > 79 characters)

reviewbotreviewbot

E501 line too long (80 > 79 characters)

reviewbotreviewbot

E501 line too long (86 > 79 characters)

reviewbotreviewbot

E501 line too long (80 > 79 characters)

reviewbotreviewbot

E501 line too long (83 > 79 characters)

reviewbotreviewbot

E501 line too long (80 > 79 characters)

reviewbotreviewbot
david
  1. 
      
  2. Show all issues

    Can you flesh out the description and testing done? See https://www.notion.so/reviewboard/Writing-Good-Change-Descriptions-10529e7c207743fa8ca90153d4b21fea

  3. reviewboard/webapi/resources/filediff.py (Diff revision 1)
     
     
     
     
     
     
    Show all issues

    Let's keep these in alphabetical order

  4. Show all issues

    This needs a period at the end.

  5. reviewboard/webapi/resources/filediff.py (Diff revision 1)
     
     
     
    Show all issues

    Please use single quotes rather than double.

  6. reviewboard/webapi/resources/filediff.py (Diff revision 1)
     
     
     
     
     
    Show all issues

    This can be simplified down to:

    qs = qs.filter(binary=(param == 'true'))
    
  7. 
      
PE
PE
brennie
  1. 
      
  2. Show all issues

    Typo in summary: "amd"

  3. Show all issues

    "is binary" or "is a binary file".

  4. reviewboard/webapi/resources/filediff.py (Diff revision 2)
     
     
     
     
    Show all issues

    Instead of doing this, you can add a binary=False keyword arugment to get_queryset() and the API field will do the rest.

  5. 
      
david
  1. 
      
  2. Show all issues

    Can you add unit tests for both the query parameter and the new field in the output?

  3. reviewboard/webapi/resources/filediff.py (Diff revision 2)
     
     
     
    Show all issues

    This should be updated to talk about the new binary query parameter.

  4. 
      
PE
Review request changed
Summary:
Return binary flag when with filediffs amd allow querying by the binary flag.
Return binary flag when with filediffs and allow querying by the binary flag.
Commits:
Summary ID
Address David's comments
8d2cdb32bc41a59bff8948bc547c5eba15c34e94
Address David's comments
2cb39d374b5799e821a07e734ed6a614904bf64c
Fix grammar mistake
4b1e111fed1df7c8cd8ae27b89c8f7b1f9dec0b5

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

chipx86
  1. 
      
  2. Show all issues

    This is including some strange delete/re-add of some changes. Can you look into what happened, try to revert those?

  3. 
      
PE
Review request changed
Commits:
Summary ID
Address David's comments
2cb39d374b5799e821a07e734ed6a614904bf64c
Fix grammar mistake
4b1e111fed1df7c8cd8ae27b89c8f7b1f9dec0b5
Address David's comments
2cb39d374b5799e821a07e734ed6a614904bf64c
Fix grammar mistake
4b1e111fed1df7c8cd8ae27b89c8f7b1f9dec0b5
Revert "Fix grammar mistake"
This reverts commit 4b1e111fed1df7c8cd8ae27b89c8f7b1f9dec0b5.
9ae87d7ca6a8c5bad33d32df78baae77f8abde4b
Fix grammar
2371a6a041e2ca4c3c9147b85eac56039872fea2

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

PE
Review request changed
Commits:
Summary ID
Address David's comments
2cb39d374b5799e821a07e734ed6a614904bf64c
Fix grammar mistake
4b1e111fed1df7c8cd8ae27b89c8f7b1f9dec0b5
Revert "Fix grammar mistake"
This reverts commit 4b1e111fed1df7c8cd8ae27b89c8f7b1f9dec0b5.
9ae87d7ca6a8c5bad33d32df78baae77f8abde4b
Fix grammar
2371a6a041e2ca4c3c9147b85eac56039872fea2
Address David's comments
2cb39d374b5799e821a07e734ed6a614904bf64c
Fix grammar mistake
4b1e111fed1df7c8cd8ae27b89c8f7b1f9dec0b5
Revert "Fix grammar mistake"
This reverts commit 4b1e111fed1df7c8cd8ae27b89c8f7b1f9dec0b5.
9ae87d7ca6a8c5bad33d32df78baae77f8abde4b
Fix grammar
2371a6a041e2ca4c3c9147b85eac56039872fea2
Add binary filter test
46a4cabcc5cf318a5fefe46ebb5b98b5789628b2
replace double quotes with single quotes
9da99cd37e3d89ecd9ea02bd65a4ff646ae5a9c2

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

PE
Review request changed
Commits:
Summary ID
Address David's comments
2cb39d374b5799e821a07e734ed6a614904bf64c
Fix grammar mistake
4b1e111fed1df7c8cd8ae27b89c8f7b1f9dec0b5
Revert "Fix grammar mistake"
This reverts commit 4b1e111fed1df7c8cd8ae27b89c8f7b1f9dec0b5.
9ae87d7ca6a8c5bad33d32df78baae77f8abde4b
Fix grammar
2371a6a041e2ca4c3c9147b85eac56039872fea2
Add binary filter test
46a4cabcc5cf318a5fefe46ebb5b98b5789628b2
replace double quotes with single quotes
9da99cd37e3d89ecd9ea02bd65a4ff646ae5a9c2
Address David's comments
2cb39d374b5799e821a07e734ed6a614904bf64c
Fix grammar mistake
4b1e111fed1df7c8cd8ae27b89c8f7b1f9dec0b5
Revert "Fix grammar mistake"
This reverts commit 4b1e111fed1df7c8cd8ae27b89c8f7b1f9dec0b5.
9ae87d7ca6a8c5bad33d32df78baae77f8abde4b
Fix grammar
2371a6a041e2ca4c3c9147b85eac56039872fea2
Add binary filter test
46a4cabcc5cf318a5fefe46ebb5b98b5789628b2
replace double quotes with single quotes
9da99cd37e3d89ecd9ea02bd65a4ff646ae5a9c2
Use djblets to map query value to parameter
90b4f57a8285cd9be9d847410144002529972775

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

PE
Review request changed
Commits:
Summary ID
Address David's comments
2cb39d374b5799e821a07e734ed6a614904bf64c
Fix grammar mistake
4b1e111fed1df7c8cd8ae27b89c8f7b1f9dec0b5
Revert "Fix grammar mistake"
This reverts commit 4b1e111fed1df7c8cd8ae27b89c8f7b1f9dec0b5.
9ae87d7ca6a8c5bad33d32df78baae77f8abde4b
Fix grammar
2371a6a041e2ca4c3c9147b85eac56039872fea2
Add binary filter test
46a4cabcc5cf318a5fefe46ebb5b98b5789628b2
replace double quotes with single quotes
9da99cd37e3d89ecd9ea02bd65a4ff646ae5a9c2
Use djblets to map query value to parameter
90b4f57a8285cd9be9d847410144002529972775
Address David's comments
2cb39d374b5799e821a07e734ed6a614904bf64c
Fix grammar mistake
4b1e111fed1df7c8cd8ae27b89c8f7b1f9dec0b5
Revert "Fix grammar mistake"
This reverts commit 4b1e111fed1df7c8cd8ae27b89c8f7b1f9dec0b5.
9ae87d7ca6a8c5bad33d32df78baae77f8abde4b
Fix grammar
2371a6a041e2ca4c3c9147b85eac56039872fea2
Add binary filter test
46a4cabcc5cf318a5fefe46ebb5b98b5789628b2
replace double quotes with single quotes
9da99cd37e3d89ecd9ea02bd65a4ff646ae5a9c2
Use djblets to map query value to parameter
90b4f57a8285cd9be9d847410144002529972775
Add binary parameter to function description
3047b4be526f39ded08bf118b14930d5252f3b79
drop get_queryset parameters done one line
bd8db61615a5ce70d82438900eb6dff63a7c105d

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

PE
Review request changed
Commits:
Summary ID
Address David's comments
2cb39d374b5799e821a07e734ed6a614904bf64c
Fix grammar mistake
4b1e111fed1df7c8cd8ae27b89c8f7b1f9dec0b5
Revert "Fix grammar mistake"
This reverts commit 4b1e111fed1df7c8cd8ae27b89c8f7b1f9dec0b5.
9ae87d7ca6a8c5bad33d32df78baae77f8abde4b
Fix grammar
2371a6a041e2ca4c3c9147b85eac56039872fea2
Add binary filter test
46a4cabcc5cf318a5fefe46ebb5b98b5789628b2
replace double quotes with single quotes
9da99cd37e3d89ecd9ea02bd65a4ff646ae5a9c2
Use djblets to map query value to parameter
90b4f57a8285cd9be9d847410144002529972775
Add binary parameter to function description
3047b4be526f39ded08bf118b14930d5252f3b79
drop get_queryset parameters done one line
bd8db61615a5ce70d82438900eb6dff63a7c105d
Address David's comments
2cb39d374b5799e821a07e734ed6a614904bf64c
Fix grammar mistake
4b1e111fed1df7c8cd8ae27b89c8f7b1f9dec0b5
Revert "Fix grammar mistake"
This reverts commit 4b1e111fed1df7c8cd8ae27b89c8f7b1f9dec0b5.
9ae87d7ca6a8c5bad33d32df78baae77f8abde4b
Fix grammar
2371a6a041e2ca4c3c9147b85eac56039872fea2
Add binary filter test
46a4cabcc5cf318a5fefe46ebb5b98b5789628b2
replace double quotes with single quotes
9da99cd37e3d89ecd9ea02bd65a4ff646ae5a9c2
Use djblets to map query value to parameter
90b4f57a8285cd9be9d847410144002529972775
Add binary parameter to function description
3047b4be526f39ded08bf118b14930d5252f3b79
drop get_queryset parameters done one line
bd8db61615a5ce70d82438900eb6dff63a7c105d
format get_queryset description to fit line length limit
0cea0271d77078b30ee22e45bbb1356d111b7dbb

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

PE
Review request changed
Description:
~  

RB Tools required getting the binary status of each filediff so it could determine which files need an accompanying file upload. This allows RB Tools to query the backend for the filediffs for a diff. The filediffs returned will contain the binart flag. An optional flag is also provide for querying only for filediffs that have the binary flag set to either true or false.

  ~

RB Tools required getting the binary status of each filediff so it

  + could determine which files need an accompanying file upload. This
  + allows RB Tools to query the backend for the filediffs for a diff. The
  + filediffs returned will contain the binart flag. An optional flag is
  + also provide for querying only for filediffs that have the binary flag
  + set to either true or false.

Testing Done:
  +

Included unit test for these changes.