Create new web api for Checklist templates

Review Request #6106 — Created July 15, 2014 and discarded

Information

rb-extension-pack

Reviewers

Declerations, and override access methods.

Added create method to post a new template list. It attaches the user to the new template list.

Added update method that :
- creates a new items when id is not present
- updates items when id, and description is present
- delete item when only id is present


 
Description From Last Updated

Col: 46 E127 continuation line over-indented for visual indent

reviewbotreviewbot

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

reviewbotreviewbot

Col: 54 W292 no newline at end of file

reviewbotreviewbot

Col: 46 E127 continuation line over-indented for visual indent

reviewbotreviewbot

Col: 17 E127 continuation line over-indented for visual indent

reviewbotreviewbot

undefined name 'ObjectDoesNotExist'

reviewbotreviewbot

undefined name 'DOES_NOT_EXIST'

reviewbotreviewbot

'PERMISSION_DENIED' imported but unused

reviewbotreviewbot

'NOT_LOGGED_IN' imported but unused

reviewbotreviewbot

You only really need braces around the from _ import _ if it spans multiple lines.

MA matthewcmaclean

Reviewboard uses trailing commas on lists and dictionaries.

MA matthewcmaclean

Reviewboard uses trailing commas on lists and dictionaries. You should add them to all your other dictionaries as well.

MA matthewcmaclean

This shouldn't be necessary. You can add a request argument (immediately after self), and then use request.user to find the …

daviddavid

PUT is used to update an object. I don't think you should support adding or deletion in update. In the …

MA matthewcmaclean

'User' imported but unused

reviewbotreviewbot

Can you change this to name = 'checklist_template'?

anselinaanselina

Very minor nitpick - there is no need for extra newlines before elif/else clauses (at least that's how it's done …

VO volodymyr

Add one more blank line before this. (There should be two blank lines between top-level blocks.)

anselinaanselina
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        checklist/checklist/ChecklistTemplateResource.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        checklist/checklist/ChecklistTemplateResource.py
    
    
  2. Col: 46
     E127 continuation line over-indented for visual indent
    
  3. Col: 80
     E501 line too long (80 > 79 characters)
    
  4. Col: 54
     W292 no newline at end of file
    
  5. 
      
SA
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        checklist/checklist/ChecklistTemplateResource.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        checklist/checklist/ChecklistTemplateResource.py
    
    
  2. Col: 46
     E127 continuation line over-indented for visual indent
    
  3. Col: 17
     E127 continuation line over-indented for visual indent
    
  4.  undefined name 'ObjectDoesNotExist'
    
  5.  undefined name 'DOES_NOT_EXIST'
    
  6. 
      
SA
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        checklist/checklist/ChecklistTemplateResource.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        checklist/checklist/ChecklistTemplateResource.py
    
    
  2.  'PERMISSION_DENIED' imported but unused
    
  3.  'NOT_LOGGED_IN' imported but unused
    
  4. 
      
SA
SA
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        checklist/checklist/ChecklistTemplateResource.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        checklist/checklist/ChecklistTemplateResource.py
    
    
  2. 
      
MA
  1. 
      
  2. You only really need braces around the from _ import _ if it spans multiple lines.

  3. Reviewboard uses trailing commas on lists and dictionaries.

  4. Reviewboard uses trailing commas on lists and dictionaries. You should add them to all your other dictionaries as well.

  5. PUT is used to update an object. I don't think you should support adding or deletion in update. In the situation that the object is invalid if it has no description so it should be deleted, you might want to make the field required instead of optional.

    1. Here the update is used to manipulate the object's properties including a list. Adding or deleting from the list is still considered an update because the parent object exists, and its properies has changed. DELETE is used to delete the whole object including the list.

  6. 
      
david
  1. 
      
  2. checklist/checklist/ChecklistTemplateResource.py (Diff revision 4)
     
     
     
     
     

    This shouldn't be necessary. You can add a request argument (immediately after self), and then use request.user to find the authenticated user.

    That also prevents people from impersonating others when creating checklist templates.

  3. 
      
SA
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        checklist/checklist/ChecklistTemplateResource.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        checklist/checklist/ChecklistTemplateResource.py
    
    
  2.  'User' imported but unused
    
  3. 
      
VO
  1. 
      
  2. checklist/checklist/ChecklistTemplateResource.py (Diff revision 5)
     
     
     
     
     
     
     

    Very minor nitpick - there is no need for extra newlines before elif/else clauses (at least that's how it's done in 99% of the codebase).

  3. 
      
anselina
  1. 
      
  2. Can you change this to name = 'checklist_template'?

  3. Add one more blank line before this. (There should be two blank lines between top-level blocks.)

  4. 
      
SA
SA
Review request changed

Status: Discarded

Loading...