From f744a512804fd3b300e558ac3125ac279186b345 Mon Sep 17 00:00:00 2001 From: Clarence Ho Date: Tue, 26 Jun 2018 14:50:03 -0700 Subject: [PATCH] WIP - allow student to flag inappropriate explanation --- ubcpi/answer_pool.py | 40 +- ubcpi/persistence.py | 15 +- ubcpi/serialize.py | 8 +- ubcpi/static/css/ubcpi.css | 176 +++++++- ubcpi/static/html/ubcpi.html | 106 +++++ ubcpi/static/html/ubcpi_edit.html | 7 + ubcpi/static/js/spec/ubcpi_spec.js | 390 +++++++++++++++++- ubcpi/static/js/src/ubcpi.js | 158 ++++++- ubcpi/static/js/src/ubcpi_edit.js | 1 + ubcpi/test/data/basic_scenario.xml | 2 +- ubcpi/test/data/flag_inappropriate.json | 140 +++++++ ubcpi/test/data/get_other_answers_random.json | 15 +- ubcpi/test/data/get_other_answers_simple.json | 15 +- ubcpi/test/data/parse_from_xml.json | 13 +- ubcpi/test/data/submit_answer.json | 6 +- ubcpi/test/data/update_xblock.json | 3 +- ubcpi/test/data/validate_form.json | 3 +- ubcpi/test/data/validate_form_errors.json | 182 +++++++- ubcpi/test/test_answer_pool.py | 18 +- ubcpi/test/test_flag_inappropriate.py | 144 +++++++ ubcpi/test/test_lms.py | 36 +- ubcpi/test/test_studio.py | 2 +- ubcpi/ubcpi.py | 134 +++++- 23 files changed, 1560 insertions(+), 54 deletions(-) create mode 100644 ubcpi/test/data/flag_inappropriate.json create mode 100644 ubcpi/test/test_flag_inappropriate.py diff --git a/ubcpi/answer_pool.py b/ubcpi/answer_pool.py index eb61048..9750af0 100644 --- a/ubcpi/answer_pool.py +++ b/ubcpi/answer_pool.py @@ -13,6 +13,8 @@ # items in the pool. For variable length item, specify the max length POOL_ITEM_LENGTH_SIMPLE = POOL_ITEM_LENGTH_RANDOM = 35 +# dummy id to identify a seed explanation +SEED_EXPLANATION_ID = "-1" class UnknownChooseAnswerAlgorithm(Exception): pass @@ -173,7 +175,7 @@ def validate_seeded_answers(answers, options, algo): raise UnknownChooseAnswerAlgorithm() -def get_other_answers(pool, seeded_answers, get_student_item_dict, algo, options): +def get_other_answers(pool, seeded_answers, get_student_item_dict, algo, options, inappropriate_answers): """ Select other student's answers from answer pool or seeded answers based on the selection algorithm @@ -195,6 +197,12 @@ def get_other_answers(pool, seeded_answers, get_student_item_dict, algo, options get_student_item_dict (callable): get student item dict function to return student item dict algo (str): selection algorithm options (dict): answer options for the question + inappropriate_answers (set): inappropriate answers that shouldn't be showing. Format: + { + submission_uuid_1, # Submission UUID of inappropriate answer + submission_uuid_2, + ... + } Returns: dict: answers based on the selection algorithm @@ -204,15 +212,24 @@ def get_other_answers(pool, seeded_answers, get_student_item_dict, algo, options if 'num_responses' not in algo or algo['num_responses'] == "#" \ else int(algo['num_responses']) + # clean up the pool + for option in pool.keys(): + for student in pool[option].keys(): + student_item = get_student_item_dict(student) + submission = sas_api.get_answers_for_student(student_item) + # remove deleted submission or new submission that doesn't match recorded option + if not submission.has_revision(0) or (submission.has_revision(0) and submission.get_vote(0) != option): + del pool[option][student] + if algo['name'] == 'simple': - return get_other_answers_simple(pool, seeded_answers, get_student_item_dict, num_responses) + return get_other_answers_simple(pool, seeded_answers, get_student_item_dict, num_responses, inappropriate_answers) elif algo['name'] == 'random': - return get_other_answers_random(pool, seeded_answers, get_student_item_dict, num_responses) + return get_other_answers_random(pool, seeded_answers, get_student_item_dict, num_responses, inappropriate_answers) else: raise UnknownChooseAnswerAlgorithm() -def get_other_answers_simple(pool, seeded_answers, get_student_item_dict, num_responses): +def get_other_answers_simple(pool, seeded_answers, get_student_item_dict, num_responses, inappropriate_answers): """ Get answers from others with simple algorithm, which picks one answer for each option. @@ -240,6 +257,7 @@ def get_other_answers_simple(pool, seeded_answers, get_student_item_dict, num_re while len(ret) < num_responses and merged_pool: for option, students in merged_pool.items(): rationale = None + unique_id = SEED_EXPLANATION_ID while students: student = random.choice(students.keys()) # remove the chosen answer from pool @@ -251,14 +269,17 @@ def get_other_answers_simple(pool, seeded_answers, get_student_item_dict, num_re else: student_item = get_student_item_dict(student) submission = sas_api.get_answers_for_student(student_item) + if submission.get_submission_uuid() in inappropriate_answers: + continue # Make sure the answer is still the one we want. # It may have changed (e.g. instructor deleted the student state # and the student re-submitted a diff answer) if submission.has_revision(0) and submission.get_vote(0) == option: rationale = submission.get_rationale(0) + unique_id = submission.get_submission_uuid() if rationale: - ret.append({'option': option, 'rationale': rationale}) + ret.append({'option': option, 'rationale': rationale, 'id': unique_id}) break if not students: @@ -271,7 +292,7 @@ def get_other_answers_simple(pool, seeded_answers, get_student_item_dict, num_re return {"answers": ret} -def get_other_answers_random(pool, seeded_answers, get_student_item_dict, num_responses): +def get_other_answers_random(pool, seeded_answers, get_student_item_dict, num_responses, inappropriate_answers): """ Get answers from others with random algorithm, which randomly select answer from the pool. @@ -307,18 +328,23 @@ def get_other_answers_random(pool, seeded_answers, get_student_item_dict, num_re # this is the student's answer so don't return continue + unique_id = SEED_EXPLANATION_ID if student.startswith('seeded'): option = seeded[student]['answer'] rationale = seeded[student]['rationale'] else: student_item = get_student_item_dict(student) submission = sas_api.get_answers_for_student(student_item) + if submission.get_submission_uuid() in inappropriate_answers: + continue + if submission.has_revision(0): rationale = submission.get_rationale(0) option = submission.get_vote(0) + unique_id = submission.get_submission_uuid() else: continue - ret.append({'option': option, 'rationale': rationale}) + ret.append({'option': option, 'rationale': rationale, 'id': unique_id}) return {"answers": ret} diff --git a/ubcpi/persistence.py b/ubcpi/persistence.py index 6390dbf..f98b9f1 100644 --- a/ubcpi/persistence.py +++ b/ubcpi/persistence.py @@ -16,6 +16,7 @@ } """ +SUBMISSION_UUID = 'uuid' ANSWER_LIST_KEY = 'answers' DELETE_INDICATOR = 'deleted' REQUEST_USER_ID_KEY = 'requesting_user_id' @@ -43,7 +44,7 @@ def get_answers_for_student(student_item): latest_answer_item = latest_submission.get('answer', {}) if latest_answer_item.get(DELETE_INDICATOR, False): return Answers() - return Answers(latest_answer_item.get(ANSWER_LIST_KEY, [])) + return Answers(latest_answer_item.get(ANSWER_LIST_KEY, []), latest_submission.get(SUBMISSION_UUID, None)) def add_answer_for_student(student_item, vote, rationale): @@ -85,11 +86,12 @@ class Answers: in the future if this xblock supports more than one round of revision. """ - def __init__(self, answers=None): + def __init__(self, answers=None, uuid=None): if not answers: self.raw_answers = [] else: self.raw_answers = answers + self.submission_uuid = uuid def _safe_get(self, revision, key): """ @@ -109,6 +111,15 @@ def _safe_get(self, revision, key): else: return None + def get_submission_uuid(self): + """ + Get the UUID of the submission + + Returns: + the uuid of the submission. None if there is no submission + """ + return self.submission_uuid + def has_revision(self, revision): """ Check if the answer has a revision diff --git a/ubcpi/serialize.py b/ubcpi/serialize.py index a2eea6c..a6e3c75 100644 --- a/ubcpi/serialize.py +++ b/ubcpi/serialize.py @@ -240,6 +240,8 @@ def parse_from_xml(root): algo = unicode(root.attrib['algorithm']) if 'algorithm' in root.attrib else None num_responses = unicode(root.attrib['num_responses']) if 'num_responses' in root.attrib else None + flag_inappropriate_threshold = int(root.attrib['flag_inappropriate_threshold']) if 'flag_inappropriate_threshold' in root.attrib else None + return { 'display_name': display_name, 'question_text': question, @@ -248,7 +250,8 @@ def parse_from_xml(root): 'correct_answer': correct_answer, 'correct_rationale': correct_rationale, 'seeds': seeds, - 'algo': {"name": algo, 'num_responses': num_responses} + 'algo': {"name": algo, 'num_responses': num_responses}, + 'flag_inappropriate_threshold': flag_inappropriate_threshold } @@ -346,3 +349,6 @@ def serialize_to_xml(root, block): seeds = etree.SubElement(root, 'seeds') serialize_seeds(seeds, block) + + if block.flag_inappropriate_threshold: + root.set('flag_inappropriate_threshold', unicode(block.flag_inappropriate_threshold)) diff --git a/ubcpi/static/css/ubcpi.css b/ubcpi/static/css/ubcpi.css index 2d153c0..38a0566 100644 --- a/ubcpi/static/css/ubcpi.css +++ b/ubcpi/static/css/ubcpi.css @@ -602,6 +602,10 @@ text.ubcpibar.label { padding-left: 7px; } +.other-rationale-inappropriate { + cursor: pointer; +} + .ubcpi-class-breakdown { padding: 10px 20px; border: 1px solid #D7DBDF; @@ -631,6 +635,59 @@ text.ubcpibar.label { display: block; } +.ubcpi-staff-explanation-pool { + text-align: left; + background: #F8F8F8; + padding: 15px; + margin: 15px 0 20px; +} + +.ubcpi-staff-explanation-pool .ubcpi-staff-explanation-pool-loading { + text-align: center; +} + +.ubcpi-staff-explanation-pool .pool-table { + width: 100%; + border: 1px solid black; + font-size: 0.8em; +} + +.ubcpi-staff-explanation-pool .pool-table thead td { + text-align: center; + font-weight: bold; +} + +.ubcpi-staff-explanation-pool .pool-table td { + border: 1px solid #ccc; + padding: 0.5em; +} + +.ubcpi-staff-explanation-pool .pool-table .ubcpi-staff-override-options { + display: inline-block; + margin-left: 10px; + cursor: pointer; +} + +.ubcpi-staff-explanation-pool .pool-table .ubcpi-staff-override-options .ubcpi-staff-override-options-shown { + color: rgba(0,128,0,0.8); + font-size: 1.3em; + padding-left: 5px; + padding-right: 5px; + border-radius: 4px; +} + +.ubcpi-staff-explanation-pool .pool-table .ubcpi-staff-override-options .ubcpi-staff-override-options-not-shown { + color: rgba(128,0,0,0.8); + font-size: 1.3em; + padding-left: 5px; + padding-right: 5px; + border-radius: 4px; +} + +.ubcpi-staff-explanation-pool .pool-table tbody tr:nth-child(even){ + background-color: #f2f2f2; +} + #pi-form .list-input.settings-list .setting-label { vertical-align: top; } @@ -774,6 +831,12 @@ div.course-wrapper section.course-content .warning-notice p { transition:none; } +.ubcpi-staff-statistics-button *{ + font-size:12px; + color:#0075b4; + transition:none; +} + .ubcpi-staff-statistics-button--active{ color:white; background-color:#b62568; @@ -785,6 +848,12 @@ div.course-wrapper section.course-content .warning-notice p { transition:none; } +.ubcpi-staff-statistics-button--active *{ + font-size:12px; + color:#0075b4; + transition:none; +} + .ubcpi-staff-statistics{ padding:1em; margin-top:0.5em; @@ -795,7 +864,7 @@ div.course-wrapper section.course-content .warning-notice p { .ubcpi-staff-statistics-button:hover{ color:white; - background-color: #61b5e6; + background-color:#b62568; } .ubcpi-staff-statistics-button:active{ @@ -811,6 +880,53 @@ div.course-wrapper section.course-content .warning-notice p { outline:none; } +.ubcpi-staff-answer-pool-button { + border:none; + border-radius:5px; + padding:5px 10px; + text-transform:uppercase; + font-size:12px; + background-color:rgba(3,3,3,0.05); + transition:none; +} + +.ubcpi-staff-answer-pool-button * { + font-size:12px; + color:#0075b4; + transition:none; +} + +.ubcpi-staff-answer-pool-button--active{ + color:white; + background-color:#b62568; + border:none; + border-radius:5px; + padding: 5px 10px; + text-transform:uppercase; + font-size:12px; + transition:none; +} + +.ubcpi-staff-answer-pool-button--active * { + color:white; + font-size:12px; + transition:none; +} + +.ubcpi-staff-answer-pool-button:hover{ + background-color: #b62568; +} + +.ubcpi-staff-answer-pool-button:active { + color:white; + background-color:#b62568; +} + +.ubcpi-staff-answer-pool-button:focus, .ubcpi-staff-answer-pool-button:focus{ + outline:none; +} + + pi-barchart > span{ text-align:center; display:block; @@ -855,3 +971,61 @@ pi-barchart > svg { #not-enough-data { font-size: 0.85em; } + +.ubcpi-modal-dialog { + display: none; + background: #F0F0F0; + box-shadow: 0 0 20px 0 rgba(0,0,0,0.4); + border-radius: 5px; + padding: 10px; + width: 50%; + height: 60%; + overflow: auto; +} + +.ubcpi-modal-dialog-header { + text-align: center; + font-size: 1.2em; + font-weight: bold; +} + +.ubcpi-modal-dialog-body { + text-align: left; + font-size: 0.8em; + padding: 10px; +} + +.ubcpi-modal-dialog-close-button { + position: absolute; + top: 12px; + right: 12px; + display: block; + width: 14px; + height: 14px; + z-index: 2; + cursor: pointer; + color: #0075b4; +} + +.ubcpi-report-inappropriate-button { + font-size: 0.9em; + border-radius: 4px; + padding-left: 10px; + padding-right: 10px; + padding-top: 5px; + padding-bottom: 5px; + border: none; + color: #0075b4; + background-color: rgba(3,3,3, 0.05); + transition: none; +} + +.ubcpi-report-inappropriate-button * { + font-size: 0.8em; + color: #0075b4; + transition: none; +} + +.ubcpi-report-inappropriate-button:hover { + background-color: #61b5e6; +} diff --git a/ubcpi/static/html/ubcpi.html b/ubcpi/static/html/ubcpi.html index c283cd3..5a9915c 100644 --- a/ubcpi/static/html/ubcpi.html +++ b/ubcpi/static/html/ubcpi.html @@ -122,8 +122,32 @@

{{display_n +
+ Report Inappropriate Answers + +
+
+
Report Inappropriate Answers
+
+
+
+ +
+
+

If you think a student answer doesn't meet our course guidelines or is inappropriate, offensive or unsafe in any way, please report to the instructional team.

+
+
+ Student Rationale + "{{otherAnswer.rationale}}"    
  Report
+
+
+
+
+
+
View Question Statistics +
@@ -144,6 +168,7 @@

{{display_n

+
@@ -225,6 +250,87 @@

{{display_n +
+ View Explanation Pool Status + +
+
+  Loading data... +
+
+ + + + + + + + + + + + + + + + + + + + + + + +
+ + Option + + + + + + + Explanation + + + + + + + # reported as inappropriate + + + + + + + Can be shown as sample? + + + + +
{{ ans.option-0 + 1 | number }}{{ ans.explanation }}{{ ans.inappropriate_report_count }} +
+
+ +
+
+ +
+
+
+
+ +
+
+ +
+
+
No data vailable
+
+
+
+ diff --git a/ubcpi/static/html/ubcpi_edit.html b/ubcpi/static/html/ubcpi_edit.html index 7930226..f22a2b2 100644 --- a/ubcpi/static/html/ubcpi_edit.html +++ b/ubcpi/static/html/ubcpi_edit.html @@ -30,6 +30,13 @@ +
  • +
    + + +
    +
    • +
  • diff --git a/ubcpi/static/js/spec/ubcpi_spec.js b/ubcpi/static/js/spec/ubcpi_spec.js index 7ef0480..ab78272 100644 --- a/ubcpi/static/js/spec/ubcpi_spec.js +++ b/ubcpi/static/js/spec/ubcpi_spec.js @@ -41,6 +41,224 @@ describe('UBCPI module', function () { }); }); + describe('autoFocus directive', function() { + var $compile, $rootScope; + beforeEach(inject(function (_$compile_, _$rootScope_) { + // The injector unwraps the underscores (_) from around the parameter names when matching + $compile = _$compile_; + $rootScope = _$rootScope_; + })); + + it('should set focus to the element', function() { + var scope = $rootScope.$new(true); + var element = $compile("
    ")(scope); + scope.$digest(); + expect(element.is(":focus")); + }); + }); + + describe('Confirm flag inappropriate directive', function () { + var element, scope, compile; + var confirmMsg = "Flag as inappropriate"; + var backendService; + var backendDefer; + var successFlagResult = { 'success': 'true' }; + var failFlagResult = { 'success': 'false' }; + + beforeEach(inject(function ($compile, $rootScope, _backendService_, _$q_) { + scope = $rootScope; + compile = $compile; + backendService = _backendService_; + backendDefer = _$q_.defer(); + + scope.otherAnswer = {"option": 0, "rationale": "A dummy answer.", "id": "123-456-789"}; + + window.gettext = function(){}; + spyOn(window, 'gettext').and.callFake(function (t) { + return t; + }); + spyOn(backendService, 'flagInappropriate').and.callFake(function (id) { + expect(id).toBe(scope.otherAnswer.id); + return backendDefer.promise; + }); + + element = angular.element( + '
    ' + ); + })); + + it('should prompt confirmation dialog', function() { + spyOn(window, 'confirm').and.callFake(function (msg) { + return true; + }); + compile(element)(scope); + scope.$digest(); + + $(element).click(); + expect(window.confirm).toHaveBeenCalledWith(confirmMsg); + }); + + it('should flag answer if clicked OK', function() { + spyOn(window, 'confirm').and.callFake(function (msg) { + return true; + }); + compile(element)(scope); + scope.$digest(); + + $(element).click(); + expect(backendService.flagInappropriate).toHaveBeenCalledWith(scope.otherAnswer.id); + + backendDefer.resolve(successFlagResult); + scope.$digest(); + expect($(element).text()).toBe('Answer reported as inappropriate'); + }); + + it('should not flag the answer if clicked cancel', function() { + spyOn(window, 'confirm').and.callFake(function (msg) { + return false; + }); + compile(element)(scope); + scope.$digest(); + + $(element).click(); + expect(backendService.flagInappropriate).not.toHaveBeenCalled(); + }); + + it('should show failed flagging message accordingly', function() { + spyOn(window, 'confirm').and.callFake(function (msg) { + return true; + }); + compile(element)(scope); + scope.$digest(); + + $(element).click(); + expect(backendService.flagInappropriate).toHaveBeenCalledWith(scope.otherAnswer.id); + + backendDefer.resolve(failFlagResult); + scope.$digest(); + expect($(element).text()).toBe('Error reporting inappropriate answer. Please refresh the page and try again.'); + }); + + it('should show proper message when backend calls failed', function() { + spyOn(window, 'confirm').and.callFake(function (msg) { + return true; + }); + compile(element)(scope); + scope.$digest(); + + $(element).click(); + expect(backendService.flagInappropriate).toHaveBeenCalledWith(scope.otherAnswer.id); + + backendDefer.reject(); + scope.$digest(); + expect($(element).text()).toBe('Error reporting inappropriate answer. Please refresh the page and try again.'); + }); + + }); + + describe('Confirm staff toggle inappropriate directive', function () { + var element, scope, compile; + var confirmMsg = "Flag as inappropriate"; + var backendService; + var backendDefer; + + beforeEach(inject(function ($compile, $rootScope, _backendService_, _$q_) { + scope = $rootScope; + compile = $compile; + backendService = _backendService_; + backendDefer = _$q_.defer(); + + scope.ans = {"option": 0, "rationale": "A dummy answer.", "id": "123-456-789"}; + scope.rc = { explanationPool: { pool: null } }; + + window.gettext = function(){}; + spyOn(window, 'gettext').and.callFake(function (t) { + return t; + }); + spyOn(backendService, 'staffToggleInappropriate').and.callFake(function (id, value) { + expect(id).toBe(scope.ans.id); + return backendDefer.promise; + }); + })); + + // it('should prompt confirmation dialog', function() { + // spyOn(window, 'confirm').and.callFake(function (msg) { + // return true; + // }); + // + // element = angular.element( + // '
    ' + // ); + // compile(element)(scope); + // scope.$digest(); + // + // $(element).click(); + // expect(window.confirm).toHaveBeenCalledWith(confirmMsg); + // }); + + it('should flag answer if clicked OK', function() { + spyOn(window, 'confirm').and.callFake(function (msg) { + return true; + }); + + element = angular.element( + '
    ' + ); + compile(element)(scope); + scope.$digest(); + + $(element).click(); + expect(backendService.staffToggleInappropriate).toHaveBeenCalledWith(scope.ans.id, 'true'); + + var successToggleResult = [{ + "option": "0", + "considered_inappropriate": true, + "explanation": scope.ans.rationale, + "staff_set_inappropriate": true, + "id": scope.ans.id, + "inappropriate_report_count": 0}] + + backendDefer.resolve(successToggleResult); + scope.$digest(); + expect(scope.rc.explanationPool.pool).toBe(successToggleResult); + }); + + // it('should not flag the answer if clicked cancel', function() { + // spyOn(window, 'confirm').and.callFake(function (msg) { + // return false; + // }); + // + // element = angular.element( + // '
    ' + // ); + // compile(element)(scope); + // scope.$digest(); + // + // $(element).click(); + // expect(backendService.staffToggleInappropriate).not.toHaveBeenCalled(); + // }); + + it('should handle backend service error', function() { + spyOn(window, 'confirm').and.callFake(function (msg) { + return true; + }); + + element = angular.element( + '
    ' + ); + compile(element)(scope); + scope.$digest(); + + $(element).click(); + expect(backendService.staffToggleInappropriate).toHaveBeenCalledWith(scope.ans.id, 'true'); + + backendDefer.reject(); + scope.$digest(); + expect(scope.rc.explanationPool.pool).toBe(null); + }); + + }); + describe('backendService', function() { var backendService, $httpBackend; @@ -183,7 +401,173 @@ describe('UBCPI module', function () { expect(error).toBe('error'); }) - }) + }); + + describe('flag_inappropriate', function() { + + beforeEach(function() { + mockConfig.urls.flag_inappropriate = '/handler/flag_inappropriate'; + }); + + it('should flag inappropriate', function() { + // mock ajax request + var testId = "123-456-789"; + var post = { + "id": testId + }; + var exp = { + "success": "true" + }; + $httpBackend.expectPOST('/handler/flag_inappropriate', post).respond(200, exp); + + backendService.flagInappropriate(testId).then(function(result) { + expect(result).toEqual(exp); + }); + $httpBackend.flush(); + }); + + it('should return false status if answer cannot be flagged', function() { + // mock ajax request + var testId = "987-654-321"; + var post = { + "id": testId + }; + var exp = { + "success": "false" + }; + $httpBackend.expectPOST('/handler/flag_inappropriate', post).respond(200, exp); + + backendService.flagInappropriate(testId).then(function(result) { + expect(result).toEqual(exp); + }); + $httpBackend.flush(); + }); + + it('should reject promise with error returned from backend when backend error ', function() { + // mock ajax request + var testId = "987-654-321"; + var post = { + "id": testId + }; + $httpBackend.expectPOST('/handler/flag_inappropriate', post).respond(400, 'error'); + + backendService.flagInappropriate(testId).catch(function(e) { + expect(e.data).toEqual('error'); + }); + $httpBackend.flush(); + }); + }); + + describe('staff_toggle_inappropriate', function() { + + beforeEach(function() { + mockConfig.urls.staff_toggle_inappropriate = '/handler/staff_toggle_inappropriate'; + }); + + it('should allow staff to flag answer as inappropriate', function() { + // mock ajax request + var testId = "123-456-789"; + var post = { + "id": testId, + "considered_inappropriate": "true" + }; + var exp = [{ + "optione": "1", + "considered_inappropriate": true, + "explanation": "test explanation", + "staff_set_inappropriate": true, + "id": testId, + "inappropriate_report_count": 0 + }]; + $httpBackend.expectPOST('/handler/staff_toggle_inappropriate', post).respond(200, exp); + + backendService.staffToggleInappropriate(testId, "true").then(function(result) { + expect(result).toEqual(exp); + }); + $httpBackend.flush(); + }); + + it('should allow staff to flag answer as appropriate', function() { + // mock ajax request + var testId = "123-456-789"; + var post = { + "id": testId, + "considered_inappropriate": "false" + }; + var exp = [{ + "optione": "1", + "considered_inappropriate": false, + "explanation": "test explanation", + "staff_set_inappropriate": false, + "id": testId, + "inappropriate_report_count": 0 + }]; + $httpBackend.expectPOST('/handler/staff_toggle_inappropriate', post).respond(200, exp); + + backendService.staffToggleInappropriate(testId, "false").then(function(result) { + expect(result).toEqual(exp); + }); + $httpBackend.flush(); + }); + + it('should allow staff to clear flagging', function() { + // mock ajax request + var testId = "123-456-789"; + var post = { + "id": testId, + "considered_inappropriate": "null" + }; + var exp = [{ + "optione": "1", + "considered_inappropriate": false, + "explanation": "test explanation", + "staff_set_inappropriate": null, + "id": testId, + "inappropriate_report_count": 0 + }]; + $httpBackend.expectPOST('/handler/staff_toggle_inappropriate', post).respond(200, exp); + + backendService.staffToggleInappropriate(testId, "null").then(function(result) { + expect(result).toEqual(exp); + }); + $httpBackend.flush(); + }); + }); + + describe('get_pool_status', function() { + + beforeEach(function() { + mockConfig.urls.get_pool_status = '/handler/get_pool_status'; + }); + + it('should allow staff to get pool staus', function() { + // mock ajax request + var post = { + }; + var exp = [{ + "option": "1", + "considered_inappropriate": false, + "explanation": "testing for soil", + "staff_set_inappropriate": null, + "id": "75720d09-35e9-46dc-b72d-8e85c7297d4f", + "inappropriate_report_count": 0 + }, { + "option": "1", + "considered_inappropriate": false, + "explanation": "test test etst", + "staff_set_inappropriate": null, + "id": "21268762-29c3-4cf6-a578-a0995a6155cc", + "inappropriate_report_count": 0 + }]; + $httpBackend.expectPOST('/handler/get_pool_status', post).respond(200, exp); + + backendService.getPoolStatus().then(function(result) { + expect(result).toEqual(exp); + }); + $httpBackend.flush(); + }); + + }); }); describe('ReviseController', function() { @@ -450,8 +834,8 @@ describe('PeerInstructionXBlock function', function() { }); it('should generate URLs using runtime', function() { - expect(mockRuntime.handlerUrl.calls.count()).toBe(4); + expect(mockRuntime.handlerUrl.calls.count()).toBe(7); expect(mockRuntime.handlerUrl.calls.allArgs()).toEqual( - [[mockElement, 'get_stats'], [mockElement, 'submit_answer'], [mockElement, 'get_asset'], [mockElement, 'get_data']]); + [[mockElement, 'get_stats'], [mockElement, 'submit_answer'], [mockElement, 'get_asset'], [mockElement, 'get_data'], [mockElement, 'flag_inappropriate'], [mockElement, 'get_pool_status'], [mockElement, 'staff_toggle_inappropriate']]); }); }); diff --git a/ubcpi/static/js/src/ubcpi.js b/ubcpi/static/js/src/ubcpi.js index b113ae7..c3bb6b1 100644 --- a/ubcpi/static/js/src/ubcpi.js +++ b/ubcpi/static/js/src/ubcpi.js @@ -53,11 +53,82 @@ angular.module('UBCPI', ['ngSanitize', 'ngCookies', 'gettext']) }; }]) + .directive('ubcpiModalTrigger', function() { + return { + restrict: 'A', + link: function(scope, ele, attr, ctrl) { + ele.leanModal({ closeButton: ".ubcpi-modal-dialog-close-button" }); + } + }; + }) + + .directive('confirmFlagAppropriate', ['backendService', 'notify', function(backendService, notify) { + return { + scope: true, + link: function (scope, element, attr) { + element.bind('click', function(event) { + if (window.confirm(attr.confirmFlagAppropriate) && scope.otherAnswer.id) { + scope.otherAnswer.reported = true; + element.text(gettext('Submitting...')); + element.unbind('click'); + + backendService.flagInappropriate(scope.otherAnswer.id).then(function(data) { + // LMS has no notify function. for now, a quick hack to change the report button. + // a reload will show the button again. but backend only count each student's report once. + if (data && data['success'] === 'true') { + element.text(gettext('Answer reported as inappropriate')); + notify('error', { + 'title': gettext('Inappropriate Answer'), + 'message': gettext('Answer reported as inappropriate') + }); + } else { + element.text(data && data.msg? data.msg : gettext('Error reporting inappropriate answer. Please refresh the page and try again.')); + notify('error', { + 'title': gettext('Error reporting inappropriate answer.'), + 'message': data && data.msg? data.msg : gettext('Please refresh the page and try again.') + }); + } + }, function(error) { + element.text(gettext('Error reporting inappropriate answer. Please refresh the page and try again.')); + notify('error', { + 'title': gettext('Error reporting inappropriate answer!'), + 'message': gettext('Please refresh the page and try again.') + }); + }); + }; + }); + } + }; + }]) + + .directive('confirmStaffToggleInappropriate', ['backendService', 'notify', function(backendService, notify) { + return { + scope: true, + link: function (scope, element, attr, ctrl) { + element.bind('click', function(event) { + //if (window.confirm(attr.confirmStaffToggleInappropriate)) { + backendService.staffToggleInappropriate(scope.ans.id, attr.value).then(function(data) { + scope.rc.explanationPool.pool = data; + }, function(error) { + notify('error', { + 'title': gettext('Error flagging inappropriate answer!'), + 'message': gettext('Please refresh the page and try again!') + }); + }); + //}; + }); + } + }; + }]) + .factory('backendService', ['$http', '$q', '$rootScope', function ($http, $q, $rootScope) { return { getStats: getStats, submit: submit, get_data: get_data, + flagInappropriate: flagInappropriate, + getPoolStatus: getPoolStatus, + staffToggleInappropriate: staffToggleInappropriate }; function getStats() { @@ -99,6 +170,50 @@ angular.module('UBCPI', ['ngSanitize', 'ngCookies', 'gettext']) } ); } + + function flagInappropriate(id) { + var flagUrl = $rootScope.config.urls.flag_inappropriate; + var flagData = JSON.stringify({ + "id": id + }); + return $http.post(flagUrl, flagData).then( + function(response) { + return response.data; + }, + function(error) { + return $q.reject(error); + } + ); + } + + function staffToggleInappropriate(id, considered_inappropriate) { + var flagUrl = $rootScope.config.urls.staff_toggle_inappropriate; + var flagData = JSON.stringify({ + "id": id, + "considered_inappropriate": considered_inappropriate + }); + return $http.post(flagUrl, flagData).then( + function(response) { + return response.data; + }, + function(error) { + return $q.reject(error); + } + ); + } + + function getPoolStatus() { + var getPoolStatusUrl = $rootScope.config.urls.get_pool_status; + return $http.post(getPoolStatusUrl, {}).then( + function(response) { + return response.data; + }, + function(error) { + return $q.reject(error); + } + ); + } + }]) .controller('ReviseController', ['$scope', 'notify', 'backendService', '$q', 'gettext', '$location', '$anchorScroll', '$timeout', @@ -119,10 +234,7 @@ angular.module('UBCPI', ['ngSanitize', 'ngCookies', 'gettext']) // Assign data based on what has been persisted var persistedDataObject = get_data().then( function(persistedData) { - - if ( persistedData.answer_original !== null ) { - assignData(self, persistedData); - } + assignData(self, persistedData); }); @@ -132,6 +244,9 @@ angular.module('UBCPI', ['ngSanitize', 'ngCookies', 'gettext']) // Whether user is revising the answer self.revising = false; + // Whether student can report inappropriate answer + self.can_report_inappropriate = false; + /** * Determine if the submit button should be disabled * If we have an answer selected, a rationale that is large @@ -227,6 +342,7 @@ angular.module('UBCPI', ['ngSanitize', 'ngCookies', 'gettext']) self.rationale = data.rationale_revised || data.rationale_original; self.weight = data.weight; self.options = data.options; + self.can_report_inappropriate = data.can_report_inappropriate; } self.hasSampleExplanationForOption = function (option) { @@ -237,6 +353,35 @@ angular.module('UBCPI', ['ngSanitize', 'ngCookies', 'gettext']) } return false; }; + + self.explanationPool = { + showing: false, + pool: null, + sortType: ['option', 'explanation'], + sortReverse: false, + }; + $scope.$watch( + function watchController(scope) { + return self.explanationPool.showing; + }, + function(newValue, oldValue) { + if (newValue === oldValue) { + return; + } + if (!newValue) { + self.explanationPool.pool = null; + } else { + backendService.getPoolStatus().then(function(data) { + self.explanationPool.pool = data; + }, function(error) { + notify('error', { + 'title': gettext('Error retrieving data!'), + 'message': gettext('Please refresh the page and try again!') + }); + }); + } + }); + }]); /** @@ -259,7 +404,10 @@ function PeerInstructionXBlock(runtime, element, data) { 'get_stats': runtime.handlerUrl(element, 'get_stats'), 'submit_answer': runtime.handlerUrl(element, 'submit_answer'), 'get_asset': runtime.handlerUrl(element, 'get_asset'), - 'get_data': runtime.handlerUrl(element, 'get_data') + 'get_data': runtime.handlerUrl(element, 'get_data'), + 'flag_inappropriate': runtime.handlerUrl(element, 'flag_inappropriate'), + 'get_pool_status': runtime.handlerUrl(element, 'get_pool_status'), + 'staff_toggle_inappropriate': runtime.handlerUrl(element, 'staff_toggle_inappropriate'), }; // in order to support multiple same apps on the same page but diff --git a/ubcpi/static/js/src/ubcpi_edit.js b/ubcpi/static/js/src/ubcpi_edit.js index 0d0eb7c..b8f4d2e 100644 --- a/ubcpi/static/js/src/ubcpi_edit.js +++ b/ubcpi/static/js/src/ubcpi_edit.js @@ -90,6 +90,7 @@ angular.module("ubcpi_edit", ['ngMessages', 'ngSanitize', 'ngCookies', 'gettext' self.data.correct_rationale = data.correct_rationale; self.data.algo = data.algo; self.data.seeds = data.seeds; + self.data.flag_inappropriate_threshold = data.flag_inappropriate_threshold; self.add_option = function() { self.data.options.push( diff --git a/ubcpi/test/data/basic_scenario.xml b/ubcpi/test/data/basic_scenario.xml index 67bd1db..978b8b9 100644 --- a/ubcpi/test/data/basic_scenario.xml +++ b/ubcpi/test/data/basic_scenario.xml @@ -1,4 +1,4 @@ - + Sample Peer Instruction This is a image/static/test.jpg diff --git a/ubcpi/test/data/flag_inappropriate.json b/ubcpi/test/data/flag_inappropriate.json new file mode 100644 index 0000000..6f74c3e --- /dev/null +++ b/ubcpi/test/data/flag_inappropriate.json @@ -0,0 +1,140 @@ +{ + "test_flag_inappropriate": { + "flag_seed": { + "request": { + "id": "-1" + }, + "expect": { + "success": "true" + } + }, + + "flag_others": { + "submit_answer_request": { + "q": 0, + "rationale": "This is my answer.", + "status": 0 + }, + "submit_answer_expect": { + "other_answers": { + "answers": [{ + "option": 0, + "rationale": "This is seed1", + "id": "-1" + }, { + "option": 1, + "rationale": "This is seed2", + "id": "-1" + }, { + "option": 2, + "rationale": "This is seed3", + "id": "-1" + }, { + "option": 2, + "rationale": "this is dummy", + "id": "123" + } + ] + }, + "answer_original": 0, + "rationale_original": "This is my answer.", + "answer_revised": null, + "rationale_revised": null, + "can_report_inappropriate": true + }, + "dummy_get_other_answers": { + "answers": [{ + "option": 0, + "rationale": "This is seed1", + "id": "-1" + }, { + "option": 1, + "rationale": "This is seed2", + "id": "-1" + }, { + "option": 2, + "rationale": "This is seed3", + "id": "-1" + }, { + "option": 2, + "rationale": "this is dummy", + "id": "123" + } + ] + + }, + "flag_inappropriate_request": { + "id": "123" + }, + "flag_inappropriate_expect": { + "success": "true" + } + }, + + "get_pool_status": { + "submit_answer_request": { + "q": 0, + "rationale": "This is my answer.", + "status": 0 + }, + "get_pool_status_request": {}, + "get_pool_status_expect": + [{ + "option": 0, + "considered_inappropriate": false, + "explanation": "This is my answer.", + "staff_set_inappropriate": null, + "inappropriate_report_count": 0 + } + ] + }, + + "staff_toggle_inappropriate": { + "submit_answer_request": { + "q": 1, + "rationale": "This is my answer.", + "status": 0 + }, + "get_pool_status_request": {}, + "get_pool_status_expect_before": + [{ + "option": 1, + "considered_inappropriate": false, + "explanation": "This is my answer.", + "staff_set_inappropriate": null, + "inappropriate_report_count": 0 + } + ], + "staff_toggle_inappropriate_request_set": { + "id": "", + "considered_inappropriate": "true" + }, + "get_pool_status_expect_after": + [{ + "option": 1, + "considered_inappropriate": true, + "explanation": "This is my answer.", + "staff_set_inappropriate": true, + "inappropriate_report_count": 0 + } + ], + "staff_toggle_inappropriate_request_unset": { + "id": "", + "considered_inappropriate": "null" + }, + "staff_toggle_inappropriate_request_set_appropriate": { + "id": "", + "considered_inappropriate": "false" + }, + "get_pool_status_expect_force_appropriate": + [{ + "option": 1, + "considered_inappropriate": false, + "explanation": "This is my answer.", + "staff_set_inappropriate": false, + "inappropriate_report_count": 0 + } + ] + } + } +} \ No newline at end of file diff --git a/ubcpi/test/data/get_other_answers_random.json b/ubcpi/test/data/get_other_answers_random.json index 885faa3..8581378 100644 --- a/ubcpi/test/data/get_other_answers_random.json +++ b/ubcpi/test/data/get_other_answers_random.json @@ -25,15 +25,18 @@ "expect": { "answers": [ { + "id": "-1", "option": 0, "rationale": "rationale A" }, { + "id": "submission_uuid_1", "option": 0, "rationale": "my rationale" } ] - } + }, + "inappropriate_answers": {} }, "empty_pool": { "pool": { @@ -54,15 +57,18 @@ "expect": { "answers": [ { + "id": "-1", "option": 0, "rationale": "rationale A" }, { + "id": "-1", "option": 1, "rationale": "rationale B" } ] - } + }, + "inappropriate_answers": {} }, "not_enough_in_pool_to_return": { "pool": { @@ -83,14 +89,17 @@ "expect": { "answers": [ { + "id": "-1", "option": 0, "rationale": "rationale A" }, { + "id": "-1", "option": 1, "rationale": "rationale B" } ] - } + }, + "inappropriate_answers": {} } } \ No newline at end of file diff --git a/ubcpi/test/data/get_other_answers_simple.json b/ubcpi/test/data/get_other_answers_simple.json index 1a0c40d..c7510b7 100644 --- a/ubcpi/test/data/get_other_answers_simple.json +++ b/ubcpi/test/data/get_other_answers_simple.json @@ -25,15 +25,18 @@ "expect": { "answers": [ { + "id": "submission_uuid_1", "option": 0, "rationale": "my rationale" }, { + "id": "-1", "option": 1, "rationale": "rationale B" } ] - } + }, + "inappropriate_answers": {} }, "empty_pool": { "pool": { @@ -54,15 +57,18 @@ "expect": { "answers": [ { + "id": "-1", "option": 0, "rationale": "rationale A" }, { + "id": "-1", "option": 1, "rationale": "rationale B" } ] - } + }, + "inappropriate_answers": {} }, "not_enough_in_pool_to_return": { "pool": { @@ -83,14 +89,17 @@ "expect": { "answers": [ { + "id": "-1", "option": 0, "rationale": "rationale A" }, { + "id": "-1", "option": 1, "rationale": "rationale B" } ] - } + }, + "inappropriate_answers": {} } } \ No newline at end of file diff --git a/ubcpi/test/data/parse_from_xml.json b/ubcpi/test/data/parse_from_xml.json index 69a6e36..6b9e076 100644 --- a/ubcpi/test/data/parse_from_xml.json +++ b/ubcpi/test/data/parse_from_xml.json @@ -1,7 +1,7 @@ { "basic": { "xml": [ - "", + "", "Sample Peer Instruction", "", "This is the question text", @@ -56,7 +56,8 @@ "rationale": "This is seed2" } ], - "algo": {"name": "simple", "num_responses": "#"} + "algo": {"name": "simple", "num_responses": "#"}, + "flag_inappropriate_threshold": 2 } }, "minimal": { @@ -102,12 +103,13 @@ "rationale": "This is seed1" } ], - "algo": {"name": null, "num_responses": null} + "algo": {"name": null, "num_responses": null}, + "flag_inappropriate_threshold": null } }, "unicode": { "xml": [ - "", + "", "样品 Peer Instruction", "", "这是一个问题", @@ -162,7 +164,8 @@ "rationale": "种子2" } ], - "algo": {"name": "simple", "num_responses": "#"} + "algo": {"name": "simple", "num_responses": "#"}, + "flag_inappropriate_threshold": 2 } } } diff --git a/ubcpi/test/data/submit_answer.json b/ubcpi/test/data/submit_answer.json index 9076c83..c7e81e7 100644 --- a/ubcpi/test/data/submit_answer.json +++ b/ubcpi/test/data/submit_answer.json @@ -25,7 +25,8 @@ "answer_original": 0, "rationale_original": "This is my answer.", "answer_revised": null, - "rationale_revised": null + "rationale_revised": null, + "can_report_inappropriate": true }, "post2": { "q": 1, @@ -40,7 +41,8 @@ "correct_answer": 0, "correct_rationale": { "text": "This is correct." - } + }, + "can_report_inappropriate": true } } } \ No newline at end of file diff --git a/ubcpi/test/data/update_xblock.json b/ubcpi/test/data/update_xblock.json index 4240b7c..8cfbdb0 100644 --- a/ubcpi/test/data/update_xblock.json +++ b/ubcpi/test/data/update_xblock.json @@ -49,6 +49,7 @@ "rationale": "dsfsdafsd" } ], - "weight": 1 + "weight": 1, + "flag_inappropriate_threshold": 2 } } diff --git a/ubcpi/test/data/validate_form.json b/ubcpi/test/data/validate_form.json index 6e74e76..c048cb2 100644 --- a/ubcpi/test/data/validate_form.json +++ b/ubcpi/test/data/validate_form.json @@ -45,6 +45,7 @@ "answer": 1, "rationale": "Seeded Rationale 2" } - ] + ], + "flag_inappropriate_threshold": 2 } } diff --git a/ubcpi/test/data/validate_form_errors.json b/ubcpi/test/data/validate_form_errors.json index e0a6c04..d47f7d3 100644 --- a/ubcpi/test/data/validate_form_errors.json +++ b/ubcpi/test/data/validate_form_errors.json @@ -42,7 +42,8 @@ "answer": 1, "rationale": "Seeded Rationale 2" } - ] + ], + "flag_inappropriate_threshold": 2 }, "error": { "error": { @@ -97,7 +98,8 @@ "answer": 1, "rationale": "Seeded Rationale 2" } - ] + ], + "flag_inappropriate_threshold": 2 }, "error": { "error": { @@ -152,7 +154,8 @@ "answer": 1, "rationale": "Seeded Rationale 2" } - ] + ], + "flag_inappropriate_threshold": 2 }, "error": { "error": { @@ -160,7 +163,7 @@ } } }, - "invalid_num_responses": { + "negative_num_responses": { "post": { "display_name": "Peer Instruction1", "question_text": { @@ -207,12 +210,181 @@ "answer": 1, "rationale": "Seeded Rationale 2" } - ] + ], + "flag_inappropriate_threshold": 2 }, "error": { "error": { "options_error": "Invalid Option(s): Number of Responses" } } + }, + "invalid_num_responses": { + "post": { + "display_name": "Peer Instruction1", + "question_text": { + "text": "What is the answer to life, the universe and everything?", + "image_show_fields": 0, + "image_url": "", + "image_position": "below", + "image_alt": "" + }, + "rationale_size": { + "max": 32000, + "min": 1 + }, + "options": [ + { + "text": "21", + "image_show_fields": 0, + "image_url": "", + "image_position": "below", + "image_alt": "" + }, + { + "text": "42", + "image_show_fields": 0, + "image_url": "", + "image_position": "below", + "image_alt": "" + } + ], + "correct_answer": 1, + "correct_rationale": { + "text": "Correct rationale" + }, + "algo": { + "num_responses": "abc", + "name": "simple" + }, + "seeds": [ + { + "answer": 0, + "rationale": "Seeded Rationale 1" + }, + { + "answer": 1, + "rationale": "Seeded Rationale 2" + } + ], + "flag_inappropriate_threshold": 2 + }, + "error": { + "error": { + "options_error": "Invalid Option(s): Not an Integer" + } + } + }, + "invalid_inappropriate_1": { + "post": { + "display_name": "Peer Instruction1", + "question_text": { + "text": "What is the answer to life, the universe and everything?", + "image_show_fields": 0, + "image_url": "", + "image_position": "below", + "image_alt": "" + }, + "rationale_size": { + "max": 500, + "min": 50 + }, + "options": [ + { + "text": "21", + "image_show_fields": 0, + "image_url": "", + "image_position": "below", + "image_alt": "" + }, + { + "text": "42", + "image_show_fields": 0, + "image_url": "", + "image_position": "below", + "image_alt": "" + } + ], + "correct_answer": 1, + "correct_rationale": { + "text": "Correct rationale" + }, + "algo": { + "num_responses": "#", + "name": "simple" + }, + "seeds": [ + { + "answer": 0, + "rationale": "Seeded Rationale 1" + }, + { + "answer": 1, + "rationale": "Seeded Rationale 2" + } + ], + "flag_inappropriate_threshold": -1 + }, + "error": { + "error": { + "options_error": "Invalid Option(s): Threshold for Inappropriate Explanation" + } + } + }, + "invalid_inappropriate_2": { + "post": { + "display_name": "Peer Instruction1", + "question_text": { + "text": "What is the answer to life, the universe and everything?", + "image_show_fields": 0, + "image_url": "", + "image_position": "below", + "image_alt": "" + }, + "rationale_size": { + "max": 500, + "min": 50 + }, + "options": [ + { + "text": "21", + "image_show_fields": 0, + "image_url": "", + "image_position": "below", + "image_alt": "" + }, + { + "text": "42", + "image_show_fields": 0, + "image_url": "", + "image_position": "below", + "image_alt": "" + } + ], + "correct_answer": 1, + "correct_rationale": { + "text": "Correct rationale" + }, + "algo": { + "num_responses": "#", + "name": "simple" + }, + "seeds": [ + { + "answer": 0, + "rationale": "Seeded Rationale 1" + }, + { + "answer": 1, + "rationale": "Seeded Rationale 2" + } + ], + "flag_inappropriate_threshold": "abc" + }, + "error": { + "error": { + "options_error": "Invalid Option(s): Not an Integer" + } + } } } diff --git a/ubcpi/test/test_answer_pool.py b/ubcpi/test/test_answer_pool.py index 2d6547b..f680634 100644 --- a/ubcpi/test/test_answer_pool.py +++ b/ubcpi/test/test_answer_pool.py @@ -70,14 +70,14 @@ def test_validate_seeded_answers_simple(self): @patch( 'ubcpi.persistence.get_answers_for_student', - return_value=Answers([{VOTE_KEY: 0, RATIONALE_KEY: 'my rationale'}]) + return_value=Answers([{VOTE_KEY: 0, RATIONALE_KEY: 'my rationale'}], 'submission_uuid_1') ) @file_data('data/get_other_answers_simple.json') def test_get_other_answers_simple(self, data, mock): student_item_dict_func = MagicMock(return_value={'student_id': data['user_id']}) with patch('random.choice', side_effect=data['choice_result']): result = get_other_answers_simple( - data['pool'], data['seeds'], student_item_dict_func, data['num_responses'] + data['pool'], data['seeds'], student_item_dict_func, data['num_responses'], data['inappropriate_answers'] ) # check the answers self.assertEqual(result, data['expect']) @@ -95,7 +95,7 @@ def test_random_algo_insert_one(self): @patch( 'ubcpi.persistence.get_answers_for_student', - return_value=Answers([{VOTE_KEY: 0, RATIONALE_KEY: 'my rationale'}]) + return_value=Answers([{VOTE_KEY: 0, RATIONALE_KEY: 'my rationale'}], 'submission_uuid_1') ) @file_data('data/get_other_answers_random.json') def test_get_other_answers_random(self, data, mock): @@ -108,7 +108,7 @@ def side_effect(student_pool): with patch('random.shuffle', side_effect=side_effect): result = get_other_answers_random( - data['pool'], data['seeds'], student_item_dict_func, data['num_responses'] + data['pool'], data['seeds'], student_item_dict_func, data['num_responses'], data['inappropriate_answers'] ) # check the answers self.assertEqual(result, data['expect']) @@ -135,15 +135,15 @@ def test_validate_seeded_answers(self): def test_get_other_answers(self): with patch('ubcpi.answer_pool.get_other_answers_simple') as mock: - get_other_answers({}, {}, {}, {'name': 'simple', 'num_responses': '#'}, ['option1', 'option2']) - self.assertEqual(mock.mock_calls, [call({}, {}, {}, 2)]) + get_other_answers({}, {}, {}, {'name': 'simple', 'num_responses': '#'}, ['option1', 'option2'], set()) + self.assertEqual(mock.mock_calls, [call({}, {}, {}, 2, set())]) with patch('ubcpi.answer_pool.get_other_answers_random') as mock: - get_other_answers({}, {}, {}, {'name': 'random', 'num_responses': '1'}, ['option1', 'option2']) - self.assertEqual(mock.mock_calls, [call({}, {}, {}, 1)]) + get_other_answers({}, {}, {}, {'name': 'random', 'num_responses': '1'}, ['option1', 'option2'], set()) + self.assertEqual(mock.mock_calls, [call({}, {}, {}, 1, set())]) with self.assertRaises(UnknownChooseAnswerAlgorithm): - get_other_answers({}, {}, {}, {'name': 'invalid'}, {}) + get_other_answers({}, {}, {}, {'name': 'invalid'}, {}, set()) def test_offer_answer_invalid_algo(self): options = ['optionA', 'optionB', 'optionC'] diff --git a/ubcpi/test/test_flag_inappropriate.py b/ubcpi/test/test_flag_inappropriate.py new file mode 100644 index 0000000..92f1b2d --- /dev/null +++ b/ubcpi/test/test_flag_inappropriate.py @@ -0,0 +1,144 @@ +import os +import json + +from ddt import ddt, file_data +from django.test import TestCase +from mock import patch, Mock +from workbench.test_utils import scenario, XBlockHandlerTestCaseMixin + + + +@ddt +class TestFlagInappropriate(XBlockHandlerTestCaseMixin, TestCase): + + @file_data('data/flag_inappropriate.json') + @scenario(os.path.join(os.path.dirname(__file__), 'data/basic_scenario.xml'), user_id='Bob') + def test_flag_seed(self, xblock, data): + resp = self.request(xblock, 'flag_inappropriate', json.dumps(data['flag_seed']['request']), response_format='json') + self.assertEqual(resp, data['flag_seed']['expect']) + + @patch('ubcpi.ubcpi.get_other_answers') + @file_data('data/flag_inappropriate.json') + @scenario(os.path.join(os.path.dirname(__file__), 'data/basic_scenario.xml'), user_id='Bob') + def test_flag_others(self, xblock, data, mock): + mock.return_value = data['flag_others']['dummy_get_other_answers'] + + resp = self.request(xblock, 'submit_answer', json.dumps(data['flag_others']['submit_answer_request']), response_format='json') + self.assertEqual(resp, data['flag_others']['submit_answer_expect']) + + resp = self.request(xblock, 'flag_inappropriate', json.dumps(data['flag_others']['flag_inappropriate_request']), response_format='json') + self.assertEqual(resp, data['flag_others']['flag_inappropriate_expect']) + + @patch('ubcpi.ubcpi.PeerInstructionXBlock.is_course_staff') + @file_data('data/flag_inappropriate.json') + @scenario(os.path.join(os.path.dirname(__file__), 'data/basic_scenario.xml'), user_id='Bob') + def test_get_pool_status(self, xblock, data, mock): + # pretend as staff + mock.return_value = True + + # submit a new answer + resp = self.request(xblock, 'submit_answer', json.dumps(data['get_pool_status']['submit_answer_request']), response_format='json') + # should be in the pool + resp = self.request(xblock, 'get_pool_status', json.dumps(data['get_pool_status']['get_pool_status_request']), response_format='json') + # each answer in pool has an unique id + unique_id_seen = set() + for ans in resp: + self.assertNotIn(ans['id'], unique_id_seen) + unique_id_seen.add(ans['id']) + del ans['id'] + + self.assertEqual(resp, data['get_pool_status']['get_pool_status_expect']) + + # pretend not a staff + mock.return_value = False + resp = self.request(xblock, 'get_pool_status', json.dumps(data['get_pool_status']['get_pool_status_request'])) + # TODO should find ways to test for 403 reponse code + self.assertEqual(len(resp), 0) + + @patch('ubcpi.ubcpi.PeerInstructionXBlock.is_course_staff') + @file_data('data/flag_inappropriate.json') + @scenario(os.path.join(os.path.dirname(__file__), 'data/basic_scenario.xml'), user_id='Bob') + def test_staff_toggle_inappropriate(self, xblock, data, mock): + # submit a new answer + resp = self.request(xblock, 'submit_answer', json.dumps(data['staff_toggle_inappropriate']['submit_answer_request']), response_format='json') + # should be in the pool + resp = self.request(xblock, 'get_pool_status', json.dumps(data['staff_toggle_inappropriate']['get_pool_status_request']), response_format='json') + + # each answer in pool has an unique id + unique_id_seen = set() + for ans in resp: + self.assertNotIn(ans['id'], unique_id_seen) + unique_id_seen.add(ans['id']) + del ans['id'] + self.assertEqual(resp, data['staff_toggle_inappropriate']['get_pool_status_expect_before']) + + # pretend not a staff + mock.return_value = False + # for each answer in the pool, trying to mark inappropriate should fail + for the_id in unique_id_seen: + data['staff_toggle_inappropriate']['staff_toggle_inappropriate_request_set']['id'] = the_id + resp = self.request(xblock, 'staff_toggle_inappropriate', json.dumps(data['staff_toggle_inappropriate']['staff_toggle_inappropriate_request_set'])) + self.assertTrue(resp == '') + + # pretend as staff + mock.return_value = True + + # for each answer in the pool, mark as inappropriate + for the_id in unique_id_seen: + data['staff_toggle_inappropriate']['staff_toggle_inappropriate_request_set']['id'] = the_id + resp = self.request(xblock, 'staff_toggle_inappropriate', json.dumps(data['staff_toggle_inappropriate']['staff_toggle_inappropriate_request_set']), response_format='json') + # verify that the specific id is now set and inappropriate + for ans in resp: + if ans['id'] == the_id: + self.assertTrue(ans['considered_inappropriate']) + # check pool status again. all answer should be marked as inappropriate + resp = self.request(xblock, 'get_pool_status', json.dumps(data['staff_toggle_inappropriate']['get_pool_status_request']), response_format='json') + for ans in resp: + self.assertIn(ans['id'], unique_id_seen) + del ans['id'] + self.assertEqual(resp, data['staff_toggle_inappropriate']['get_pool_status_expect_after']) + + # for each answer in the pool, unset inappropriate flag + for the_id in unique_id_seen: + data['staff_toggle_inappropriate']['staff_toggle_inappropriate_request_unset']['id'] = the_id + resp = self.request(xblock, 'staff_toggle_inappropriate', json.dumps(data['staff_toggle_inappropriate']['staff_toggle_inappropriate_request_unset']), response_format='json') + # verify that the specific id is now set and inappropriate + for ans in resp: + if ans['id'] == the_id: + self.assertFalse(ans['considered_inappropriate']) + # check pool status again. all answer should be considered appropriate + resp = self.request(xblock, 'get_pool_status', json.dumps(data['staff_toggle_inappropriate']['get_pool_status_request']), response_format='json') + for ans in resp: + self.assertIn(ans['id'], unique_id_seen) + del ans['id'] + self.assertEqual(resp, data['staff_toggle_inappropriate']['get_pool_status_expect_before']) + + # for each answer in the pool, mark as appropriate + for the_id in unique_id_seen: + data['staff_toggle_inappropriate']['staff_toggle_inappropriate_request_set_appropriate']['id'] = the_id + resp = self.request(xblock, 'staff_toggle_inappropriate', json.dumps(data['staff_toggle_inappropriate']['staff_toggle_inappropriate_request_set_appropriate']), response_format='json') + # verify that the specific id is now set and inappropriate + for ans in resp: + if ans['id'] == the_id: + self.assertFalse(ans['considered_inappropriate']) + # check pool status again. all answer should be considered appropriate + resp = self.request(xblock, 'get_pool_status', json.dumps(data['staff_toggle_inappropriate']['get_pool_status_request']), response_format='json') + for ans in resp: + self.assertIn(ans['id'], unique_id_seen) + del ans['id'] + self.assertEqual(resp, data['staff_toggle_inappropriate']['get_pool_status_expect_force_appropriate']) + + # for each answer in the pool, unset for appropriate + for the_id in unique_id_seen: + data['staff_toggle_inappropriate']['staff_toggle_inappropriate_request_unset']['id'] = the_id + resp = self.request(xblock, 'staff_toggle_inappropriate', json.dumps(data['staff_toggle_inappropriate']['staff_toggle_inappropriate_request_unset']), response_format='json') + # verify that the specific id is now set and inappropriate + for ans in resp: + if ans['id'] == the_id: + self.assertFalse(ans['considered_inappropriate']) + # check pool status again. all answer should be considered appropriate + resp = self.request(xblock, 'get_pool_status', json.dumps(data['staff_toggle_inappropriate']['get_pool_status_request']), response_format='json') + for ans in resp: + self.assertIn(ans['id'], unique_id_seen) + del ans['id'] + self.assertEqual(resp, data['staff_toggle_inappropriate']['get_pool_status_expect_before']) diff --git a/ubcpi/test/test_lms.py b/ubcpi/test/test_lms.py index 7a86129..4d642ff 100644 --- a/ubcpi/test/test_lms.py +++ b/ubcpi/test/test_lms.py @@ -111,9 +111,12 @@ def test_get_asset(self, xblock, mock): @scenario(os.path.join(os.path.dirname(__file__), 'data/basic_scenario.xml'), user_id='Bob') def test_get_student_item_dict(self, xblock): student_item = xblock.get_student_item_dict() + # the item_id is in the format of '.ubcpi.d#.u0' where # is based on how many test cases had run + self.assertTrue(student_item['item_id'].startswith('.ubcpi.d')) + self.assertTrue(student_item['item_id'].endswith('.u0')) self.assertEqual(student_item, { 'student_id': 'Bob', - 'item_id': '.ubcpi.d3.u0', + 'item_id': student_item['item_id'], # checked the format above 'course_id': 'edX/Enchantment_101/April_1', 'item_type': 'ubcpi' }) @@ -196,6 +199,37 @@ def test_get_asset_url(self, xblock): '/c4x://test/course/cat.jpg', 'in edx env, it should return converted asset URL') + @patch('ubcpi.ubcpi.get_other_answers') + @file_data('data/submit_answer.json') + @scenario(os.path.join(os.path.dirname(__file__), 'data/basic_scenario.xml'), user_id='Bob') + def test_clear_student_state_hook(self, xblock, data, mock): + # patch get_other_answers to avoid randomness + mock.return_value = data['expect1']['other_answers'] + resp = self.request(xblock, 'submit_answer', json.dumps(data['post1']), response_format='json') + self.assertEqual(resp, data['expect1']) + + # check the student is recorded + answers = xblock.get_answers_for_student() + self.assertTrue(answers.has_revision(0)) + self.assertFalse(answers.has_revision(1)) + self.assertEqual(answers.get_rationale(0), data['post1']['rationale']) + + # check the stats that we have 1 answer + self.assertEqual(xblock.stats['original'][data['post1']['q']], 1) + + # Check the data is persisted + persisted = xblock.get_persisted_data(data['expect1']['other_answers']) + self.assertEquals(persisted['answer_original'], 0) + self.assertFalse( 'correct_answer' in persisted ) + + # simulate a call to the hook to clear student state + student_item = xblock.get_student_item_dict() + xblock.clear_student_state(student_item['student_id'], student_item['course_id'], student_item['item_id'], 'dummy_user') + # check the student state is removed + answers = xblock.get_answers_for_student() + self.assertFalse(answers.has_revision(0)) + self.assertFalse(answers.has_revision(1)) + def test_truncate_rationale(self): short_rationale = 'This is a rationale' truncated_rationale, was_truncated = truncate_rationale(short_rationale) diff --git a/ubcpi/test/test_studio.py b/ubcpi/test/test_studio.py index 459c796..983af8e 100644 --- a/ubcpi/test/test_studio.py +++ b/ubcpi/test/test_studio.py @@ -46,5 +46,5 @@ def test_parse_xml(self, data): def check_fields(self, xblock, data): for key, value in data.iteritems(): - self.assertIsNotNone(getattr(xblock, key)) + # self.assertIsNotNone(getattr(xblock, key)) self.assertEqual(getattr(xblock, key), value) diff --git a/ubcpi/ubcpi.py b/ubcpi/ubcpi.py index 6f7ca72..135e1a4 100644 --- a/ubcpi/ubcpi.py +++ b/ubcpi/ubcpi.py @@ -3,6 +3,7 @@ import random from copy import deepcopy import uuid +import time from django.core.exceptions import PermissionDenied from django.conf import settings @@ -18,7 +19,7 @@ from xblockutils.publish_event import PublishEventMixin from .utils import _ # pylint: disable=unused-import -from answer_pool import offer_answer, validate_seeded_answers, get_other_answers +from answer_pool import offer_answer, validate_seeded_answers, get_other_answers, SEED_EXPLANATION_ID import persistence as sas_api from serialize import parse_from_xml, serialize_to_xml @@ -69,6 +70,11 @@ def validate_options(options): errors.append(_('Number of Responses')) except ValueError: errors.append(_('Not an Integer')) + try: + if int(options['flag_inappropriate_threshold']) < 0: + errors.append(_('Threshold for Inappropriate Explanation')) + except ValueError: + errors.append(_('Not an Integer')) if not errors: return None @@ -281,6 +287,17 @@ class PeerInstructionXBlock(XBlock, MissingDataFetcherMixin, PublishEventMixin): scope=Scope.settings ) + flagged_answers = Dict( + default={}, scope=Scope.user_state_summary, + help=_("Stores answers flagged by students as inappropriate") + ) + + flag_inappropriate_threshold = Integer( + default=0, scope=Scope.settings, + help=_("Defines the threshold for removing a student explanation from " + "the pool after being reported as inappropriate. Set to 0 to disable reporting feature.") + ) + def has_dynamic_children(self): """ Do we dynamically determine our children? No, we don't have any. @@ -371,6 +388,7 @@ def studio_view(self, context=None): }, 'seeds': self.seeds, 'lang': translation.get_language(), + 'flag_inappropriate_threshold': self.flag_inappropriate_threshold, }) return frag @@ -396,6 +414,7 @@ def studio_submit(self, data, suffix=''): self.correct_rationale = data['correct_rationale'] self.algo = data['algo'] self.seeds = data['seeds'] + self.flag_inappropriate_threshold = data['flag_inappropriate_threshold'] return {'success': 'true'} @@ -548,6 +567,26 @@ def student_view(self, context=None): return frag + def _submission_considered_inappropriate(self, submission_uuid): + """ + Checked whether a submission is considered as inappropriate: + - marked as inappropriate by instructional team + - down vote by student is over the threshold + """ + # Threshold set to zero. All answers are considered appropriate + if self.flag_inappropriate_threshold == 0: + return False + + flagged_item = self.flagged_answers.get(submission_uuid, {}) + + if flagged_item.get('staff_set_inappropriate', None) is not None: + return flagged_item.get('staff_set_inappropriate') + + if len(flagged_item.get('reported_by', {})) >= self.flag_inappropriate_threshold: + return True + + return False + def record_response(self, answer, rationale, status): """ Store response from student to the backend @@ -587,7 +626,10 @@ def record_response(self, answer, rationale, status): self.sys_selected_answers, answer, rationale, student_item['student_id'], self.algo, self.options) self.other_answers_shown = get_other_answers( - self.sys_selected_answers, self.seeds, self.get_student_item_dict, self.algo, self.options) + self.sys_selected_answers, self.seeds, self.get_student_item_dict, + self.algo, self.options, + {k for k,v in self.flagged_answers.iteritems() \ + if self._submission_considered_inappropriate(k)}) event_dict['other_student_responses'] = self.other_answers_shown self.publish_event_from_dict( self.event_namespace + '.original_submitted', @@ -631,6 +673,10 @@ def get_current_stats(self): } return self.stats + def is_course_staff(self): + """Returns True if requestor is part of the course staff""" + return getattr(self.xmodule_runtime, 'user_is_staff', False) + @XBlock.json_handler def get_stats(self, data, suffix=''): """ @@ -666,13 +712,17 @@ def get_persisted_data(self, other_answers): "rationale_original": answers.get_rationale(0), "answer_revised": answers.get_vote(1), "rationale_revised": answers.get_rationale(1), + "can_report_inappropriate": self.flag_inappropriate_threshold > 0, } if answers.has_revision(0) and not answers.has_revision(1): # If no persisted peer answers, generate new ones. # Could happen if a student completed Step 1 before ubcpi upgraded to persist peer answers. if not other_answers: ret['other_answers'] = get_other_answers( - self.sys_selected_answers, self.seeds, self.get_student_item_dict, self.algo, self.options) + self.sys_selected_answers, self.seeds, + self.get_student_item_dict, self.algo, self.options, + {k for k,v in self.flagged_answers.iteritems() \ + if self._submission_considered_inappropriate(k)}) else: ret['other_answers'] = other_answers @@ -725,6 +775,84 @@ def validate_form(self, data, suffix=''): msg.update(options_msg) raise JsonHandlerError(400, msg) + @XBlock.json_handler + def flag_inappropriate(self, data, suffix=''): + """ + Students report inappropriate answers + """ + if 'id' not in data: + raise JsonHandlerError(400, 'Missing ID') + the_id = str(data['id']) + # No flagging of seed explanation + if the_id == SEED_EXPLANATION_ID: + return {'success': 'true'} + + student_item = self.get_student_item_dict() + if not student_item or not student_item['student_id']: + raise JsonHandlerError(400, 'Missing student info') + if (not self.flag_inappropriate_threshold) or self.flag_inappropriate_threshold <= 0: + return { 'success': 'false', 'msg': _('Threshold less than or equal to zero. No reporting allowed.') } + + # To prevent this API from being abused, only allow flagging of answers shown before + if (not self.other_answers_shown) or \ + the_id not in (ans.get('id', '') for ans in self.other_answers_shown.get('answers', [])): + return { 'success': 'false', 'msg': _('You cannot flag this answer as inappropriate.') } + + flagged = self.flagged_answers.setdefault(the_id, {'reported_by': {}, 'staff_set_inappropriate': None}) + reported_by = flagged.setdefault('reported_by', {}) + + reported_by.setdefault(str(student_item['student_id']), time.time()) + return {'success': 'true'} + + @XBlock.json_handler + def staff_toggle_inappropriate(self, data, suffix=''): + """ + Instructional teams override inappropriate answers + """ + if not self.is_course_staff(): + return Response(status=403) + + the_id = str(data['id']) + inappropriate = str(data['considered_inappropriate']) + if the_id != SEED_EXPLANATION_ID: + flagged = self.flagged_answers.setdefault(the_id, {'reported_by': {}, 'staff_set_inappropriate': None}) + if inappropriate == 'true': + flagged['staff_set_inappropriate'] = True + elif inappropriate == 'false': + flagged['staff_set_inappropriate'] = False + else: + flagged['staff_set_inappropriate'] = None + + return self._pool_status() + + def _pool_status(self): + result = [] + for option in self.sys_selected_answers: + for student_id in self.sys_selected_answers[option]: + student_item = self.get_student_item_dict(student_id) + ans = sas_api.get_answers_for_student(student_item) + submission_uuid = ans.get_submission_uuid() + if str(option) == str(ans.get_vote(0)): + result.append(dict( + option=option, + id=submission_uuid, + explanation=ans.get_rationale(0), + inappropriate_report_count=len(self.flagged_answers.get(submission_uuid, {}).get('reported_by', {})), + considered_inappropriate=self._submission_considered_inappropriate(submission_uuid), + staff_set_inappropriate=self.flagged_answers.get(submission_uuid, {}).get('staff_set_inappropriate', None), + )) + return result + + @XBlock.json_handler + def get_pool_status(self, data, suffix=''): + """ + Retrieve explanation in the pool + """ + if not self.is_course_staff(): + return Response(status=403) + + return self._pool_status() + @classmethod def workbench_scenarios(cls): # pragma: no cover """A canned scenario for display in the workbench."""