Skip to content
This repository has been archived by the owner on May 10, 2022. It is now read-only.

Feature/exercises #348

Merged
merged 16 commits into from
May 27, 2014
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,4 @@ db/.data.yml.gz
brakeman.html
cucumber_rerun.txt
rspec.failures
vendor/cache
1 change: 1 addition & 0 deletions .ruby-version
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1.9.3-p194
1 change: 0 additions & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ group :development, :test do
gem 'rspec-rails'
gem 'rspec-rerun'
gem 'cucumber-rails', :require => false
gem 'capybara-webkit'
gem 'capybara-screenshot'
gem 'poltergeist'
gem 'launchy'
Expand Down
4 changes: 0 additions & 4 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,6 @@ GEM
xpath (~> 0.1.4)
capybara-screenshot (0.2.2)
capybara (>= 1.0)
capybara-webkit (0.12.1)
capybara (>= 1.0.0, < 1.2)
json
carrierwave (0.7.1)
activemodel (>= 3.2.0)
activesupport (>= 3.2.0)
Expand Down Expand Up @@ -359,7 +356,6 @@ DEPENDENCIES
brakeman
capistrano (~> 2.12.0)
capybara-screenshot
capybara-webkit
carrierwave
cheat
chronic
Expand Down
13 changes: 13 additions & 0 deletions app/controllers/assignment_exercises_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Copyright 2011-2014 Rice University. Licensed under the Affero General Public
# License version 3 or later. See the COPYRIGHT file for details.


class AssignmentExercisesController < ApplicationController

def show
@assignment_exercises = AssignmentExercise.find(params[:id]).topic_exercise.assignment_exercises
raise SecurityTransgression unless present_user.can_read?(@assignment_exercises.first)
@include_mathjax = true
end

end
33 changes: 32 additions & 1 deletion app/models/assignment_exercise.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright 2011-2012 Rice University. Licensed under the Affero General Public
# Copyright 2011-2014 Rice University. Licensed under the Affero General Public
# License version 3 or later. See the COPYRIGHT file for details.

class AssignmentExercise < ActiveRecord::Base
Expand Down Expand Up @@ -35,4 +35,35 @@ def add_tags(tags)
def has_tag?(tag)
tag_list.include?(tag.downcase)
end


# Get the list of student exercises:
# - grouped by student status
# - filtered by visibility for the given user
# - sorted by student
def student_exercises_by_student_status(user)
exercises = student_exercises.visible_by_student(user)

exercises.group_by do |ex|
case
when ex.student.has_dropped
:dropped
when ex.student.is_auditing
:auditing
else
:registered
end
end
end


#############################################################################
# Access control methods
#############################################################################
def can_be_read_by?(user)
!user.is_anonymous? && (user.is_administrator? ||
user.is_researcher? ||
assignment.is_educator?(user))
end

end
22 changes: 21 additions & 1 deletion app/models/student_exercise.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ class StudentExercise < ActiveRecord::Base
belongs_to :assignment_exercise
has_many :response_times, :as => :response_timeable, :dependent => :destroy
has_many :free_responses, :dependent => :destroy, :order => :number
has_one :student, :through => :student_assignment
has_one :consent, :through => :student

before_destroy :destroyable?, prepend: true

Expand Down Expand Up @@ -38,7 +40,25 @@ class StudentExercise < ActiveRecord::Base

validate :changes_match_state, :on => :update
validate :has_at_least_one_free_response, :on => :update


# Gets the list of student exercises visible for the given user,
# sorted by student status (dropped / auditing) and the last name.
scope :visible_by_student, lambda { |user|
if user.is_researcher? || user.is_visitor?
# The dummy "1 = 1" condition is added in order to use the
# merge method.
visible = where("1 = 1").merge(Student.visible(user))
Copy link
Member

Choose a reason for hiding this comment

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

Just replace merge with a subquery using IN and you don't need all this handcoded SQL.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Dantemss - I need to order the resulting student exercises by the student last name and student status - so, having the students in the subquery won't be sufficient. Does that make sense?

Copy link
Member

Choose a reason for hiding this comment

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

Please avoid any hand-coded SQL. Even if we need to break the query up into steps, I'd rather keep the queries only using the abstracted query syntax provided by Rails.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the query I get from the above:

SELECT 
       "student_exercises".* 
FROM   "student_exercises" 
INNER JOIN 
      "student_assignments" ON "student_assignments"."id" = "student_exercises"."student_assignment_id" 
INNER JOIN 
      "students" ON "students"."id" = "student_assignments"."student_id" 
INNER JOIN "users" ON "students"."user_id" = "users"."id" 
WHERE "student_exercises"."assignment_exercise_id" = 10 
ORDER BY "students"."has_dropped", "students"."is_auditing", lower("users"."last_name")

This is exactly the query that's needed. I can understand the avoid handcoded sql argument. But don't you guys think in this scenario (where rails is unable to produce the optimal sql output) - judicious sprinkling of handcoded sql makes for a performant page?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is the one for researchers:

SELECT 
       "student_exercises".* 
FROM "student_exercises" 
INNER JOIN 
      "student_assignments" ON "student_assignments"."id" = "student_exercises"."student_assignment_id" 
INNER JOIN 
      "students" ON "students"."id" = "student_assignments"."student_id" 
INNER JOIN 
      "consents" ON "consents"."consentable_id" = "students"."id" 
AND  "consents"."consentable_type" = 'Student' 
INNER JOIN "users" ON "students"."user_id" = "users"."id" 
WHERE "student_exercises"."assignment_exercise_id" = 10 
AND "consents"."did_consent" = 't' 
AND (1 = 1) 
ORDER BY "students"."has_dropped", "students"."is_auditing", lower("users"."last_name")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah - makes sense. I will try the :visible route. Since we are bringing the consent logic here - it seems like good idea.

On that note, @jpslav / @Dantemss - good idea to think about AcsAsConsentable or something like that?

Copy link
Member

Choose a reason for hiding this comment

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

You mean making a new gem? Probably not for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Something in the lib dir (like ActsAsNumberable). But yeah - there are just two occurrences, I guess we can wait until there is one more.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I agree we should hold off on library-ifying the consenting infrastructure. The only user at this point is Tutor. Maybe when we redo Tutor it'll make sense to abstract the consent stuff into a gem.

Copy link
Member

Choose a reason for hiding this comment

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

There's a whole mess of consenting infrastructure around, so a gem is the right idea just don't know if we're at the point where we want to spend time consolidating and generalizing it.

else
visible = joins{student}
end
# Handcoded join avoids duplicate joins on student_assignment and
# student tables.
visible
.joins("INNER JOIN \"users\" ON \"students\".\"user_id\" = \"users\".\"id\"")
.includes{student_assignment.student.user}
.order{[students.has_dropped, students.is_auditing, lower(students.users.last_name)]}
}

after_save :update_cached_conditions

before_save :lock_choice_if_indicated, :on => :update
Expand Down
57 changes: 57 additions & 0 deletions app/views/assignment_exercises/_response_list.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
<%# Copyright 2011-2014 Rice University. Licensed under the Affero General Public
License version 3 or later. See the COPYRIGHT file for details. %>

<%#

Params:
responses: required.

%>
<table class="list" width="90%">
<thead>
<tr>
<th width="8%">Student</th>
<th width="60%">Response</th>
<th width="8%">Choice</th>
<th width="8%">Score</th>
<th width="8%">Times</th>
</tr>
<thead>
<tbody>
<% responses.each_with_index do |se, ii| %>
<% student = se.student_assignment.student %>
<tr>
<td>
<%= link_to student.full_name(present_user), se.student_assignment %>
</td>
<td>
<div class='free_response'>
<%= '(blank)' if se.free_responses.none? %>
<% se.free_responses.each do |free_response| %>
<%= render :partial => 'free_responses/show',
:locals => { :free_response => free_response } %>
<% end %>
</div>
<% if se.requires_follow_up_question? %>
<div class='follow_up_question'>
<%= se.follow_up_answer || '(blank)' %>
</div>
<% end %>
</td>
<td><%= se.selected_answer.nil? ? "--" : "(#{choice_letter(se.selected_answer)})" %></td>
<td><%= link_to se.score, student_exercise_score_detail_path(se), :remote => true %></td>
<td>
<%= link_to_function "Show", "$('#se_#{ii}_rts').show(); $(this).hide(); $(this).next().show(); " %>
<%= link_to_function "Hide", "$('#se_#{ii}_rts').hide(); $(this).hide(); $(this).prev().show(); ", :style => "display:none" %>
</td>
</tr>
<tr id="se_<%= ii %>_rts" style="display:none">
<td></td>
<td colspan="4">
<%= render :partial => 'response_times/list',
:locals => {:response_times => se.response_times, :show_page => true} %>
</td>
</tr>
<% end %>
</tbody>
</table>
54 changes: 54 additions & 0 deletions app/views/assignment_exercises/show.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<%# Copyright 2011-2014 Rice University. Licensed under the Affero General Public
License version 3 or later. See the COPYRIGHT file for details. %>

<%
assignment_exercise = @assignment_exercises.first
exercise = assignment_exercise.topic_exercise.exercise
%>
<%= pageHeading("Student responses for
Assignment: #{assignment_exercise.assignment.assignment_plan.name},
Exercise: #{assignment_exercise.number}",
{:sub_heading_text => full_class_name(assignment_exercise.assignment)}) %>


<%= render 'exercises/exercise_blurb', :exercise => exercise,
:show_correct_answer => true,
:show_choices => true %>

<%= section "Responses", :classes => 'section-exercise-responses no_bar' do %>

NOTE: In the 'Response' column, free responses are always shown. If
required by the learning conditions, follow-up answers appear in
gray below the free responses. In both cases, a missing response
will be depicted by the string '(blank)'.

<% @assignment_exercises.each do |ae| %>
<%= section "Cohort: #{ae.assignment.cohort.name}" do %>
<% exercises = ae.student_exercises_by_student_status(present_user) %>
<% exercises.each do |status, lst| %>
<% if !lst.nil? && lst.count > 0 %>
<% message = "From #{status} students" %>
<%= render :partial => 'response_list',
:layout => 'layouts/section',
:locals => {:responses => lst,
:classes => 'no_bar',
:section_name => message } %>
<% end %>
<% end %>
<% end %>
<% end %>

<% end %>

<% prev_ae = assignment_exercise.prev %>
<% next_ae = assignment_exercise.next %>
<%
navitem { link_to "Class", klass_path(assignment_exercise.assignment.klass) }
navitem { link_to "Assignment", assignment_path(assignment_exercise.assignment) }
if !prev_ae.nil?
navitem { link_to "Previous Exercise", assignment_exercise_path(prev_ae) }
end
if !next_ae.nil?
navitem { link_to "Next Exercise", assignment_exercise_path(next_ae) }
end
%>
6 changes: 4 additions & 2 deletions app/views/assignments/_student_results.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@

<tr>
<th>Name</th>
<% (1..num_exercises).each do |ii| %>
<th style="text-align:center"><%= ii.to_s %></th>
<% assignment.assignment_exercises.each_with_index do |ae, ii| %>
<th style="text-align:center">
<%= link_to (ii + 1).to_s, ae %>
</th>
<% end %>
</tr>

Expand Down
27 changes: 27 additions & 0 deletions app/views/exercises/_choices.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<%# Copyright 2011-2014 Rice University. Licensed under the Affero General Public
License version 3 or later. See the COPYRIGHT file for details. %>

<%#

Params:

exercise: required
mark_correct: optional. Marks the correct answer with a class. default: false.
%>

<%
exercise_data = exercise.content
solution = exercise.correct_choice_index
mark_correct = false if local_assigns[:mark_correct].nil?
%>

<div class:="question multiple-choice">
<ul class="unstyled">
<% exercise_data["simple_question"]["answer_choices"].each_with_index do |answer_choice, ii| %>
<li class="row <% if mark_correct && solution == ii %>correct-answer<% end %>">
<div class="choice-letter span1"><%= choice_letter(ii) %>)</div>
<div class="answer-choice span6 text-left"><%= answer_choice["html"].html_safe %></div>
<li>
<% end %>
</ul>
</div>
19 changes: 19 additions & 0 deletions app/views/exercises/_correct_answer.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<%# Copyright 2011-2014 Rice University. Licensed under the Affero General Public
License version 3 or later. See the COPYRIGHT file for details. %>

<%#

Params:

exercise: required.
phrase: required.

%>

<%
solution = exercise.correct_choice_index
%>

<div class="exercise-answer">
<p><%= phrase %> <b>(<%= choice_letter(solution) %>)</b></p>
</div>
52 changes: 52 additions & 0 deletions app/views/exercises/_exercise_blurb.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<%# Copyright 2011-2014 Rice University. Licensed under the Affero General Public
License version 3 or later. See the COPYRIGHT file for details. %>

<%#

Params:

show_correct_answer: optional. default: false
show_choices: optional. defailt: false

%>

<%
show_correct_answer = false if local_assigns[:show_correct_answer].nil?
show_choices = false if local_assigns[:show_choices].nil?

%>

<%= render :partial => 'exercises/question',
:layout => 'layouts/section',
:locals => {
:exercise => exercise,
:section_name => "Exercise",
:collapsed => false,
:classes => 'first no_bar'
} %>


<% if show_correct_answer %>
<%= render :partial => 'exercises/correct_answer',
:layout => 'layouts/section',
:locals => {
:phrase => 'The correct answer is:',
:exercise => exercise,
:section_name => "Correct Answer",
:collapsed => false,
:classes => 'first no_bar'
} %>
<% end %>



<% if show_choices %>
<%= render :partial => 'exercises/choices',
:layout => 'layouts/section',
:locals => {
:exercise => exercise,
:section_name => "Answer Choices",
:collapsed => true,
:classes => 'first no_bar'
} %>
<% end %>
24 changes: 24 additions & 0 deletions app/views/exercises/_question.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<%# Copyright 2011-2014 Rice University. Licensed under the Affero General Public
License version 3 or later. See the COPYRIGHT file for details. %>

<%# Clients must supply:
exercise
%>

<%
question = exercise.content["simple_question"]
%>

<div class="exercise-question">
<div class="question question-box">
<div class="question intro">
<% if !question["introduction"].nil? %>
<%= question["introduction"]["html"].html_safe %>
<% end %>
</div>

<div class="question question-text">
<%= question["content"]["html"].html_safe %>
</div>
</div>
</div>
Loading