Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't show results for anonymous surveys #19753

Merged
merged 4 commits into from Jan 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions apps/i18n/common/en_us.json
Expand Up @@ -914,9 +914,12 @@
"submitAssessment": "Submit your assessment",
"submittableComplete": "You cannot edit your assessment after submitting it. Are you sure?",
"submittableIncomplete": "You left some questions incomplete. You cannot edit your assessment after submitting it. Are you sure?",
"submittableSurveyComplete": "You cannot edit your survey after submitting it. To preserve anonymity, your responses will also be cleared from this page. Are you sure you want to Submit?",
"submittableSurveyIncomplete": "You left some questions incomplete. You cannot edit your survey after submitting it. To preserve anonymity, your responses will also be cleared from this page. Are you sure you want to Submit?",
"submittableUnsubmit": "Unsubmitting your assessment will reset the submitted time and date. Are you sure?",
"submitted": "Submitted",
"submitting": "Submitting...",
"submitSurvey": "Submit your survey",
"submitYourProject": "Submit your project",
"submitYourProjectConfirm": "You cannot edit your project after submitting it, really submit?",
"subtitle": "a visual programming environment",
Expand Down
18 changes: 1 addition & 17 deletions apps/src/lib/ui/LegacyDialogContents.js
Expand Up @@ -7,7 +7,7 @@ import React, { PropTypes } from 'react';
import i18n from '@cdo/locale';
import ProtectedStatefulDiv from '@cdo/apps/templates/ProtectedStatefulDiv';

const SingleLevelGroupDialog = ({id, title, body}) => (
export const SingleLevelGroupDialog = ({id, title, body}) => (
<ProtectedStatefulDiv id={id}>
<div className="modal-content no-modal-icon">
<p className="dialog-title">{title}</p>
Expand All @@ -27,22 +27,6 @@ SingleLevelGroupDialog.propTypes = {
body: PropTypes.string.isRequired,
};

export const IncompleteDialog = (
<SingleLevelGroupDialog
id="levelgroup-submit-incomplete-dialogcontent"
title={i18n.submitAssessment()}
body={i18n.submittableIncomplete()}
/>
);

export const CompleteDialog = (
<SingleLevelGroupDialog
id="levelgroup-submit-complete-dialogcontent"
title={i18n.submitAssessment()}
body={i18n.submittableComplete()}
/>
);

export const UnsubmitDialog = (
<SingleLevelGroupDialog
id="unsubmit-dialogcontent"
Expand Down
106 changes: 56 additions & 50 deletions apps/src/sites/studio/pages/levels/_dialog.js
Expand Up @@ -8,63 +8,69 @@ import { UnsubmitDialog } from '@cdo/apps/lib/ui/LegacyDialogContents';
* This file does some handling of submit button interactions.
*/

// Are we read-only? This can be because we're a teacher OR because an answer
// has been previously submitted.
if (appOptions.readonlyWorkspace) {
// hide the Submit button.
$('.submitButton').hide();
// It's worth noting that in levelgroups containing external type levels, this
// JS can get loaded more than once. It's important that this code runs after
// the DOM is done loading, otherwise we might not actually have all of our
// buttons yet
$(document).ready(() => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to this file are probably most easily viewed with whitespace diffs ignored https://github.com/code-dot-org/code-dot-org/pull/19753/files?w=1#diff-00a2cfbb66f812306b2ac0fea8473c03R15 as it's entirely wrapping things in the document.ready

// Are we read-only? This can be because we're a teacher OR because an answer
// has been previously submitted.
if (appOptions.readonlyWorkspace) {
// hide the Submit button.
$('.submitButton').hide();

// Are we a student viewing their own previously-submitted work?
if (appOptions.submitted) {
// show the Unsubmit button.
$('.unsubmitButton').show();
}
// Are we a student viewing their own previously-submitted work?
if (appOptions.submitted) {
// show the Unsubmit button.
$('.unsubmitButton').show();
}

// Set the entire page background to be light grey.
$('.full_container').addClass('submitted_readonly');
}
// Set the entire page background to be light grey.
$('.full_container').addClass('submitted_readonly');
}

// Unsubmit button should only be available when this is a standalone level.
$('.unsubmitButton').click(function () {
showDialog(UnsubmitDialog, function () {
$.post(window.appOptions.unsubmitUrl,
{"_method": 'PUT', user_level: {submitted: false}},
function () {
// Just reload so that the progress in the header is shown correctly.
location.reload();
}
);
// Unsubmit button should only be available when this is a standalone level.
$('.unsubmitButton').click(function () {
showDialog(UnsubmitDialog, function () {
$.post(window.appOptions.unsubmitUrl,
{"_method": 'PUT', user_level: {submitted: false}},
function () {
// Just reload so that the progress in the header is shown correctly.
location.reload();
}
);
});
});
});

// TODO(dave): Dashboard shouldn't be reaching into the internal implementation of
// individual levels. Instead levels should call appOptions.onAttempt.
$(document).on('click', '.submitButton', function () {
var submitButton = $('.submitButton');
if (submitButton.attr('disabled')) {
return;
}

var result = getResult();
if (result.confirmationDialog) {
// This is only used by level_group.js, and only uses the React approach to
// showDialog
if (typeof result.confirmationDialog === 'string') {
throw new Error('result.confirmationDialog only supports React approach to showDialog');
// TODO(dave): Dashboard shouldn't be reaching into the internal implementation of
// individual levels. Instead levels should call appOptions.onAttempt.
$(document).on('click', '.submitButton', function () {
var submitButton = $('.submitButton');
if (submitButton.attr('disabled')) {
return;
}
showDialog(result.confirmationDialog, function () {
processResults(undefined, result.beforeProcessResultsHook);
});
} else {
// Avoid multiple simultaneous submissions.
submitButton.attr('disabled', true);

var onComplete = function (willRedirect) {
if (!willRedirect) {
$('.submitButton').attr('disabled', false);
var result = getResult();
if (result.confirmationDialog) {
// This is only used by level_group.js, and only uses the React approach to
// showDialog
if (typeof result.confirmationDialog === 'string') {
throw new Error('result.confirmationDialog only supports React approach to showDialog');
}
};
showDialog(result.confirmationDialog, function () {
processResults(undefined, result.beforeProcessResultsHook);
});
} else {
// Avoid multiple simultaneous submissions.
submitButton.attr('disabled', true);

processResults(onComplete, result.beforeProcessResultsHook);
}
var onComplete = function (willRedirect) {
if (!willRedirect) {
$('.submitButton').attr('disabled', false);
}
};

processResults(onComplete, result.beforeProcessResultsHook);
}
});
});
24 changes: 22 additions & 2 deletions apps/src/sites/studio/pages/levels/_level_group.js
@@ -1,10 +1,13 @@
/* global appOptions */

import $ from 'jquery';
import React from 'react';
import throttle from 'lodash/throttle';
import getScriptData from '@cdo/apps/util/getScriptData';
import * as codeStudioLevels from '@cdo/apps/code-studio/levels/codeStudioLevels';
import {IncompleteDialog, CompleteDialog} from '@cdo/apps/lib/ui/LegacyDialogContents';
import {SingleLevelGroupDialog} from '@cdo/apps/lib/ui/LegacyDialogContents';
import i18n from '@cdo/locale';

window.Multi = require('@cdo/apps/code-studio/levels/multi.js');
window.TextMatch = require('@cdo/apps/code-studio/levels/textMatch.js');
var saveAnswers = require('@cdo/apps/code-studio/levels/saveAnswers.js').saveAnswers;
Expand Down Expand Up @@ -128,7 +131,24 @@ window.initLevelGroup = function (levelCount, currentPage, lastAttempt) {
}
}

const confirmationDialog = (validCount === levelCount) ? CompleteDialog : IncompleteDialog;
let id, title, body;
const isSurvey = appOptions.level.anonymous === true ||
appOptions.level.anonymous === 'true';
title = isSurvey ? i18n.submitSurvey() : i18n.submitAssessment();
if (validCount === levelCount) {
id = "levelgroup-submit-complete-dialogcontent";
body = isSurvey ? i18n.submittableSurveyComplete() : i18n.submittableComplete();
} else {
id = "levelgroup-submit-incomplete-dialogcontent";
body = isSurvey ? i18n.submittableSurveyIncomplete() : i18n.submittableIncomplete();
}
const confirmationDialog = (
<SingleLevelGroupDialog
id={id}
title={title}
body={body}
/>
);

return {
response: encodeURIComponent(JSON.stringify(lastAttempt)),
Expand Down
5 changes: 5 additions & 0 deletions dashboard/app/assets/stylesheets/application.scss
Expand Up @@ -3134,3 +3134,8 @@ code {
font-family: "Gotham 4r", sans-serif;
}
}

#survey-submission-thankyou {
color: $purple;
font-size: 18px;
}
18 changes: 7 additions & 11 deletions dashboard/app/models/script_level.rb
Expand Up @@ -367,22 +367,18 @@ def to_param
position.to_s
end

# Given the signed-in user and an optional user that is being viewed
# (e.g. a student viewed by a teacher), tell us whether we are allowed
# to view their prior answer.
def can_view_last_attempt(user, viewed_user)
# If it's an anonymous survey, then teachers can't view student answers
return false if user && viewed_user && user != viewed_user && anonymous?

# Everything else is okay.
return true
end

# Is this script_level hidden for the current section, either because the stage
# it is contained in is hidden, or the script it is contained in is hidden.
def hidden_for_section?(section_id)
return false if section_id.nil?
!SectionHiddenStage.find_by(stage_id: stage.id, section_id: section_id).nil? ||
!SectionHiddenScript.find_by(script_id: stage.script.id, section_id: section_id).nil?
end

# Given the signed-in user and an optional user that is being viewed
# (e.g. a student viewed by a teacher), tell us whether we should be hiding
# prior answers
def should_hide_survey(user, viewed_user)
anonymous? && user.teacher? && user != viewed_user
end
end
13 changes: 11 additions & 2 deletions dashboard/app/views/levels/_level_group.haml
Expand Up @@ -10,7 +10,7 @@
text. It's very important that we don't lose or miss this check.
Check that @script_level exists, because it won't for a /level/ view.

- unless @script_level && @script_level.can_view_last_attempt(current_user, @user)
- if @script_level && @script_level.should_hide_survey(current_user, @user)
- page = @pages.first
%h2= t('heading_x_of_y', heading: data["title"], x: page.page_number, y: @level.pages.length)
%br/
Expand All @@ -22,6 +22,9 @@
-# page. The controller takes care of that for us.
- current_page = params[:puzzle_page] || 1

-# Is this a survey where we've already submitted
- submitted_survey = @script_level.try(:anonymous?) && current_user.user_level_for(@script_level, @level).try(:submitted?)

-# anonymous users shouldn't ever see lockable stages, but give them the locked_stage message if they do
- if @script_level && @script_level.stage.lockable? && (!current_user || @script_level.locked?(current_user))
= render 'levels/locked_stage'
Expand All @@ -35,8 +38,14 @@
%h2= t('heading_x_of_y', heading: data["title"], x: page.page_number, y: @level.pages.length)
%br/

- if submitted_survey
#survey-submission-thankyou
= I18n.t('level_group.survey_submission_thankyou')

- page.levels.each_with_index do |level, index|
- sublevel_last_attempt = LevelGroup.get_sublevel_last_attempt(current_user, @user, level)
-# Don't show last attempt for submitted surveys, even for the current user, otherwise teachers
-# could access student responses
- sublevel_last_attempt = LevelGroup.get_sublevel_last_attempt(current_user, @user, level) unless submitted_survey

-# Show any text from an external level.
- if data["texts"]
Expand Down
1 change: 1 addition & 0 deletions dashboard/config/locales/en.yml
Expand Up @@ -677,6 +677,7 @@ en:
correct_title: "Thank you"
correct_body: "Thank you for submitting answers."
hide_survey_last_answer: "This survey is anonymous. Please visit the teacher dashboard for aggregate results. Note that results are only visible if at least five students submitted the survey."
survey_submission_thankyou: "Thank you for submitting this survey! The survey has been cleared so that your responses stay anonymous. Note that your teacher will still be able to see responses for the whole class, but without names attached."
level_group-submittable:
wrong_title: "Thank you"
wrong_body: "Thank you for submitting answers."
Expand Down
8 changes: 4 additions & 4 deletions dashboard/test/models/script_level_test.rb
Expand Up @@ -366,7 +366,7 @@ class ScriptLevelTest < ActiveSupport::TestCase

student = create :student

assert script_level.can_view_last_attempt(student, nil)
refute script_level.should_hide_survey(student, nil)
end

test 'can view other user last attempt for regular levelgroup' do
Expand All @@ -381,7 +381,7 @@ class ScriptLevelTest < ActiveSupport::TestCase
teacher = create :teacher
student = create :student

assert script_level.can_view_last_attempt(teacher, student)
refute script_level.should_hide_survey(teacher, student)
end

test 'can view my last attempt for anonymous levelgroup' do
Expand All @@ -396,7 +396,7 @@ class ScriptLevelTest < ActiveSupport::TestCase

student = create :student

assert script_level.can_view_last_attempt(student, nil)
refute script_level.should_hide_survey(student, nil)
end

test 'can not view other user last attempt for anonymous levelgroup' do
Expand All @@ -412,7 +412,7 @@ class ScriptLevelTest < ActiveSupport::TestCase
student = create :student
teacher = create :teacher

assert_not script_level.can_view_last_attempt(teacher, student)
assert script_level.should_hide_survey(teacher, student)
end

test 'anonymous levels must be assessments' do
Expand Down