Skip to content
Browse files

refactor + fix message students who dialog for quiz show

test plan:
  - as a teacher make a new quiz and publish it.
  - make sure one or two students in the course have taken the quiz
  - in the cog menu on the quiz show page
    (/courses/:course_id/quizzes/:quiz_id), click on "Message Students
    Who..."
  - You should see students who have taken the quiz in the dialog when
    you select "Have taken the quiz". You should see students who
    haven't taken the quiz when you select "Have NOT taken the quiz"
  - Try submitting the message with a blank message. You should get a
    warning box. Now, make a message to each group of students and save
    the message. Check that each student received the message.
  - Now make the rest of the students finish the quiz. Open the dialog
    again and fill out a message in the textbox. Select "students who
    have NOT taken the quiz". It should be empty. Try sending the
    message by clicking "Send Message". You should immediately get an
    error that the group of students you selected is empty and you
    should choose a different group of students.

fixes CNVS-4743

Change-Id: I2199bd7a18089726251bd0244bc618446db26d07
Reviewed-on: https://gerrit.instructure.com/18869
Reviewed-by: Simon Williams <simon@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
QA-Review: Amber Taniuchi <amber@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
  • Loading branch information...
1 parent 0869ec1 commit be37cbb8f28e0a76de3726db61191e85074439a6 Stanley Stuart committed with simonista
View
7 app/coffeescripts/conversations/Conversation.coffee
@@ -5,6 +5,13 @@ define [
], (Backbone) ->
class Conversation extends Backbone.Model
+
+ # NOTE: This class should be considered deprecated. Please be careful
+ # when modifying it, especially adding functionality.
+ #
+ # Try adding to app/coffeescripts/models/Conversation.coffee first,
+ # which is a version of this model that uses the API.
+
defaults:
audience: []
View
33 app/coffeescripts/models/Conversation.coffee
@@ -0,0 +1,33 @@
+define [
+ 'i18n!conversations'
+ 'Backbone'
+ 'underscore'
+ 'jquery'
+], (I18n,{Model},_,$) ->
+
+ class Conversation extends Model
+
+ # This new class is here instead of reusing
+ # coffeescripts/models/conversations/Conversation.coffee in order to
+ # take advantage of the API.
+ #
+ # For a full list of supported attributes, see the Conversation API
+ # documentation.
+
+ url: '/api/v1/conversations'
+
+ BLANK_BODY_ERR = I18n.t 'cannot_be_empty', 'Message cannot be blank'
+ NO_RECIPIENTS_ERR = I18n.t('no_recipients_choose_another_group',
+ 'No recipients are in this group. Please choose another group.')
+
+ validate: (attrs,options) ->
+ errors = {}
+ if !attrs.body or !$.trim(attrs.body.toString())
+ errors.body = [ message: BLANK_BODY_ERR ]
+ if !attrs.recipients || !attrs.recipients.length
+ errors.recipients = [ message: NO_RECIPIENTS_ERR ]
+ if _.keys(errors).length
+ errors
+ else
+ undefined
+
View
64 app/coffeescripts/views/MessageStudentsDialog.coffee
@@ -0,0 +1,64 @@
+define [
+ 'i18n!quizzes'
+ 'compiled/views/ValidatedFormView'
+ 'jst/messageStudentsDialog'
+ 'compiled/models/Conversation'
+ 'jst/_messageStudentsWhoRecipientList'
+ 'underscore'
+ 'compiled/jquery/serializeForm'
+], (I18n,ValidatedFormView, messageStudentsDialog,Conversation,recipientList,_) ->
+
+ class MessageStudentsDialog extends ValidatedFormView
+
+ # A list of "recipientGroups" that have two properties:
+ # name: String # Describes the group of users
+ # recipients: Array of Objects
+ # These objects must have two keys:
+ # id: String or Number # user's id
+ # short_name: String # represents a short version of the user's name
+ @optionProperty 'recipientGroups'
+
+ # The title of whatever the message is "for", renders the text as
+ # for <title> when the dialog is rendered.
+ @optionProperty 'title'
+
+ els:
+ '[name=recipientGroupName]': '$recipientGroupName'
+ '#message-recipients': '$messageRecipients'
+ '[name=body]': '$messageBody'
+
+ template: messageStudentsDialog
+
+ initialize: (opts) ->
+ super
+ @recipients = @recipientGroups[0].recipients
+ @model or= new Conversation
+
+ events: _.extend({}, ValidatedFormView::events,
+ 'change [name=recipientGroupName]': 'updateListOfRecipients')
+
+ toJSON: =>
+ json = {}
+ json[key] = @[key] for key in [ 'title','recipients','recipientGroups' ]
+ json
+
+ validateBeforeSave: (data, errors) =>
+ errs = @model.validate data
+ if errs
+ errors.body = errs.body if errs.body
+ errors.recipientGroupName = errs.recipients if errs.recipients
+ errors
+
+ _findRecipientGroupByName: (name) =>
+ _.detect @recipientGroups, (grp) -> grp.name is name
+
+ getFormData: =>
+ {recipientGroupName, body} = @$el.toJSON()
+ {recipients} = @_findRecipientGroupByName recipientGroupName
+ body: body, recipients: _.pluck(recipients,'id')
+
+ updateListOfRecipients: =>
+ groupName = @$recipientGroupName.val()
+ {recipients} = @_findRecipientGroupByName groupName
+ @$messageRecipients.html recipientList recipients: recipients
+
View
9 app/controllers/quizzes_controller.rb
@@ -183,7 +183,9 @@ def show
@stored_params = (@submission.temporary_data rescue nil) if params[:take] && @submission && (@submission.untaken? || @submission.preview?)
@stored_params ||= {}
log_asset_access(@quiz, "quizzes", "quizzes")
- js_env :QUIZZES_URL => polymorphic_url([@context, :quizzes])
+ js_env({:QUIZZES_URL => polymorphic_url([@context, :quizzes]),
+ :IS_SURVEY => @quiz.survey?,
+ :QUIZ => quiz_json(@quiz,@context,@current_user,session)})
if params[:take] && can_take_quiz?
# allow starting the quiz via a GET request, but only when using a lockdown browser
if request.post? || (@quiz.require_lockdown_browser? && !quiz_submission_active?)
@@ -196,6 +198,7 @@ def show
end
def managed_quiz_data
+ extend Api::V1::User
if authorized_action(@quiz, @current_user, [:grade, :read_statistics])
students = @context.students_visible_to(@current_user).order_by_sortable_name.to_a.uniq
@submissions = @quiz.quiz_submissions.for_user_ids(students.map(&:id)).where("workflow_state<>'settings_only'").all
@@ -206,6 +209,10 @@ def managed_quiz_data
@submitted_students = @submitted_students.sort_by{|stu| submission_ids[stu.id] }
end
@unsubmitted_students = students.reject{|stu| submission_ids[stu.id] }
+ submitted_students_json = @submitted_students.map { |u| user_json(u, @current_user, session) }
+ unsubmitted_students_json = @unsubmitted_students.map { |u| user_json(u, @current_user, session) }
+ @quiz_submission_list = {:UNSUBMITTED_STUDENTS => unsubmitted_students_json,
+ :SUBMITTED_STUDENTS => submitted_students_json}.to_json
render :layout => false
end
end
View
6 app/stylesheets/jst/messageStudentsDialog.scss
@@ -0,0 +1,6 @@
+.message-students-dialog {
+ textarea {
+ width: 95%;
+ height: 200px;
+ }
+}
View
5 app/views/jst/_messageStudentsWhoRecipientList.handlebars
@@ -0,0 +1,5 @@
+{{#each recipients}}
+ <span class="label">
+ {{short_name}}
+ </span>
+{{/each}}
View
45 app/views/jst/messageStudentsDialog.handlebars
@@ -0,0 +1,45 @@
+<div class="message-students-dialog">
+ <form class="bootstrap-form">
+
+ <h2>{{#t "message_students_who"}}Message Students Who..{{/t}}</h2>
+ <p>
+ {{#t "for_title"}}
+ for {{title}}
+ {{/t}}
+ </p>
+
+ <div class="control-group">
+ <div class="controls">
+ <select name="recipientGroupName">
+ {{#each recipientGroups}}
+ <option value="{{name}}">{{name}}</option>
+ {{/each}}
+ </select>
+ </div>
+ </div>
+
+ <div id="message-recipients">
+ {{> messageStudentsWhoRecipientList this}}
+ </div>
+
+ <div class="control-group">
+ <div class="controls">
+ <textarea name=body></textarea>
+ </div>
+ </div>
+
+ <div class="button-container">
+ <button class="btn dialog_closer btn-secondary"
+ data-text-while-loading='{{#t "#buttons.sending"}}Sending...{{/t}}'>
+ {{#t "#buttons.cancel"}}Cancel{{/t}}
+ </button>
+ <button class="btn btn-primary"
+ data-text-while-loading='{{#t "#buttons.sending"}}Sending...{{/t}}'
+ data-text-when-loaded='{{#t "#buttons.sent"}}Sent!{{/t}}'>
+ {{#t "#buttons.send_message"}}Send Message{{/t}}
+ </button>
+
+ </div>
+
+ </form>
+</div>
View
7 app/views/quizzes/managed_quiz_data.html.erb
@@ -70,3 +70,10 @@
</div>
</div>
+
+<script>
+<%# Doing this here instead of js_env because js_env doesn't get called
+ with render :layout => false %>
+window.ENV = window.ENV || {};
+window.ENV.QUIZ_SUBMISSION_LIST = <%= raw(@quiz_submission_list) %>;
+</script>
View
5 app/views/quizzes/show.html.erb
@@ -187,7 +187,10 @@
<% if @any_submissions_pending_review %>
<i class="icon-warning"></i>
<% end %>
- <i class="icon-group"></i> <%= @quiz.survey? ? t('links.show_student_survey_results', "Show Student Survey Results") : t('links.show_student_quiz_results', "Show Student Quiz Results") %>
+ <i class="icon-group"></i>
+ <span id="quiz_details_text">
+ <%= @quiz.survey? ? t('links.show_student_survey_results', "Show Student Survey Results") : t('links.show_student_quiz_results', "Show Student Quiz Results") %>
+ </span>
<div style="font-size: 0.8em; padding-left: 20px;">(<%= t(:students_submitted_so_far, {:one => "1 student submitted so far", :other => "%{count} students submitted so far"}, :count => @submitted_student_count) %>)</div>
</a>
</li>
View
5 lib/messageable_user.rb
@@ -96,7 +96,10 @@ def self.context_recipients(recipients)
end
def self.individual_recipients(recipients)
- recipients.grep(Calculator::INDIVIDUAL_RECIPIENT).map(&:to_i)
+ recipients.select{ |id|
+ !id.is_a?(String) ||
+ id =~ Calculator::INDIVIDUAL_RECIPIENT
+ }.map(&:to_i)
end
def common_groups
View
66 public/javascripts/quiz_show.js
@@ -19,31 +19,21 @@
define([
'i18n!quizzes.show',
'jquery' /* $ */,
+ 'compiled/views/MessageStudentsDialog',
'quiz_arrows',
'quiz_inputs',
'jquery.instructure_date_and_time' /* dateString, time_field, datetime_field */,
'jqueryui/dialog',
+ 'compiled/jquery/fixDialogButtons',
+ 'compiled/jquery.rails_flash_notifications',
'jquery.instructure_misc_helpers' /* scrollSidebar */,
'jquery.instructure_misc_plugins' /* ifExists, confirmDelete */,
'jquery.disableWhileLoading',
'message_students' /* messageStudents */
-], function(I18n, $, showAnswerArrows, inputMethods) {
+], function(I18n, $, MessageStudentsDialog, showAnswerArrows, inputMethods) {
$(document).ready(function () {
- function loadStudents(callback) {
- return ensureStudentsLoaded(function() {
- var students = $('#quiz_details .student_list .student').not('.blank').map(function() {
- return {
- id : $(this).attr('data-id'),
- name : $.trim($(this).find(".name").text()),
- submitted: $(this).closest(".student_list").hasClass('submitted')
- };
- });
- callback(students)
- });
- }
-
function ensureStudentsLoaded(callback) {
if ($('#quiz_details').length) {
return callback();
@@ -55,30 +45,6 @@ $(document).ready(function () {
};
}
- function showMessageStudentsForm(students) {
- var title = $("#quiz_title").text();
-
- window.messageStudents({
- options: [
- {text: I18n.t('have_taken_the_quiz', "Have taken the quiz")},
- {text: I18n.t('have_not_taken_the_quiz', "Have NOT taken the quiz")}
- ],
- title: title,
- students: students,
- callback: function(selected, cutoff, students) {
- students = $.grep(students, function($student, idx) {
- var student = $student.user_data;
- if(selected == I18n.t('have_taken_the_quiz', "Have taken the quiz")) {
- return student.submitted;
- } else if(selected == I18n.t('have_not_taken_the_quiz', "Have NOT taken the quiz")) {
- return !student.submitted;
- }
- });
- return $.map(students, function(student) { return student.user_data.id; });
- }
- });
- }
-
showAnswerArrows();
inputMethods.disableInputs('[type=radio], [type=checkbox]');
inputMethods.setWidths();
@@ -106,7 +72,7 @@ $(document).ready(function () {
event.preventDefault();
$("#quiz_details_wrapper").disableWhileLoading(
ensureStudentsLoaded(function() {
- var $quizResultsText = $('#quiz_results_text');
+ var $quizResultsText = $('#quiz_details_text');
$("#quiz_details").slideToggle();
if (hasOpenedQuizDetails) {
if (ENV.IS_SURVEY) {
@@ -131,7 +97,27 @@ $(document).ready(function () {
});
$(".message_students_link").click(function(event) {
event.preventDefault();
- $(this).disableWhileLoading(loadStudents(showMessageStudentsForm));
+ ensureStudentsLoaded(function(){
+ var submissionList = ENV.QUIZ_SUBMISSION_LIST;
+ var unsubmittedStudents = submissionList.UNSUBMITTED_STUDENTS;
+ var submittedStudents = submissionList.SUBMITTED_STUDENTS;
+ var haveTakenQuiz = I18n.t('have_taken_the_quiz', "Have taken the quiz");
+ var haveNotTakenQuiz =
+ I18n.t('have_not_taken_the_quiz', "Have NOT taken the quiz");
+ var dialog = new MessageStudentsDialog({
+ title: ENV.QUIZ.title,
+ recipientGroups: [
+ { name: haveTakenQuiz, recipients: submittedStudents },
+ { name: haveNotTakenQuiz, recipients: unsubmittedStudents }
+ ]
+ });
+ dialog.onSaveSuccess = function() {
+ dialog.$el.dialog('close');
+ dialog.remove();
+ $.flashMessage(I18n.t('notices.message_sent', "Message Sent!"));
+ };
+ dialog.render().$el.dialog({width: 'auto', modal: 'true'}).fixDialogButtons();
+ });
});
$.scrollSidebar();
View
24 spec/coffeescripts/models/ConversationSpec.coffee
@@ -0,0 +1,24 @@
+define [
+ 'compiled/models/Conversation'
+], (Conversation) ->
+
+ module "Conversation",
+ setup: ->
+ @conversation = new Conversation
+
+ test "#validate validates body length", ->
+ ok @conversation.validate(body: '')
+ ok @conversation.validate(body: null).body
+ ok @conversation.validate(body: 'body', recipients: [{}]) == undefined
+
+ test "#validate validates there must be at least one recipient object", ->
+ testData = body: 'i love testing javascript', recipients: [ {} ]
+ ok @conversation.validate(testData) == undefined
+ testData.recipients = []
+ ok @conversation.validate(testData).recipients
+ delete testData.recipients
+ ok @conversation.validate(testData).recipients
+
+ test "#url is the correct API url", ->
+ equal @conversation.url, '/api/v1/conversations'
+
View
68 spec/coffeescripts/views/MessageStudentsDialogSpec.coffee
@@ -0,0 +1,68 @@
+define [
+ 'compiled/views/MessageStudentsDialog'
+ 'jquery'
+ 'underscore'
+], (MessageStudentsDialog,$,_) ->
+
+ module "MessageStudentsDialog",
+ setup: ->
+ @testData =
+ title: 'The Quiz'
+ recipientGroups: [
+ {
+ name: 'have taken the quiz'
+ recipients: [ {id: 1, short_name: 'bob'}]
+ }
+ {
+ name: "haven't taken the quiz"
+ recipients: [ {id: 2, short_name: 'alice'} ]
+ }
+ ]
+ @dialog = new MessageStudentsDialog @testData
+ @dialog.render()
+ $('#fixtures').append @dialog.$el
+
+ teardown: ->
+ @dialog.remove()
+ $('#fixtures').empty()
+
+ test "#initialize", ->
+
+ deepEqual @dialog.recipientGroups, @testData.recipientGroups,
+ 'saves recipientGroups'
+
+ deepEqual @dialog.recipients, @testData.recipientGroups[0].recipients,
+ 'saves first recipientGroups recipients to be displayed'
+
+ deepEqual @dialog.title, @testData.title, 'saves the title to be displayed'
+
+ ok @dialog.model, 'creates conversation automatically'
+
+ test "updates list of recipients when dropdown changes", ->
+ @dialog.$recipientGroupName.val("haven't taken the quiz").trigger 'change'
+ html = @dialog.$el.html()
+
+ ok html.match('alice'), 'updated with the new list of recipients'
+ ok !html.match('bob'), "doesn't contain old list of recipients"
+
+ test "#getFormValues returns correct values", ->
+ messageBody = 'Students please take your quiz, dang it!'
+ @dialog.$messageBody.val messageBody
+ json = @dialog.getFormData()
+ {body,recipients} = json
+
+ strictEqual json.body, messageBody, 'includes message body'
+ strictEqual json.recipientGroupName, undefined,
+ "doesn't include recipientGroupName"
+ deepEqual json.recipients,
+ _.pluck(@testData.recipientGroups[0].recipients, 'id'),
+ 'includes list of ids'
+
+ test "validateBeforeSave", ->
+ errors = @dialog.validateBeforeSave({body: ''},{})
+ ok errors.body[0].message, 'validates empty body'
+ errors = @dialog.validateBeforeSave({body: 'take your dang quiz'},{recipients: []})
+ ok errors.recipientGroupName[0].message, 'validates when sending to empty list of users'
+
+
+
View
35 spec/selenium/teacher_quizzes_spec.rb
@@ -120,41 +120,6 @@ def create_quiz_with_default_due_dates
f('#main .description').should include_text(test_text)
end
- it "message students who... should do something" do
- @context = @course
- q = quiz_model
- q.generate_quiz_data
- q.save!
- # add a student to the course
- student = student_in_course(:active_enrollment => true).user
- student.conversations.size.should == 0
-
- get "/courses/#{@course.id}/quizzes/#{q.id}"
-
- f('.al-trigger').click
- driver.find_element(:partial_link_text, "Message Students Who...").click
- wait_for_ajaximations
- dialog = ffj("#message_students_dialog:visible")
- dialog.length.should == 1
- dialog = dialog.first
-
- click_option('.message_types', 'Have taken the quiz')
- students = ffj(".student_list > .student:visible")
-
- students.length.should == 0
-
- click_option('.message_types', 'Have NOT taken the quiz')
- students = ffj(".student_list > .student:visible")
- students.length.should == 1
-
- dialog.find_element(:css, 'textarea#body').send_keys('This is a test message.')
-
- submit_dialog(dialog)
- wait_for_ajax_requests
-
- student.conversations.size.should == 1
- end
-
it "should asynchronously load student quiz results" do
@context = @course
q = quiz_model

0 comments on commit be37cbb

Please sign in to comment.
Something went wrong with that request. Please try again.