Add webAPI resources for archived and muted review requests

Review Request #7087 — Created March 19, 2015 and submitted

Information

Review Board
master
e23d735...

Reviewers

This adds webAPI resources for the creation and deletion of archived and muted review requests.

Each resource is a child of the User resource and inherits from a base archived resource. Resources accept POST and DELETE requests, both of which modify ReviewRequestVisit database entries accordingly.

Added standard POST and DELETE tests to reviewboard.webapi.tests. All tests passed.

Description From Last Updated

Col: 80 E501 line too long (82 > 79 characters)

reviewbotreviewbot

Needs an added_in field. This should be 2.5.x (until we know when it is going to land).

brenniebrennie

Docstring needs more information. Blank line between these.

brenniebrennie

This doesn't appear to be used.

brenniebrennie

Fields should use hyphens instead of underscores.

brenniebrennie

Archive instead of watch?

brenniebrennie

Needs a docstring.

brenniebrennie

This should be (ReviewRequest.DoesNotExist, User.DoesNotExist). ObjectDoesNotExist is much too generic here. The same goes for other places you use ObjectDoesNotExist.

brenniebrennie

We should check if the user has view permissions for the review request, otherwise we leak information that a review …

brenniebrennie

Needs a docstring.

brenniebrennie

Needs an added_in = '2.5.x' field.

brenniebrennie

I don't believe this is used.

brenniebrennie

Needs a docstring.

brenniebrennie

Needs a docstring.

brenniebrennie

'ObjectDoesNotExist' imported but unused

reviewbotreviewbot

Is there any model that these WebAPI resources should be based on? Also, you can add model_parent_key to point to …

VT VTL-Developer

undefined name 'ReviewRequest'

reviewbotreviewbot

undefined name 'User'

reviewbotreviewbot

undefined name 'ReviewRequest'

reviewbotreviewbot

undefined name 'User'

reviewbotreviewbot

Col: 23 W292 no newline at end of file

reviewbotreviewbot

Col: 67 W292 no newline at end of file

reviewbotreviewbot

Col: 61 W292 no newline at end of file

reviewbotreviewbot

Should be in alphabetical order.

chipx86chipx86

'archived_list_mimetype' imported but unused

reviewbotreviewbot

'get_archived_review_request_item_url' imported but unused

reviewbotreviewbot

local variable 'archived_rsp' is assigned to but never used

reviewbotreviewbot

Col: 53 E128 continuation line under-indented for visual indent

reviewbotreviewbot

Col: 49 E128 continuation line under-indented for visual indent

reviewbotreviewbot

Col: 80 E501 line too long (80 > 79 characters)

reviewbotreviewbot

Col: 80 E501 line too long (81 > 79 characters)

reviewbotreviewbot

Col: 67 W292 no newline at end of file

reviewbotreviewbot

Col: 61 W292 no newline at end of file

reviewbotreviewbot

'archived_list_mimetype' imported but unused

reviewbotreviewbot

Col: 53 E128 continuation line under-indented for visual indent

reviewbotreviewbot

Col: 49 E128 continuation line under-indented for visual indent

reviewbotreviewbot

Col: 67 W292 no newline at end of file

reviewbotreviewbot

Col: 61 W292 no newline at end of file

reviewbotreviewbot

'archived_list_mimetype' imported but unused

reviewbotreviewbot

Col: 53 E128 continuation line under-indented for visual indent

reviewbotreviewbot

Col: 49 E128 continuation line under-indented for visual indent

reviewbotreviewbot

local variable 'visit' is assigned to but never used

reviewbotreviewbot

Creates -> Create

JY jyuen

This should probably check if user is an instance of basestring instead, to better communicate intentions.

chipx86chipx86

This can be one statement.

chipx86chipx86

List -> Lists manipulates -> manipulate

JY jyuen

Marks -> Mark

JY jyuen

Deletes -> Delete

JY jyuen

You can save a query in the common case by doing: visit, is_new = ReviewRequestVisit.objects.get_or_create( user=user, review_request=review_request, defaults={ 'visibility': self.visibility, …

chipx86chipx86

Same note as above.

chipx86chipx86

Lists -> List manipulates -> manipulate

JY jyuen

That's not really true, though. This is returning review requests.

chipx86chipx86

Marks -> Mark

JY jyuen

Deletes -> Delete

JY jyuen

Col: 9 E303 too many blank lines (2)

reviewbotreviewbot
CR
CR
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/webapi/resources/muted_review_request.py
        reviewboard/accounts/evolutions/__init__.py
        reviewboard/accounts/evolutions/reviewrequestvisit_visibility.py
        reviewboard/datagrids/grids.py
        reviewboard/accounts/models.py
        reviewboard/webapi/resources/user.py
        reviewboard/webapi/resources/archived_review_request.py
    
    Ignored Files:
        reviewboard/templates/base.html
        reviewboard/templates/reviews/review_header.html
        reviewboard/templates/datagrids/hideable_review_request_listview.html
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/templates/reviews/reviewable_page_data.js
        reviewboard/static/rb/css/common.less
        reviewboard/templates/reviews/review_request_actions_secondary.html
        reviewboard/static/rb/js/models/userSessionModel.js
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/webapi/resources/muted_review_request.py
        reviewboard/accounts/evolutions/__init__.py
        reviewboard/accounts/evolutions/reviewrequestvisit_visibility.py
        reviewboard/datagrids/grids.py
        reviewboard/accounts/models.py
        reviewboard/webapi/resources/user.py
        reviewboard/webapi/resources/archived_review_request.py
    
    Ignored Files:
        reviewboard/templates/base.html
        reviewboard/templates/reviews/review_header.html
        reviewboard/templates/datagrids/hideable_review_request_listview.html
        reviewboard/static/rb/js/views/reviewRequestEditorView.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/templates/reviews/reviewable_page_data.js
        reviewboard/static/rb/css/common.less
        reviewboard/templates/reviews/review_request_actions_secondary.html
        reviewboard/static/rb/js/models/userSessionModel.js
    
    
  2. reviewboard/datagrids/grids.py (Diff revision 1)
     
     
    Show all issues
    Col: 80
     E501 line too long (82 > 79 characters)
    
  3. 
      
CR
CR
CR
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/resources/user.py
        reviewboard/webapi/resources/muted_review_request.py
        reviewboard/webapi/resources/archived_review_request.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/resources/user.py
        reviewboard/webapi/resources/muted_review_request.py
        reviewboard/webapi/resources/archived_review_request.py
    
    
  2. 
      
brennie
  1. The MutedReviewRequestResource and ArchivedReviewRequestResource are very similar and they should maybe be refactored to inherit from a base class that does most of the machinery. Then the subclasses can implement the methods via the @augment_method_from decorator.

  2. Show all issues

    Needs an added_in field. This should be 2.5.x (until we know when it is going to land).

    1. Actually, this should be 3.0 because we are currently doing the feature freeze for 2.5.x.

  3. Show all issues

    Docstring needs more information.

    Blank line between these.

  4. Show all issues

    This doesn't appear to be used.

  5. Show all issues

    Fields should use hyphens instead of underscores.

    1. Is reviewboard gradually switching over to hyphens as well? Just that I see most other fields use underscores in other resources

    2. Oops! Sorry, you can ignore this one. Fields are supposed to use underscores.

  6. Show all issues

    Archive instead of watch?

  7. Show all issues

    Needs a docstring.

  8. reviewboard/webapi/resources/archived_review_request.py (Diff revision 2)
     
     
     
     
     
     

    This is becuase you can't put a keyword argument after *args right?

  9. Show all issues

    This should be (ReviewRequest.DoesNotExist, User.DoesNotExist). ObjectDoesNotExist is much too generic here.

    The same goes for other places you use ObjectDoesNotExist.

  10. Show all issues

    We should check if the user has view permissions for the review request, otherwise we leak information that a review request that they do not have permissions to view exists.

    Same elsewhere.

    1. does @webapi_check_local_site does this?

    2. I'm not sure. You should ask someone who knows more about local sites than I.

  11. Show all issues

    Needs a docstring.

  12. Show all issues

    Needs an added_in = '2.5.x' field.

    1. Actually, this should be 3.0 because we are currently doing the feature freeze for 2.5.x.

  13. Show all issues

    I don't believe this is used.

  14. Show all issues

    Needs a docstring.

  15. Show all issues

    Needs a docstring.

  16. 
      
CR
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/resources/muted_review_request.py
        reviewboard/webapi/resources/base_hidden_object.py
        reviewboard/webapi/resources/user.py
        reviewboard/webapi/resources/archived_review_request.py
    
    Ignored Files:
        reviewboard/templates/base.html
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/resources/muted_review_request.py
        reviewboard/webapi/resources/base_hidden_object.py
        reviewboard/webapi/resources/user.py
        reviewboard/webapi/resources/archived_review_request.py
    
    Ignored Files:
        reviewboard/templates/base.html
    
    
  2. Show all issues
     'ObjectDoesNotExist' imported but unused
    
  3. Show all issues
     undefined name 'ReviewRequest'
    
  4. Show all issues
     undefined name 'User'
    
  5. Show all issues
     undefined name 'ReviewRequest'
    
  6. Show all issues
     undefined name 'User'
    
  7. Show all issues
    Col: 23
     W292 no newline at end of file
    
  8. 
      
VT
  1. 
      
  2. reviewboard/webapi/resources/base_hidden_object.py (Diff revision 3)
     
     
     
     
     
    Show all issues

    Is there any model that these WebAPI resources should be based on?

    Also, you can add model_parent_key to point to the parent.

    You can check out https://github.com/djblets/djblets/blob/master/djblets/webapi/resources.py#L383 for a list of fields that may also be relevant.

  3. 
      
CR
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/tests/test_archived_review_request.py
        reviewboard/webapi/resources/muted_review_request.py
        reviewboard/webapi/resources/base_archived_object.py
        reviewboard/webapi/resources/user.py
        reviewboard/webapi/resources/archived_review_request.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/tests/test_archived_review_request.py
        reviewboard/webapi/resources/muted_review_request.py
        reviewboard/webapi/resources/base_archived_object.py
        reviewboard/webapi/resources/user.py
        reviewboard/webapi/resources/archived_review_request.py
    
    
  2. Show all issues
    Col: 67
     W292 no newline at end of file
    
  3. Show all issues
    Col: 61
     W292 no newline at end of file
    
  4. Show all issues
     'archived_list_mimetype' imported but unused
    
  5. Show all issues
     'get_archived_review_request_item_url' imported but unused
    
  6. Show all issues
     local variable 'archived_rsp' is assigned to but never used
    
  7. Show all issues
    Col: 53
     E128 continuation line under-indented for visual indent
    
  8. Show all issues
    Col: 49
     E128 continuation line under-indented for visual indent
    
  9. Show all issues
    Col: 80
     E501 line too long (80 > 79 characters)
    
  10. Show all issues
    Col: 80
     E501 line too long (81 > 79 characters)
    
  11. 
      
CR
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/testing/testcase.py
        reviewboard/webapi/tests/test_archived_review_request.py
        reviewboard/webapi/resources/muted_review_request.py
        reviewboard/webapi/tests/urls.py
        reviewboard/webapi/tests/mimetypes.py
        reviewboard/webapi/resources/user.py
        reviewboard/webapi/resources/archived_review_request.py
        reviewboard/webapi/resources/base_archived_object.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/testing/testcase.py
        reviewboard/webapi/tests/test_archived_review_request.py
        reviewboard/webapi/resources/muted_review_request.py
        reviewboard/webapi/tests/urls.py
        reviewboard/webapi/tests/mimetypes.py
        reviewboard/webapi/resources/user.py
        reviewboard/webapi/resources/archived_review_request.py
        reviewboard/webapi/resources/base_archived_object.py
    
    
  2. Show all issues
    Col: 67
     W292 no newline at end of file
    
  3. Show all issues
    Col: 61
     W292 no newline at end of file
    
  4. Show all issues
     'archived_list_mimetype' imported but unused
    
  5. Show all issues
    Col: 53
     E128 continuation line under-indented for visual indent
    
  6. Show all issues
    Col: 49
     E128 continuation line under-indented for visual indent
    
  7. 
      
CR
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/testing/testcase.py
        reviewboard/webapi/tests/test_archived_review_request.py
        reviewboard/webapi/resources/muted_review_request.py
        reviewboard/webapi/tests/urls.py
        reviewboard/webapi/tests/mimetypes.py
        reviewboard/webapi/resources/user.py
        reviewboard/webapi/resources/archived_review_request.py
        reviewboard/webapi/resources/base_archived_object.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/testing/testcase.py
        reviewboard/webapi/tests/test_archived_review_request.py
        reviewboard/webapi/resources/muted_review_request.py
        reviewboard/webapi/tests/urls.py
        reviewboard/webapi/tests/mimetypes.py
        reviewboard/webapi/resources/user.py
        reviewboard/webapi/resources/archived_review_request.py
        reviewboard/webapi/resources/base_archived_object.py
    
    
  2. Show all issues
    Col: 67
     W292 no newline at end of file
    
  3. Show all issues
    Col: 61
     W292 no newline at end of file
    
  4. Show all issues
     'archived_list_mimetype' imported but unused
    
  5. Show all issues
    Col: 53
     E128 continuation line under-indented for visual indent
    
  6. Show all issues
    Col: 49
     E128 continuation line under-indented for visual indent
    
  7. 
      
CR
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/testing/testcase.py
        reviewboard/webapi/tests/test_archived_review_request.py
        reviewboard/webapi/resources/muted_review_request.py
        reviewboard/webapi/tests/urls.py
        reviewboard/webapi/tests/mimetypes.py
        reviewboard/webapi/resources/user.py
        reviewboard/webapi/resources/archived_review_request.py
        reviewboard/webapi/resources/base_archived_object.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/testing/testcase.py
        reviewboard/webapi/tests/test_archived_review_request.py
        reviewboard/webapi/resources/muted_review_request.py
        reviewboard/webapi/tests/urls.py
        reviewboard/webapi/tests/mimetypes.py
        reviewboard/webapi/resources/user.py
        reviewboard/webapi/resources/archived_review_request.py
        reviewboard/webapi/resources/base_archived_object.py
    
    
  2. 
      
CR
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/testing/testcase.py
        reviewboard/webapi/tests/test_archived_review_request.py
        reviewboard/webapi/resources/muted_review_request.py
        reviewboard/webapi/tests/urls.py
        reviewboard/webapi/tests/mimetypes.py
        reviewboard/webapi/resources/user.py
        reviewboard/webapi/resources/archived_review_request.py
        reviewboard/webapi/resources/base_archived_object.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/testing/testcase.py
        reviewboard/webapi/tests/test_archived_review_request.py
        reviewboard/webapi/resources/muted_review_request.py
        reviewboard/webapi/tests/urls.py
        reviewboard/webapi/tests/mimetypes.py
        reviewboard/webapi/resources/user.py
        reviewboard/webapi/resources/archived_review_request.py
        reviewboard/webapi/resources/base_archived_object.py
    
    
  2. Show all issues
     local variable 'visit' is assigned to but never used
    
  3. 
      
CR
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/testing/testcase.py
        reviewboard/webapi/tests/test_archived_review_request.py
        reviewboard/webapi/resources/muted_review_request.py
        reviewboard/webapi/tests/urls.py
        reviewboard/webapi/tests/mimetypes.py
        reviewboard/webapi/resources/user.py
        reviewboard/webapi/resources/archived_review_request.py
        reviewboard/webapi/resources/base_archived_object.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/testing/testcase.py
        reviewboard/webapi/tests/test_archived_review_request.py
        reviewboard/webapi/resources/muted_review_request.py
        reviewboard/webapi/tests/urls.py
        reviewboard/webapi/tests/mimetypes.py
        reviewboard/webapi/resources/user.py
        reviewboard/webapi/resources/archived_review_request.py
        reviewboard/webapi/resources/base_archived_object.py
    
    
  2. 
      
JY
  1. Some nitpicky stuff for PEP-257 compliancy

  2. reviewboard/testing/testcase.py (Diff revision 9)
     
     
    Show all issues

    Creates -> Create

  3. Show all issues

    List -> Lists
    manipulates -> manipulate

  4. Show all issues

    Marks -> Mark

  5. Show all issues

    Deletes -> Delete

  6. Show all issues

    Lists -> List
    manipulates -> manipulate

  7. Show all issues

    Marks -> Mark

  8. Show all issues

    Deletes -> Delete

  9. 
      
chipx86
  1. 
      
  2. reviewboard/webapi/resources/user.py (Diff revision 4)
     
     
     
     
     
    Show all issues

    Should be in alphabetical order.

  3. reviewboard/testing/testcase.py (Diff revision 9)
     
     
     
    Show all issues

    This should probably check if user is an instance of basestring instead, to better communicate intentions.

  4. reviewboard/testing/testcase.py (Diff revision 9)
     
     
     
     
     
     
     
    Show all issues

    This can be one statement.

  5. reviewboard/webapi/resources/base_archived_object.py (Diff revision 9)
     
     
     
     
     
    Show all issues

    You can save a query in the common case by doing:

    visit, is_new = ReviewRequestVisit.objects.get_or_create(
        user=user,
        review_request=review_request,
        defaults={
            'visibility': self.visibility,
        })
    
    if not is_new and visit.visibility != self.visibility:
        visit.visibility = self.visibility
        visit.save(update_fields=['visibility'])
    
  6. reviewboard/webapi/resources/base_archived_object.py (Diff revision 9)
     
     
     
     
     
    Show all issues

    Same note as above.

  7. reviewboard/webapi/resources/muted_review_request.py (Diff revision 9)
     
     
     
     
     
    Show all issues

    That's not really true, though. This is returning review requests.

  8. 
      
CR
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/testing/testcase.py
        reviewboard/webapi/tests/test_archived_review_request.py
        reviewboard/webapi/resources/muted_review_request.py
        reviewboard/webapi/tests/urls.py
        reviewboard/webapi/tests/mimetypes.py
        reviewboard/webapi/resources/user.py
        reviewboard/webapi/resources/archived_review_request.py
        reviewboard/webapi/resources/base_archived_object.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/testing/testcase.py
        reviewboard/webapi/tests/test_archived_review_request.py
        reviewboard/webapi/resources/muted_review_request.py
        reviewboard/webapi/tests/urls.py
        reviewboard/webapi/tests/mimetypes.py
        reviewboard/webapi/resources/user.py
        reviewboard/webapi/resources/archived_review_request.py
        reviewboard/webapi/resources/base_archived_object.py
    
    
  2. Show all issues
    Col: 9
     E303 too many blank lines (2)
    
  3. 
      
CR
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/testing/testcase.py
        reviewboard/webapi/tests/test_archived_review_request.py
        reviewboard/webapi/resources/muted_review_request.py
        reviewboard/webapi/tests/urls.py
        reviewboard/webapi/tests/mimetypes.py
        reviewboard/webapi/resources/user.py
        reviewboard/webapi/resources/archived_review_request.py
        reviewboard/webapi/resources/base_archived_object.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/testing/testcase.py
        reviewboard/webapi/tests/test_archived_review_request.py
        reviewboard/webapi/resources/muted_review_request.py
        reviewboard/webapi/tests/urls.py
        reviewboard/webapi/tests/mimetypes.py
        reviewboard/webapi/resources/user.py
        reviewboard/webapi/resources/archived_review_request.py
        reviewboard/webapi/resources/base_archived_object.py
    
    
  2. 
      
CR
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to ucosp/cristocrat/archive-and-mute (08266e0)
Loading...