Top Level Reviews API Endpoint
Review Request #12028 — Created Feb. 3, 2022 and discarded
- Added top level reviews endpoint.
- Ability to filter by username using ?user=username parameter.
- Ability to filter by repository name using ?repository=repoName parameter.
- Ability to filter by date range using ?last-updated-from and ?last-updated-to parameters.
- Ability to filter by review group using ?review-group=group_name parameter.
- Added tests for the new
root_reviews
resource. - Added documentation for top level reviews endpoint.
- Wrote unit tests for all new functionality
- Ran unit tests for
root_reviews
resource and ensure they pass - Tested new API endpoint manually and confirmed results were as expected.
Summary | ID |
---|---|
ec03d8deeb71d5a57b81acdc543a4e957b9cda98 | |
1cc3bc3ff5e466b8a100801946320fc110ba3261 | |
3de9b81399c00e0ba216cfeb369ab6a9d508bbf4 | |
9a18d8d58df4645b6ffea30ba1011058bf3d944c | |
02a7ebed1a9622585df30c63e04d4529bc22873e | |
1c91b9b3233a47860d390f5edb55e3f3411fed78 | |
4a167d944a0c5b6d31891ceec2df29d5d9e3936d | |
c628f9f7d225b38d7f977dcf231b5c9cfeb4908c | |
792e6a22a36639861060a11ad0ed484900112071 | |
7b8db5e8d21439436caf0fc04e506a7196d52d68 | |
c56e7745cc1782bfbcc4cc5954b87a2e66059444 | |
27fa75dc9644639e7392f8cc1c34cbf82c09c77f | |
cacca7efd27207c02b718e271275dbe66416c69e |
Description | From | Last Updated |
---|---|---|
F401 'django.utils.six' imported but unused |
reviewbot | |
F401 'djblets.util.decorators.augment_method_from' imported but unused |
reviewbot | |
F401 'djblets.webapi.decorators.webapi_response_errors' imported but unused |
reviewbot | |
F401 'djblets.webapi.errors.INVALID_FORM_DATA' imported but unused |
reviewbot | |
F401 'reviewboard.reviews.errors.RevokeShipItError' imported but unused |
reviewbot | |
F401 'reviewboard.webapi.decorators.webapi_check_local_site' imported but unused |
reviewbot | |
F401 'reviewboard.webapi.errors.REVOKE_SHIP_IT_ERROR' imported but unused |
reviewbot | |
F401 'datetime.datetime' imported but unused |
reviewbot | |
F401 'datetime.timedelta' imported but unused |
reviewbot | |
F401 'django.core.mail' imported but unused |
reviewbot | |
F401 'django.utils.timezone' imported but unused |
reviewbot | |
F401 'djblets.util.dates.get_tz_aware_utcnow' imported but unused |
reviewbot | |
F401 'djblets.testing.decorators.add_fixtures' imported but unused |
reviewbot | |
F401 'djblets.webapi.errors.PERMISSION_DENIED' imported but unused |
reviewbot | |
F401 'djblets.webapi.errors.INVALID_FORM_DATA' imported but unused |
reviewbot | |
F401 'djblets.webapi.errors.DOES_NOT_EXIST' imported but unused |
reviewbot | |
F401 'djblets.webapi.testing.decorators.webapi_test_template' imported but unused |
reviewbot | |
F401 'kgb.SpyAgency' imported but unused |
reviewbot | |
F401 'kgb.spy_on' imported but unused |
reviewbot | |
F401 'reviewboard.reviews.models.Review' imported but unused |
reviewbot | |
F401 'reviewboard.reviews.models.ReviewRequest' imported but unused |
reviewbot | |
F401 'reviewboard.reviews.signals.review_ship_it_revoking' imported but unused |
reviewbot | |
F401 'reviewboard.webapi.errors.REVOKE_SHIP_IT_ERROR' imported but unused |
reviewbot | |
F401 'reviewboard.webapi.tests.mimetypes.review_item_mimetype' imported but unused |
reviewbot | |
F401 'reviewboard.webapi.tests.mixins.ReviewRequestChildItemMixin' imported but unused |
reviewbot | |
F401 'reviewboard.webapi.tests.mixins.ReviewRequestChildListMixin' imported but unused |
reviewbot | |
F401 'reviewboard.webapi.tests.mixins.BasicTestsMetaclass' imported but unused |
reviewbot | |
F401 'reviewboard.webapi.tests.mixins_review.ReviewItemMixin' imported but unused |
reviewbot | |
F401 'reviewboard.webapi.tests.mixins_review.ReviewListMixin' imported but unused |
reviewbot | |
E501 line too long (80 > 79 characters) |
reviewbot | |
F401 'django.utils.six' imported but unused |
reviewbot | |
F401 'djblets.util.decorators.augment_method_from' imported but unused |
reviewbot | |
F401 'djblets.webapi.decorators.webapi_response_errors' imported but unused |
reviewbot | |
F401 'djblets.webapi.errors.INVALID_FORM_DATA' imported but unused |
reviewbot | |
F401 'reviewboard.reviews.errors.RevokeShipItError' imported but unused |
reviewbot | |
F401 'reviewboard.webapi.decorators.webapi_check_local_site' imported but unused |
reviewbot | |
F401 'reviewboard.webapi.errors.REVOKE_SHIP_IT_ERROR' imported but unused |
reviewbot | |
F401 'datetime.datetime' imported but unused |
reviewbot | |
F401 'datetime.timedelta' imported but unused |
reviewbot | |
F401 'django.core.mail' imported but unused |
reviewbot | |
F401 'django.utils.timezone' imported but unused |
reviewbot | |
F401 'djblets.util.dates.get_tz_aware_utcnow' imported but unused |
reviewbot | |
F401 'djblets.webapi.errors.PERMISSION_DENIED' imported but unused |
reviewbot | |
F401 'djblets.webapi.errors.INVALID_FORM_DATA' imported but unused |
reviewbot | |
F401 'djblets.webapi.errors.DOES_NOT_EXIST' imported but unused |
reviewbot | |
F401 'djblets.webapi.testing.decorators.webapi_test_template' imported but unused |
reviewbot | |
F401 'kgb.SpyAgency' imported but unused |
reviewbot | |
F401 'kgb.spy_on' imported but unused |
reviewbot | |
F401 'reviewboard.reviews.models.Review' imported but unused |
reviewbot | |
F401 'reviewboard.reviews.models.ReviewRequest' imported but unused |
reviewbot | |
F401 'reviewboard.reviews.signals.review_ship_it_revoking' imported but unused |
reviewbot | |
F401 'reviewboard.webapi.errors.REVOKE_SHIP_IT_ERROR' imported but unused |
reviewbot | |
F401 'reviewboard.webapi.tests.mimetypes.review_item_mimetype' imported but unused |
reviewbot | |
F401 'reviewboard.webapi.tests.mixins.ReviewRequestChildItemMixin' imported but unused |
reviewbot | |
F401 'reviewboard.webapi.tests.mixins.ReviewRequestChildListMixin' imported but unused |
reviewbot | |
F401 'reviewboard.webapi.tests.mixins.BasicTestsMetaclass' imported but unused |
reviewbot | |
F401 'reviewboard.webapi.tests.mixins_review.ReviewItemMixin' imported but unused |
reviewbot | |
F401 'reviewboard.webapi.tests.mixins_review.ReviewListMixin' imported but unused |
reviewbot | |
E501 line too long (80 > 79 characters) |
reviewbot | |
E501 line too long (95 > 79 characters) |
reviewbot | |
On the master branch, we no longer need the unicode_literals imports. |
david | |
This was just copied from the other review resource, but should probably be clarified since this one is not a … |
david | |
E501 line too long (84 > 79 characters) |
reviewbot | |
E501 line too long (80 > 79 characters) |
reviewbot | |
E501 line too long (95 > 79 characters) |
reviewbot | |
E501 line too long (90 > 79 characters) |
reviewbot | |
E501 line too long (82 > 79 characters) |
reviewbot | |
E501 line too long (83 > 79 characters) |
reviewbot | |
E501 line too long (95 > 79 characters) |
reviewbot | |
E501 line too long (96 > 79 characters) |
reviewbot | |
E501 line too long (94 > 79 characters) |
reviewbot | |
E501 line too long (93 > 79 characters) |
reviewbot | |
E501 line too long (91 > 79 characters) |
reviewbot | |
E501 line too long (82 > 79 characters) |
reviewbot | |
E501 line too long (95 > 79 characters) |
reviewbot | |
E501 line too long (86 > 79 characters) |
reviewbot | |
E501 line too long (89 > 79 characters) |
reviewbot | |
E501 line too long (84 > 79 characters) |
reviewbot | |
E501 line too long (80 > 79 characters) |
reviewbot | |
E501 line too long (95 > 79 characters) |
reviewbot | |
E501 line too long (90 > 79 characters) |
reviewbot | |
E501 line too long (82 > 79 characters) |
reviewbot | |
E501 line too long (83 > 79 characters) |
reviewbot | |
E501 line too long (95 > 79 characters) |
reviewbot | |
E501 line too long (96 > 79 characters) |
reviewbot | |
E501 line too long (94 > 79 characters) |
reviewbot | |
E501 line too long (109 > 79 characters) |
reviewbot | |
E501 line too long (121 > 79 characters) |
reviewbot | |
E501 line too long (117 > 79 characters) |
reviewbot | |
E501 line too long (82 > 79 characters) |
reviewbot | |
E501 line too long (139 > 79 characters) |
reviewbot | |
E501 line too long (84 > 79 characters) |
reviewbot | |
F401 'importlib' imported but unused |
reviewbot | |
F401 'django.utils.six' imported but unused |
reviewbot | |
Please add a module docstring at the top of this file. Something like """Root-level resource for querying reviews.""" |
david | |
Imports from reviewboard should be in a separate group after third-party packages. So: from django... from djblets... from reviewboard... Everything … |
david | |
This can be wrapped much later. Line width is 79 columns. |
david | |
I know you probably copied this from our other code, but this should use our new docstring standards: https://www.notion.so/reviewboard/Writing-Python-Docs-845c696271194d85b69503246db1d0e6 |
david | |
Looks great! Only one suggestion here, once speficied in webapi_request_fields, we can get parsed parameters in kwargds. Using kwargds['drafts'] here … |
Chaoyu | |
This needs a docstring. |
david | |
Please update for new docstring standards. |
david | |
This file needs a module docstring. |
david | |
Please sort alphabetically - t before u |
david | |
This needs a blank line between these two. |
david | |
Please add a docstring. |
david | |
When we wrap a single-line docstring, please put the final """ on its own line: def test_get_with_counts_only(self): """Testing the GET … |
david | |
""" on its own line. |
david | |
""" on its own line. Please go through the rest of this file and change where appropriate. |
david | |
webapi.decorators before webapi.fields |
david | |
This should be in the imperative mood ("Return" instead of "Returns") |
david | |
This line can dedent 4 spaces. |
david | |
Returns -> Return |
david | |
Returns -> Return |
david | |
Instead of listing "reviews/" in each docstring, let's use "<URL>/" and add on the @webapi_test_template decorator. |
david | |
I think everything looks good overall. I just have a small suggestion to improve the unit tests readability: I think … |
sheenaNg | |
The review_request api endpoint filters by the ID of the repository instead of the name. Should we filter by ID … |
maubin | |
I'm going to specify that this is the "review group name of users" rather than "the group of users" |
maubin | |
It might be worthwhile to add fields for "time-added-to" and "time-added-from" just so that there's more options for date filtering. … |
maubin |
- Description:
-
- Added rudimentary top level reviews endpoint.
~ - Added some basic tests for the new
root_reviews
resource.
~ - Still missing a lot of basic functionality, documentation, and tests
~ - Ability to filter by username using ?user=username parameter
~ - Ability to filter by repository name using ?repository=repoName parameter
+ - Added some basic tests for the new
root_reviews
resource.
+ - Still missing date range filtering, review group filtering, documentation, some unit tests
- Testing Done:
-
~ - Ran unit tests for
root_reviews
resource.
~ - Tested API new endpoint and confirmed results were as expected.
~ - Wrote unit tests for all new functionality
~ - Ran unit tests for
root_reviews
resource and ensure they pass
+ - Tested new API endpoint manually and confirmed results were as expected.
- Ran unit tests for
- Commits:
-
Summary ID 4a98ef5189063d4f605a70f87cd78c696ead9e5d 206769261bfca454dc42dca506e10bb67bb6f20f 62f5caf5437416438fe386d3169a3d2c95de44c7 4a98ef5189063d4f605a70f87cd78c696ead9e5d 206769261bfca454dc42dca506e10bb67bb6f20f 62f5caf5437416438fe386d3169a3d2c95de44c7 64e37475a38cd7180e70065a0b85c0557b7726bd
Checks run (1 failed, 1 succeeded)
flake8
-
Warning: Showing 30 of 37 failures.
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
- Description:
-
- Added rudimentary top level reviews endpoint.
- Ability to filter by username using ?user=username parameter
- Ability to filter by repository name using ?repository=repoName parameter
~ - Added some basic tests for the new
root_reviews
resource.
~ - Still missing date range filtering, review group filtering, documentation, some unit tests
~ - Ability to filter by date range using ?last-updated-from and ?last-updated-to
~ - Ability to filter by review group using ?review-group=group_name
+ - Added some basic tests for the new
root_reviews
resource.
+ - Still missing documentation, some unit tests
- Commits:
-
Summary ID 4a98ef5189063d4f605a70f87cd78c696ead9e5d 206769261bfca454dc42dca506e10bb67bb6f20f 62f5caf5437416438fe386d3169a3d2c95de44c7 64e37475a38cd7180e70065a0b85c0557b7726bd 4a98ef5189063d4f605a70f87cd78c696ead9e5d 206769261bfca454dc42dca506e10bb67bb6f20f 62f5caf5437416438fe386d3169a3d2c95de44c7 64e37475a38cd7180e70065a0b85c0557b7726bd ef5f0394cfbcf904b9ecffee4c01530494e1db62 d01ead0c5789f6f273dde85ffc40820b514e2f1f
Checks run (1 failed, 1 succeeded)
flake8
- Description:
-
- Added rudimentary top level reviews endpoint.
- Ability to filter by username using ?user=username parameter
- Ability to filter by repository name using ?repository=repoName parameter
- Ability to filter by date range using ?last-updated-from and ?last-updated-to
- Ability to filter by review group using ?review-group=group_name
~ - Added some basic tests for the new
root_reviews
resource.
~ - Added tests for the new
root_reviews
resource.
- Still missing documentation, some unit tests
- Commits:
-
Summary ID 4a98ef5189063d4f605a70f87cd78c696ead9e5d 206769261bfca454dc42dca506e10bb67bb6f20f 62f5caf5437416438fe386d3169a3d2c95de44c7 64e37475a38cd7180e70065a0b85c0557b7726bd ef5f0394cfbcf904b9ecffee4c01530494e1db62 d01ead0c5789f6f273dde85ffc40820b514e2f1f 4a98ef5189063d4f605a70f87cd78c696ead9e5d 206769261bfca454dc42dca506e10bb67bb6f20f 62f5caf5437416438fe386d3169a3d2c95de44c7 64e37475a38cd7180e70065a0b85c0557b7726bd ef5f0394cfbcf904b9ecffee4c01530494e1db62 d01ead0c5789f6f273dde85ffc40820b514e2f1f 46ef7bf956bd684356ad088143acc16ca8d53ca7
Checks run (1 failed, 1 succeeded)
flake8
- Commits:
-
Summary ID 4a98ef5189063d4f605a70f87cd78c696ead9e5d 206769261bfca454dc42dca506e10bb67bb6f20f 62f5caf5437416438fe386d3169a3d2c95de44c7 64e37475a38cd7180e70065a0b85c0557b7726bd ef5f0394cfbcf904b9ecffee4c01530494e1db62 d01ead0c5789f6f273dde85ffc40820b514e2f1f 46ef7bf956bd684356ad088143acc16ca8d53ca7 4a98ef5189063d4f605a70f87cd78c696ead9e5d 206769261bfca454dc42dca506e10bb67bb6f20f 62f5caf5437416438fe386d3169a3d2c95de44c7 64e37475a38cd7180e70065a0b85c0557b7726bd ef5f0394cfbcf904b9ecffee4c01530494e1db62 d01ead0c5789f6f273dde85ffc40820b514e2f1f 46ef7bf956bd684356ad088143acc16ca8d53ca7 a82f040f7ad92cc37ac053662d0d7380c8d551b7
Checks run (2 succeeded)
- Description:
-
- Added rudimentary top level reviews endpoint.
- Ability to filter by username using ?user=username parameter
- Ability to filter by repository name using ?repository=repoName parameter
- Ability to filter by date range using ?last-updated-from and ?last-updated-to
- Ability to filter by review group using ?review-group=group_name
- Added tests for the new
root_reviews
resource.
~ - Still missing documentation, some unit tests
~ - Still missing documentation
- Summary:
-
[WIP] Top Level Reviews API EndpointTop Level Reviews API Endpoint
- Description:
-
~ - Added rudimentary top level reviews endpoint.
~ - Ability to filter by username using ?user=username parameter
~ - Ability to filter by repository name using ?repository=repoName parameter
~ - Ability to filter by date range using ?last-updated-from and ?last-updated-to
~ - Ability to filter by review group using ?review-group=group_name
~ - Added top level reviews endpoint.
~ - Ability to filter by username using ?user=username parameter.
~ - Ability to filter by repository name using ?repository=repoName parameter.
~ - Ability to filter by date range using ?last-updated-from and ?last-updated-to parameters.
~ - Ability to filter by review group using ?review-group=group_name parameter.
- Added tests for the new
root_reviews
resource.
~ - Still missing documentation
~ - Added documentation for top level reviews endpoint.
- Commits:
-
Summary ID 4a98ef5189063d4f605a70f87cd78c696ead9e5d 206769261bfca454dc42dca506e10bb67bb6f20f 62f5caf5437416438fe386d3169a3d2c95de44c7 64e37475a38cd7180e70065a0b85c0557b7726bd ef5f0394cfbcf904b9ecffee4c01530494e1db62 d01ead0c5789f6f273dde85ffc40820b514e2f1f 46ef7bf956bd684356ad088143acc16ca8d53ca7 a82f040f7ad92cc37ac053662d0d7380c8d551b7 4a98ef5189063d4f605a70f87cd78c696ead9e5d 206769261bfca454dc42dca506e10bb67bb6f20f 62f5caf5437416438fe386d3169a3d2c95de44c7 64e37475a38cd7180e70065a0b85c0557b7726bd ef5f0394cfbcf904b9ecffee4c01530494e1db62 d01ead0c5789f6f273dde85ffc40820b514e2f1f 46ef7bf956bd684356ad088143acc16ca8d53ca7 a82f040f7ad92cc37ac053662d0d7380c8d551b7 55c5247da821df4fb376238c31ac7a6a8f806f32
- Change Summary:
-
Rebased off of origin/master.
- Commits:
-
Summary ID 4a98ef5189063d4f605a70f87cd78c696ead9e5d 206769261bfca454dc42dca506e10bb67bb6f20f 62f5caf5437416438fe386d3169a3d2c95de44c7 64e37475a38cd7180e70065a0b85c0557b7726bd ef5f0394cfbcf904b9ecffee4c01530494e1db62 d01ead0c5789f6f273dde85ffc40820b514e2f1f 46ef7bf956bd684356ad088143acc16ca8d53ca7 a82f040f7ad92cc37ac053662d0d7380c8d551b7 55c5247da821df4fb376238c31ac7a6a8f806f32 ec03d8deeb71d5a57b81acdc543a4e957b9cda98 1cc3bc3ff5e466b8a100801946320fc110ba3261 3de9b81399c00e0ba216cfeb369ab6a9d508bbf4 9a18d8d58df4645b6ffea30ba1011058bf3d944c 02a7ebed1a9622585df30c63e04d4529bc22873e 1c91b9b3233a47860d390f5edb55e3f3411fed78 4a167d944a0c5b6d31891ceec2df29d5d9e3936d c628f9f7d225b38d7f977dcf231b5c9cfeb4908c 792e6a22a36639861060a11ad0ed484900112071
Checks run (2 succeeded)
- Change Summary:
-
confirmed the compiled HTML of the documentation displays as expected.
- Commits:
-
Summary ID ec03d8deeb71d5a57b81acdc543a4e957b9cda98 1cc3bc3ff5e466b8a100801946320fc110ba3261 3de9b81399c00e0ba216cfeb369ab6a9d508bbf4 9a18d8d58df4645b6ffea30ba1011058bf3d944c 02a7ebed1a9622585df30c63e04d4529bc22873e 1c91b9b3233a47860d390f5edb55e3f3411fed78 4a167d944a0c5b6d31891ceec2df29d5d9e3936d c628f9f7d225b38d7f977dcf231b5c9cfeb4908c 792e6a22a36639861060a11ad0ed484900112071 ec03d8deeb71d5a57b81acdc543a4e957b9cda98 1cc3bc3ff5e466b8a100801946320fc110ba3261 3de9b81399c00e0ba216cfeb369ab6a9d508bbf4 9a18d8d58df4645b6ffea30ba1011058bf3d944c 02a7ebed1a9622585df30c63e04d4529bc22873e 1c91b9b3233a47860d390f5edb55e3f3411fed78 4a167d944a0c5b6d31891ceec2df29d5d9e3936d c628f9f7d225b38d7f977dcf231b5c9cfeb4908c 792e6a22a36639861060a11ad0ed484900112071 7b8db5e8d21439436caf0fc04e506a7196d52d68
Checks run (2 succeeded)
-
Looking great! I have a few comments but they're all trivial style things.
-
Please add a module docstring at the top of this file. Something like
"""Root-level resource for querying reviews."""
-
Imports from
reviewboard
should be in a separate group after third-party packages. So:from django... from djblets... from reviewboard...
Everything within those two groups should be sorted alphabetically.
-
-
I know you probably copied this from our other code, but this should use our new docstring standards: https://www.notion.so/reviewboard/Writing-Python-Docs-845c696271194d85b69503246db1d0e6
-
-
-
-
-
-
-
When we wrap a single-line docstring, please put the final """ on its own line:
def test_get_with_counts_only(self): """Testing the GET reviews/?counts_only=1 API returns expected counts """
-
-
- Commits:
-
Summary ID ec03d8deeb71d5a57b81acdc543a4e957b9cda98 1cc3bc3ff5e466b8a100801946320fc110ba3261 3de9b81399c00e0ba216cfeb369ab6a9d508bbf4 9a18d8d58df4645b6ffea30ba1011058bf3d944c 02a7ebed1a9622585df30c63e04d4529bc22873e 1c91b9b3233a47860d390f5edb55e3f3411fed78 4a167d944a0c5b6d31891ceec2df29d5d9e3936d c628f9f7d225b38d7f977dcf231b5c9cfeb4908c 792e6a22a36639861060a11ad0ed484900112071 7b8db5e8d21439436caf0fc04e506a7196d52d68 ec03d8deeb71d5a57b81acdc543a4e957b9cda98 1cc3bc3ff5e466b8a100801946320fc110ba3261 3de9b81399c00e0ba216cfeb369ab6a9d508bbf4 9a18d8d58df4645b6ffea30ba1011058bf3d944c 02a7ebed1a9622585df30c63e04d4529bc22873e 1c91b9b3233a47860d390f5edb55e3f3411fed78 4a167d944a0c5b6d31891ceec2df29d5d9e3936d c628f9f7d225b38d7f977dcf231b5c9cfeb4908c 792e6a22a36639861060a11ad0ed484900112071 7b8db5e8d21439436caf0fc04e506a7196d52d68 c56e7745cc1782bfbcc4cc5954b87a2e66059444
Checks run (2 succeeded)
- Commits:
-
Summary ID ec03d8deeb71d5a57b81acdc543a4e957b9cda98 1cc3bc3ff5e466b8a100801946320fc110ba3261 3de9b81399c00e0ba216cfeb369ab6a9d508bbf4 9a18d8d58df4645b6ffea30ba1011058bf3d944c 02a7ebed1a9622585df30c63e04d4529bc22873e 1c91b9b3233a47860d390f5edb55e3f3411fed78 4a167d944a0c5b6d31891ceec2df29d5d9e3936d c628f9f7d225b38d7f977dcf231b5c9cfeb4908c 792e6a22a36639861060a11ad0ed484900112071 7b8db5e8d21439436caf0fc04e506a7196d52d68 c56e7745cc1782bfbcc4cc5954b87a2e66059444 ec03d8deeb71d5a57b81acdc543a4e957b9cda98 1cc3bc3ff5e466b8a100801946320fc110ba3261 3de9b81399c00e0ba216cfeb369ab6a9d508bbf4 9a18d8d58df4645b6ffea30ba1011058bf3d944c 02a7ebed1a9622585df30c63e04d4529bc22873e 1c91b9b3233a47860d390f5edb55e3f3411fed78 4a167d944a0c5b6d31891ceec2df29d5d9e3936d c628f9f7d225b38d7f977dcf231b5c9cfeb4908c 792e6a22a36639861060a11ad0ed484900112071 7b8db5e8d21439436caf0fc04e506a7196d52d68 c56e7745cc1782bfbcc4cc5954b87a2e66059444 27fa75dc9644639e7392f8cc1c34cbf82c09c77f
Checks run (2 succeeded)
-
-
I think everything looks good overall.
I just have a small suggestion to improve the unit tests readability: I think it would be easier to read if you add some blank lines to separate different groups of codes in the unit tests.
For example, add blank line after the codes that create review_request, review etc. and before calling the API endpoint. Then, add another blank line after calling the API endpoint and before the assertions.
I see that you have added some blank lines in some unit tests but not all. It will improve the consistency if this is applied to the rest of the unit tests.
- Commits:
-
Summary ID ec03d8deeb71d5a57b81acdc543a4e957b9cda98 1cc3bc3ff5e466b8a100801946320fc110ba3261 3de9b81399c00e0ba216cfeb369ab6a9d508bbf4 9a18d8d58df4645b6ffea30ba1011058bf3d944c 02a7ebed1a9622585df30c63e04d4529bc22873e 1c91b9b3233a47860d390f5edb55e3f3411fed78 4a167d944a0c5b6d31891ceec2df29d5d9e3936d c628f9f7d225b38d7f977dcf231b5c9cfeb4908c 792e6a22a36639861060a11ad0ed484900112071 7b8db5e8d21439436caf0fc04e506a7196d52d68 c56e7745cc1782bfbcc4cc5954b87a2e66059444 27fa75dc9644639e7392f8cc1c34cbf82c09c77f ec03d8deeb71d5a57b81acdc543a4e957b9cda98 1cc3bc3ff5e466b8a100801946320fc110ba3261 3de9b81399c00e0ba216cfeb369ab6a9d508bbf4 9a18d8d58df4645b6ffea30ba1011058bf3d944c 02a7ebed1a9622585df30c63e04d4529bc22873e 1c91b9b3233a47860d390f5edb55e3f3411fed78 4a167d944a0c5b6d31891ceec2df29d5d9e3936d c628f9f7d225b38d7f977dcf231b5c9cfeb4908c 792e6a22a36639861060a11ad0ed484900112071 7b8db5e8d21439436caf0fc04e506a7196d52d68 c56e7745cc1782bfbcc4cc5954b87a2e66059444 27fa75dc9644639e7392f8cc1c34cbf82c09c77f cacca7efd27207c02b718e271275dbe66416c69e
Checks run (2 succeeded)
-
-
The review_request api endpoint filters by the ID of the repository instead of the name. Should we filter by ID here too for consistency or should we just specify in the description that this field is the name of the repository?
-
-
It might be worthwhile to add fields for "time-added-to" and "time-added-from" just so that there's more options for date filtering. Should I add these fields?