Allow extra fields returned from comments service

Previously, an error was raised if the comments service returned data
including an unexpected field, which unnecessarily complicated the
release path for new features, since the list of allowed fields would
need to be modified before cs_comments_service could be modified, and
only then could edx-platform take advantage of the new CS feature. We
still log a warning if an unexpected field is returned, so we will
still be able to tell if the CS returns a corrupt response.

JIRA: FOR-180

lms/djangoapps/django_comment_client/base/views.py

@@ -88,14 +88,15 @@ def create_thread(request, course_id, commentable_id):
     if 'body' not in post or not post['body'].strip():
         return JsonError(_("Body can't be empty"))
 
-    thread = cc.Thread(**extract(post, ['body', 'title']))
-    thread.update_attributes(**{
-        'anonymous': anonymous,
-        'anonymous_to_peers': anonymous_to_peers,
-        'commentable_id': commentable_id,
-        'course_id': course_id,
-        'user_id': request.user.id,
-    })
+    thread = cc.Thread(
+        anonymous=anonymous,
+        anonymous_to_peers=anonymous_to_peers,
+        commentable_id=commentable_id,
+        course_id=course_id,
+        user_id=request.user.id,
+        body=post["body"],
+        title=post["title"]
+    )
 
     user = cc.User.from_django_user(request.user)
 
@@ -118,7 +119,7 @@ def create_thread(request, course_id, commentable_id):
             group_id = user_group_id
 
         if group_id:
-            thread.update_attributes(group_id=group_id)
+            thread.group_id = group_id
 
     thread.save()
 
@@ -149,7 +150,8 @@ def update_thread(request, course_id, thread_id):
     if 'body' not in request.POST or not request.POST['body'].strip():
         return JsonError(_("Body can't be empty"))
     thread = cc.Thread.find(thread_id)
-    thread.update_attributes(**extract(request.POST, ['body', 'title']))
+    thread.body = request.POST["body"]
+    thread.title = request.POST["title"]
     thread.save()
     if request.is_ajax():
         return ajax_content_response(request, course_id, thread.to_dict())
@@ -166,7 +168,6 @@ def _create_comment(request, course_id, thread_id=None, parent_id=None):
 
     if 'body' not in post or not post['body'].strip():
         return JsonError(_("Body can't be empty"))
-    comment = cc.Comment(**extract(post, ['body']))
 
     course = get_course_with_access(request.user, course_id, 'load')
     if course.allow_anonymous:
@@ -179,14 +180,15 @@ def _create_comment(request, course_id, thread_id=None, parent_id=None):
     else:
         anonymous_to_peers = False
 
-    comment.update_attributes(**{
-        'anonymous': anonymous,
-        'anonymous_to_peers': anonymous_to_peers,
-        'user_id': request.user.id,
-        'course_id': course_id,
-        'thread_id': thread_id,
-        'parent_id': parent_id,
-    })
+    comment = cc.Comment(
+        anonymous=anonymous,
+        anonymous_to_peers=anonymous_to_peers,
+        user_id=request.user.id,
+        course_id=course_id,
+        thread_id=thread_id,
+        parent_id=parent_id,
+        body=post["body"]
+    )
     comment.save()
     if post.get('auto_subscribe', 'false').lower() == 'true':
         user = cc.User.from_django_user(request.user)
@@ -235,7 +237,7 @@ def update_comment(request, course_id, comment_id):
     comment = cc.Comment.find(comment_id)
     if 'body' not in request.POST or not request.POST['body'].strip():
         return JsonError(_("Body can't be empty"))
-    comment.update_attributes(**extract(request.POST, ['body']))
+    comment.body = request.POST["body"]
     comment.save()
     if request.is_ajax():
         return ajax_content_response(request, course_id, comment.to_dict())

lms/lib/comment_client/comment.py

@@ -52,14 +52,14 @@ class Comment(models.Model):
         else:
             raise CommentClientRequestError("Can only flag/unflag threads or comments")
         params = {'user_id': user.id}
-        request = perform_request(
+        response = perform_request(
             'put',
             url,
             params,
             metric_tags=self._metric_tags,
             metric_action='comment.abuse.flagged'
         )
-        voteable.update_attributes(request)
+        voteable._update_from_response(response)
 
     def unFlagAbuse(self, user, voteable, removeAll):
         if voteable.type == 'thread':
@@ -73,14 +73,14 @@ class Comment(models.Model):
         if removeAll:
             params['all'] = True
 
-        request = perform_request(
+        response = perform_request(
             'put',
             url,
             params,
             metric_tags=self._metric_tags,
             metric_action='comment.abuse.unflagged'
         )
-        voteable.update_attributes(request)
+        voteable._update_from_response(response)
 
 
 def _url_for_thread_comments(thread_id):

lms/lib/comment_client/models.py

+import logging
+
 from .utils import extract, perform_request, CommentClientRequestError
 
 
+log = logging.getLogger(__name__)
+
+
 class Model(object):
 
     accessible_fields = ['id']
@@ -70,7 +75,7 @@ class Model(object):
             metric_tags=self._metric_tags,
             metric_action='model.retrieve'
         )
-        self.update_attributes(**response)
+        self._update_from_response(response)
 
     @property
     def _metric_tags(self):
@@ -93,12 +98,17 @@ class Model(object):
     def find(cls, id):
         return cls(id=id)
 
-    def update_attributes(self, *args, **kwargs):
-        for k, v in kwargs.items():
+    def _update_from_response(self, response_data):
+        for k, v in response_data.items():
             if k in self.accessible_fields:
                 self.__setattr__(k, v)
             else:
-                raise AttributeError("Field {0} does not exist".format(k))
+                log.warning(
+                    "Unexpected field {field_name} in model {model_name}".format(
+                        field_name=k,
+                        model_name=self.__class__.__name__
+                    )
+                )
 
     def updatable_attributes(self):
         return extract(self.attributes, self.updatable_fields)
@@ -135,14 +145,14 @@ class Model(object):
                 metric_action='model.insert'
             )
         self.retrieved = True
-        self.update_attributes(**response)
+        self._update_from_response(response)
         self.after_save(self)
 
     def delete(self):
         url = self.url(action='delete', params=self.attributes)
         response = perform_request('delete', url, metric_tags=self._metric_tags, metric_action='model.delete')
         self.retrieved = True
-        self.update_attributes(**response)
+        self._update_from_response(response)
 
     @classmethod
     def url_with_id(cls, params={}):

lms/lib/comment_client/thread.py

@@ -98,7 +98,7 @@ class Thread(models.Model):
             metric_action='model.retrieve',
             metric_tags=self._metric_tags
         )
-        self.update_attributes(**response)
+        self._update_from_response(response)
 
     def flagAbuse(self, user, voteable):
         if voteable.type == 'thread':
@@ -108,14 +108,14 @@ class Thread(models.Model):
         else:
             raise CommentClientRequestError("Can only flag/unflag threads or comments")
         params = {'user_id': user.id}
-        request = perform_request(
+        response = perform_request(
             'put',
             url,
             params,
             metric_action='thread.abuse.flagged',
             metric_tags=self._metric_tags
         )
-        voteable.update_attributes(request)
+        voteable._update_from_response(response)
 
     def unFlagAbuse(self, user, voteable, removeAll):
         if voteable.type == 'thread':
@@ -129,38 +129,38 @@ class Thread(models.Model):
         if removeAll:
             params['all'] = True
 
-        request = perform_request(
+        response = perform_request(
             'put',
             url,
             params,
             metric_tags=self._metric_tags,
             metric_action='thread.abuse.unflagged'
         )
-        voteable.update_attributes(request)
+        voteable._update_from_response(response)
 
     def pin(self, user, thread_id):
         url = _url_for_pin_thread(thread_id)
         params = {'user_id': user.id}
-        request = perform_request(
+        response = perform_request(
             'put',
             url,
             params,
             metric_tags=self._metric_tags,
             metric_action='thread.pin'
         )
-        self.update_attributes(request)
+        self._update_from_response(response)
 
     def un_pin(self, user, thread_id):
         url = _url_for_un_pin_thread(thread_id)
         params = {'user_id': user.id}
-        request = perform_request(
+        response = perform_request(
             'put',
             url,
             params,
             metric_tags=self._metric_tags,
             metric_action='thread.unpin'
         )
-        self.update_attributes(request)
+        self._update_from_response(response)
 
 
 def _url_for_flag_abuse_thread(thread_id):

lms/lib/comment_client/user.py

@@ -56,14 +56,14 @@ class User(models.Model):
         else:
             raise CommentClientRequestError("Can only vote / unvote for threads or comments")
         params = {'user_id': self.id, 'value': value}
-        request = perform_request(
+        response = perform_request(
             'put',
             url,
             params,
             metric_action='user.vote',
             metric_tags=self._metric_tags + ['target.type:{}'.format(voteable.type)],
         )
-        voteable.update_attributes(request)
+        voteable._update_from_response(response)
 
     def unvote(self, voteable):
         if voteable.type == 'thread':
@@ -73,14 +73,14 @@ class User(models.Model):
         else:
             raise CommentClientRequestError("Can only vote / unvote for threads or comments")
         params = {'user_id': self.id}
-        request = perform_request(
+        response = perform_request(
             'delete',
             url,
             params,
             metric_action='user.unvote',
             metric_tags=self._metric_tags + ['target.type:{}'.format(voteable.type)],
         )
-        voteable.update_attributes(request)
+        voteable._update_from_response(response)
 
     def active_threads(self, query_params={}):
         if not self.course_id:
@@ -141,7 +141,7 @@ class User(models.Model):
                 )
             else:
                 raise
-        self.update_attributes(**response)
+        self._update_from_response(response)
 
 
 def _url_for_vote_comment(comment_id):