Add webAPI resources for archived and muted review requests

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

cristocrat
Review Board
master
7109
7086
e23d735...
reviewboard, students

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)
     
     
    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. 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. Docstring needs more information.

    Blank line between these.

  4. This doesn't appear to be used.

  5. 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. Archive instead of watch?

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

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

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

    The same goes for other places you use ObjectDoesNotExist.

  9. 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.

  10. 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.

  11. I don't believe this is used.

  12. Needs a docstring.

  13. Needs a docstring.

  14. 
      
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.  'ObjectDoesNotExist' imported but unused
    
  3.  undefined name 'ReviewRequest'
    
  4.  undefined name 'User'
    
  5.  undefined name 'ReviewRequest'
    
  6.  undefined name 'User'
    
  7. Col: 23
     W292 no newline at end of file
    
  8. 
      
VT
  1. 
      
  2. reviewboard/webapi/resources/base_hidden_object.py (Diff revision 3)
     
     
     
     
     

    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. Col: 67
     W292 no newline at end of file
    
  3. Col: 61
     W292 no newline at end of file
    
  4.  'archived_list_mimetype' imported but unused
    
  5.  'get_archived_review_request_item_url' imported but unused
    
  6.  local variable 'archived_rsp' is assigned to but never used
    
  7. Col: 53
     E128 continuation line under-indented for visual indent
    
  8. Col: 49
     E128 continuation line under-indented for visual indent
    
  9. Col: 80
     E501 line too long (80 > 79 characters)
    
  10. 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. Col: 67
     W292 no newline at end of file
    
  3. Col: 61
     W292 no newline at end of file
    
  4.  'archived_list_mimetype' imported but unused
    
  5. Col: 53
     E128 continuation line under-indented for visual indent
    
  6. 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. Col: 67
     W292 no newline at end of file
    
  3. Col: 61
     W292 no newline at end of file
    
  4.  'archived_list_mimetype' imported but unused
    
  5. Col: 53
     E128 continuation line under-indented for visual indent
    
  6. 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.  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)
     
     

    Creates -> Create

  3. List -> Lists
    manipulates -> manipulate

  4. Lists -> List
    manipulates -> manipulate

  5. Deletes -> Delete

  6. 
      
chipx86
  1. 
      
  2. reviewboard/webapi/resources/user.py (Diff revision 4)
     
     
     
     
     

    Should be in alphabetical order.

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

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

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

    This can be one statement.

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

    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)
     
     
     
     
     

    Same note as above.

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

    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. 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...