• 
      

    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:
    Completed
    Change Summary:
    Pushed to ucosp/cristocrat/archive-and-mute (08266e0)