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] 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>