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

Don't show results for anonymous surveys #19753

merged 4 commits into from Jan 5, 2018

Conversation

Bjvanminnen
Copy link
Contributor

When giving students surveys, we clearly state that their results will be anonymous and teachers will not be able to see them. We make sure to only show teachers aggregated results after they have responses from 5+ students. We also make sure that unlike other puzzle types, when the teacher navigates to that level we don't let them see the student's last attempt

Poorva recently realized however that teachers generally have access to their students passwords. This means they could log in as the student, and still see the last attempt (i.e. their answers). This PR makes it so that once a survey has been submitted, we won't show the previous responses anymore - even to the student who submitted them.

One remaining hole is that if you later unsubmit the survey (or unlock it in the case of a lockable level), the previously submitted answers will again be revealed. Avoiding this is difficult for the following reason:

  • Right now every time you answer a question, we make a milestone request (even before you've submitted)
  • This is done at least in part so that on multi-page surveys you can navigate between pages and still see your previous/yet unsubmitted responses.
  • The ideal behavior would be that we show you your answer if you've changed it since you last submitted (and otherwise not), but we don't have any good way to do 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

Copy link
Contributor

@islemaster islemaster left a comment

Choose a reason for hiding this comment

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

Awesome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants