Status Updates part 6: Add resource for status updates.

Review Request #8407 — Created Sept. 18, 2016 and submitted

Information

Review Board
release-3.0.x
2d8b8cc...

Reviewers

This change adds an API endpoint for creating and manipulating status updates.
This is intended to be used by consumers who want to create status updates from
a purely third-party source without writing an extension.

Ran unit tests.

Description From Last Updated

We should have unit tests for POSTing/PUTing with: 1) Bad change IDs 2) Bad review IDs 3) Bad statuses Also, …

chipx86chipx86

We should have a docstring here. (I know the others don't, but that's something we need to do at some …

chipx86chipx86

Since this is public docs, this should be greatly fleshed out to cover more about what the resource is about. …

chipx86chipx86

Can we add some filtering options for timestamps, change description IDs, states, and service IDs, and document those? (This would …

chipx86chipx86

Here and in other HTTP handler methods, we should use "Returns." While "Return" is appropriate for codebase docs, API docs …

chipx86chipx86

Missing Args and Returns.

chipx86chipx86

This should be fleshed out to go into more details about usage. For instance, indicating when a change/review ID would …

chipx86chipx86

For consistency with the other resources, can we make this: return 201, { self.item_result_key: status_update, }

chipx86chipx86

This needs a docstring for the public API docs.

chipx86chipx86

Same comment as above.

chipx86chipx86

Missing docstring.

chipx86chipx86

This can use the pattern other resources use: for field_name in ('service_id', 'summary', 'description', ...): if field_name in kwargs: setattr(status_update, …

chipx86chipx86

This needs to check and return a suitable error.

chipx86chipx86

Needs docs.

chipx86chipx86

User or unicode

brenniebrennie

In part 5, you use done-success and done-failure in the StatusUpdate model.

brenniebrennie

Missing Returns

chipx86chipx86
chipx86
  1. 
      
  2. We should have unit tests for POSTing/PUTing with:

    1) Bad change IDs
    2) Bad review IDs
    3) Bad statuses

    Also, can you run ./contrib/internal/webapi-lint.py and make sure this doesn't introduce new warnings?

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

    We should have a docstring here. (I know the others don't, but that's something we need to do at some point too.)

  4. Since this is public docs, this should be greatly fleshed out to cover more about what the resource is about. Purpose of status updates, how you'd use the resource, etc. would be good.

  5. reviewboard/webapi/resources/status_update.py (Diff revision 1)
     
     
     
     
     
     

    Can we add some filtering options for timestamps, change description IDs, states, and service IDs, and document those? (This would go in get_queryset, but should be documented here.)

    1. I'd like to add that in a later change.

  6. Here and in other HTTP handler methods, we should use "Returns." While "Return" is appropriate for codebase docs, API docs aren't using it.

  7. Missing Args and Returns.

  8. This should be fleshed out to go into more details about usage. For instance, indicating when a change/review ID would be provided, ordering of operations.

    1. I'm not sure what you mean by "ordering of operations". Can you clarify?

  9. For consistency with the other resources, can we make this:

    return 201, {
        self.item_result_key: status_update,
    }
    
  10. This needs a docstring for the public API docs.

  11. Same comment as above.

  12. Missing docstring.

  13. reviewboard/webapi/resources/status_update.py (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    This can use the pattern other resources use:

    for field_name in ('service_id', 'summary', 'description', ...):
        if field_name in kwargs:
            setattr(status_update, field_name, kwargs[field_name])
    
  14. reviewboard/webapi/resources/status_update.py (Diff revision 1)
     
     
     
     
     
     
     
     
     

    This needs to check and return a suitable error.

  15. 
      
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/testing/testcase.py
        reviewboard/webapi/resources/__init__.py
        reviewboard/webapi/tests/test_review.py
        reviewboard/webapi/resources/review_request.py
        reviewboard/webapi/tests/test_status_update.py
        reviewboard/webapi/tests/urls.py
        reviewboard/webapi/tests/mimetypes.py
        reviewboard/webapi/resources/status_update.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/testing/testcase.py
        reviewboard/webapi/resources/__init__.py
        reviewboard/webapi/tests/test_review.py
        reviewboard/webapi/resources/review_request.py
        reviewboard/webapi/tests/test_status_update.py
        reviewboard/webapi/tests/urls.py
        reviewboard/webapi/tests/mimetypes.py
        reviewboard/webapi/resources/status_update.py
    
    
  2. 
      
david
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/testing/testcase.py
        reviewboard/webapi/resources/__init__.py
        reviewboard/webapi/tests/test_review.py
        reviewboard/webapi/resources/review_request.py
        reviewboard/webapi/tests/test_status_update.py
        reviewboard/webapi/tests/urls.py
        reviewboard/webapi/tests/mimetypes.py
        reviewboard/webapi/resources/status_update.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/testing/testcase.py
        reviewboard/webapi/resources/__init__.py
        reviewboard/webapi/tests/test_review.py
        reviewboard/webapi/resources/review_request.py
        reviewboard/webapi/tests/test_status_update.py
        reviewboard/webapi/tests/urls.py
        reviewboard/webapi/tests/mimetypes.py
        reviewboard/webapi/resources/status_update.py
    
    
  2. 
      
david
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/testing/testcase.py
        reviewboard/webapi/resources/__init__.py
        reviewboard/webapi/tests/test_review.py
        reviewboard/webapi/resources/review_request.py
        reviewboard/webapi/tests/test_status_update.py
        reviewboard/webapi/tests/urls.py
        reviewboard/webapi/tests/mimetypes.py
        reviewboard/webapi/resources/status_update.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/testing/testcase.py
        reviewboard/webapi/resources/__init__.py
        reviewboard/webapi/tests/test_review.py
        reviewboard/webapi/resources/review_request.py
        reviewboard/webapi/tests/test_status_update.py
        reviewboard/webapi/tests/urls.py
        reviewboard/webapi/tests/mimetypes.py
        reviewboard/webapi/resources/status_update.py
    
    
  2. 
      
brennie
  1. 
      
  2. reviewboard/testing/testcase.py (Diff revision 3)
     
     

    User or unicode

  3. In part 5, you use done-success and done-failure in the StatusUpdate model.

    1. Looks like you started reviewing this just before I posted an updated diff with this fixed.

  4. 
      
david
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/testing/testcase.py
        reviewboard/webapi/resources/__init__.py
        reviewboard/webapi/tests/test_review.py
        reviewboard/webapi/resources/review_request.py
        reviewboard/webapi/tests/test_status_update.py
        reviewboard/webapi/tests/urls.py
        reviewboard/webapi/tests/mimetypes.py
        reviewboard/webapi/resources/status_update.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/testing/testcase.py
        reviewboard/webapi/resources/__init__.py
        reviewboard/webapi/tests/test_review.py
        reviewboard/webapi/resources/review_request.py
        reviewboard/webapi/tests/test_status_update.py
        reviewboard/webapi/tests/urls.py
        reviewboard/webapi/tests/mimetypes.py
        reviewboard/webapi/resources/status_update.py
    
    
  2. 
      
chipx86
  1. 
      
  2. reviewboard/testing/testcase.py (Diff revision 4)
     
     
     
     
     
     
     
     
     

    Missing Returns

  3. 
      
david
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/testing/testcase.py
        reviewboard/webapi/resources/__init__.py
        reviewboard/webapi/tests/test_review.py
        reviewboard/webapi/resources/review_request.py
        reviewboard/webapi/tests/test_status_update.py
        reviewboard/webapi/tests/urls.py
        reviewboard/webapi/tests/mimetypes.py
        reviewboard/webapi/resources/status_update.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/testing/testcase.py
        reviewboard/webapi/resources/__init__.py
        reviewboard/webapi/tests/test_review.py
        reviewboard/webapi/resources/review_request.py
        reviewboard/webapi/tests/test_status_update.py
        reviewboard/webapi/tests/urls.py
        reviewboard/webapi/tests/mimetypes.py
        reviewboard/webapi/resources/status_update.py
    
    
  2. 
      
chipx86
  1. Ship It!
  2. 
      
david
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-3.0.x (a3177aa)
Loading...