From 582036d19269229da87df3b4d61ba301e8c284cf Mon Sep 17 00:00:00 2001 From: Andrew Gardener Date: Wed, 25 Jan 2017 14:41:20 -0800 Subject: [PATCH] Add peer feedback report Report shows student feedback given to peers. - Grouping is Assignment then user who gave the feedback - When filtering by groups, will show feedback given by students in the group - Some HTML entities will be unescaped and all HTML tags will be removed from feedback cotnent. Completes: #499 --- compair/api/report.py | 97 +++++++- .../static/modules/report/report-module.js | 3 +- compair/tests/api/test_report.py | 219 +++++++++++++++++- data/fixtures/test_data.py | 55 ++++- 4 files changed, 361 insertions(+), 13 deletions(-) diff --git a/compair/api/report.py b/compair/api/report.py index 9033c62e0..109525491 100644 --- a/compair/api/report.py +++ b/compair/api/report.py @@ -1,6 +1,7 @@ import csv import os import time +import re from bouncer.constants import MANAGE from flask import Blueprint, current_app, abort @@ -8,11 +9,12 @@ from flask_restful import Resource, reqparse -from sqlalchemy import func +from sqlalchemy import func, and_, or_ +from sqlalchemy.orm import joinedload from compair.authorization import require from compair.core import event -from compair.models import CourseRole, Assignment, UserCourse, Course, Answer, \ +from compair.models import User, CourseRole, Assignment, UserCourse, Course, Answer, \ AnswerComment, AssignmentCriterion, Comparison, AnswerCommentType from .util import new_restful_api @@ -108,6 +110,21 @@ def post(self, course_uuid): title_row2.append("Self Evaluation Submitted") titles = [title_row1, title_row2] + elif report_type == "peer_feedback": + titles1 = [ + "", + "Sender", "", "", + "Receiver", "", "", + "", "" + ] + titles2 = [ + "Assignment", + "Last Name", "First Name", "Student No", + "Last Name", "First Name", "Student No", + "Type", "Feedback" + ] + data = peer_feedback_report(course, assignments, group_name) + titles = [titles1, titles2] else: return {'error': 'The requested report type cannot be found'}, 400 @@ -329,3 +346,79 @@ def participation_report(course, assignments, group_name): report.append(temp) return report + +def peer_feedback_report(course, assignments, group_name): + report = [] + + senders = User.query \ + .join("user_courses") \ + .filter(and_( + UserCourse.course_id == course.id, + UserCourse.course_role == CourseRole.student + )) \ + .order_by(User.lastname, User.firstname, User.id) + if group_name: + senders = senders.filter(UserCourse.group_name == group_name) + senders = senders.all() + sender_user_ids = [u.id for u in senders] + + assignment_ids = [assignment.id for assignment in assignments] + + answer_comments = AnswerComment.query \ + .join(Answer, AnswerComment.answer_id == Answer.id) \ + .join(User, User.id == Answer.user_id) \ + .with_entities( + Answer.user_id.label("receiver_user_id"), + AnswerComment.user_id.label("sender_user_id"), + Answer.assignment_id.label("assignment_id"), + AnswerComment.comment_type, + AnswerComment.content, + User.firstname.label("receiver_firstname"), + User.lastname.label("receiver_lastname"), + User.student_number.label("receiver_student_number"), + ) \ + .filter(Answer.assignment_id.in_(assignment_ids)) \ + .filter(AnswerComment.user_id.in_(sender_user_ids)) \ + .filter(AnswerComment.comment_type != AnswerCommentType.self_evaluation) \ + .filter(Answer.draft == False) \ + .filter(Answer.practice == False) \ + .filter(AnswerComment.draft == False) \ + .order_by(AnswerComment.created) \ + .all() + + for assignment in assignments: + for user in senders: + user_sent_feedback = [ac for ac in answer_comments \ + if ac.sender_user_id == user.id and ac.assignment_id == assignment.id] + + if len(user_sent_feedback) > 0: + for feedback in user_sent_feedback: + temp = [ + assignment.name, + user.lastname, user.firstname, user.student_number, + feedback.receiver_lastname, feedback.receiver_firstname, feedback.receiver_student_number, + feedback.comment_type.value, strip_html(feedback.content) + ] + report.append(temp) + + else: + # enter blank row + temp = [ + assignment.name, + user.lastname, user.firstname, user.student_number, + "---", "---", "---", + "", "" + ] + report.append(temp) + + return report + +def strip_html(text): + text = re.sub('<[^>]+>', '', text) + text = text.replace(' ', ' ') + text = text.replace('&', '&') + text = text.replace('<', '<') + text = text.replace('>', '>') + text = text.replace('"', '"') + text = text.replace(''', '\'') + return text \ No newline at end of file diff --git a/compair/static/modules/report/report-module.js b/compair/static/modules/report/report-module.js index 9c0a657a9..7c6e31ba8 100644 --- a/compair/static/modules/report/report-module.js +++ b/compair/static/modules/report/report-module.js @@ -44,7 +44,8 @@ module.controller( var allGroups = {'name': 'All Groups', 'value': 'all'}; $scope.types = [ {'id': 'participation', 'name': 'Participation Report (Regular)'}, - {'id': 'participation_stat', 'name': 'Participation Report (Research)'} + {'id': 'participation_stat', 'name': 'Participation Report (Research)'}, + {'id': 'peer_feedback', 'name': 'Peer Feedback Report'} ]; UserResource.getTeachingUserCourses().$promise.then( diff --git a/compair/tests/api/test_report.py b/compair/tests/api/test_report.py index 1c635aa22..1184aa6a0 100644 --- a/compair/tests/api/test_report.py +++ b/compair/tests/api/test_report.py @@ -2,6 +2,7 @@ import json import io import os +import re from data.fixtures.test_data import TestFixture from compair.tests.test_compair import ComPAIRAPITestCase @@ -13,7 +14,7 @@ class ReportAPITest(ComPAIRAPITestCase): def setUp(self): super(ReportAPITest, self).setUp() self.fixtures = TestFixture().add_course(num_students=30, num_assignments=2, num_additional_criteria=1, num_groups=2, num_answers=25, - with_draft_student=True) + with_draft_student=True, with_comments=True) self.url = "/api/courses/" + self.fixtures.course.uuid + "/report" self.files_to_cleanup = [] @@ -28,8 +29,6 @@ def tearDown(self): except Exception as e: print(e) - - def test_generate_report(self): # test login required rv = self.client.post(self.url) @@ -326,7 +325,163 @@ def test_generate_report(self): next_row = next(reader) user_stats = self._check_participation_stat_report_user_row(self.fixtures.assignments[0], student, next_row, overall_stats) + # peer_feedback with valid instructor + with self.login(self.fixtures.instructor.username): + input = { + 'group_name': None, + 'type': "peer_feedback", + 'assignment': None + } + + # test authorized user entire course + rv = self.client.post(self.url, data=json.dumps(input), content_type='application/json') + self.assert200(rv) + self.assertIsNotNone(rv.json['file']) + file_name = rv.json['file'].split("/")[-1] + self.files_to_cleanup.append(file_name) + + tmp_name = os.path.join(current_app.config['REPORT_FOLDER'], file_name) + with open(tmp_name, 'rt') as csvfile: + reader = csv.reader(csvfile, delimiter=',') + + heading1 = next(reader) + heading2 = next(reader) + self._check_peer_feedback_report_heading_rows(heading1, heading2) + + sorted_students = sorted(self.fixtures.students, + key=lambda student: (student.lastname, student.firstname, student.id) + ) + + for assignment in self.fixtures.assignments: + for student in sorted_students: + self._check_peer_feedback_report_user_rows(assignment, student, reader) + + # test authorized user one assignment + single_assignment_input = input.copy() + single_assignment_input['assignment'] = self.fixtures.assignments[0].uuid + rv = self.client.post(self.url, data=json.dumps(single_assignment_input), content_type='application/json') + self.assert200(rv) + self.assertIsNotNone(rv.json['file']) + file_name = rv.json['file'].split("/")[-1] + self.files_to_cleanup.append(file_name) + + tmp_name = os.path.join(current_app.config['REPORT_FOLDER'], file_name) + with open(tmp_name, 'rt') as csvfile: + reader = csv.reader(csvfile, delimiter=',') + + heading1 = next(reader) + heading2 = next(reader) + self._check_peer_feedback_report_heading_rows(heading1, heading2) + + sorted_students = sorted(self.fixtures.students, + key=lambda student: (student.lastname, student.firstname, student.id) + ) + + for student in sorted_students: + self._check_peer_feedback_report_user_rows(self.fixtures.assignments[0], student, reader) + + # test authorized user entire course with group_name filter + group_name_input = input.copy() + group_name_input['group_name'] = self.fixtures.groups[0] + rv = self.client.post(self.url, data=json.dumps(group_name_input), content_type='application/json') + self.assert200(rv) + self.assertIsNotNone(rv.json['file']) + file_name = rv.json['file'].split("/")[-1] + self.files_to_cleanup.append(file_name) + + tmp_name = os.path.join(current_app.config['REPORT_FOLDER'], file_name) + with open(tmp_name, 'rt') as csvfile: + reader = csv.reader(csvfile, delimiter=',') + + heading1 = next(reader) + heading2 = next(reader) + self._check_peer_feedback_report_heading_rows(heading1, heading2) + sorted_students = sorted(self.fixtures.students, + key=lambda student: (student.lastname, student.firstname, student.id) + ) + + for assignment in self.fixtures.assignments: + for student in sorted_students: + if student.user_courses[0].group_name != self.fixtures.groups[0]: + continue + self._check_peer_feedback_report_user_rows(assignment, student, reader) + + # test authorized user one assignment + group_name_input = input.copy() + group_name_input['group_name'] = self.fixtures.groups[0] + group_name_input['assignment'] = self.fixtures.assignments[0].uuid + rv = self.client.post(self.url, data=json.dumps(group_name_input), content_type='application/json') + self.assert200(rv) + self.assertIsNotNone(rv.json['file']) + file_name = rv.json['file'].split("/")[-1] + self.files_to_cleanup.append(file_name) + + tmp_name = os.path.join(current_app.config['REPORT_FOLDER'], file_name) + with open(tmp_name, 'rt') as csvfile: + reader = csv.reader(csvfile, delimiter=',') + + heading1 = next(reader) + heading2 = next(reader) + self._check_peer_feedback_report_heading_rows(heading1, heading2) + + sorted_students = sorted(self.fixtures.students, + key=lambda student: (student.lastname, student.firstname, student.id) + ) + + for student in sorted_students: + if student.user_courses[0].group_name != self.fixtures.groups[0]: + continue + self._check_peer_feedback_report_user_rows(self.fixtures.assignments[0], student, reader) + + # test authorized user one assignment (content's html parsed) + original_content = {} + for answer_comment in self.fixtures.answer_comments: + if answer_comment.user.user_courses[0].group_name != self.fixtures.groups[0] or \ + answer_comment.assignment_id != self.fixtures.assignments[0].id: + continue + original_content[answer_comment.id] = answer_comment.content + answer_comment.content = '

'"><& <\/p>'+answer_comment.content + db.session.commit() + + group_name_input = input.copy() + group_name_input['group_name'] = self.fixtures.groups[0] + group_name_input['assignment'] = self.fixtures.assignments[0].uuid + rv = self.client.post(self.url, data=json.dumps(group_name_input), content_type='application/json') + self.assert200(rv) + self.assertIsNotNone(rv.json['file']) + file_name = rv.json['file'].split("/")[-1] + self.files_to_cleanup.append(file_name) + + tmp_name = os.path.join(current_app.config['REPORT_FOLDER'], file_name) + with open(tmp_name, 'rt') as csvfile: + reader = csv.reader(csvfile, delimiter=',') + + heading1 = next(reader) + heading2 = next(reader) + self._check_peer_feedback_report_heading_rows(heading1, heading2) + + sorted_students = sorted(self.fixtures.students, + key=lambda student: (student.lastname, student.firstname, student.id) + ) + + for student in sorted_students: + if student.user_courses[0].group_name != self.fixtures.groups[0]: + continue + + answer_comments = sorted( + [ac for ac in self.fixtures.answer_comments if ac.user_id == student.id and + ac.assignment_id == self.fixtures.assignments[0].id], + key=lambda ac: (ac.created) + ) + + if len(answer_comments) > 0: + for answer_comment in answer_comments: + row = next(reader) + self.assertEqual(row[8], '\'"><& '+original_content[answer_comment.id]) + else: + # skip user with no comments + row = next(reader) def _check_participation_stat_report_heading_rows(self, heading): expected_heading = ['Assignment', 'User UUID', 'Last Name', 'First Name', @@ -488,3 +643,61 @@ def _check_participation_report_user_row(self, assignments, student, row): self.assertEqual(row[index], str(evaluations_submitted)) index += 1 + + def _check_peer_feedback_report_heading_rows(self, heading1, heading2): + expected_heading1 = [ + "", + "Sender", "", "", + "Receiver", "", "", + "", "" + ] + self.assertEqual(expected_heading1, heading1) + expected_heading2 = [ + "Assignment", + "Last Name", "First Name", "Student No", + "Last Name", "First Name", "Student No", + "Type", "Feedback" + ] + self.assertEqual(expected_heading2, heading2) + + def _check_peer_feedback_report_user_rows(self, assignment, student, reader): + + answer_comments = sorted( + [ac for ac in self.fixtures.answer_comments if ac.user_id == student.id and ac.assignment_id == assignment.id], + key=lambda ac: (ac.created) + ) + + if len(answer_comments) > 0: + for answer_comment in answer_comments: + row = next(reader) + answer_user = answer_comment.answer.user + + excepted_row = [ + assignment.name, + student.lastname, student.firstname, student.student_number, + answer_user.lastname, answer_user.firstname, answer_user.student_number, + answer_comment.comment_type.value, self._strip_html(answer_comment.content) + ] + + self.assertEqual(row, excepted_row) + else: + row = next(reader) + + excepted_row = [ + assignment.name, + student.lastname, student.firstname, student.student_number, + "---", "---", "---", + "", "" + ] + + self.assertEqual(row, excepted_row) + + def _strip_html(self, text): + text = re.sub('<[^>]+>', '', text) + text = text.replace(' ', ' ') + text = text.replace('&', '&') + text = text.replace('<', '<') + text = text.replace('>', '>') + text = text.replace('"', '"') + text = text.replace(''', '\'') + return text \ No newline at end of file diff --git a/data/fixtures/test_data.py b/data/fixtures/test_data.py index ca4e7f1ae..cabc91824 100644 --- a/data/fixtures/test_data.py +++ b/data/fixtures/test_data.py @@ -6,7 +6,8 @@ from compair import db from six.moves import range -from compair.models import SystemRole, CourseRole, Criterion, Course, Comparison, ThirdPartyType +from compair.models import SystemRole, CourseRole, Criterion, Course, \ + Comparison, ThirdPartyType, AnswerCommentType from data.factories import CourseFactory, UserFactory, UserCourseFactory, AssignmentFactory, \ AnswerFactory, CriterionFactory, ComparisonFactory, AssignmentCriterionFactory, FileFactory, \ AssignmentCommentFactory, AnswerCommentFactory, ScoreFactory, ComparisonExampleFactory, \ @@ -510,9 +511,11 @@ def __init__(self): self.unauthorized_student = UserFactory(system_role=SystemRole.student) self.dropped_instructor = UserFactory(system_role=SystemRole.instructor) self.draft_student = None + self.answer_comments = [] db.session.commit() - def add_course(self, num_students=5, num_assignments=1, num_additional_criteria=0, num_groups=0, num_answers='#', with_draft_student=False, with_comparisons=False): + def add_course(self, num_students=5, num_assignments=1, num_additional_criteria=0, num_groups=0, num_answers='#', + with_comments=False, with_draft_student=False, with_comparisons=False): self.course = CourseFactory() self.instructor = UserFactory(system_role=SystemRole.instructor) self.enrol_user(self.instructor, self.course, CourseRole.instructor) @@ -527,10 +530,10 @@ def add_course(self, num_students=5, num_assignments=1, num_additional_criteria= # create a shortcut for first assignment as it is frequently used self.assignment = self.assignments[0] - self.add_answers(num_answers) + self.add_answers(num_answers, with_comments=with_comments) if with_comparisons: - self.add_comparisons() + self.add_comparisons(with_comments=with_comments) if with_draft_student: self.add_students(1) @@ -543,7 +546,7 @@ def add_course(self, num_students=5, num_assignments=1, num_additional_criteria= return self - def add_answers(self, num_answers): + def add_answers(self, num_answers, with_comments=False): if num_answers == '#': num_answers = len(self.students) * len(self.assignments) if len(self.students) * len(self.assignments) < num_answers: @@ -553,9 +556,10 @@ def add_answers(self, num_answers): ) for assignment in self.assignments: for i in range(num_answers): + student = self.students[i % len(self.students)] answer = AnswerFactory( assignment=assignment, - user=self.students[i % len(self.students)] + user=student ) # half of the answers have scores if i < num_answers/2: @@ -566,6 +570,30 @@ def add_answers(self, num_answers): criterion=criterion, score=random.random() * 5 ) + + if with_comments: + other_students = [s for s in self.students if s.id != student.id] + # half of the answers has a public comment + if i < num_answers/2: + random.shuffle(other_students) + answer_comment = AnswerCommentFactory( + user=other_students[0], + answer=answer, + comment_type=AnswerCommentType.public + ) + self.answer_comments.append(answer_comment) + + # half of the answers has a private comment + # (middle half so there is partial overlap with public comments) + if num_answers/4 < i and i < (num_answers * 3)/4: + random.shuffle(other_students) + answer_comment = AnswerCommentFactory( + user=other_students[0], + answer=answer, + comment_type=AnswerCommentType.private + ) + self.answer_comments.append(answer_comment) + self.answers.append(answer) for dropped_student in self.dropped_students: @@ -578,15 +606,28 @@ def add_answers(self, num_answers): return self - def add_comparisons(self): + def add_comparisons(self, with_comments=False): for assignment in self.assignments: for student in self.students: + answers = set() for i in range(assignment.total_comparisons_required): comparisons = Comparison.create_new_comparison_set(assignment.id, student.id, False) for comparison in comparisons: comparison.completed = True comparison.winner_id = min([comparisons[0].answer1_id, comparisons[0].answer2_id]) db.session.add(comparison) + + answers.add(comparisons[0].answer1) + answers.add(comparisons[0].answer2) + + if with_comments: + for answer in answers: + answer_comment = AnswerCommentFactory( + user=student, + answer=answer, + comment_type=AnswerCommentType.evaluation + ) + self.answer_comments.append(answer_comment) db.session.commit() return self