Merge pull request #654 from ubc/allow-comparison-of-instructor-answers

Allow comparison of instructor answers

alembic/versions/2561c39ac4d9_allow_instructor_answers_to_be_included_.py

@@ -0,0 +1,53 @@
+"""Allow instructor answers to be included in comparisons
+
+Revision ID: 2561c39ac4d9
+Revises: f6145781f130
+Create Date: 2017-12-06 21:07:23.219244
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '2561c39ac4d9'
+down_revision = 'f6145781f130'
+
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.sql import text
+
+from compair.models import convention
+
+def upgrade():
+    # add a new "comparable" column. default as true
+    with op.batch_alter_table('answer', naming_convention=convention) as batch_op:
+        batch_op.add_column(sa.Column('comparable', sa.Integer(), nullable=False, default='1', server_default='1'))
+
+    # Patch existing answers from instructors and TAs as non-comparable.
+    # Note that existing answers from sys admin are considered comparable (i.e. no need to patch).
+    # sqlite doesn't support in-clause with multiple columns...
+    # update = text(
+    #     "UPDATE answer SET comparable = 0 "
+    #     "WHERE (assignment_id, user_id) IN ( "
+    #     "  SELECT a.id, uc.user_id "
+    #     "  FROM user_course uc "
+    #     "  JOIN assignment a "
+    #     "    ON a.course_id = uc.course_id "
+    #     "  WHERE uc.course_role IN ('Instructor', 'Teaching Assistant'))"
+    # )
+    # ... use a potentially slower query
+    update = text(
+        "UPDATE answer SET comparable = 0 "
+        "WHERE EXISTS ( "
+        "  SELECT 1 "
+        "  FROM user_course "
+        "  JOIN assignment "
+        "    ON assignment.course_id = user_course.course_id "
+        "  WHERE "
+        "    assignment.id = answer.assignment_id "
+        "    AND user_course.user_id = answer.user_id "
+        "    AND user_course.course_role IN ('Instructor', 'Teaching Assistant'))"
+    )
+    op.get_bind().execute(update)
+
+def downgrade():
+    with op.batch_alter_table('answer', naming_convention=convention) as batch_op:
+        batch_op.drop_column('comparable')

compair/api/answer.py

@@ -21,13 +21,15 @@
 
 new_answer_parser = RequestParser()
 new_answer_parser.add_argument('user_id', default=None)
+new_answer_parser.add_argument('comparable', type=bool, default=True)
 new_answer_parser.add_argument('content', default=None)
 new_answer_parser.add_argument('file_id', default=None)
 new_answer_parser.add_argument('draft', type=bool, default=False)
 
 existing_answer_parser = RequestParser()
 existing_answer_parser.add_argument('id', required=True, help="Answer id is required.")
 existing_answer_parser.add_argument('user_id', default=None)
+existing_answer_parser.add_argument('comparable', type=bool, default=True)
 existing_answer_parser.add_argument('content', default=None)
 existing_answer_parser.add_argument('file_id', default=None)
 existing_answer_parser.add_argument('draft', type=bool, default=False)
@@ -72,7 +74,7 @@ def get(self, course_uuid, assignment_uuid):
         """
         Return a list of answers for a assignment based on search criteria. The
         list of the answers are paginated. If there is any answers from instructor
-        or TA, their answers will be on top of the list.
+        or TA, their answers will be on top of the list (unless they are comparable).
 
         :param course_uuid: course uuid
         :param assignment_uuid: assignment uuid
@@ -103,8 +105,14 @@ def get(self, course_uuid, assignment_uuid):
                 UserCourse.course_id == course.id
             )) \
             .add_columns(
-                UserCourse.course_role.__eq__(CourseRole.instructor).label("instructor_role"),
-                UserCourse.course_role.__eq__(CourseRole.teaching_assistant).label("ta_role")
+                and_(
+                    UserCourse.course_role.__eq__(CourseRole.instructor),
+                    not Answer.comparable
+                ).label("instructor_role"),
+                and_(
+                    UserCourse.course_role.__eq__(CourseRole.teaching_assistant),
+                    not Answer.comparable
+                ).label("ta_role")
             ) \
             .filter(and_(
                 Answer.assignment_id == assignment.id,
@@ -194,12 +202,20 @@ def post(self, course_uuid, assignment_uuid):
         if user_uuid and not allow(MANAGE, Answer(course_id=course.id)):
             abort(400, title="Answer Not Submitted", message="Only instructors and teaching assistants can submit an answer on behalf of another.")
 
+        explicitSubmitAsStudent = False
         if user_uuid:
             user = User.get_by_uuid_or_404(user_uuid)
             answer.user_id = user.id
+            explicitSubmitAsStudent = user.get_course_role(course.id) == CourseRole.student
         else:
             answer.user_id = current_user.id
 
+        # instructor / TA can mark the answer as non-comparable, unless the answer is for a student
+        if allow(MANAGE, Answer(course_id=course.id)) and not explicitSubmitAsStudent:
+            answer.comparable = params.get("comparable")
+        else:
+            answer.comparable = True
+
         user_course = UserCourse.query \
             .filter_by(
                 course_id=course.id,
@@ -302,6 +318,15 @@ def post(self, course_uuid, assignment_uuid, answer_uuid):
         answer.content = params.get("content")
 
         user_uuid = params.get("user_id")
+        explicitSubmitAsStudent = False
+        if user_uuid:
+            user = User.get_by_uuid_or_404(user_uuid)
+            explicitSubmitAsStudent = user.get_course_role(course.id) == CourseRole.student
+
+        # instructor / TA can mark the answer as non-comparable if it is not a student answer
+        if allow(MANAGE, Answer(course_id=course.id)) and not explicitSubmitAsStudent:
+            answer.comparable = params.get("comparable")
+
         # we allow instructor and TA to submit multiple answers for other users in the class
         if user_uuid and user_uuid != answer.user_uuid:
             if not allow(MANAGE, answer) or not answer.draft:

compair/api/assignment.py

@@ -484,8 +484,8 @@ def get(self, course_uuid, assignment_uuid):
 
         comparison_count = assignment.completed_comparison_count_for_user(current_user.id)
         comparison_draft_count = assignment.draft_comparison_count_for_user(current_user.id)
-        other_student_answers = assignment.student_answer_count - answer_count
-        comparison_available = comparison_count < other_student_answers * (other_student_answers - 1) / 2
+        other_comparable_answers = assignment.comparable_answer_count - answer_count
+        comparison_available = comparison_count < other_comparable_answers * (other_comparable_answers - 1) / 2
 
         status = {
             'answers': {
@@ -659,8 +659,8 @@ def get(self, course_uuid):
             assignment_drafts = [draft for draft in drafts if draft.assignment_id == assignment.id]
             comparison_count = assignment.completed_comparison_count_for_user(current_user.id)
             comparison_draft_count = assignment.draft_comparison_count_for_user(current_user.id)
-            other_student_answers = assignment.student_answer_count - answer_count
-            comparison_available = comparison_count < other_student_answers * (other_student_answers - 1) / 2
+            other_comparable_answers = assignment.comparable_answer_count - answer_count
+            comparison_available = comparison_count < other_comparable_answers * (other_comparable_answers - 1) / 2
 
             statuses[assignment.uuid] = {
                 'answers': {

compair/api/classlist.py

@@ -9,7 +9,7 @@
 from flask_login import login_required, current_user
 from flask_restful import Resource, marshal
 from six import BytesIO
-from sqlalchemy import and_
+from sqlalchemy import and_, or_
 from sqlalchemy.orm import joinedload
 from werkzeug.utils import secure_filename
 from flask_restful.reqparse import RequestParser
@@ -67,6 +67,7 @@
 on_classlist_instructor_label = event.signal('CLASSLIST_INSTRUCTOR_LABEL_GET')
 on_classlist_instructor = event.signal('CLASSLIST_INSTRUCTOR_GET')
 on_classlist_student = event.signal('CLASSLIST_STUDENT_GET')
+on_classlist_instructional = event.signal('CLASSLIST_INSTRUCTIONAL_GET')
 on_classlist_update_users_course_roles = event.signal('CLASSLIST_UPDATE_USERS_COURSE_ROLES')
 
 
@@ -619,6 +620,59 @@ def get(self, course_uuid):
 
 api.add_resource(StudentsAPI, '/students')
 
+# /instructionals - return list of Instructors and TAs of the course
+class InstructionalsAPI(Resource):
+    @login_required
+    def get(self, course_uuid):
+        course = Course.get_active_by_uuid_or_404(course_uuid)
+        require(READ, course,
+            title="Instructional users Unavailable",
+            message="Instructional users can only be seen here by those enrolled in the course. Please double-check your enrollment in this course.")
+        restrict_user = not allow(MANAGE, course)
+
+        instructionals = User.query \
+            .with_entities(User, UserCourse.group_name, UserCourse.course_role) \
+            .join(UserCourse, UserCourse.user_id == User.id) \
+            .filter(
+                UserCourse.course_id == course.id,
+                or_(
+                    UserCourse.course_role == CourseRole.instructor,
+                    UserCourse.course_role == CourseRole.teaching_assistant
+                )
+            ) \
+            .order_by(User.lastname, User.firstname) \
+            .all()
+
+        users = []
+        user_course = UserCourse(course_id=course.id)
+        for u in instructionals:
+            if allow(READ, user_course):
+                users.append({
+                    'id': u.User.uuid,
+                    'name': u.User.fullname_sortable,
+                    'group_name': u.group_name,
+                    'role': u.course_role.value
+                })
+            else:
+                name = u.User.displayname
+                users.append({
+                    'id': u.User.uuid,
+                    'name': name,
+                    'group_name': u.group_name,
+                    'role': u.course_role.value
+                })
+
+        on_classlist_instructional.send(
+            self,
+            event_name=on_classlist_instructional.name,
+            user=current_user,
+            course_id=course.id
+        )
+
+        return { 'objects': users }
+
+
+api.add_resource(InstructionalsAPI, '/instructionals')
 
 # /roles - set course role for multi users at once
 class UserCourseRoleAPI(Resource):

compair/api/comparison.py

@@ -104,7 +104,7 @@ def get(self, course_uuid, assignment_uuid):
                 abort(500, title="Comparisons Unavailable", message="Generating scored pairs failed, this really shouldn't happen.")
 
         return {
-            'comparison': marshal(comparison, dataformat.get_comparison(restrict_user)),
+            'comparison': marshal(comparison, dataformat.get_comparison(restrict_user, include_answer_user=False, include_score=False)),
             'new_pair': new_pair,
             'current': comparison_count+1
         }
@@ -248,6 +248,6 @@ def post(self, course_uuid, assignment_uuid):
             is_comparison_example=is_comparison_example,
             data=marshal(comparison, dataformat.get_comparison(restrict_user)))
 
-        return {'comparison': marshal(comparison, dataformat.get_comparison(restrict_user))}
+        return {'comparison': marshal(comparison, dataformat.get_comparison(restrict_user, include_answer_user=False, include_score=False))}
 
 api.add_resource(CompareRootAPI, '')

compair/api/dataformat/__init__.py

@@ -178,29 +178,35 @@ def get_assignment(restrict_user=True):
         'created': fields.DateTime(dt_format='iso8601', attribute=lambda x: replace_tzinfo(x.created))
     }
 
-def get_answer(restrict_user=True):
-    return {
+def get_answer(restrict_user=True, include_answer_user=True, include_score=True):
+    ret = {
         'id': fields.String(attribute="uuid"),
         'course_id': fields.String(attribute="course_uuid"),
         'assignment_id': fields.String(attribute="assignment_uuid"),
-        'user_id': fields.String(attribute="user_uuid"),
 
         'content': fields.String,
         'file': fields.Nested(get_file(), allow_null=True),
         'flagged': fields.Boolean,
         'draft': fields.Boolean,
         'top_answer': fields.Boolean,
 
-        'score': fields.Nested(get_score(restrict_user), allow_null=True),
-
         'comment_count': fields.Integer,
         'private_comment_count': fields.Integer,
         'public_comment_count': fields.Integer,
 
-        'user': get_partial_user(restrict_user),
-        'created': fields.DateTime(dt_format='iso8601', attribute=lambda x: replace_tzinfo(x.created))
+        'created': fields.DateTime(dt_format='iso8601', attribute=lambda x: replace_tzinfo(x.created)),
+        'comparable': fields.Boolean
     }
 
+    if include_score:
+        ret['score'] = fields.Nested(get_score(restrict_user), allow_null=True)
+
+    if include_answer_user:
+        ret['user_id'] = fields.String(attribute="user_uuid")
+        ret['user'] = get_partial_user(restrict_user)
+
+    return ret
+
 
 def get_assignment_comment(restrict_user=True):
     return {
@@ -253,7 +259,7 @@ def get_kaltura_media():
     }
 
 
-def get_comparison(restrict_user=True, with_answers=True, with_feedback=False):
+def get_comparison(restrict_user=True, with_answers=True, with_feedback=False, include_answer_user=True, include_score=True):
     ret = {
         'id': fields.String(attribute="uuid"),
         'course_id': fields.String(attribute="course_uuid"),
@@ -269,8 +275,8 @@ def get_comparison(restrict_user=True, with_answers=True, with_feedback=False):
     }
 
     if with_answers:
-        ret['answer1'] = fields.Nested(get_answer(restrict_user))
-        ret['answer2'] = fields.Nested(get_answer(restrict_user))
+        ret['answer1'] = fields.Nested(get_answer(restrict_user, include_answer_user, include_score))
+        ret['answer2'] = fields.Nested(get_answer(restrict_user, include_answer_user, include_score))
 
     if with_feedback:
         ret['answer1_feedback'] = fields.List(fields.Nested(get_answer_comment(restrict_user)))