[checklist] Refactoring of checklist

Refactoring includes

- Use of `request.user` instead of passing `user_id` in every HTTP request
- Updating of the view only after successful API response
- Renamed status of each item from `finished` to `checked`
- Use of `ChecklistItem` non-database resource API instead of using HTTP
  request parameters
- Moving of front-end API logic from view to model
- CSS hyphens everywhere instead of underscores
- Improving Python code (renaming variables, removing redundant code)
- Start checklist item IDs from 0 instead of 1.

Testing done:
Basic CRUD functionality remains. Ensured that the views will not update unless
there is a successful response from the server.

Reviewed at https://reviews.reviewboard.org/r/7176/

checklist/checklist/extension.py

@@ -1,3 +1,5 @@
+from __future__ import unicode_literals
+
 from djblets.webapi.resources import (register_resource_for_model,
                                       unregister_resource_for_model)
 from reviewboard.accounts.forms.pages import AccountPageForm
@@ -7,7 +9,8 @@
                                           TemplateHook)
 from reviewboard.urls import reviewable_url_names
 
-from checklist.resources import checklist_resource, checklist_template_resource
+from checklist.resources import (checklist_item_resource, checklist_resource,
+                                 checklist_template_resource)
 from checklist.models import ChecklistTemplate, ReviewChecklist
 
 
@@ -57,7 +60,8 @@ class Checklist(Extension):
         },
     }
 
-    resources = [checklist_resource, checklist_template_resource]
+    resources = [checklist_resource, checklist_item_resource,
+                 checklist_template_resource]
 
     def __init__(self, *args, **kwargs):
         super(Checklist, self).__init__(*args, **kwargs)

checklist/checklist/models.py

@@ -1,59 +1,80 @@
+from __future__ import unicode_literals
+
 from django.contrib.auth.models import User
 from django.db import models
+from django.utils import six
 from djblets.db.fields import JSONField
 from reviewboard.reviews.models import ReviewRequest
 
 
 class ReviewChecklist(models.Model):
-    """A checklist is a list of items to keep track of during a review. """
+    """A checklist is a list of items to keep track of during a review.
+
+    Items are stored in JSON format in the following manner:
+
+    ::
+
+      checklist_items: {
+          id: {
+              'id': six.text_type,
+              'checked': bool,
+              'description': six.text_type
+          },
+          ...
+      }
+    """
 
     # The user making the review.
     user = models.ForeignKey(User)
 
     # The review request the user is reviewing.
     review_request = models.ForeignKey(ReviewRequest)
 
-    # Keeps track of the number of items *added* in the
-    # checklist, not the current number of items in it. This
-    # allows us to give unique IDs to new items.
-    items_counter = models.IntegerField(default=0)
+    # Used to provide unique ids for each checklist item.
+    next_item_id = models.IntegerField(default=0)
 
-    # A JSON of all the items in the checklist. Key = ID.
-    # Value = { id, finished status, description }
+    # A JSON blob of all the items in the checklist.
     checklist_items = JSONField()
 
     def add_item(self, item_description):
-        self.items_counter += 1
-        self.checklist_items[self.items_counter] = {
-            'id': self.items_counter,
-            'finished': False,
+        """Add and return the new checklist item."""
+        item = {
+            'id': self.next_item_id,
+            'checked': False,
             'description': item_description
         }
+        self.checklist_items[self.next_item_id] = item
+        self.next_item_id += 1
+
         self.save()
+        return item
 
-    def edit_item_desc(self, itemID, item_description):
-        if str(itemID) in self.checklist_items:
-            itemDict = self.checklist_items.get(str(itemID))
-            itemDict['description'] = item_description
-            self.save()
+    def edit_item(self, item_id, item_description, checked):
+        """Modify and return the checklist item specified."""
+        if six.text_type(item_id) in self.checklist_items:
+            item = self.checklist_items.get(six.text_type(item_id))
+
+            if item_description is not None:
+                item['description'] = item_description
+
+            if checked is not None:
+                item['checked'] = checked
 
-    def toggle_item_status(self, itemID):
-        if str(itemID) in self.checklist_items:
-            itemDict = self.checklist_items.get(str(itemID))
-            itemDict['finished'] = not itemDict['finished']
             self.save()
+            return item
 
-    def delete_item(self, itemID):
-        if str(itemID) in self.checklist_items:
-            self.checklist_items.pop(str(itemID))
+    def delete_item(self, item_id):
+        """Delete the checklist item."""
+        if six.text_type(item_id) in self.checklist_items:
+            self.checklist_items.pop(six.text_type(item_id))
             self.save()
 
 
 class ChecklistTemplate(models.Model):
-    """A template for checklist items.
+    """A checklist template defines a collection of checklist items.
 
-    Templates can be imported into checklist instances to quickly populate them
-    with common items.
+    Each template can be imported into a checklist. Items are stored in JSON
+    format as a single array of checklist items.
     """
 
     owner = models.ForeignKey(User)

checklist/checklist/resources/__init__.py

@@ -1,5 +1,9 @@
+from __future__ import unicode_literals
+
+from checklist_item import checklist_item_resource
 from checklist_resource import checklist_resource
 from checklist_template import checklist_template_resource
 
 
-__all__ = ['checklist_resource', 'checklist_template_resource']
+__all__ = ['checklist_resource', 'checklist_item_resource',
+           'checklist_template_resource']

checklist/checklist/resources/checklist_item.py

@@ -0,0 +1,132 @@
+from __future__ import unicode_literals
+
+from django.utils import six
+from djblets.webapi.decorators import (webapi_login_required,
+                                       webapi_response_errors,
+                                       webapi_request_fields)
+from djblets.webapi.errors import DOES_NOT_EXIST
+from reviewboard.webapi.base import WebAPIResource
+from reviewboard.webapi.decorators import webapi_check_local_site
+
+from checklist.models import ReviewChecklist as Checklist
+
+
+class ChecklistItemResource(WebAPIResource):
+    """Provide information for individual checklist items of a checklist."""
+
+    name = 'checklist_item'
+    uri_object_key = 'checklist_item_id'
+    allowed_methods = ('GET', 'POST', 'PUT', 'DELETE')
+
+    fields = {
+        'id': {
+            'type': six.text_type,
+            'description': 'The id of the checklist item.',
+        },
+        'description': {
+            'type': six.text_type,
+            'description': 'The description of the checklist item.',
+        },
+        'checked': {
+            'type': bool,
+            'description': 'Whether the checklist item is finished.',
+        },
+    }
+
+    def get_parent_object(self, pk):
+        """Return the parent checklist object."""
+        return self._parent_resource.model.objects.get(pk=pk)
+
+    @webapi_login_required
+    @webapi_response_errors(DOES_NOT_EXIST)
+    @webapi_check_local_site
+    def get(self, request, api_format, checklist_id, checklist_item_id, *args,
+            **kwargs):
+        """Return the individual checklist item."""
+        try:
+            checklist = self.get_parent_object(checklist_id)
+        except Checklist.ObjectDoesNotExist:
+            return DOES_NOT_EXIST
+
+        if six.text_type(checklist_item_id) not in checklist.checklist_items:
+            return DOES_NOT_EXIST
+
+        item = checklist.checklist_items.get(six.text_type(checklist_item_id))
+        return 200, {self.item_result_key: item}
+
+    @webapi_login_required
+    @webapi_response_errors(DOES_NOT_EXIST)
+    @webapi_check_local_site
+    def get_list(self, request, api_format, checklist_id, *args, **kwargs):
+        """Return a list of checklist items in the checklist specified."""
+        try:
+            checklist = self.get_parent_object(checklist_id)
+        except Checklist.ObjectDoesNotExist:
+            return DOES_NOT_EXIST
+
+        return 200, {self.item_result_key: checklist.checklist_items}
+
+    @webapi_request_fields(
+        required={
+            'description': {
+                'type': six.text_type,
+                'description': 'The description of the checklist item.',
+            },
+        }
+    )
+    @webapi_login_required
+    @webapi_response_errors(DOES_NOT_EXIST)
+    @webapi_check_local_site
+    def create(self, request, api_format, checklist_id, description=None,
+               *args, **kwargs):
+        """Add a new checklist item to the checklist."""
+        try:
+            checklist = self.get_parent_object(checklist_id)
+        except Checklist.ObjectDoesNotExist:
+            return DOES_NOT_EXIST
+
+        item = checklist.add_item(description)
+        return 201, {self.item_result_key: item}
+
+    @webapi_request_fields(
+        optional={
+            'description': {
+                'type': six.text_type,
+                'description': 'The description of the checklist item.',
+            },
+            'checked': {
+                'type': bool,
+                'description': 'Whether the checklist item is completed.',
+            },
+        }
+    )
+    @webapi_login_required
+    @webapi_response_errors(DOES_NOT_EXIST)
+    @webapi_check_local_site
+    def update(self, request, api_format, checklist_id, checklist_item_id,
+               description=None, checked=None, *args, **kwargs):
+        """Update a checklist item, whether edited or toggled."""
+        try:
+            checklist = self.get_parent_object(checklist_id)
+        except Checklist.ObjectDoesNotExist:
+            return DOES_NOT_EXIST
+
+        item = checklist.edit_item(checklist_item_id, description, checked)
+        return 200, {self.item_result_key: item}
+
+    @webapi_login_required
+    @webapi_response_errors(DOES_NOT_EXIST)
+    @webapi_check_local_site
+    def delete(self, request, api_format, checklist_id, checklist_item_id,
+               *args, **kwargs):
+        """Delete a checklist item in the checklist."""
+        try:
+            checklist = self.get_parent_object(checklist_id)
+        except Checklist.ObjectDoesNotExist:
+            return DOES_NOT_EXIST
+
+        checklist.delete_item(checklist_item_id)
+        return 204, {}
+
+
+checklist_item_resource = ChecklistItemResource()