From ffd2dbbaab653d245cc17d050c7e2f9e10cd866a Mon Sep 17 00:00:00 2001 From: Clarence Ho Date: Wed, 11 Apr 2018 15:54:11 -0700 Subject: [PATCH 1/3] Improve visual consistency - Revise UI to improve visual consistency - Fix problem of response missing when switching between units - Add test cases for new chart format - Generate and return peer answers on-the-fly if nothing persisted --- ubcpi/static/css/ubcpi.css | 102 +++++- ubcpi/static/html/ubcpi.html | 312 ++++++++---------- ubcpi/static/js/spec/d3-pibar_spec.js | 93 ++++++ .../js/spec/ubcpi-barchar-directive_spec.js | 147 +++++++++ ubcpi/static/js/spec/ubcpi_spec.js | 1 - ubcpi/static/js/src/d3-pibar.js | 136 ++++++++ .../static/js/src/ubcpi-barchart-directive.js | 55 +++ ubcpi/static/js/src/ubcpi.js | 47 ++- ubcpi/ubcpi.py | 8 +- 9 files changed, 682 insertions(+), 219 deletions(-) diff --git a/ubcpi/static/css/ubcpi.css b/ubcpi/static/css/ubcpi.css index 1740057..2d153c0 100644 --- a/ubcpi/static/css/ubcpi.css +++ b/ubcpi/static/css/ubcpi.css @@ -25,11 +25,6 @@ h2.question-text { padding: 0; } -.choicegroup fieldset .ubcpi-possible-options { - padding-left: 20px; - border-left: 2px solid #e5e5e5; -} - div.course-wrapper section.course-content fieldset .ubcpi-option { margin: 0; } @@ -77,6 +72,20 @@ fieldset .ubcpi-option:last-of-type .ubcpi-label { float: none; } +.ubcpi-no-pointer { + cursor: default; +} + +.ubcpi-answer img { + display: block; +} + +.ubcpi-label.ubcpi-explain-label { + border: none; + margin-bottom: 0; + padding: 0px; +} + .results-container .ubcpi-label { margin-bottom: 15px; } @@ -116,7 +125,8 @@ textarea.ubcpi-field { -webkit-font-smoothing: antialiased; } -.choicegroup .ubcpi_submit { +.choicegroup .ubcpi_submit, +.others-responses .ubcpi_submit { padding: 10px 40px; border: 1px solid #2b8dbb; color: #fff; @@ -140,7 +150,10 @@ textarea.ubcpi-field { .ubcpi_block .choicegroup .ubcpi_submit:hover, .ubcpi_block .choicegroup .ubcpi_submit:active, -.ubcpi_block .choicegroup .ubcpi_submit:focus { +.ubcpi_block .choicegroup .ubcpi_submit:focus, +.others-responses .ubcpi_submit:hover, +.others-responses .ubcpi_submit:active, +.others-responses .ubcpi_submit:focus { border-color: #00a7f6; box-shadow: none; background: #00a7f6 none; @@ -178,8 +191,30 @@ div.course-wrapper section.course-content .vert-mod > div ul.ubcpi-other-answers border-bottom: 1px solid #cfc6c6; } -div.course-wrapper section.course-content .vert-mod > div ul.ubcpi-other-answers li:last-of-type { - border-bottom: none; +div.course-wrapper section.course-content .vert-mod .sample-answer-list > :not(:first-child) { + border-top: 1px solid #cfc6c6; +} + +div.course-wrapper section.course-content .vert-mod .sample-answer { + margin: 1em; + display: block; +} + +div.course-wrapper section.course-content .vert-mod .no-sample-answer { + font-style: italic; + margin: 1em; + display: block; +} + +div.course-wrapper section.course-content .vert-mod .sample-answer .other-rationale { + line-height: 140%; + padding-left: 1em; +} + +div.course-wrapper section.course-content .vert-mod .own-answer { + padding-top: 15px; + margin: 1em; + font-weight: bold; } .ubcpi-other-answers, @@ -198,8 +233,15 @@ div.course-wrapper section.course-content .vert-mod > div ul.ubcpi-other-answers } .ubcpi-other-answers h4, -.ubcpi-solution-your-initial-answer, +.ubcpi-solution-your-initial-answer { + font-weight: 900; +} + .ubcpi-solution-your-final-answer { + border-top: 1px solid #ddd; + margin-top: 10px; + padding-top: 5px; + font-size: 0.85em; font-weight: 900; } @@ -385,7 +427,6 @@ textarea.pi-options { position: relative; margin: 20px 0; border: 1px solid #d7dbdf; - border-left: 10px solid #b9c1c8; box-shadow: inset 0 1px 2px 1px rgba(2, 2, 3, 0.1); padding: 20px; background: #fdfdfd; @@ -499,8 +540,26 @@ label.ubcpi-label { fill: #50c67b; } -.ubcpibar:hover { - opacity: 1.0; +.ubcpibar.original { + opacity: 0.4; +} + +text.ubcpibar { + font-size: 12px; + font-weight: 600; + fill: #000000; +} + +text.ubcpibar.correct-answer { + font-size: 12px; + font-weight: 600; + fill: #000000; +} + +text.ubcpibar.label { + font-size: 12px; + font-weight: 600; + fill: #000000; } .ubcpi-correct-answer-option { @@ -509,6 +568,11 @@ label.ubcpi-label { } .ubcpi-show-correct { + font-size: 0.85em; +} + +.ubcpi-correct-answer-highlight { + font-weight: bold; color: #50c67b; } @@ -525,11 +589,13 @@ label.ubcpi-label { .ubcpi-correct-answer-rationale { display: block; margin-bottom: 20px; + font-size: 0.85em; } .ubcpi-solution-rationales { padding-left: 10px; word-break: break-all; + font-weight: normal; } .other-rationale { @@ -630,6 +696,12 @@ div.course-wrapper section.course-content .warning-notice p { font-style: italic; } +#decision-prompt { + font-weight: normal; + font-size: 120%; + padding-right: 4.8em; +} + .response-text { padding-left: 22px; } @@ -780,4 +852,6 @@ pi-barchart > svg { text-transform: capitalize; } - +#not-enough-data { + font-size: 0.85em; +} diff --git a/ubcpi/static/html/ubcpi.html b/ubcpi/static/html/ubcpi.html index 73a35d1..3340585 100644 --- a/ubcpi/static/html/ubcpi.html +++ b/ubcpi/static/html/ubcpi.html @@ -5,59 +5,36 @@

{{display_name}}

-
-
 ({{weight}} point possible)
-
 ({{weight}} points possible)
+
+
({{weight}} point possible)
+
({{weight}} points possible)

Question

- {{question_text.image_alt}} + {{question_text.image_alt}} - + - {{question_text.image_alt}} + {{question_text.image_alt}}
- + +
- +
    -

  1. Answer, Completed
    2. +

  2. Answer, In Progress
    3. -

  3. Reflection, In Progress
    4. +

  4. Reflection, Upcoming

  5. Results, Upcoming
 -
- -
- - Step 2) Read Other Students Answers -

These are samples of other student answers for this question. Read them and then compare with your answer below.

- -
    -
  • - {{options[answer.option].image_alt}} - -
    Student Answer: {{options[answer.option].text}}
     - - {{options[answer.option].image_alt}} - Student Rationale - "{{answer.rationale}}" - -
    • -
- -
- -
- - +
    -

  1. Answer, In Progress
    2. +

  2. Answer, Completed
    3. -

  3. Reflection, Upcoming
    4. +

  4. Reflection, In Progress

  5. Results, Upcoming
@@ -68,69 +45,106 @@

{{display_n
- Step 1) Your Initial Answer You can change this answer later, if you change your mind. - Step 3) Your Final Answer You now have the option to change your initial selection and explanation, if you wish. + Step 1) Give Initial Answer You can change this answer later, if you change your mind. + Step 2) Read Other Student Answers +

These are samples of other student answers for this question. Read them and then compare with your answer below.

+ -
-

-

+
+ -

+
- - -

You are approaching the limit of {{rationale_size.max}} characters for this answer. Characters left: {{rationale_size.max - rc.rationale.length}}

-

Error: Please edit your explanation so that it is less than {{rationale_size.max}} characters.

- -

- - Hint In the next step, you will be shown a selection of other responses that may help you refine your answer. - -

- -
-
- -
-
-
- View Question Statistics -
- -
- -

Original Answer

-
- -
- -

Revised Answer

-
+
+ View Question Statistics +
+ +
+
+ + + +
-
- +
+
@@ -139,24 +153,22 @@

{{display_n

{{display_name}}

-
- -
({{weight}} point possible)
- -
({{weight}} points possible)
+
+
({{weight}} point possible)
+
({{weight}} points possible)

Question

- {{question_text.image_alt}} + {{question_text.image_alt}} - + - {{question_text.image_alt}} + {{question_text.image_alt}}
-
- +
+

  1. Answer, Completed
    2. @@ -167,100 +179,50 @@

    {{display_n

+
-
Correct Answer
-
-
Instructor's Explanation
+ + Step 3) View Course-Wide Results +

This is a look at how your classmates answered the question initially and after revision.

+ + -
- {{options[rc.correct_answer].image_alt}} + {{rc.correct_rationale.text}} -

{{options[rc.correct_answer].text}}

+
+
+
+ Correct Answer: + {{option.text}} - {{rc.correct_rationale.text}} - -
-
    -
  • -

    - Your initial answer: - {{options[rc.answer_original].text}} - {{options[rc.answer_original].text}} - {{options[rc.answer_original].text}} - ({{'Option' | translate }} {{rc.answer_original + 1}}) -

    - Student Rationale"{{rc.rationale_original}}" -
    • -
  • + {{option.image_alt}} + + + {{rc.correct_rationale.text}} + + + +

    - Your final answer: - {{options[rc.answer_revised].text}} - {{options[rc.answer_revised].text}} - {{options[rc.answer_revised].text}} - ({{'Option' | translate }} {{rc.answer_revised + 1}}) + Your final explanation: + "{{rc.rationale_revised}}"

    - Student Rationale"{{rc.rationale_revised}}" -
    • -
-

See how your classmates answered below

+
+
+
-
- - -
Class Breakdown
-

This is a look at how your classmates answered the question during the initial and final rounds.

-
Answer Options
-
- -

- {{option.image_alt}} - - Option {{$index + 1}} (correct) - {{option.text}} - {{rc.calc($index)}} - {{option.text}} - {{rc.calc($index)}} - - {{option.image_alt}} - - (You chose this option) -

- -
- -
-

-

  • - Option {{$index + 1}}: {{rc.calc($index)}} - (correct) -
    • -
    - -
    Initial Answer Selection
    - -
    - -
    Final Answer Selection
    - - -
    -
    diff --git a/ubcpi/static/js/spec/d3-pibar_spec.js b/ubcpi/static/js/spec/d3-pibar_spec.js index 16fab60..9452f6a 100644 --- a/ubcpi/static/js/spec/d3-pibar_spec.js +++ b/ubcpi/static/js/spec/d3-pibar_spec.js @@ -105,3 +105,96 @@ describe('D3 bar chart', function () { // expect(line.empty()).not.toBe(true); //}); }); + + +describe('D3 per answer chart', function () { + var chart; + var chartContainer; + + describe('instructor view', function(){ + + beforeEach(function() { + var scope = {role:'instructor'}; + chart = d3.custom.perAnswerChart(scope); + chartContainer = d3.select('body') + .append('div') + .attr('class', 'testContainer'); + }); + + afterEach(function() { + // clean up + chartContainer.remove(); + }); + + it('should provide getters and setters', function() { + var defaultChartWidth = chart.width(); + var defaultChartHeight = chart.height(); + var defaultMinTotalFrequency = chart.minTotalFrequency(); + + chart.width(100) + .height(50) + .minTotalFrequency(20); + + var newChartWidth = chart.width(); + var newChartHeight = chart.height(); + var newMinTotalFrequency = chart.minTotalFrequency(); + + + expect(defaultChartWidth).not.toBe(100); + expect(defaultChartHeight).not.toBe(50); + expect(defaultMinTotalFrequency).not.toBe(20); + expect(newChartWidth).toBe(100); + expect(newChartHeight).toBe(50); + expect(newMinTotalFrequency).toBe(20); + }); + + + it('should have a minTotalFrequency of 1', function(){ + var defaultMinTotalFrequency = chart.minTotalFrequency(); + expect(defaultMinTotalFrequency).toBe(1); + }); + }); + + describe('student view', function(){ + + beforeEach(function() { + var scope = {role:'student'}; + chart = d3.custom.barChart(scope); + chartContainer = d3.select('body') + .append('div') + .attr('class', 'testContainer'); + }); + + afterEach(function() { + // clean up + chartContainer.remove(); + }); + + it('should provide getters and setters', function() { + var defaultChartWidth = chart.width(); + var defaultChartHeight = chart.height(); + var defaultMinTotalFrequency = chart.minTotalFrequency(); + + chart.width(100) + .height(50) + .minTotalFrequency(20); + + var newChartWidth = chart.width(); + var newChartHeight = chart.height(); + var newMinTotalFrequency = chart.minTotalFrequency(); + + + expect(defaultChartWidth).not.toBe(100); + expect(defaultChartHeight).not.toBe(50); + expect(defaultMinTotalFrequency).not.toBe(20); + expect(newChartWidth).toBe(100); + expect(newChartHeight).toBe(50); + expect(newMinTotalFrequency).toBe(20); + }); + + it('should have a minTotalFrequency of 10', function(){ + var defaultMinTotalFrequency = chart.minTotalFrequency(); + expect(defaultMinTotalFrequency).toBe(10); + }); + }); +}); diff --git a/ubcpi/static/js/spec/ubcpi-barchar-directive_spec.js b/ubcpi/static/js/spec/ubcpi-barchar-directive_spec.js index 5039542..ac4cb06 100644 --- a/ubcpi/static/js/spec/ubcpi-barchar-directive_spec.js +++ b/ubcpi/static/js/spec/ubcpi-barchar-directive_spec.js @@ -115,4 +115,151 @@ describe('UBCPI', function () { }) }) }); + + + describe('pi-per-answer-chart', function () { + var element, scope; + + beforeEach(inject(function ($compile, $rootScope) { + scope = $rootScope; + + element = angular.element( + '' + ); + $compile(element)(scope); + scope.$digest(); + })); + + it('should not render anything when stats is empty', function () { + expect(element.find('svg').length).toBe(0); + expect(element.find('g').length).toBe(0); + }); + + describe('directive', function () { + var options = [{ + "text": "21", + "image_alt": "", + "image_url": "", + "image_position": "below", + "image_show_fields": 0 + }, { + "text": "42", + "image_alt": "", + "image_url": "", + "image_position": "below", + "image_show_fields": 0 + }, { + "text": "63", + "image_alt": "", + "image_url": "", + "image_position": "below", + "image_show_fields": 0 + }]; + var answers = {"original": 2, "revised": 1}; + var correct = 0; + + describe('with enough submissions', function() { + var stats = { + "original": {0: 4, 1: 5, 2: 1}, + "revised": {0: 1, 1: 8, 2: 1}, + }; + beforeEach(function() { + scope.$apply(function () { + scope.options = options; + scope.stats = stats; + scope.answers = answers; + scope.correct = correct; + scope.per_answer_stats = 1; + }); + }); + + it('should render the template with given data', function() { + // one graph for given answer per_answer_stats. two bars: one for initial choice, one for revision + expect(element.find('> svg').length).toBe(1); + expect(element.find('> svg > g').length).toBe(2); + }); + + it('should calculate percentage correctly for incorrect answer', function() { + expect(element.find('> svg > g').eq(0).find('> rect').length).toBe(2); + expect( + element.find('> svg > g').eq(0).find('> rect.ubcpibar').attr('width') / + element.find('> svg > g').eq(0).find('> rect:not(.ubcpibar)').attr('width')).toBe(0.5); + expect(element.find('> svg > g').eq(0).find('> svg > text').text()).toContain('50%'); + expect( + element.find('> svg > g').eq(1).find('> rect.ubcpibar').attr('width') / + element.find('> svg > g').eq(1).find('> rect:not(.ubcpibar)').attr('width')).toBe(0.8); + expect(element.find('> svg > g').eq(1).find('> svg > text').text()).toContain('80%'); + }); + }); + + describe('with enough submissions', function() { + var stats = { + "original": {0: 4, 1: 5, 2: 1}, + "revised": {0: 1, 1: 8, 2: 1}, + }; + beforeEach(function() { + scope.$apply(function () { + scope.options = options; + scope.stats = stats; + scope.answers = answers; + scope.correct = correct; + scope.per_answer_stats = correct; + }); + }); + + it('should calculate percentage correctly for correct answer', function() { + expect(element.find('> svg > g').eq(0).find('> rect').length).toBe(2); + expect( + element.find('> svg > g').eq(0).find('> rect.correct-answer').attr('width') / + element.find('> svg > g').eq(0).find('> rect:not(.correct-answer)').attr('width')).toBe(0.4); + expect(element.find('> svg > g').eq(0).find('> svg > text').text()).toContain('40%'); + expect( + element.find('> svg > g').eq(1).find('> rect.correct-answer').attr('width') / + element.find('> svg > g').eq(1).find('> rect:not(.correct-answer)').attr('width')).toBe(0.1); + expect(element.find('> svg > g').eq(1).find('> svg > text').text()).toContain('10%'); + }); + + it('should update when stats changed', function() { + scope.$apply(function() { + scope.stats = { + "original": {0: 10, 1: 6, 2: 4}, + "revised": {0: 4, 1: 14, 2: 2}, + }; + }); + expect(element.find('> svg > g').eq(0).find('> rect').length).toBe(2); + expect( + element.find('> svg > g').eq(0).find('> rect.correct-answer').attr('width') / + element.find('> svg > g').eq(0).find('> rect:not(.correct-answer)').attr('width')).toBe(0.5); + expect(element.find('> svg > g').eq(0).find('> svg > text').text()).toContain('50%'); + expect( + element.find('> svg > g').eq(1).find('> rect.correct-answer').attr('width') / + element.find('> svg > g').eq(1).find('> rect:not(.correct-answer)').attr('width')).toBe(0.2); + expect(element.find('> svg > g').eq(1).find('> svg > text').text()).toContain('20%'); + }); + }); + + describe('with enough submissions and showing stats for user\'s revision', function() { + var stats = { + "original": {0: 4, 1: 5, 2: 1}, + "revised": {0: 1, 1: 8, 2: 1}, + }; + beforeEach(function() { + scope.$apply(function () { + scope.options = options; + scope.stats = stats; + scope.answers = answers; + scope.correct = correct; + scope.per_answer_stats = answers['revised']; + }); + }); + + it('should not indicate it as user\'s initial answer', function() { + expect(element.find('> svg > g').eq(0).find('> svg > text').text()).not.toContain('including you'); + }); + it('should indicate it as user\'s revision', function() { + expect(element.find('> svg > g').eq(1).find('> svg > text').text()).toContain('including you'); + }); + }); + }) + }); }); diff --git a/ubcpi/static/js/spec/ubcpi_spec.js b/ubcpi/static/js/spec/ubcpi_spec.js index cf368ae..7ef0480 100644 --- a/ubcpi/static/js/spec/ubcpi_spec.js +++ b/ubcpi/static/js/spec/ubcpi_spec.js @@ -400,7 +400,6 @@ describe('UBCPI module', function () { }); controller.getStats(); expect(controller.stats).toEqual(response); - expect(controller.calc(0)).toBe(" Initial Answer Selection: 100% Final Answer Selection: 0%"); }); it('should call notify with error when backend errors', function() { diff --git a/ubcpi/static/js/src/d3-pibar.js b/ubcpi/static/js/src/d3-pibar.js index d76bb17..896fac7 100644 --- a/ubcpi/static/js/src/d3-pibar.js +++ b/ubcpi/static/js/src/d3-pibar.js @@ -156,3 +156,139 @@ d3.custom.barChart = function(scope, gettext) { return chart; }; +/** + * Data format: + * [ + * {percentage: 40, order: 0, label: '(including you) initial choise', class: 'ubcpibar'}, + * {percentage: 75, order: 1, label: 'after revision', class: 'ubcpibar'}, + * ] + */ +d3.custom.perAnswerChart = function(scope, gettext, allAnswerCount) { + // Private Variables + var chartWidth = 750; + var chartHeight = 56; + var labelWidth = 150; + var minTotalFrequency = 10; + + if(scope.role == 'instructor' || scope.role == 'staff'){ minTotalFrequency = 1} + + function chart(selection) { + selection.each(function(data) { + var totalFreq = allAnswerCount; + + // sort bars based on given order + data = data.sort(function(a, b) { + return d3.ascending(a.order, b.order); + }); + + if (totalFreq < minTotalFrequency) { + d3.select(this) + .append("span") + .attr("id", 'not-enough-data') + .text(gettext("Not enough data to generate the chart. Please check back later.")); + return; + } else { + var notEnoughDataSpan = d3.select('#not-enough-data'); + if (typeof notEnoughDataSpan !== 'undefined') { + notEnoughDataSpan.remove(); + } + } + + var svg = d3.select(this) + .classed("svg-container", true) + .append("svg") + .attr("preserveAspectRatio", "xMaxYMax meet") + .attr("viewBox", "0 0 " + chartWidth + " " + chartHeight) + .classed("svg-content-responsive", true); + + // x and y scale + var x = d3.scale.linear() + .range([0, chartWidth-labelWidth]) + .domain([0, 100]); + var y = d3.scale.ordinal() + .rangeRoundBands([0, chartHeight], 0.2) + .domain(data.map(function (d) { + return d.type; + })); + + var bars = svg.selectAll("g.bar") + .data(data) + .enter() + .append("g") + .attr("transform", function(d) { + return "translate(0," + y(d.type) + ")"; + }); + + // label of each bar + bars.append("text") + .attr("x", "0em") + .attr("dy", "1.2em") + .attr("text-anchor", "left") + .attr("class", function(d) { + return d.label_class; + }) + .text(function(d) { return d.type; }); + + // background of each bar at 100% full length + bars.append("rect") + .style("fill", "#dddddd") + .attr("x", labelWidth) + .attr("rx", 3) + .attr("ry", 3) + .attr("height", y.rangeBand()) + .attr("width", function(d) { + return x(100); + }); + + // the actual bar based on given percentage + bars.append("rect") + .attr("class", function (d) { + return d.class; + }) + .attr("x", labelWidth) + .attr("rx", 3) + .attr("ry", 3) + .attr("height", y.rangeBand()) + .attr("width", function (d) { + return x(d.percentage); + }); + + // text on each bar + bars.append("svg") + .attr({ height: y.rangeBand() }) + .append("text") + .attr("x", labelWidth+10) + .attr("dy", "1.2em") + .attr("text-anchor", "left") + .attr("class", function(d) { + return d.class; + }) + .text(function(d) { return Math.round(d.percentage) + "% " + d.text; }); + }); + + } + + // Public Variables/ (Getters and Setters) + chart.width = function(width) { + if (!arguments.length) return chartWidth; + chartWidth = width; + + return this; + }; + + chart.height = function(height) { + if (!arguments.length) return chartHeight; + chartHeight = height; + + return this; + }; + + chart.minTotalFrequency = function(min) { + if (!arguments.length) return minTotalFrequency; + minTotalFrequency = min; + + return this; + }; + + return chart; +}; diff --git a/ubcpi/static/js/src/ubcpi-barchart-directive.js b/ubcpi/static/js/src/ubcpi-barchart-directive.js index c88325b..6fb860f 100644 --- a/ubcpi/static/js/src/ubcpi-barchart-directive.js +++ b/ubcpi/static/js/src/ubcpi-barchart-directive.js @@ -37,3 +37,58 @@ angular.module('UBCPI'). } } }]); + + +angular.module('UBCPI'). + directive('piPerAnswerChart', ['gettext', function(gettext){ + return { + restrict: 'E', + scope: { + options: '=', + stats: '=', + correct: '=', + role: '=', + answers: '=', + perAnswerStats: '=' + }, + // no overwrite template + replace: false, + link: function(scope, element) { + // watch the stats as it could be async populated + scope.$watch('stats', function(stats) { + if(!stats) { + return; + } + + var data = []; + var allAnswerCount = 0; + for (var k in stats) { + var total = 0; + for (var op in stats[k]) { + total += stats[k][op]; + } + if (total > allAnswerCount) { + allAnswerCount = total; + } + data.push({ + percentage: (total > 0 && typeof stats[k][scope.perAnswerStats] !== 'undefined'? + (stats[k][scope.perAnswerStats] / total) * 100 : 0), + order: k === 'original'? 0 : 1, + text: (scope.answers[k] == scope.perAnswerStats? gettext('(including you) ') : ''), + class: 'ubcpibar' + (scope.correct == scope.perAnswerStats ? ' correct-answer' : '') + + (k === 'original'? ' original' : ''), + label_class: 'ubcpibar label', + type: k === 'original'? gettext('Chose initially') : gettext('Chose after revision') + }); + } + + d3.select(element[0]).select("svg").remove(); // remove old chart + // generate the chart + var chartLayout = d3.custom.perAnswerChart(scope, gettext, allAnswerCount); + d3.select(element[0]) + .datum(data) + .call(chartLayout); + }, true) + } + } + }]); diff --git a/ubcpi/static/js/src/ubcpi.js b/ubcpi/static/js/src/ubcpi.js index 6087b65..4519aef 100644 --- a/ubcpi/static/js/src/ubcpi.js +++ b/ubcpi/static/js/src/ubcpi.js @@ -151,6 +151,9 @@ angular.module('UBCPI', ['ngSanitize', 'ngCookies', 'gettext']) // By default, we're not submitting, this changes when someone presses the submit button self.submitting = false; + // Whether user is revising the answer + self.revising = false; + /** * Determine if the submit button should be disabled * If we have an answer selected, a rationale that is large @@ -191,6 +194,14 @@ angular.module('UBCPI', ['ngSanitize', 'ngCookies', 'gettext']) self.getStats = function () { return backendService.getStats().then(function(data) { self.stats = data; + + self.perAnswerStats = {}; + for (var i = 0; i < $scope.options.length; i++) { + self.perAnswerStats[i] = { + 'original': (typeof self.stats.original[i] !== 'undefined'? self.stats.original[i] : 0), + 'revised' : (typeof self.stats.revised[i] !== 'undefined'? self.stats.revised[i] : 0) + } + } }, function(error) { notify('error', { 'title': gettext('Error retrieving statistics!'), @@ -200,34 +211,6 @@ angular.module('UBCPI', ['ngSanitize', 'ngCookies', 'gettext']) }); }; - self.calc = function(s) { - var originalPercentage = gettext(" Initial Answer Selection: "); - var revisedPercentage = gettext(" Final Answer Selection: "); - if (typeof self.stats !== 'undefined' && typeof s !== 'undefined' && typeof self.stats.original[s] !== 'undefined') { - var totalCounts = 0; - for (var i = 0; i < data.options.length; i++) { - if (typeof self.stats.original[i] !== 'undefined') - totalCounts += self.stats.original[i]; - } - originalPercentage += self.stats.original[s] / totalCounts * 100 + "%"; - } - else - originalPercentage += "0%"; - - if (typeof self.stats !== 'undefined' && typeof s !== 'undefined' && typeof self.stats.revised[s] !== 'undefined') { - var totalCounts = 0; - for (var i = 0; i < data.options.length; i++) { - if (typeof self.stats.revised[i] !== 'undefined') - totalCounts += self.stats.revised[i]; - } - revisedPercentage += self.stats.revised[s] / totalCounts * 100 + "%"; - } - else - revisedPercentage += "0%"; - - return originalPercentage + " " + revisedPercentage; - }; - function get_data() { return backendService.get_data().then(function(data) { return data; @@ -257,6 +240,14 @@ angular.module('UBCPI', ['ngSanitize', 'ngCookies', 'gettext']) self.options = data.options; } + self.hasSampleExplanationForOption = function (option) { + for (var index in self.other_answers.answers) { + if (option == self.other_answers.answers[index].option) { + return true; + } + } + return false; + }; }]); /** diff --git a/ubcpi/ubcpi.py b/ubcpi/ubcpi.py index a630a85..b560ad3 100644 --- a/ubcpi/ubcpi.py +++ b/ubcpi/ubcpi.py @@ -618,7 +618,13 @@ def get_persisted_data(self, other_answers): "rationale_revised": answers.get_rationale(1), } if answers.has_revision(0) and not answers.has_revision(1): - ret['other_answers'] = other_answers + # 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 self.other_answers_shown: + ret['other_answers'] = get_other_answers( + self.sys_selected_answers, self.seeds, self.get_student_item_dict, self.algo, self.options) + else: + ret['other_answers'] = other_answers # reveal the correct answer in the end if answers.has_revision(1): From f80be56634b0020dc1e374e6ea557afddaf901b2 Mon Sep 17 00:00:00 2001 From: Clarence Ho Date: Wed, 25 Apr 2018 16:48:25 -0700 Subject: [PATCH 2/3] Delete learner's state in staff debug - implement the callback clear_student_state - add a new submission to indicate existing answers are deleted - add (but commented out) code to partially support revising score manually. But full support will require adding logic to persist the score within xblock - fix issue with same explanation displayed multiple times in Step 2 - simplify the Simple algo for picking sample explanation. Instead of using arbitrary max loop count of 100 and hope to pick unique explanations, remove item from the pool once they are chosen. --- ubcpi/answer_pool.py | 70 ++++++++++++++++++++++---------------------- ubcpi/persistence.py | 17 +++++++++++ ubcpi/ubcpi.py | 57 ++++++++++++++++++++++++++++++++++-- 3 files changed, 106 insertions(+), 38 deletions(-) diff --git a/ubcpi/answer_pool.py b/ubcpi/answer_pool.py index f6c32a7..eb61048 100644 --- a/ubcpi/answer_pool.py +++ b/ubcpi/answer_pool.py @@ -227,48 +227,45 @@ def get_other_answers_simple(pool, seeded_answers, get_student_item_dict, num_re ret = [] # clean up answers so that all keys are int pool = {int(k): v for k, v in pool.items()} - total_in_pool = len(seeded_answers) merged_pool = convert_seeded_answers(seeded_answers) student_id = get_student_item_dict()['student_id'] # merge the dictionaries in the answer dictionary for key in pool: - total_in_pool += len(pool[key]) - # if student_id has value, we assume the student just submitted an answer. So removing it - # from total number in the pool - if student_id in pool[key].keys(): - total_in_pool -= 1 - if key in merged_pool: - merged_pool[key].update(pool[key].items()) - else: - merged_pool[key] = pool[key] - - # remember which option+student_id is selected, so that we don't have duplicates in the result - selected = [] + merged_pool.setdefault(key, {}) + merged_pool[key].update(pool[key]) + # Pop student's own answer, if exists + merged_pool[key].pop(student_id, None) - # loop until we have enough answers to return - while len(ret) < min(num_responses, total_in_pool): + # loop until we have enough answers to return or when there is nothing more to return + while len(ret) < num_responses and merged_pool: for option, students in merged_pool.items(): - student = student_id - i = 0 - while (student == student_id or i > 100) and (str(option) + student) not in selected: - # retry until we got a different one or after 100 retries - # we are suppose to get a different student answer or a seeded one in a few tries - # as we have at least one seeded answer for each option in the algo. And it is not - # suppose to overflow i order to break the loop + rationale = None + while students: student = random.choice(students.keys()) - i += 1 - selected.append(str(option)+student) - if student.startswith('seeded'): - # seeded answer, get the rationale from local - rationale = students[student] - else: - student_item = get_student_item_dict(student) - submission = sas_api.get_answers_for_student(student_item) - rationale = submission.get_rationale(0) - ret.append({'option': option, 'rationale': rationale}) + # remove the chosen answer from pool + content = students.pop(student, None) + + if student.startswith('seeded'): + # seeded answer, get the rationale from local + rationale = content + else: + student_item = get_student_item_dict(student) + submission = sas_api.get_answers_for_student(student_item) + # 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) + + if rationale: + ret.append({'option': option, 'rationale': rationale}) + break + + if not students: + del merged_pool[option] # check if we have enough answers - if len(ret) >= min(num_responses, total_in_pool): + if len(ret) >= num_responses: break return {"answers": ret} @@ -316,8 +313,11 @@ def get_other_answers_random(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) - rationale = submission.get_rationale(0) - option = submission.get_vote(0) + if submission.has_revision(0): + rationale = submission.get_rationale(0) + option = submission.get_vote(0) + else: + continue ret.append({'option': option, 'rationale': rationale}) return {"answers": ret} diff --git a/ubcpi/persistence.py b/ubcpi/persistence.py index eec4063..6390dbf 100644 --- a/ubcpi/persistence.py +++ b/ubcpi/persistence.py @@ -17,6 +17,8 @@ """ ANSWER_LIST_KEY = 'answers' +DELETE_INDICATOR = 'deleted' +REQUEST_USER_ID_KEY = 'requesting_user_id' VOTE_KEY = 'vote' RATIONALE_KEY = 'rationale' @@ -39,6 +41,8 @@ def get_answers_for_student(student_item): latest_submission = submissions[0] 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, [])) @@ -59,6 +63,19 @@ def add_answer_for_student(student_item, vote, rationale): ANSWER_LIST_KEY: answers.get_answers_as_list() }) +def delete_answer_for_student(student_item, requesting_user_id): + """ + Create a new submission to indicate student's answer is deleted + + Args: + student_item (dict): The location of the problem this submission is + associated with, as defined by a course, student, and item. + requesting_user_id: The user that is requesting to delete student answer + """ + sub_api.create_submission(student_item, { + DELETE_INDICATOR: True, + REQUEST_USER_ID_KEY: requesting_user_id, + }) class Answers: """ diff --git a/ubcpi/ubcpi.py b/ubcpi/ubcpi.py index b560ad3..80140fd 100644 --- a/ubcpi/ubcpi.py +++ b/ubcpi/ubcpi.py @@ -11,6 +11,8 @@ from webob import Response from xblock.core import XBlock from xblock.exceptions import JsonHandlerError +# For supporting manual revision of scores. Commented out for now. +# from xblock.scorable import ScorableXBlockMixin, Score from xblock.fields import Scope, String, List, Dict, Integer, DateTime, Float from xblock.fragment import Fragment from xblockutils.publish_event import PublishEventMixin @@ -256,8 +258,6 @@ class PeerInstructionXBlock(XBlock, MissingDataFetcherMixin, PublishEventMixin): help=_("The algorithm for selecting which answers to be presented to students"), ) - # Declare that we are not part of the grading System. Disabled for now as for the concern about the loading - # speed of the progress page. has_score = True start = DateTime( @@ -293,6 +293,57 @@ def max_score(self): """ return 1 + # def calculate_score(self): + # answers = self.get_answers_for_student() + # if answers.has_revision(0) and answers.has_revision(1): + # return Score(1, 1) + # return Score(0, 1) + # + # def set_score(self, score): + # # TODO persisting score + # pass + # + # def get_score(self): + # # TODO Since we are not persisting score, always return 1. + # # That means the Overriding Score function will always set the score to 1 + # # Instructors can reset the score to 0 by deleting learner's state + # return Score(1, 1) + + def clear_student_state(self, user_id, course_id, item_id, requesting_user_id): + """ + Being notified that student state is going to be deleted. Mark student's + submissions as deleted + """ + student_item = dict( + student_id=user_id, + item_id=item_id, + course_id=course_id, + item_type='ubcpi' + ) + + # TODO currently not possible to revise the stats as they are defined with scope Scope.user_state_summary. + # The stats are not available when clear_student_state is called + # answers = sas_api.get_answers_for_student(student_item) + # stats = self.get_current_stats() + # if answers.has_revision(0): + # num_resp = stats['original'].setdefault(answers.get_vote(0), 0) + # if num_resp > 0: + # stats['original'][answers.get_vote(0)] = num_resp - 1 + # if answers.has_revision(1): + # num_resp = stats['revised'].setdefault(answers.get_vote(1), 0) + # if num_resp > 0: + # stats['revised'][answers.get_vote(1)] = num_resp - 1 + + # mark existing submission as deleted + sas_api.delete_answer_for_student(student_item, requesting_user_id) + + # def has_submitted_answer(self): + # answers = self.get_answers_for_student() + # return answers.has_revision(0) and answers.has_revision(1) + # + # def publish_grade(self): + # self._publish_grade(self.get_score()) + def studio_view(self, context=None): """ view function for studio edit @@ -620,7 +671,7 @@ def get_persisted_data(self, other_answers): 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 self.other_answers_shown: + 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) else: From 32ba633237f934c36e3b6afdadd4c319b9cc379f Mon Sep 17 00:00:00 2001 From: Clarence Ho Date: Wed, 6 Nov 2019 17:13:59 -0800 Subject: [PATCH 3/3] Allow students to fresh rationales shown - on step 2, display a link to allow students to refresh the answers shown --- ubcpi/answer_pool.py | 137 +++++++++++++++++++++ ubcpi/static/css/ubcpi.css | 23 +++- ubcpi/static/html/ubcpi.html | 18 +-- ubcpi/static/html/ubcpi_edit.html | 2 +- ubcpi/static/js/spec/ubcpi_spec.js | 99 ++++++++++++++- ubcpi/static/js/src/d3-pibar.js | 3 +- ubcpi/static/js/src/ubcpi.js | 82 +++++++++++- ubcpi/test/data/refresh_other_answers.json | 35 ++++++ ubcpi/test/data/submit_answer.json | 3 +- ubcpi/test/test_answer_pool.py | 68 +++++++++- ubcpi/test/test_lms.py | 28 +++++ ubcpi/ubcpi.py | 86 ++++++++++++- 12 files changed, 564 insertions(+), 20 deletions(-) create mode 100644 ubcpi/test/data/refresh_other_answers.json diff --git a/ubcpi/answer_pool.py b/ubcpi/answer_pool.py index eb61048..76d393e 100644 --- a/ubcpi/answer_pool.py +++ b/ubcpi/answer_pool.py @@ -1,4 +1,5 @@ import random +import copy import persistence as sas_api from utils import _ # pylint: disable=unused-import @@ -172,6 +173,52 @@ def validate_seeded_answers(answers, options, algo): else: raise UnknownChooseAnswerAlgorithm() +def get_other_answers_count(pool, seeded_answers, get_student_item_dict): + """ + Count of available answers and seeds in the pool for each option + + Args: + pool (dict): answer pool, format: + { + option1_index: { + student_id: { can store algorithm specific info here } + }, + option2_index: { + student_id: { ... } + } + } + seeded_answers (list): seeded answers from instructor + [ + {'answer': 0, 'rationale': 'rationale A'}, + {'answer': 1, 'rationale': 'rationale B'}, + ] + get_student_item_dict (callable): get student item dict function to return student item dict + + Returns: + dict: count for each option + { + 0: 4, + 1: 2, + 3: 1, + ... + } + + """ + ret = {} + + # clean up answers so that all keys are int + pool = {int(k): v for k, v in pool.items()} + merged_pool = convert_seeded_answers(seeded_answers) + student_id = get_student_item_dict()['student_id'] + for key in pool: + merged_pool.setdefault(key, {}) + merged_pool[key].update(pool[key]) + # Pop student's own answer, if exists + merged_pool[key].pop(student_id, None) + + for key in merged_pool: + ret[key] = len(merged_pool.get(key, {})) + return ret def get_other_answers(pool, seeded_answers, get_student_item_dict, algo, options): """ @@ -323,6 +370,96 @@ def get_other_answers_random(pool, seeded_answers, get_student_item_dict, num_re return {"answers": ret} +def refresh_answers(answers_shown, option, pool, seeded_answers, get_student_item_dict, seeded_first=False): + """ + Refresh the answers shown for given option + + Args: + answers_shown (dict): answers being shown that need to be refreshed. Format: + {'answers': [ + {'option': 0, 'rationale': 'rationale A'}, + {'option': 1, 'rationale': 'rationale B'}, + ]} + option (int): the option to refresh + pool (dict): answer pool, format: + { + option1_index: { + student_id: { can store algorithm specific info here } + }, + option2_index: { + student_id: { ... } + } + } + seeded_answers (list): seeded answers from instructor + [ + {'answer': 0, 'rationale': 'rationale A'}, + {'answer': 1, 'rationale': 'rationale B'}, + ] + get_student_item_dict (callable): get student item dict function to return student item dict + seeded_first (boolean): refresh with answers from seeded_answers first, when exhausted, pick from pool + + Returns: + dict: refreshed answers lists + { + 'answers': + [ + {'option': 0, 'rationale': 'rationale A'}, + {'option': 1, 'rationale': 'rationale B'}, + ] + } + """ + ret = copy.deepcopy(answers_shown) + # clean up answers so that all keys are int + pool = {int(k): v for k, v in pool.items()} + seeded_pool = convert_seeded_answers(seeded_answers) + student_id = get_student_item_dict()['student_id'] + + available_students = copy.deepcopy(pool.get(option, {})) + available_students.pop(student_id, None) + # if seed answers have higher priority, fill the available seeds. + # otherwise merge them into available students + available_seeds = {} + if seeded_first and seeded_pool.get(option, {}): + available_seeds = copy.deepcopy(seeded_pool.get(option, {})) + else: + for key in seeded_pool.get(option, {}): + available_students[key] = seeded_pool.get(option, {}).get(key, None) + + for answer in ret.get('answers', []): + if answer.get('option', None) == option: + rationale = None + + while available_seeds: + key = random.choice(available_seeds.keys()) + rationale = available_seeds.pop(key, None) + if rationale is not None: + answer['rationale'] = rationale + break; + + while available_students and rationale is None: + key = random.choice(available_students.keys()) + # remove the chosen answer from pool + content = available_students.pop(key, None) + + if key.startswith('seeded'): + rationale = content + else: + student_item = get_student_item_dict(key) + submission = sas_api.get_answers_for_student(student_item) + # 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) + + if rationale: + answer['rationale'] = rationale + break + + # random.shuffle(ret['answers']) + return ret + + def convert_seeded_answers(answers): """ Convert seeded answers into the format that can be merged into student answers. diff --git a/ubcpi/static/css/ubcpi.css b/ubcpi/static/css/ubcpi.css index 2d153c0..a9710dd 100644 --- a/ubcpi/static/css/ubcpi.css +++ b/ubcpi/static/css/ubcpi.css @@ -52,6 +52,7 @@ fieldset .ubcpi-label { border-radius: 3px; padding: 10px; width: 100%; + background-color: #fbfbfb; } fieldset .ubcpi-label:hover { @@ -191,10 +192,6 @@ div.course-wrapper section.course-content .vert-mod > div ul.ubcpi-other-answers border-bottom: 1px solid #cfc6c6; } -div.course-wrapper section.course-content .vert-mod .sample-answer-list > :not(:first-child) { - border-top: 1px solid #cfc6c6; -} - div.course-wrapper section.course-content .vert-mod .sample-answer { margin: 1em; display: block; @@ -625,12 +622,30 @@ text.ubcpibar.label { border: 2px solid #e5e5e5; border-radius: 3px; padding: 10px; + background-color: #fbfbfb; } .ubcpi-breakdown-answer-options .ubcpi-option .ubcpi-breakdown-answer-text { display: block; } +.ubcpi-refresh-section { + margin: 1em; + border-top: 1px solid #cfc6c6; + color: #0075b4; +} + +.ubcpi-refresh-option-button * { + font-size: 0.9em; + transition: none; + cursor: pointer; +} + +.ubcpi-refresh-option-button-disabled { + pointer-events: none; + color: #888; +} + #pi-form .list-input.settings-list .setting-label { vertical-align: top; } diff --git a/ubcpi/static/html/ubcpi.html b/ubcpi/static/html/ubcpi.html index 3340585..d016a48 100644 --- a/ubcpi/static/html/ubcpi.html +++ b/ubcpi/static/html/ubcpi.html @@ -20,7 +20,7 @@

    {{display_n -
    +

    1. Answer, In Progress
      2. @@ -47,13 +47,13 @@

      {{display_n Step 1) Give Initial Answer You can change this answer later, if you change your mind. Step 2) Read Other Student Answers -

      These are samples of other student answers for this question. Read them and then compare with your answer below.

      +

      These are randomly chosen samples of other student answers for this question. Read them and compare with your answer below. Then you may revise your answer, if you wish.
      -
      +
      + +

    (no student explanations were randomly selected for this answer) @@ -98,7 +101,7 @@

    {{display_n

    - +

    @@ -110,9 +113,10 @@

    {{display_n

    What would you like to do?

    + - - + +

    @@ -167,7 +171,7 @@

    {{display_n {{question_text.image_alt}}

    -
    +

    1. Answer, Completed
      2. diff --git a/ubcpi/static/html/ubcpi_edit.html b/ubcpi/static/html/ubcpi_edit.html index 7930226..a66b8e0 100644 --- a/ubcpi/static/html/ubcpi_edit.html +++ b/ubcpi/static/html/ubcpi_edit.html @@ -138,7 +138,7 @@
      - This is the number of examples shown to the students after they answer. Enter the # symbol to use the same number as the answer possibilities you've set. + This is the number of examples shown on screen to the students after they answer. Students can choose to refresh for some other samples. Enter the # symbol to use the same number as the answer possibilities you've set.
      diff --git a/ubcpi/static/js/spec/ubcpi_spec.js b/ubcpi/static/js/spec/ubcpi_spec.js index 7ef0480..31f1296 100644 --- a/ubcpi/static/js/spec/ubcpi_spec.js +++ b/ubcpi/static/js/spec/ubcpi_spec.js @@ -41,6 +41,69 @@ describe('UBCPI module', function () { }); }); + describe('ubcpi-refresh-rationale directive', function() { + var $compile, + $rootScope, + backendService, + $httpBackend; + + // Store references to $rootScope and $compile + // so they are available to all tests in this describe block + beforeEach(inject(function (_$compile_, _$rootScope_, _$httpBackend_) { + // The injector unwraps the underscores (_) from around the parameter names when matching + $compile = _$compile_; + $rootScope = _$rootScope_; + $httpBackend = _$httpBackend_; + + mockConfig.urls.refresh_other_answers = '/handler/refresh_other_answers'; + })); + + afterEach(function() { + $httpBackend.verifyNoOutstandingExpectation(); + $httpBackend.verifyNoOutstandingRequest(); + }); + + it('should show button to refresh answers shown', function () { + var scope = $rootScope.$new(true); + scope.rc = { + other_answers: { answer: [] } + }; + // Compile a piece of HTML containing the directive + var element = $compile("
      ")(scope); + scope.$digest(); + expect(element.html()).toContain('Show other samples'); + }); + + it('should allow refreshing of answers shown', function () { + var scope = $rootScope.$new(true); + scope.rc = { + other_answers: { answers: [] } + }; + var param = {"option": "1"}; + var exp = { + "other_answers": { + "answers": [ + {"option": 0, "rationale": "Tree gets carbon from air."}, + {"option": 1, "rationale": "Tree gets minerals from soil."}, + {"option": 2, "rationale": "Tree drinks water."}] + }, + "rationale_revised": null, + "answer_original": 1, + "rationale_original": "testing", + "answer_revised": null + }; + var data = undefined; + $httpBackend.expectPOST('/handler/refresh_other_answers', JSON.stringify(param)).respond(200, exp); + + // Compile a piece of HTML containing the directive + var element = $compile("
      ")(scope); + scope.$digest(); + $(element).click(); + $httpBackend.flush(); + expect(scope.rc.other_answers).toEqual(exp.other_answers); + }); + }); + describe('backendService', function() { var backendService, $httpBackend; @@ -128,6 +191,38 @@ describe('UBCPI module', function () { }); }); + describe('refresh_other_answers', function() { + + beforeEach(function() { + mockConfig.urls.refresh_other_answers = '/handler/refresh_other_answers'; + }); + + it('should be able to refresh other answers', function() { + var param = {"option": 1}; + var exp = { + "other_answers": { + "answers": [ + {"option": 0, "rationale": "Tree gets carbon from air."}, + {"option": 1, "rationale": "Tree gets minerals from soil."}, + {"option": 2, "rationale": "Tree drinks water."}] + }, + "rationale_revised": null, + "answer_original": 1, + "rationale_original": "testing", + "answer_revised": null + }; + var data = undefined; + $httpBackend.expectPOST('/handler/refresh_other_answers', JSON.stringify(param)).respond(200, exp); + + backendService.refreshOtherAnswers(1).then(function(d) { + data = d; + }); + $httpBackend.flush(); + + expect(data).toEqual(exp); + }); + }); + describe('submit', function() { var post = { "q": 0, @@ -450,8 +545,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(5); 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, 'refresh_other_answers']]); }); }); diff --git a/ubcpi/static/js/src/d3-pibar.js b/ubcpi/static/js/src/d3-pibar.js index 896fac7..f12d5d5 100644 --- a/ubcpi/static/js/src/d3-pibar.js +++ b/ubcpi/static/js/src/d3-pibar.js @@ -182,13 +182,14 @@ d3.custom.perAnswerChart = function(scope, gettext, allAnswerCount) { }); if (totalFreq < minTotalFrequency) { + d3.select(this).selectAll("svg > *").remove(); d3.select(this) .append("span") .attr("id", 'not-enough-data') .text(gettext("Not enough data to generate the chart. Please check back later.")); return; } else { - var notEnoughDataSpan = d3.select('#not-enough-data'); + var notEnoughDataSpan = d3.select(this).select('#not-enough-data'); if (typeof notEnoughDataSpan !== 'undefined') { notEnoughDataSpan.remove(); } diff --git a/ubcpi/static/js/src/ubcpi.js b/ubcpi/static/js/src/ubcpi.js index 4519aef..7f06fd2 100644 --- a/ubcpi/static/js/src/ubcpi.js +++ b/ubcpi/static/js/src/ubcpi.js @@ -69,17 +69,78 @@ angular.module('UBCPI', ['ngSanitize', 'ngCookies', 'gettext']) if (!target) { target = ele; } - $('html,body').animate({scrollTop: target.offset().top}, "slow"); + if (target && target.offset()) { + $('html,body').animate({scrollTop: target.offset().top}, "slow"); + } }); } }; }) + /** + * Scroll to the xblock progress bar + */ + .directive('scrollToProgressBar', function() { + return { + restrict: 'A', + scope: { + scrollToProgressBar: '@' + }, + link: function(scope, ele, attr, ctrl) { + ele.on(scope.scrollToProgressBar? scope.scrollToProgressBar : 'click', function() { + var target; + target = ele.parents('.ubcpi_block').find('.ubcpi_progress_bar'); + if (!target) { + target = ele; + } + if (target && target.offset()) { + $('html,body').animate({scrollTop: target.offset().top}, "slow"); + } + }); + } + }; + }) + + .directive('ubcpiRefreshRationale', ['backendService', 'notify', 'gettext', function(backendService, notify, gettext) { + return { + retrict: 'A', + replace: false, + scope: { + ubcpiRefreshModel: '=', + }, + link: function(scope, ele, attr, ctrl) { + var option = attr.ubcpiOption; + var desc = '' + gettext('Show other samples') + ''; + var desc_loading = '' + gettext('Refreshing...') + ''; + + function call_refresh() { + ele.empty().append(' ' + desc_loading); + backendService.refreshOtherAnswers(option).then(function(data) { + if (data && data.other_answers && data.other_answers.answers) { + scope.ubcpiRefreshModel = data.other_answers.answers + } + }, function(error) { + notify('error', { + 'title': gettext('Error refreshing answers from other students'), + 'message': gettext('Please refresh the page and try again!') + }); + }).finally(function() { + ele.empty().append(' ' + desc); + }); + } + + ele.on('click', call_refresh); + ele.empty().append(' ' + desc); + } + } + }]) + .factory('backendService', ['$http', '$q', '$rootScope', function ($http, $q, $rootScope) { return { getStats: getStats, submit: submit, get_data: get_data, + refreshOtherAnswers: refreshOtherAnswers, }; function getStats() { @@ -121,6 +182,21 @@ angular.module('UBCPI', ['ngSanitize', 'ngCookies', 'gettext']) } ); } + + function refreshOtherAnswers(option) { + var refreshUrl = $rootScope.config.urls.refresh_other_answers; + var refreshParam = JSON.stringify({ + "option": option, + }); + return $http.post(refreshUrl, refreshParam).then( + function(response) { + return response.data; + }, + function(error) { + return $q.reject(error); + } + ); + } }]) .controller('ReviseController', ['$scope', 'notify', 'backendService', '$q', 'gettext', '$location', @@ -238,6 +314,7 @@ angular.module('UBCPI', ['ngSanitize', 'ngCookies', 'gettext']) self.rationale = data.rationale_revised || data.rationale_original; self.weight = data.weight; self.options = data.options; + self.alt_answers_available = data.alt_answers_available; } self.hasSampleExplanationForOption = function (option) { @@ -270,7 +347,8 @@ 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'), + 'refresh_other_answers': runtime.handlerUrl(element, 'refresh_other_answers'), }; // in order to support multiple same apps on the same page but diff --git a/ubcpi/test/data/refresh_other_answers.json b/ubcpi/test/data/refresh_other_answers.json new file mode 100644 index 0000000..cb698b2 --- /dev/null +++ b/ubcpi/test/data/refresh_other_answers.json @@ -0,0 +1,35 @@ +{ + "valid": { + "submit_answer_param": { + "q": 0, + "rationale": "This is my answer.", + "status": 0 + }, + "refresh_params": { + "option": "2" + }, + "expect": { + "other_answers": { + "answers": [ + { + "option": 0, + "rationale": "This is answer shown for option 0" + }, + { + "option": 1, + "rationale": "This is answer shown for option 1" + }, + { + "option": 2, + "rationale": "This is answer shown for option 2" + } + ] + }, + "answer_original": 0, + "rationale_original": "This is my answer.", + "answer_revised": null, + "rationale_revised": null, + "alt_answers_available": {"1": false, "0": true, "2": false} + } + } +} \ No newline at end of file diff --git a/ubcpi/test/data/submit_answer.json b/ubcpi/test/data/submit_answer.json index 9076c83..ce4dbda 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, + "alt_answers_available": {"1": false, "0": true, "2": false} }, "post2": { "q": 1, diff --git a/ubcpi/test/test_answer_pool.py b/ubcpi/test/test_answer_pool.py index 2d6547b..6b80f2a 100644 --- a/ubcpi/test/test_answer_pool.py +++ b/ubcpi/test/test_answer_pool.py @@ -4,7 +4,7 @@ from mock import patch, call, MagicMock from ubcpi.answer_pool import offer_answer, validate_seeded_answers_simple, UnknownChooseAnswerAlgorithm, \ validate_seeded_answers_random, validate_seeded_answers, get_other_answers, get_other_answers_simple, \ - get_other_answers_random, get_max_size, POOL_ITEM_LENGTH_SIMPLE + get_other_answers_random, get_max_size, POOL_ITEM_LENGTH_SIMPLE, refresh_answers from ubcpi.persistence import Answers, VOTE_KEY, RATIONALE_KEY @@ -154,3 +154,69 @@ def test_get_max_size(self): self.assertEqual(get_max_size({}, 3, POOL_ITEM_LENGTH_SIMPLE), 102) self.assertEqual(get_max_size({}, 10, POOL_ITEM_LENGTH_SIMPLE), 67) self.assertEqual(get_max_size({1: {i: {} for i in xrange(10)}}, 10, POOL_ITEM_LENGTH_SIMPLE), 62) + + @patch( + 'ubcpi.persistence.get_answers_for_student', + return_value=Answers([{VOTE_KEY: 0, RATIONALE_KEY: 'my rationale'}]) + ) + @file_data('data/get_other_answers_simple.json') + def test_refresh_answers_normal(self, data, mock): + answer_shown = {'answers': [ + {'option': 0, 'rationale': 'rationale to be refreshed'}, + {'option': 1, 'rationale': 'rationale should stay'}, + ]} + option = 0 + pool = data['pool'] + student_item_dict_func = MagicMock(return_value={'student_id': data['user_id']}) + seeded_answers = data['seeds'] + refreshed = refresh_answers(answer_shown, option, pool, seeded_answers, student_item_dict_func) + for answer in refreshed['answers']: + if answer['option'] == 0: + self.assertNotEqual(answer['rationale'], 'rationale to be refreshed') + else: + self.assertEqual(answer['rationale'], 'rationale should stay') + + @patch( + 'ubcpi.persistence.get_answers_for_student', + return_value=Answers([{VOTE_KEY: 0, RATIONALE_KEY: 'my rationale'}]) + ) + @file_data('data/get_other_answers_simple.json') + def test_refresh_answers_no_seed(self, data, mock): + answer_shown = {'answers': [ + {'option': 0, 'rationale': 'rationale to be refreshed if pool not empty'}, + {'option': 1, 'rationale': 'rationale should stay'}, + ]} + option = 0 + pool = data['pool'] + student_item_dict_func = MagicMock(return_value={'student_id': data['user_id']}) + seeded_answers = [] + refreshed = refresh_answers(answer_shown, option, pool, seeded_answers, student_item_dict_func) + for answer in refreshed['answers']: + if answer['option'] == 0: + if len(pool) > 0: + self.assertNotEqual(answer['rationale'], 'rationale to be refreshed if pool not empty') + else: + self.assertEqual(answer['rationale'], 'rationale to be refreshed if pool not empty') + else: + self.assertEqual(answer['rationale'], 'rationale should stay') + + @patch( + 'ubcpi.persistence.get_answers_for_student', + return_value=Answers([{VOTE_KEY: 0, RATIONALE_KEY: 'my rationale'}]) + ) + @file_data('data/get_other_answers_simple.json') + def test_refresh_answers_seeded_first(self, data, mock): + answer_shown = {'answers': [ + {'option': 0, 'rationale': 'rationale to be refreshed'}, + {'option': 1, 'rationale': 'rationale should stay'}, + ]} + option = 0 + pool = data['pool'] + student_item_dict_func = MagicMock(return_value={'student_id': data['user_id']}) + seeded_answers = data['seeds'] + refreshed = refresh_answers(answer_shown, option, pool, seeded_answers, student_item_dict_func, True) + for answer in refreshed['answers']: + if answer['option'] == 0: + self.assertNotEqual(answer['rationale'], 'rationale to be refreshed') + else: + self.assertEqual(answer['rationale'], 'rationale should stay') diff --git a/ubcpi/test/test_lms.py b/ubcpi/test/test_lms.py index 7a86129..c615f3c 100644 --- a/ubcpi/test/test_lms.py +++ b/ubcpi/test/test_lms.py @@ -211,3 +211,31 @@ def check_fields(self, xblock, data): for key, value in data.iteritems(): self.assertIsNotNone(getattr(xblock, key)) self.assertEqual(getattr(xblock, key), value) + + @patch('ubcpi.ubcpi.get_other_answers') + @file_data('data/refresh_other_answers.json') + @scenario(os.path.join(os.path.dirname(__file__), 'data/basic_scenario.xml'), user_id='Bob') + def test_refresh_other_answers(self, xblock, data, mock): + # patch get_other_answers to avoid randomness + mock.return_value = data['expect']['other_answers'] + resp = self.request(xblock, 'submit_answer', json.dumps(data['submit_answer_param']), response_format='json') + self.assertEqual(resp, data['expect']) + + original_other_ans = [ans['rationale'] for ans in xblock.other_answers_shown['answers'] if ans['option'] == int(data['refresh_params']['option'])] + + resp = self.request(xblock, 'refresh_other_answers', json.dumps({}), response_format='json') + self.assertEqual(resp, {'error': 'Missing option'}) + resp = self.request(xblock, 'refresh_other_answers', json.dumps({'option': -1}), response_format='json') + self.assertEqual(resp, {'error': 'Invalid option'}) + + resp = self.request(xblock, 'refresh_other_answers', json.dumps(data['refresh_params']), response_format='json') + self.assertEqual(xblock.other_answers_refresh_count[data['refresh_params']['option']] , 1) + self.assertEqual(len(xblock.other_answers_shown_history), 3) + refreshed_other_ans = [ans['rationale'] for ans in xblock.other_answers_shown['answers'] if ans['option'] == int(data['refresh_params']['option'])] + # rationale of the refreshed option should be chanaged + self.assertNotEqual(original_other_ans, refreshed_other_ans) + + resp = self.request(xblock, 'refresh_other_answers', json.dumps(data['refresh_params']), response_format='json') + # refresh count of the option should be increased + self.assertEqual(xblock.other_answers_refresh_count[data['refresh_params']['option']] , 2) + self.assertEqual(len(xblock.other_answers_shown_history), 3) diff --git a/ubcpi/ubcpi.py b/ubcpi/ubcpi.py index 80140fd..ae246eb 100644 --- a/ubcpi/ubcpi.py +++ b/ubcpi/ubcpi.py @@ -18,7 +18,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, get_other_answers_count, refresh_answers import persistence as sas_api from serialize import parse_from_xml, serialize_to_xml @@ -32,6 +32,10 @@ MAX_RATIONALE_SIZE = 32000 MAX_RATIONALE_SIZE_IN_EVENT = settings.TRACK_MAX_EVENT / 4 +# max number of times the student can refresh to see other student answers shown to them. +# afterward, will fallback to only return seeded answers +MAX_REFRESH_PER_OPTION = 5 + def truncate_rationale(rationale, max_length=MAX_RATIONALE_SIZE_IN_EVENT): """ Truncates the rationale for analytics event emission if necessary @@ -253,6 +257,16 @@ class PeerInstructionXBlock(XBlock, MissingDataFetcherMixin, PublishEventMixin): help=_("Stores the specific answers of other students shown, for a given student."), ) + other_answers_shown_history = Dict( + default={}, scope=Scope.user_state, + help=_("In case the student requested to see more answers, these are the answers previously shown."), + ) + + other_answers_refresh_count = Dict( + default={}, scope=Scope.user_state, + help=_("Keep track of the number of refreshes for each option") + ) + algo = Dict( default={'name': 'simple', 'num_responses': '#'}, scope=Scope.content, help=_("The algorithm for selecting which answers to be presented to students"), @@ -589,6 +603,10 @@ def record_response(self, answer, rationale, status): 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) + # reset the shown answers history and clear the refresh counts + self._reset_answers_shown_history() + self._record_answers_shown(self.other_answers_shown) + event_dict['other_student_responses'] = self.other_answers_shown self.publish_event_from_dict( self.event_namespace + '.original_submitted', @@ -662,6 +680,7 @@ def get_persisted_data(self, other_answers): Adds the other answers and correct answer/rationale when needed """ answers = self.get_answers_for_student() + pool_count = get_other_answers_count(self.sys_selected_answers, self.seeds, self.get_student_item_dict) ret = { "answer_original": answers.get_vote(0), "rationale_original": answers.get_rationale(0), @@ -677,6 +696,15 @@ def get_persisted_data(self, other_answers): else: ret['other_answers'] = other_answers + ret['alt_answers_available'] = {} + # count how many answers we are showing for each option + showing_count = {} + for ans in ret['other_answers'].get('answers', []): + if ans.get('option', None) is not None: + showing_count[ans['option']] = showing_count.get(ans['option'], 0) + 1 + for key in pool_count: + ret['alt_answers_available'][key] = True if pool_count[key] > showing_count.get(key, 0) else False + # reveal the correct answer in the end if answers.has_revision(1): ret['correct_answer'] = self.correct_answer @@ -758,3 +786,59 @@ def add_xml_to_node(self, node): Serialize the XBlock to XML for exporting. """ serialize_to_xml(node, self) + + @XBlock.json_handler + def refresh_other_answers(self, data, suffix=''): + """ + Refresh other answers shown, if possible + """ + if 'option' not in data: + raise JsonHandlerError(400, 'Missing option') + the_option = int(data['option']) + if the_option < 0 or the_option >= len(self.options): + raise JsonHandlerError(400, 'Invalid option') + + student_item = self.get_student_item_dict() + if not student_item or not student_item['student_id']: + raise JsonHandlerError(400, 'Missing student info') + + # can refresh only after intial submission and before final submission + answers = self.get_answers_for_student() + if not(answers.has_revision(0) and not answers.has_revision(1)): + return self.get_persisted_data(self.other_answers_shown) + + seeded_first = self._over_answers_shown_refresh_limit(the_option) + self.other_answers_shown = \ + refresh_answers(self.other_answers_shown, the_option, self.sys_selected_answers, self.seeds, self.get_student_item_dict, seeded_first) + + self._incr_answers_shown_refresh_count(the_option) + self._record_answers_shown(self.other_answers_shown) + + return self.get_persisted_data(self.other_answers_shown) + + def _reset_answers_shown_history(self): + self.other_answers_shown_history = {} + self.other_answers_refresh_count = {} + + def _record_answers_shown(self, answers_shown): + import time + answers = answers_shown.get("answers", None) + + for option in range(len(self.options)): + key = str(option) + answers_shown_for_option = self.other_answers_shown_history.setdefault(key, {}) + # see if we are showing new answers for this option + for ans in answers: + if ans.get('option', None) is not None and str(ans['option']) == key: + answers_shown_for_option[ans.get('rationale', None)] = time.time() + + def _incr_answers_shown_refresh_count(self, option): + key = str(option) + count = self.other_answers_refresh_count.get(key, 0) + 1 + self.other_answers_refresh_count[key] = count + + def _over_answers_shown_refresh_limit(self, option): + key = str(option) + count = self.other_answers_refresh_count.get(key, 0) + return count > MAX_REFRESH_PER_OPTION +