From 2b2decdd860997c64f9f65485ca4dac54c854086 Mon Sep 17 00:00:00 2001
From: lakshmivyas <lakshmi.vyas@gmail.com>
Date: Thu, 12 Jun 2014 11:14:59 +0530
Subject: [PATCH 1/4] lml/ost#345: Add questions with links to exercises
 section.

- Add `family` scope and method to the assignment model.
- Add `_questions` partial to show the exercise questions.
- Add `show_url` param and ability to the `_question` partial.
- Add `authority?` view helper function.
- Use `authority?` view helper function.
- Fix copyrights.

TODO:

- The styles for questions are a little weird.  Leaving things
  as is to wait for the UX review.
---
 app/controllers/assignments_controller.rb |  3 ++-
 app/helpers/assignments_helper.rb         |  7 ++++++-
 app/models/assignment.rb                  | 12 ++++++++++--
 app/views/assignments/_questions.html.erb | 21 +++++++++++++++++++++
 app/views/assignments/show.html.erb       | 14 ++++++++++----
 app/views/exercises/_question.html.erb    | 14 ++++++++++++--
 app/views/exercises/_url.html.erb         | 12 ++++++++++++
 7 files changed, 73 insertions(+), 10 deletions(-)
 create mode 100644 app/views/assignments/_questions.html.erb
 create mode 100644 app/views/exercises/_url.html.erb

diff --git a/app/controllers/assignments_controller.rb b/app/controllers/assignments_controller.rb
index b900d04f..f2c1d955 100644
--- a/app/controllers/assignments_controller.rb
+++ b/app/controllers/assignments_controller.rb
@@ -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.
 
 
@@ -19,6 +19,7 @@ def show
     if !student.nil?
       @student_assignment = StudentAssignment.for_student(student).for_assignment(@assignment).first
     end    
+    @include_mathjax = view_context.authority?
   end
 
   def grades
diff --git a/app/helpers/assignments_helper.rb b/app/helpers/assignments_helper.rb
index 707838c7..09bce916 100644
--- a/app/helpers/assignments_helper.rb
+++ b/app/helpers/assignments_helper.rb
@@ -1,5 +1,10 @@
-# 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.
 
 module AssignmentsHelper
+  def authority?
+    @assignment.cohort.klass.is_educator?(present_user) ||
+      Researcher.is_one?(present_user) ||
+      present_user.is_administrator?
+  end
 end
diff --git a/app/models/assignment.rb b/app/models/assignment.rb
index 70acf98c..1212c10b 100644
--- a/app/models/assignment.rb
+++ b/app/models/assignment.rb
@@ -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 Assignment < ActiveRecord::Base
@@ -21,7 +21,15 @@ class Assignment < ActiveRecord::Base
   before_destroy :destroyable?, prepend: true
   
   attr_accessor :dry_run
-  
+
+  scope :family, lambda { |assignment|
+    where{assignment_plan_id == assignment.assignment_plan.id}
+  }
+
+  def family
+    Assignment.family(self)
+  end
+
   def klass
     cohort.klass
   end
diff --git a/app/views/assignments/_questions.html.erb b/app/views/assignments/_questions.html.erb
new file mode 100644
index 00000000..02240d54
--- /dev/null
+++ b/app/views/assignments/_questions.html.erb
@@ -0,0 +1,21 @@
+<%# Copyright 2011-2014 Rice University. Licensed under the Affero General Public
+    License version 3 or later.  See the COPYRIGHT file for details. %>
+
+<%#
+
+Params:
+
+    assignment
+%>
+<% assignment.family.each do |thissignment| %>
+  <%= sub_section "Cohort: #{thissignment.cohort.name}" do %>
+    <ol class="cohort-questions">
+    <% thissignment.assignment_exercises.each do |ae| %>
+      <% exercise = ae.topic_exercise.exercise %>
+      <li>
+        <%= render 'exercises/question', :exercise => exercise, :show_url => true %>
+      </li>
+    <% end  %>
+    </ol>
+  <% end %>
+<% end %>
diff --git a/app/views/assignments/show.html.erb b/app/views/assignments/show.html.erb
index 997b70ba..0d7a8bb0 100644
--- a/app/views/assignments/show.html.erb
+++ b/app/views/assignments/show.html.erb
@@ -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. %>
 
 <% addTestMeta {{:page_type  => "show"}} %>
@@ -82,11 +82,15 @@
   <% end %>
 <% end %>
 
-<%= section "Exercises", {:classes => 'no_bar exercises'} do %>
+<%= section "Exercises", {:classes => 'no_bar exercises', :collapsible => authority?, :collapsed => authority?} do %>
 
   <% if @student_assignment.blank? %>
     <p>This assignment has <%= pluralize(@assignment.assignment_exercises.count, "exercise") %>.</p>
-    <p>You cannot see or work the exercises because they were not assigned to you.</p>
+    <% if authority? %>
+      <%= render 'questions', :assignment => @assignment %>
+    <% else  %>%>
+      <p>You cannot see or work the exercises because they were not assigned to you.</p>
+    <% end %>
   <% else %>    
     <%= render :partial => 'student_exercises/list', 
                :locals => { :student_assignment   => @student_assignment,
@@ -95,7 +99,7 @@
 
 <% end %>
 
-<% if klass.is_educator?(present_user) || Researcher.is_one?(present_user) || present_user.is_administrator? %>
+<% if authority? %>
   <%= render 'results', :assignment_plan => assignment_plan %>
 <% end %>
 
@@ -104,3 +108,5 @@
   navitem(:can_read_children?, @assignment, :grades) { link_to "Grades", grades_assignment_path(@assignment, :format => :xls) }
 %>
 
+
+
diff --git a/app/views/exercises/_question.html.erb b/app/views/exercises/_question.html.erb
index cb593450..78667673 100644
--- a/app/views/exercises/_question.html.erb
+++ b/app/views/exercises/_question.html.erb
@@ -1,12 +1,17 @@
 <%# 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
+<%#
+
+Params:
+
+  exercise
+  show_url: optional. default: false.
 %>
 
 <%
   question = exercise.content["simple_question"]
+  show_url = false if local_assigns[:show_url].nil?
 %>
 
 <div class="exercise-question">
@@ -20,5 +25,10 @@
     <div class="question question-text">
       <%= question["content"]["html"].html_safe %>
     </div>
+    <% if show_url  %>
+    <div class="question question-link">
+      <%= render 'exercises/url', :exercise => exercise %>
+    </div>
+    <% end %>
   </div>
 </div>
diff --git a/app/views/exercises/_url.html.erb b/app/views/exercises/_url.html.erb
new file mode 100644
index 00000000..287f3dc9
--- /dev/null
+++ b/app/views/exercises/_url.html.erb
@@ -0,0 +1,12 @@
+<%# 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"]
+%>
+
+<a href="<%= question['url'] %>" class="question-url"><%= question["url"] %></a>

From 801dfe0bc85830f1940b76550817e8860d3c5e2f Mon Sep 17 00:00:00 2001
From: lakshmivyas <lakshmi.vyas@gmail.com>
Date: Fri, 13 Jun 2014 08:59:06 +0530
Subject: [PATCH 2/4] lml/ost#345: Fixes from @dantemss review.

- Remove the newly introduced `family` scope in favor
  of using association accessors.
- Remove extra closing tag (typo).
---
 app/models/assignment.rb                  | 8 --------
 app/views/assignments/_questions.html.erb | 2 +-
 app/views/assignments/show.html.erb       | 2 +-
 3 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/app/models/assignment.rb b/app/models/assignment.rb
index 1212c10b..b5d8c344 100644
--- a/app/models/assignment.rb
+++ b/app/models/assignment.rb
@@ -22,14 +22,6 @@ class Assignment < ActiveRecord::Base
   
   attr_accessor :dry_run
 
-  scope :family, lambda { |assignment|
-    where{assignment_plan_id == assignment.assignment_plan.id}
-  }
-
-  def family
-    Assignment.family(self)
-  end
-
   def klass
     cohort.klass
   end
diff --git a/app/views/assignments/_questions.html.erb b/app/views/assignments/_questions.html.erb
index 02240d54..b16d3997 100644
--- a/app/views/assignments/_questions.html.erb
+++ b/app/views/assignments/_questions.html.erb
@@ -7,7 +7,7 @@ Params:
 
     assignment
 %>
-<% assignment.family.each do |thissignment| %>
+<% assignment.assignment_plan.assignments.each do |thissignment| %>
   <%= sub_section "Cohort: #{thissignment.cohort.name}" do %>
     <ol class="cohort-questions">
     <% thissignment.assignment_exercises.each do |ae| %>
diff --git a/app/views/assignments/show.html.erb b/app/views/assignments/show.html.erb
index 0d7a8bb0..aa98fbdf 100644
--- a/app/views/assignments/show.html.erb
+++ b/app/views/assignments/show.html.erb
@@ -88,7 +88,7 @@
     <p>This assignment has <%= pluralize(@assignment.assignment_exercises.count, "exercise") %>.</p>
     <% if authority? %>
       <%= render 'questions', :assignment => @assignment %>
-    <% else  %>%>
+    <% else  %>
       <p>You cannot see or work the exercises because they were not assigned to you.</p>
     <% end %>
   <% else %>    

From 2d324f97dde3d7a0aeb5d810bebb3e52206f13ac Mon Sep 17 00:00:00 2001
From: lakshmivyas <lakshmi.vyas@gmail.com>
Date: Mon, 23 Jun 2014 07:51:20 +0530
Subject: [PATCH 3/4] Add memoization for the `authority?` function.

- Add dependency for the `memoist` gem.
---
 Gemfile                           | 2 ++
 Gemfile.lock                      | 2 ++
 app/helpers/assignments_helper.rb | 4 +++-
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/Gemfile b/Gemfile
index 574b8982..c5925de4 100644
--- a/Gemfile
+++ b/Gemfile
@@ -47,6 +47,8 @@ gem 'acts-as-taggable-on', '~> 2.3.1'
 
 gem 'sqlite3', '~> 1.3.6'
 
+gem 'memoist'
+
 gem 'rack-mini-profiler'
 gem 'newrelic_rpm'
 
diff --git a/Gemfile.lock b/Gemfile.lock
index 32a345ee..412fd1a8 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -193,6 +193,7 @@ GEM
     mail (2.5.4)
       mime-types (~> 1.16)
       treetop (~> 1.4.8)
+    memoist (0.9.3)
     mime-types (1.25.1)
     mini_magick (3.4)
       subexec (~> 0.2.1)
@@ -370,6 +371,7 @@ DEPENDENCIES
   jquery-rails (~> 2.0.2)
   launchy
   lograge!
+  memoist
   mime-types
   mini_magick
   mysql2 (~> 0.3.11)
diff --git a/app/helpers/assignments_helper.rb b/app/helpers/assignments_helper.rb
index 09bce916..fa50d8ec 100644
--- a/app/helpers/assignments_helper.rb
+++ b/app/helpers/assignments_helper.rb
@@ -1,10 +1,12 @@
 # Copyright 2011-2014 Rice University. Licensed under the Affero General Public 
 # License version 3 or later.  See the COPYRIGHT file for details.
-
+require 'memoist'
 module AssignmentsHelper
+  extend Memoist
   def authority?
     @assignment.cohort.klass.is_educator?(present_user) ||
       Researcher.is_one?(present_user) ||
       present_user.is_administrator?
   end
+  memoize :authority?
 end

From e100b4045c0d80864d0e425608810cc9024be9ff Mon Sep 17 00:00:00 2001
From: lakshmivyas <lakshmi.vyas@gmail.com>
Date: Sat, 28 Jun 2014 15:11:15 +0530
Subject: [PATCH 4/4] lml/ost#345: Handle JP's review comments.

- Use local variable in view/controller instead of helper.
- Separate exercise section rendering for instructor and
  student roles.
- Remove memoist gem.
- Remove url partial and inline using `exercise.url`
---
 Gemfile                                   |  2 --
 Gemfile.lock                              |  2 --
 app/controllers/assignments_controller.rb | 13 +++++++++++-
 app/helpers/assignments_helper.rb         |  8 --------
 app/views/assignments/show.html.erb       | 25 ++++++++++++-----------
 app/views/exercises/_question.html.erb    |  2 +-
 app/views/exercises/_url.html.erb         | 12 -----------
 7 files changed, 26 insertions(+), 38 deletions(-)
 delete mode 100644 app/views/exercises/_url.html.erb

diff --git a/Gemfile b/Gemfile
index c5925de4..574b8982 100644
--- a/Gemfile
+++ b/Gemfile
@@ -47,8 +47,6 @@ gem 'acts-as-taggable-on', '~> 2.3.1'
 
 gem 'sqlite3', '~> 1.3.6'
 
-gem 'memoist'
-
 gem 'rack-mini-profiler'
 gem 'newrelic_rpm'
 
diff --git a/Gemfile.lock b/Gemfile.lock
index 412fd1a8..32a345ee 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -193,7 +193,6 @@ GEM
     mail (2.5.4)
       mime-types (~> 1.16)
       treetop (~> 1.4.8)
-    memoist (0.9.3)
     mime-types (1.25.1)
     mini_magick (3.4)
       subexec (~> 0.2.1)
@@ -371,7 +370,6 @@ DEPENDENCIES
   jquery-rails (~> 2.0.2)
   launchy
   lograge!
-  memoist
   mime-types
   mini_magick
   mysql2 (~> 0.3.11)
diff --git a/app/controllers/assignments_controller.rb b/app/controllers/assignments_controller.rb
index f2c1d955..f19af40c 100644
--- a/app/controllers/assignments_controller.rb
+++ b/app/controllers/assignments_controller.rb
@@ -19,7 +19,8 @@ def show
     if !student.nil?
       @student_assignment = StudentAssignment.for_student(student).for_assignment(@assignment).first
     end    
-    @include_mathjax = view_context.authority?
+
+    @include_mathjax = self.authority?
   end
 
   def grades
@@ -29,5 +30,15 @@ def grades
       format.xls
     end
   end
+  
+  protected
+
+  def authority?
+    @assignment.cohort.klass.is_educator?(present_user) ||
+      Researcher.is_one?(present_user) ||
+      present_user.is_administrator?
+  end
+
+
 
 end
diff --git a/app/helpers/assignments_helper.rb b/app/helpers/assignments_helper.rb
index fa50d8ec..c2a55531 100644
--- a/app/helpers/assignments_helper.rb
+++ b/app/helpers/assignments_helper.rb
@@ -1,12 +1,4 @@
 # Copyright 2011-2014 Rice University. Licensed under the Affero General Public 
 # License version 3 or later.  See the COPYRIGHT file for details.
-require 'memoist'
 module AssignmentsHelper
-  extend Memoist
-  def authority?
-    @assignment.cohort.klass.is_educator?(present_user) ||
-      Researcher.is_one?(present_user) ||
-      present_user.is_administrator?
-  end
-  memoize :authority?
 end
diff --git a/app/views/assignments/show.html.erb b/app/views/assignments/show.html.erb
index aa98fbdf..0188a34d 100644
--- a/app/views/assignments/show.html.erb
+++ b/app/views/assignments/show.html.erb
@@ -8,6 +8,9 @@
 <%
   assignment_plan = @assignment.assignment_plan
   klass = @assignment.cohort.klass
+  authority = @assignment.cohort.klass.is_educator?(present_user) ||
+              Researcher.is_one?(present_user) ||
+              present_user.is_administrator?
 %>
 
 
@@ -82,24 +85,22 @@
   <% end %>
 <% end %>
 
-<%= section "Exercises", {:classes => 'no_bar exercises', :collapsible => authority?, :collapsed => authority?} do %>
-
-  <% if @student_assignment.blank? %>
-    <p>This assignment has <%= pluralize(@assignment.assignment_exercises.count, "exercise") %>.</p>
-    <% if authority? %>
-      <%= render 'questions', :assignment => @assignment %>
-    <% else  %>
-      <p>You cannot see or work the exercises because they were not assigned to you.</p>
-    <% end %>
-  <% else %>    
+<% if !@student_assignment.blank? %>
+  <%= section "Your Exercises", {:classes => 'no_bar exercises'} do %>
     <%= render :partial => 'student_exercises/list', 
                :locals => { :student_assignment   => @student_assignment,
-                            :show_feedback_status => true } %>  
+                            :show_feedback_status => true } %>
   <% end %>
+<% end %>
 
+<% if authority %>
+  <%= section "Exercises", {:classes => 'no_bar exercises', :collapsible => true, :collapsed => true} do %>
+    <p>This assignment has <%= pluralize(@assignment.assignment_exercises.count, "exercise") %>.</p>
+    <%= render 'questions', :assignment => @assignment %>
+  <% end %>
 <% end %>
 
-<% if authority? %>
+<% if authority %>
   <%= render 'results', :assignment_plan => assignment_plan %>
 <% end %>
 
diff --git a/app/views/exercises/_question.html.erb b/app/views/exercises/_question.html.erb
index 78667673..027e460d 100644
--- a/app/views/exercises/_question.html.erb
+++ b/app/views/exercises/_question.html.erb
@@ -27,7 +27,7 @@ Params:
     </div>
     <% if show_url  %>
     <div class="question question-link">
-      <%= render 'exercises/url', :exercise => exercise %>
+      <a href="<%= exercise['url'] %>" class="question-url"><%= exercise["url"] %></a>
     </div>
     <% end %>
   </div>
diff --git a/app/views/exercises/_url.html.erb b/app/views/exercises/_url.html.erb
deleted file mode 100644
index 287f3dc9..00000000
--- a/app/views/exercises/_url.html.erb
+++ /dev/null
@@ -1,12 +0,0 @@
-<%# 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"]
-%>
-
-<a href="<%= question['url'] %>" class="question-url"><%= question["url"] %></a>