diff --git a/app/models/plan.rb b/app/models/plan.rb
index 5c97fb2..20b5fb3 100644
--- a/app/models/plan.rb
+++ b/app/models/plan.rb
@@ -94,10 +94,10 @@
   # returns the template for this plan, or generates an empty template and returns that
   #
   # @return [Dmptemplate] the template associated with this plan
-	def dmptemplate
-		#self.project.try(:dmptemplate) || Dmptemplate.new
-		self.template
-	end
+  def dmptemplate
+    #self.project.try(:dmptemplate) || Dmptemplate.new
+    self.template
+  end
 
 
 
@@ -119,24 +119,24 @@
   # @param qid [Integer] the id for the question to find the answer for
   # @param create_if_missing [Boolean] if true, will genereate a default answer to the question
   # @return [Answer,nil] the most recent answer to the question, or a new question with default value, or nil
-	def answer(qid, create_if_missing = true)
-  	answer = answers.where(:question_id => qid).order("created_at DESC").first
-  	question = Question.find(qid)
-		if answer.nil? && create_if_missing then
-			answer = Answer.new
-			answer.plan_id = id
-			answer.question_id = qid
-			answer.text = question.default_value
-			default_options = Array.new
-			question.question_options.each do |option|
-				if option.is_default
-					default_options << option
-				end
-			end
-			answer.question_options = default_options
-		end
-		return answer
-	end
+  def answer(qid, create_if_missing = true)
+    answer = answers.where(:question_id => qid).order("created_at DESC").first
+    question = Question.find(qid)
+    if answer.nil? && create_if_missing then
+      answer = Answer.new
+      answer.plan_id = id
+      answer.question_id = qid
+      answer.text = question.default_value
+      default_options = Array.new
+      question.question_options.each do |option|
+        if option.is_default
+          default_options << option
+        end
+      end
+      answer.question_options = default_options
+    end
+    return answer
+  end
 
 # TODO: This just retrieves all of the guidance associated with the themes within the template
 #       so why are we transferring it here to the plan?
@@ -343,32 +343,46 @@
   #
   # @param user_id [Integer] the id for a user
   # @return [Boolean] true if user can edit the plan
-	def editable_by?(user_id)
+  def editable_by?(user_id)
     user_id = user_id.id if user_id.is_a?(User)
     has_role(user_id, :editor)
-	end
+  end
 
   ##
   # determines if the plan is readable by the specified user
-  # TODO: introduce explicit readable rather than implicit
-  # currently role with no flags = readable
   #
   # @param user_id [Integer] the id for a user
   # @return [Boolean] true if the user can read the plan
-	def readable_by?(user_id)
+  def readable_by?(user_id)
+    user = user_id.is_a?(User) ? user_id : User.find(user_id)
+    owner_orgs = self.owner_and_coowners.collect(&:org)
+    
+    # Super Admins can view plans read-only, Org Admins can view their Org's plans 
+    # otherwise the user must have the commenter role
+    (user.can_super_admin? ||
+     user.can_org_admin? && owner_orgs.include?(user.org) ||
+     has_role(user.id, :commenter))
+  end
+
+  ##
+  # determines if the plan is readable by the specified user
+  #
+  # @param user_id [Integer] the id for a user
+  # @return [Boolean] true if the user can read the plan
+  def commentable_by?(user_id)
     user_id = user_id.id if user_id.is_a?(User)
     has_role(user_id, :commenter)
-	end
+  end
 
   ##
   # determines if the plan is administerable by the specified user
   #
   # @param user_id [Integer] the id for the user
   # @return [Boolean] true if the user can administer the plan
-	def administerable_by?(user_id)
+  def administerable_by?(user_id)
     user_id = user_id.id if user_id.is_a?(User)
     has_role(user_id, :administrator)
-	end
+  end
 
   ##
   # determines if the plan is owned by the specified user
@@ -764,7 +778,7 @@
   def has_role(user_id, role_as_sym)
     if user_id.is_a?(Integer) && role_as_sym.is_a?(Symbol)
       vals = Role.access_values_for(role_as_sym)
-      self.roles.where(user_id: user_id, access: vals).first.present?
+      self.roles.where(user_id: user_id, access: vals, active: true).first.present?
     else
       false
     end
@@ -841,57 +855,57 @@
 
 
   ##
-	# Based on the height of the text gathered so far and the available vertical
-	# space of the pdf, estimate a percentage of how much space has been used.
-	# This is highly dependent on the layout in the pdf. A more accurate approach
-	# would be to render the pdf and check how much space had been used, but that
-	# could be very slow.
-	# NOTE: This is only an estimate, rounded up to the nearest 5%; it is intended
-	# for guidance when editing plan data, not to be 100% accurate.
+  # Based on the height of the text gathered so far and the available vertical
+  # space of the pdf, estimate a percentage of how much space has been used.
+  # This is highly dependent on the layout in the pdf. A more accurate approach
+  # would be to render the pdf and check how much space had been used, but that
+  # could be very slow.
+  # NOTE: This is only an estimate, rounded up to the nearest 5%; it is intended
+  # for guidance when editing plan data, not to be 100% accurate.
   #
   # @param used_height [Integer] an estimate of the height used so far
   # @return [Integer] the estimate of space used of an A4 portrain
-	def estimate_space_used(used_height)
-		@formatting ||= self.settings(:export).formatting
+  def estimate_space_used(used_height)
+    @formatting ||= self.settings(:export).formatting
 
-		return 0 unless @formatting[:font_size] > 0
+    return 0 unless @formatting[:font_size] > 0
 
-		margin_height    = @formatting[:margin][:top].to_i + @formatting[:margin][:bottom].to_i
-		page_height      = A4_PAGE_HEIGHT - margin_height # 297mm for A4 portrait
-		available_height = page_height * self.dmptemplate.settings(:export).max_pages
+    margin_height    = @formatting[:margin][:top].to_i + @formatting[:margin][:bottom].to_i
+    page_height      = A4_PAGE_HEIGHT - margin_height # 297mm for A4 portrait
+    available_height = page_height * self.dmptemplate.settings(:export).max_pages
 
-		percentage = (used_height / available_height) * 100
-		(percentage / ROUNDING).ceil * ROUNDING # round up to nearest five
-	end
+    percentage = (used_height / available_height) * 100
+    (percentage / ROUNDING).ceil * ROUNDING # round up to nearest five
+  end
 
   ##
-	# Take a guess at the vertical height (in mm) of the given text based on the
-	# font-size and left/right margins stored in the plan's settings.
-	# This assumes a fixed-width for each glyph, which is obviously
-	# incorrect for the font-face choices available; the idea is that
-	# they'll hopefully average out to that in the long-run.
-	# Allows for hinting different font sizes (offset from base via font_size_inc)
-	# and vertical margins (i.e. for heading text)
+  # Take a guess at the vertical height (in mm) of the given text based on the
+  # font-size and left/right margins stored in the plan's settings.
+  # This assumes a fixed-width for each glyph, which is obviously
+  # incorrect for the font-face choices available; the idea is that
+  # they'll hopefully average out to that in the long-run.
+  # Allows for hinting different font sizes (offset from base via font_size_inc)
+  # and vertical margins (i.e. for heading text)
   #
   # @param text [String] the text to estimate size of
   # @param font_size_inc [Integer] the size of the font of the text, defaults to 0
   # @param vertical_margin [Integer] the top margin above the text, defaults to 0
-	def height_of_text(text, font_size_inc = 0, vertical_margin = 0)
-		@formatting     ||= self.settings(:export).formatting
-		@margin_width   ||= @formatting[:margin][:left].to_i + @formatting[:margin][:right].to_i
-		@base_font_size ||= @formatting[:font_size]
+  def height_of_text(text, font_size_inc = 0, vertical_margin = 0)
+    @formatting     ||= self.settings(:export).formatting
+    @margin_width   ||= @formatting[:margin][:left].to_i + @formatting[:margin][:right].to_i
+    @base_font_size ||= @formatting[:font_size]
 
-		return 0 unless @base_font_size > 0
+    return 0 unless @base_font_size > 0
 
-		font_height = FONT_HEIGHT_CONVERSION_FACTOR * (@base_font_size + font_size_inc)
-		font_width  = font_height * FONT_WIDTH_HEIGHT_RATIO # Assume glyph width averages at 2/5s the height
-		leading     = font_height / 2
+    font_height = FONT_HEIGHT_CONVERSION_FACTOR * (@base_font_size + font_size_inc)
+    font_width  = font_height * FONT_WIDTH_HEIGHT_RATIO # Assume glyph width averages at 2/5s the height
+    leading     = font_height / 2
 
-		chars_in_line = (A4_PAGE_WIDTH - @margin_width) / font_width # 210mm for A4 portrait
-		num_lines = (text.length / chars_in_line).ceil
+    chars_in_line = (A4_PAGE_WIDTH - @margin_width) / font_width # 210mm for A4 portrait
+    num_lines = (text.length / chars_in_line).ceil
 
-		(num_lines * font_height) + vertical_margin + leading
-	end
+    (num_lines * font_height) + vertical_margin + leading
+  end
 
   # Initialize the title and dirty flags for new templates
   # --------------------------------------------------------
diff --git a/app/policies/note_policy.rb b/app/policies/note_policy.rb
index 88a9e11..2a25489 100644
--- a/app/policies/note_policy.rb
+++ b/app/policies/note_policy.rb
@@ -9,15 +9,15 @@
   end
 
   def create?
-    @note.answer.plan.readable_by?(@user.id)
+    @note.answer.plan.commentable_by?(@user.id)
   end
 
   def update?
-    Plan.find(@note.answer.plan_id).readable_by?(@user.id)
+    Plan.find(@note.answer.plan_id).commentable_by?(@user.id)
   end
 
   def archive?
-    Plan.find(@note.answer.plan_id).readable_by?(@user.id)
+    Plan.find(@note.answer.plan_id).commentable_by?(@user.id)
   end
 
 end
diff --git a/app/policies/plan_policy.rb b/app/policies/plan_policy.rb
index 3da3562..8860b22 100644
--- a/app/policies/plan_policy.rb
+++ b/app/policies/plan_policy.rb
@@ -10,62 +10,62 @@
   end
 
   def show?
-    @plan.readable_by?(@user.id) && Role.find_by(user_id: @user.id, plan_id: @plan.id).active
+    @plan.readable_by?(@user.id)
   end
 
   def share?
-    @plan.editable_by?(@user.id) && Role.find_by(user_id: @user.id, plan_id: @plan.id).active
+    @plan.editable_by?(@user.id)
   end
 
   def export?
-    @plan.readable_by?(@user.id) && Role.find_by(user_id: @user.id, plan_id: @plan.id).active
+    @plan.readable_by?(@user.id)
   end
 
   def download?
-    @plan.readable_by?(@user.id) && Role.find_by(user_id: @user.id, plan_id: @plan.id).active
+    @plan.readable_by?(@user.id)
   end
 
   def edit?
-    @plan.readable_by?(@user.id) && Role.find_by(user_id: @user.id, plan_id: @plan.id).active
+    @plan.readable_by?(@user.id)
   end
 
   def update?
-    @plan.editable_by?(@user.id) && Role.find_by(user_id: @user.id, plan_id: @plan.id).active
+    @plan.editable_by?(@user.id)
   end
 
   def destroy?
-    @plan.editable_by?(@user.id) && Role.find_by(user_id: @user.id, plan_id: @plan.id).active
+    @plan.editable_by?(@user.id)
   end
 
   def status?
-    @plan.readable_by?(@user.id) && Role.find_by(user_id: @user.id, plan_id: @plan.id).active
+    @plan.readable_by?(@user.id)
   end
 
   def duplicate?
-    @plan.editable_by?(@user.id) && Role.find_by(user_id: @user.id, plan_id: @plan.id).active
+    @plan.editable_by?(@user.id)
   end
 
   def visibility?
-    @plan.administerable_by?(@user.id) && Role.find_by(user_id: @user.id, plan_id: @plan.id).active
+    @plan.administerable_by?(@user.id)
   end
 
   def set_test?
-    @plan.administerable_by?(@user.id)&& Role.find_by(user_id: @user.id, plan_id: @plan.id).active
+    @plan.administerable_by?(@user.id)
   end
 
   def answer?
-    @plan.readable_by?(@user.id) && Role.find_by(user_id: @user.id, plan_id: @plan.id).active
+    @plan.readable_by?(@user.id)
   end
 
   def request_feedback?
-    @plan.administerable_by?(@user.id) && Role.find_by(user_id: @user.id, plan_id: @plan.id).active
+    @plan.administerable_by?(@user.id)
   end
   
   def feedback_complete?
-    @plan.reviewable_by?(@user.id) && Role.find_by(user_id: @user.id, plan_id: @plan.id).active
+    @plan.reviewable_by?(@user.id)
   end
 
   def overview?
-    @plan.readable_by?(@user.id) && Role.find_by(user_id: @user.id, plan_id: @plan.id).active
+    @plan.readable_by?(@user.id)
   end
 end
diff --git a/app/views/notes/_layout.html.erb b/app/views/notes/_layout.html.erb
index 5115277..93c1f62 100644
--- a/app/views/notes/_layout.html.erb
+++ b/app/views/notes/_layout.html.erb
@@ -5,17 +5,19 @@
     <%= render partial: "/notes/list", locals: { question_id: question.id, notes: notes, plan: plan }, formats: [:html] %>  
   </div>
 </div>
-<div class="row">
-  <div class="col-md-12">
-    <div class="note_new" id="<%= "note_new#{question.id}" %>" data-question-id="<%= question.id %>">
-        <%= render partial: "/notes/new", locals: { question: question, answer: answer, plan: plan }, formats: [:html] %>
+<% if plan.commentable_by?(current_user) %>
+  <div class="row">
+    <div class="col-md-12">
+      <div class="note_new" id="<%= "note_new#{question.id}" %>" data-question-id="<%= question.id %>">
+          <%= render partial: "/notes/new", locals: { question: question, answer: answer, plan: plan }, formats: [:html] %>
+      </div>
     </div>
   </div>
-</div>
-<div class="row">
-  <div class="col-md-12">
-    <div class="pull-right">
-      <%= link_to(_('Add Comment'), "#note_new#{question.id}", class: "btn btn-default note_new_link", role: "button", style: "visibility: hidden") %>
+  <div class="row">
+    <div class="col-md-12">
+      <div class="pull-right">
+        <%= link_to(_('Add Comment'), "#note_new#{question.id}", class: "btn btn-default note_new_link", role: "button", style: "visibility: hidden") %>
+      </div>
     </div>
   </div>
-</div>
\ No newline at end of file
+<% end %>
\ No newline at end of file
diff --git a/app/views/notes/_list.html.erb b/app/views/notes/_list.html.erb
index d087d96..73132f3 100644
--- a/app/views/notes/_list.html.erb
+++ b/app/views/notes/_list.html.erb
@@ -14,13 +14,15 @@
                         <div class="pull-right"><!-- note actions -->
                             <ul class="list-inline">
                                 <li><%= link_to(_('Show'), "#note_show#{note.id}", class: 'note_show_link') %></li>
-                                <% if current_user.id == note.user_id %>
-                                    <li><%= link_to(_('Edit'), "#note_edit#{note.id}", class: 'note_edit_link') %></li>
-                                    <li><%= link_to(_('Remove'), "#note_archive#{note.id}", class: 'note_archive_link') %></li>
-                                <% else  %>
-                                    <% if plan.administerable_by?(current_user.id) %>
-                                        <li><%= link_to(_('Remove'), "#note_archive#{note.id}", class: 'note_archive_link') %></li>
-                                    <% end %>
+                                <% if plan.commentable_by?(current_user) %>
+                                  <% if current_user.id == note.user_id %>
+                                      <li><%= link_to(_('Edit'), "#note_edit#{note.id}", class: 'note_edit_link') %></li>
+                                      <li><%= link_to(_('Remove'), "#note_archive#{note.id}", class: 'note_archive_link') %></li>
+                                  <% else  %>
+                                      <% if plan.administerable_by?(current_user.id) %>
+                                          <li><%= link_to(_('Remove'), "#note_archive#{note.id}", class: 'note_archive_link') %></li>
+                                      <% end %>
+                                  <% end %>
                                 <% end %>
                             </ul>
                         </div>
diff --git a/app/views/org_admin/plans/index.html.erb b/app/views/org_admin/plans/index.html.erb
index b99edc9..2659490 100644
--- a/app/views/org_admin/plans/index.html.erb
+++ b/app/views/org_admin/plans/index.html.erb
@@ -59,7 +59,7 @@
             <% @plans.each do |plan| %>
               <tr>
                 <td>
-                  <%= "#{plan.title.length > 60 ? "#{plan.title[0..59]} ..." : plan.title}" %>
+                  <%= link_to "#{plan.title.length > 60 ? "#{plan.title[0..59]} ..." : plan.title}", plan_path(plan) %>
                 </td>
                 <td><%= plan.template.title %></td>
                 <td><%= plan.users.first.org.name %></td>
diff --git a/test/functional/plans_controller_test.rb b/test/functional/plans_controller_test.rb
index 3d188a7..82c545f 100644
--- a/test/functional/plans_controller_test.rb
+++ b/test/functional/plans_controller_test.rb
@@ -102,7 +102,8 @@
   # ----------------------------------------------------------
   test 'show the plan page' do
     # Should redirect user to the root path if they are not logged in!
-    try_no_user_and_unauthorized(plan_path(@plan))
+    get plan_path(@plan)
+    assert_unauthorized_redirect_to_root_path
 
     sign_in @user
     get plan_path(@plan)
@@ -230,7 +231,8 @@
   # ----------------------------------------------------------
   test "get the share plan page" do
     # Should redirect user to the root path if they are not logged in!
-    try_no_user_and_unauthorized(share_plan_path(@plan))
+    get share_plan_path(@plan)
+    assert_unauthorized_redirect_to_root_path
 
     sign_in @user
     get share_plan_path(@plan)
@@ -242,7 +244,8 @@
   # ----------------------------------------------------------
   test "get the plan status" do
     # Should redirect user to the root path if they are not logged in!
-    try_no_user_and_unauthorized(status_plan_path(@plan, format: :json))
+    get status_plan_path(@plan, format: :json)
+    assert_unauthorized_redirect_to_root_path
 
     sign_in @user
     get status_plan_path(@plan, format: :json)
@@ -254,7 +257,8 @@
   # ----------------------------------------------------------
   test "get the answer to the specified question for the plan" do
     # Should redirect user to the root path if they are not logged in!
-    try_no_user_and_unauthorized(answer_plan_path(@plan, format: :json))
+    get answer_plan_path(@plan, format: :json)
+    assert_unauthorized_redirect_to_root_path
 
     sign_in @user
     get answer_plan_path(@plan, format: :json)
@@ -266,7 +270,9 @@
   # ----------------------------------------------------------
   test "export the plan" do
     # Should redirect user to the root path if they are not logged in!
-    try_no_user_and_unauthorized(export_plan_path(@plan))
+    get export_plan_path(@plan), {'format': 'pdf'}
+    assert_unauthorized_redirect_to_root_path
+    
     export_params = {"utf8"=>"✓",
        "phase_id"=>"5470",
        "export"=>{"project_details"=>"true",
@@ -295,7 +301,8 @@
   # ----------------------------------------------------------
   test "show the download plan page" do
     # Should redirect user to the root path if they are not logged in!
-    try_no_user_and_unauthorized(download_plan_path(@plan))
+    get download_plan_path(@plan)
+    assert_unauthorized_redirect_to_root_path
 
     sign_in @user
     get download_plan_path(@plan)
@@ -320,19 +327,4 @@
     get overview_plan_path(@plan)
     assert_response(:success)
   end
-
-  private
-    def try_no_user_and_unauthorized(target)
-      # Should redirect user to the root path if they are not logged in!
-      get target
-      assert_unauthorized_redirect_to_root_path
-
-      # User who is does not have access to the plan
-      sign_in User.first
-      get target
-      assert_equal _('You are not authorized to perform this action.'), flash[:alert]
-      assert_response :redirect
-      assert_redirected_to plans_url
-    end
-
 end