diff --git a/app/controllers/org_admin/plans_controller.rb b/app/controllers/org_admin/plans_controller.rb
index 7788ad1ff8..37a9df126e 100644
--- a/app/controllers/org_admin/plans_controller.rb
+++ b/app/controllers/org_admin/plans_controller.rb
@@ -10,8 +10,10 @@ def index
       raise Pundit::NotAuthorizedError
     end
 
-    feedback_ids = Role.where(user_id: current_user.id).reviewer.pluck(:plan_id).uniq
-    @feedback_plans = Plan.where(id: feedback_ids)
+    feedback_ids = Role.creator.joins(:user,:plan)
+      .where('users.org_id = ? AND plans.feedback_requested is TRUE',
+              current_user.org_id).pluck(:plan_id)
+    @feedback_plans = Plan.find(feedback_ids)
     @plans = current_user.org.plans.page(1)
   end
 
diff --git a/app/controllers/plans_controller.rb b/app/controllers/plans_controller.rb
index 39a8e51cd1..17ab569c18 100644
--- a/app/controllers/plans_controller.rb
+++ b/app/controllers/plans_controller.rb
@@ -258,8 +258,7 @@ def share
     @plan = Plan.find(params[:id])
     if @plan.present?
       authorize @plan
-      # Get the roles where the user is not a reviewer
-      @plan_roles = @plan.roles.select { |r| !r.reviewer? }
+      @plan_roles = @plan.roles
     else
       redirect_to(plans_path)
     end
diff --git a/app/helpers/mailer_helper.rb b/app/helpers/mailer_helper.rb
index 688baf89af..4cf108ad72 100644
--- a/app/helpers/mailer_helper.rb
+++ b/app/helpers/mailer_helper.rb
@@ -15,13 +15,7 @@ def privileges_list(user)
 
   # Returns the messaging for the specified role
   def role_text(role)
-    if role.reviewer?
-      {
-        type: _('reviewer'),
-        placeholder1: _('read the plan and provide feedback.'),
-        placeholder2: nil
-      }
-    elsif role.administrator?
+    if role.administrator?
       {
         type: _('co-owner'),
         placeholder1: _('write and edit the plan in a collaborative manner.'),
@@ -41,4 +35,4 @@ def role_text(role)
       }
     end
   end
-end
\ No newline at end of file
+end
diff --git a/app/helpers/perms_helper.rb b/app/helpers/perms_helper.rb
index 6a007b895d..38af33189f 100644
--- a/app/helpers/perms_helper.rb
+++ b/app/helpers/perms_helper.rb
@@ -2,14 +2,15 @@ module PermsHelper
   # Returns a hash whose keys are the names associated to Perms and values are the text to be displayed to the end user
   def name_and_text
     {
-      :add_organisations => _('Add organisations'),
-      :change_org_affiliation => _('Change affiliation'),
-      :grant_permissions => _('Manage user privileges'),
-      :modify_templates => _('Manage templates'),
-      :modify_guidance => _('Manage guidance'),
-      :use_api => _('API rights'),
-      :change_org_details => _('Manage organisation details'),
-      :grant_api_to_orgs => _('Grant API to organisations')
+      add_organisations:  _('Add organisations'),
+      change_org_affiliation: _('Change affiliation'),
+      grant_permissions: _('Manage user privileges'),
+      modify_templates: _('Manage templates'),
+      modify_guidance: _('Manage guidance'),
+      use_api: _('API rights'),
+      change_org_details: _('Manage organisation details'),
+      grant_api_to_orgs:  _('Grant API to organisations'),
+      review_org_plans: _('')
     }
   end
-end
\ No newline at end of file
+end
diff --git a/app/models/perm.rb b/app/models/perm.rb
index a2fdced5e6..0c40c131c4 100644
--- a/app/models/perm.rb
+++ b/app/models/perm.rb
@@ -60,4 +60,8 @@ def self.change_org_details
   def self.grant_api
     Perm.find_by(name: 'grant_api_to_orgs')
   end
+
+  def self.review_plans
+    Perm.find_by(name: 'review_org_plans')
+  end
 end
diff --git a/app/models/plan.rb b/app/models/plan.rb
index cd63c47e28..3051f29769 100644
--- a/app/models/plan.rb
+++ b/app/models/plan.rb
@@ -141,8 +141,7 @@ class Plan < ActiveRecord::Base
   # Retrieves any plan in which the user has an active role and
   # is not a reviewer
   scope :active, lambda { |user|
-    plan_ids = Role.where(active: true, user_id: user.id)
-                   .not_reviewer.pluck(:plan_id)
+    plan_ids = Role.where(active: true, user_id: user.id).pluck(:plan_id)
 
     includes(:template, :roles)
     .where(id: plan_ids)
@@ -278,13 +277,6 @@ def request_feedback(user)
     Plan.transaction do
       begin
         self.feedback_requested = true
-        # Share the plan with each org admin as the reviewer role
-        admins = user.org.org_admins
-        admins.each do |admin|
-          unless admin == user
-            add_user!(admin.id, :reviewer)
-          end
-        end
         if save!
           # Send an email to the org-admin contact
           if user.org.contact_email.present?
@@ -311,10 +303,6 @@ def complete_feedback(org_admin)
     Plan.transaction do
       begin
         self.feedback_requested = false
-
-        # Remove the org admins reviewer role from the plan
-        roles.delete(Role.where(plan: self).reviewer)
-
         if save!
           # Send an email confirmation to the owners and co-owners
           deliver_if(recipients: owner_and_coowners,
@@ -376,7 +364,7 @@ def readable_by?(user_id)
   #
   # Returns Boolean
   def commentable_by?(user_id)
-    Role.commenter.where(plan_id: id, user_id: user_id, active: true).any?
+    Role.commenter.where(plan_id: id, user_id: user_id, active: true).any? || reviewable_by?(user_id)
   end
 
   # determines if the plan is administerable by the specified user
@@ -394,7 +382,10 @@ def administerable_by?(user_id)
   #
   # Returns Boolean
   def reviewable_by?(user_id)
-    Role.reviewer.where(plan_id: id, user_id: user_id, active: true).any?
+    reviewer = User.find(user_id)
+    feedback_requested? &&
+    reviewer.org_id == owner.org_id &&
+    reviewer.can_review_plans?
   end
 
   # the datetime for the latest update of this plan
@@ -440,8 +431,6 @@ def add_user!(user_id, access_type = :commenter)
         role.editor = true
       when :editor
         role.editor = true
-      when :reviewer
-        role.reviewer = true
       end
       role.commenter = true
       role.save
diff --git a/app/models/user.rb b/app/models/user.rb
index 870e05a98b..4966d724c1 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -295,6 +295,15 @@ def can_grant_api_to_orgs?
     perms.include? Perm.grant_api
   end
 
+
+  ##
+  # Can the user review their organisation's plans?
+  #
+  # Returns Boolean
+  def can_review_plans?
+    perms.include? Perm.review_plans
+  end
+
   # Removes the api_token from the user
   #
   # Returns nil
diff --git a/app/policies/note_policy.rb b/app/policies/note_policy.rb
index 2a25489114..972e584886 100644
--- a/app/policies/note_policy.rb
+++ b/app/policies/note_policy.rb
@@ -13,7 +13,7 @@ def create?
   end
 
   def update?
-    Plan.find(@note.answer.plan_id).commentable_by?(@user.id)
+    Plan.find(@note.answer.plan_id).commentable_by?(@user.id) && @note.user_id = @user.id
   end
 
   def archive?
diff --git a/app/views/paginable/users/_index.html.erb b/app/views/paginable/users/_index.html.erb
index fe9170bb77..e4282fc4b3 100644
--- a/app/views/paginable/users/_index.html.erb
+++ b/app/views/paginable/users/_index.html.erb
@@ -41,7 +41,7 @@
                   </td>
                   <td class="text-center">
                       <% unless user.roles.nil? %>
-                      <%= user.roles.where(Role.not_reviewer_condition).length %>
+                      <%= user.roles.length %>
                       <% end %>
                   </td>
                   <%# The content of this column get updated through AJAX whenever the permission for an user are updated %>
diff --git a/app/views/plans/_share_form.html.erb b/app/views/plans/_share_form.html.erb
index 7988af6768..88a7272404 100644
--- a/app/views/plans/_share_form.html.erb
+++ b/app/views/plans/_share_form.html.erb
@@ -125,22 +125,21 @@
   <% end %>
    
 
-   <div class="col-xs-12">
-     <% if plan.owner_and_coowners.include?(current_user) && current_user.org.present? && current_user.org.feedback_enabled? %>
-       <h2><%= _('Request expert feedback') %></h2>
-       <p><%= _('Click below to give data management staff at your organisation access to read and comment on your plan.') %></p>
-       <div class="well well-sm">
-         <%= sanitize current_user.org.feedback_email_msg.to_s % { user_name: current_user.name(false), plan_name: plan.title } %>
-       </div>
-       <p><%= _('You can continue to edit and download the plan in the interim.') %></p>
-
-       <div class="form-group col-xs-8">
-         <%= link_to _('Request feedback'),
+  <div class="col-xs-12">
+    <% if plan.owner_and_coowners.include?(current_user) && plan.owner.org.feedback_enabled? %>
+      <h2><%= _('Request expert feedback') %></h2>
+      <p><%= _("Click below to give data management staff at #{plan.owner.org.name}, the Plan Owner's org, access to read and comment on your plan.") %></p>
+      <div class="well well-sm">
+        <%= sanitize plan.owner.org.feedback_email_msg.to_s % { user_name: current_user.name(false), plan_name: plan.title } %>
+      </div>
+      <p><%= _('You can continue to edit and download the plan in the interim.') %></p>
+      <div class="form-group col-xs-8">
+        <%= link_to _('Request feedback'),
                      feedback_requests_path(plan_id: @plan.id),
                      data: { method: 'post' },
                      class: "btn btn-default#{' disabled' if @plan.feedback_requested?}" %>
-         <span><%= _("Feedback has been requested.") if @plan.feedback_requested? %></span>
-       </div>
+        <span><%= _("Feedback has been requested.") if @plan.feedback_requested? %></span>
+      </div>
     <% end %>
   </div>
 <% end %>
diff --git a/app/views/users/_admin_grant_permissions.html.erb b/app/views/users/_admin_grant_permissions.html.erb
index 15ae0a4773..e03477b61a 100644
--- a/app/views/users/_admin_grant_permissions.html.erb
+++ b/app/views/users/_admin_grant_permissions.html.erb
@@ -2,19 +2,19 @@
   <div class="modal-content">
 
     <div class="modal-header">
-      <button type="button" class="close" 
-              data-dismiss="modal" 
+      <button type="button" class="close"
+              data-dismiss="modal"
               aria-label="Close">
               <span aria-hidden="true">&times;</span></button>
       <h2 class="modal-title"><%= _('Editing privileges for %{username}') % { username: user.name(false) } %></h2>
     </div>
-    
+
     <%= form_for user, url: admin_update_permissions_user_path(user), html: { method: :put, remote: true, class: 'admin_update_permissions' } do |f| %>
       <div class="modal-body" style="text-align: left">
         <ul class="list-group">
          <input type="checkbox" name="org_admin_privileges" id="org_admin_privileges" />
           <%= label_tag :organisational_admin_privileges %>
-          <% perms.each do |perm| %>           
+          <% perms.each do |perm| %>
             <% case perm.name when 'grant_permissions' %>
               <li class="list-group-item" style="border: none" title="<%= _('Allows the user to assign permissions to other users within the same organisation. Users can only assign permissions they own themselves') %>">
                 <%= check_box_tag "perm_ids[]", perm.id, user.perms.include?(perm), class: 'org_grant_privileges' %>
@@ -35,13 +35,17 @@
               <li class="list-group-item" style="border: none" title="<%= _('Allows the user to amend the organisation details (name, URL etc) and add basic branding such as the logo') %>">
                 <%= check_box_tag "perm_ids[]", perm.id, user.perms.include?(perm), class: 'org_grant_privileges' %>
                 <%= _('Manage organisation details') %></li>
+            <% when 'review_org_plans' %>
+              <li class="list-group-item" style="border: none" title="<%= _('Allows the user to review plans submitted by organisational users') %>">
+                <%= check_box_tag "perm_ids[]", perm.id, user.perms.include?(perm), class: 'org_grant_privileges' %>
+                <%= _('Manage organisation details') %></li>
             <% end %>
           <% end %>
 
 
           <% if current_user.can_super_admin? %>
             <input type="checkbox" name="super_admin_privileges" id="super_admin_privileges" />
-            <%= label_tag :super_admin_privileges %> 
+            <%= label_tag :super_admin_privileges %>
             <% perms.each do |perm| %>
               <% case perm.name when 'add_organisations' %>
                 <li class="list-group-item" style="border: none" title="<%= _('Allows the user to create new organisations') %>">
@@ -62,8 +66,8 @@
       </div>
       <div class="modal-footer">
         <%= submit_tag _('Save'), class: "btn btn-primary" %>
-        <%= button_tag(_('Cancel'), class: 'btn btn-primary', 'data-dismiss': 'modal') %>      
+        <%= button_tag(_('Cancel'), class: 'btn btn-primary', 'data-dismiss': 'modal') %>
       </div>
     <% end %>
   </div>
-</div>
\ No newline at end of file
+</div>
diff --git a/app/views/users/admin_grant_permissions.html.erb b/app/views/users/admin_grant_permissions.html.erb
deleted file mode 100644
index 4114fa999f..0000000000
--- a/app/views/users/admin_grant_permissions.html.erb
+++ /dev/null
@@ -1,59 +0,0 @@
-<% title _('Edit User Privileges') %>
-
-<% namesHash = name_and_text %>
-<div class="row">
-  <div class="col-md-12">
-    <h1><%= _('Edit User Privileges') %></h1>
-  </div>
-</div>
-
-<div class="row">
-  <div class="col-md-12">
-    <%= form_tag( admin_update_permissions_user_path(@user), method: :put) do %>
-    <div class='table-responsive'>
-      <table class="table table-hover table-bordered">
-        <thead>
-          <tr>
-            <th scope="col"><%= _('Name') %></th>
-          
-            <% @perms.each do |perm| %>
-              <% case perm.name when 'grant_permissions' %>
-                <th scope="col" class="text-center" data-toggle="tooltip" title="<%= _('Allows the user to assign permissions to other users within the same organisation. Users can only assign permissions they own themselves') %>">
-                  <%= namesHash[perm.name.to_sym] %>
-                </th>
-              <% when 'modify_templates' %>
-                <th scope="col" class="text-center" data-toggle="tooltip" title="<%= _('Allows the user to create new organisational templates, edit existing ones and customise funder templates') %>">
-                  <%= namesHash[perm.name.to_sym] %>
-                </th>
-              <% when 'modify_guidance' %>
-                <th scope="col" class="text-center" data-toggle="tooltip" title="<%= _('Allows the user to create and edit guidance') %>">
-                  <%= namesHash[perm.name.to_sym] %>
-                </th>
-              <% when 'use_api' %>
-                <th scope="col" class="text-center" data-toggle="tooltip" title="<%= _('Provides the user with an API token and grants rights to harvest information from the tool') %>">
-                  <%= namesHash[perm.name.to_sym] %>
-                </th>
-              <% when 'change_org_details' %>
-                <th scope="col" class="text-center" data-toggle="tooltip" title="<%= _('Allows the user to amend the organisation details (name, URL etc) and add basic branding such as the logo') %>">
-                  <%= namesHash[perm.name.to_sym] %>
-                </th>
-              <% end %>
-            <% end %>
-          </tr>
-        </thead>
-        <tbody>
-          <tr>
-            <td><%= @user.name(false) %></td>
-            <% @perms.each do |perm| %>
-              <td class="text-center"><%= check_box_tag "perm_ids[]", perm.id, @user.perms.include?(perm) %></td>
-            <% end %>
-          </tr>
-        </tbody>
-      </table>
-    </div>
-    <div class="form-group">
-      <%= submit_tag _('Save'), class: "btn btn-primary" %>
-    </div>
-    <% end %>
-  </div>
-</div>
diff --git a/db/seeds.rb b/db/seeds.rb
index ad0fce5ddf..86cb245307 100755
--- a/db/seeds.rb
+++ b/db/seeds.rb
@@ -182,7 +182,8 @@
   {name: 'modify_guidance'},
   {name: 'use_api'},
   {name: 'change_org_details'},
-  {name: 'grant_api_to_orgs'}
+  {name: 'grant_api_to_orgs'},
+  {name: 'review_org_plans'}
 ]
 
 perms.map{ |p| create(:perm, p) }
diff --git a/lib/tasks/upgrade.rake b/lib/tasks/upgrade.rake
index 9c64bf0c70..788d1b26d7 100644
--- a/lib/tasks/upgrade.rake
+++ b/lib/tasks/upgrade.rake
@@ -4,6 +4,9 @@ namespace :upgrade do
   desc "Upgrade to v2.1.3"
   task v2_1_3: :environment do
     Rake::Task['upgrade:fill_blank_plan_identifiers'].execute
+    Rake::Task["upgrade:add_reviewer_perm"].execute
+    Rake::Task["upgrade:add_reviewer_to_existing_admin_perms"].execute
+    Rake::Task["upgrade:migrate_reviewer_roles"].execute
   end
 
   desc "Upgrade to v2.1.2:"
@@ -662,11 +665,37 @@ namespace :upgrade do
     end
   end
 
+
   desc "Fill blank or nil plan identifiers with plan_id"
   task fill_blank_plan_identifiers: :environment do
     Plan.where(identifier: ["",nil]).update_all('identifier = id')
   end
 
+  desc "Adds a new permission for plan reviewers"
+  task add_reviewer_perm: :environment do
+    perm_name = 'review_org_plans'
+    unless Perm.find_by(name: perm_name).present?
+      Perm.create(name: perm_name)
+    end
+  end
+
+  desc "adds the new reviewer perm to all existing admin perms"
+  task add_reviewer_to_existing_admin_perms: :environment do
+    Perm.change_org_details.users.each do |u|
+      u.perms << Perm.review_plans
+    end
+  end
+
+  desc "remove the old reviewer roles and ensure these are marked feedback-enabled"
+  task migrate_reviewer_roles: :environment do
+    # remove all roles with nil plan_id
+    Role.reviewer.where(plan_id: nil).destroy_all
+    # Pluck all other plan_ids
+    review_plan_ids = Role.reviewer.pluck(:plan_id).uniq
+    Plan.where(id: review_plan_ids).update_all(feedback_requested: true)
+    Role.reviewer.destroy_all
+  end
+
   private
 
   def fuzzy_match?(text_a, text_b, min = 3)
diff --git a/spec/factories/perms.rb b/spec/factories/perms.rb
index 115fa73795..481735c797 100644
--- a/spec/factories/perms.rb
+++ b/spec/factories/perms.rb
@@ -51,5 +51,10 @@
       name { 'grant_api_to_orgs' }
       initialize_with { Perm.find_or_create_by(name: name) }
     end
+
+    trait :review_org_plans do
+      name { 'review_org_plans' }
+      initialize_with { Perm.find_or_create_by(name: name) }
+    end
   end
 end
diff --git a/spec/models/plan_spec.rb b/spec/models/plan_spec.rb
index a49afa3727..bcad1f1799 100644
--- a/spec/models/plan_spec.rb
+++ b/spec/models/plan_spec.rb
@@ -363,16 +363,6 @@
 
     end
 
-    context "where user role is reviewer" do
-
-      before do
-        create(:role, :active, :reviewer, user: user, plan: plan)
-      end
-
-      it { is_expected.not_to include(plan) }
-
-    end
-
   end
 
   describe ".load_for_phase" do
@@ -650,7 +640,6 @@
       create(:role, :creator, plan: plan, user: user)
       # This person gets the email notification
       create(:role, :administrator, plan: plan, user: admin)
-      create_list(:role, 2, :reviewer, plan: plan)
     end
 
     it "changes plan's feedback_requested value to false" do
@@ -659,10 +648,6 @@
       }.from(true).to(false)
     end
 
-    it "destroys the reviewer Roles" do
-      expect { subject }.to change { plan.roles.count }.by(-2)
-    end
-
     it "doesn't send any emails" do
       User.any_instance.stubs(:get_preferences)
           .returns(:users => { :feedback_provided => false })
@@ -692,7 +677,7 @@
 
   describe "#editable_by?" do
 
-    let!(:plan) { build_plan(true, true, true, true) }
+    let!(:plan) { build_plan(true, true, true) }
 
     subject { plan }
 
@@ -732,20 +717,12 @@
       end
     end
 
-    it "when user is a reviewer" do
-      # Reviewers should only be able to edit if they are also
-      # a creator, administrator or editor
-      subject.roles.reviewer.each do |role|
-        expect(subject.editable_by?(role.user.id)).to eql(role.editor?)
-      end
-    end
-
   end
 
   describe "#readable_by?" do
 
     let!(:user) { create(:user, org: create(:org)) }
-    let!(:plan) { build_plan(true, true, true, true) }
+    let!(:plan) { build_plan(true, true, true) }
 
     subject { plan }
 
@@ -797,51 +774,83 @@
         role = subject.roles.commenter.first
         role.deactivate!
         user = role.user
-        expect(subject.commentable_by?(user.id)).to eql(false)
+        expect(subject.readable_by?(user.id)).to eql(false)
       end
 
       it "when user is a creator" do
         # All creators should be able to read
         subject.roles.creator.pluck(:user_id).each do |user_id|
-          expect(subject.commentable_by?(user_id)).to eql(true)
+          expect(subject.readable_by?(user_id)).to eql(true)
         end
       end
 
       it "when user is a administrator" do
         # All administrators should be able to read
         subject.roles.administrator.pluck(:user_id).each do |user_id|
-          expect(subject.commentable_by?(user_id)).to eql(true)
+          expect(subject.readable_by?(user_id)).to eql(true)
         end
       end
 
       it "when user is a editor" do
         # All editors should be able to read
         subject.roles.editor.pluck(:user_id).each do |user_id|
-          expect(subject.commentable_by?(user_id)).to eql(true)
+          expect(subject.readable_by?(user_id)).to eql(true)
         end
       end
 
       it "when user is a commenter" do
         # All commenters should be able to read
         subject.roles.commenter.pluck(:user_id).each do |user_id|
-          expect(subject.commentable_by?(user_id)).to eql(true)
+          expect(subject.readable_by?(user_id)).to eql(true)
         end
       end
 
-      it "when user is a reviewer" do
-        # All reviewers should be able to read
-        subject.roles.reviewer.pluck(:user_id).each do |user_id|
-          expect(subject.commentable_by?(user_id)).to eql(true)
+      context "When user is a reviewer" do
+        before do
+          user.org = plan.owner.org
+          user.save
+          user.perms << create(:perm, :review_org_plans)
+        end
+
+        it "when user is a reviewer and feedback requested" do
+          # All reviewers of the same org should be able to comment
+          plan.feedback_requested = true
+          plan.save
+          expect(subject.readable_by?(user.id)).to eql(true)
+        end
+
+        it "when user is a reviewer and feedback not requested" do
+          plan.feedback_requested = false
+          plan.save
+          expect(subject.readable_by?(user.id)).to eql(false)
+        end
+
+        it "when user is a reviewer of a different org and feedback requested" do
+          # reviewers of other orgs should have no access
+          user.org = create(:org)
+          user.save
+          user.perms << create(:perm, :review_org_plans)
+          plan.feedback_requested = true
+          plan.save
+          expect(subject.readable_by?(user.id)).to eql(false)
         end
       end
 
+      it "when user is not reviewer, has no roles on the plan and feedback requested" do
+        # All reviewers should be able to comment
+        user.org = plan.owner.org
+        user.save
+        plan.feedback_requested = true
+        plan.save
+        expect(subject.readable_by?(user.id)).to eql(false)
+      end
     end
 
   end
 
   describe "#commentable_by?" do
 
-    let!(:plan) { build_plan(true, true, true, true) }
+    let!(:plan) { build_plan(true, true, true) }
 
     subject { plan }
 
@@ -880,18 +889,55 @@
       end
     end
 
-    it "when user is a reviewer" do
-      # All reviewers should be able to comment
-      subject.roles.reviewer.pluck(:user_id).each do |user_id|
-        expect(subject.commentable_by?(user_id)).to eql(true)
+    let(:user) { create(:user) }
+
+    context "when user is a reviewer" do
+
+      before do
+        user.org = plan.owner.org
+        user.save
+        user.perms << create(:perm, :review_org_plans)
+      end
+      it "of the same org and feedback requested" do
+        # All reviewers of the same org should be able to comment
+        plan.feedback_requested = true
+        plan.save
+        expect(subject.commentable_by?(user.id)).to eql(true)
+      end
+
+      it "of the same org and feedback not requested" do
+        plan.feedback_requested = false
+        plan.save
+        expect(subject.commentable_by?(user.id)).to eql(false)
       end
+
+      it "of a different org and feedback requested" do
+        # All reviewers of other orgs should not be able to comment
+        user.org = create(:org)
+        user.save
+        # re-add permissions as org-admins will have these removed on save
+        user.perms << create(:perm, :review_org_plans)
+        plan.feedback_requested = true
+        plan.save
+        expect(subject.commentable_by?(user.id)).to eql(false)
+      end
+
+    end
+
+    it "when user is not reviewer, has no roles on the plan and feedback requested" do
+      # All reviewers should be able to comment
+      user.org = plan.owner.org
+      user.save
+      plan.feedback_requested = true
+      plan.save
+      expect(subject.commentable_by?(user.id)).to eql(false)
     end
 
   end
 
   describe "#administerable_by?" do
 
-    let!(:plan) { build_plan(true, true, true, true) }
+    let!(:plan) { build_plan(true, true, true) }
 
     subject { plan }
 
@@ -932,58 +978,33 @@
       end
     end
 
-    it "when user is a reviewer" do
-      # Reviewers should only be able to administer if they are also
-      # a creator or administrator
-      subject.roles.reviewer.each do |role|
-        expect(subject.administerable_by?(role.user.id)).to eql(role.administrator?)
-      end
-    end
-
   end
 
   describe "#reviewable_by?" do
 
-    let!(:plan) { build_plan(true, true, true, true) }
-
-    subject { plan }
-
-    it "when role is inactive" do
-      role = subject.roles.reviewer.first
-      role.deactivate!
-      user = role.user
-      expect(subject.reviewable_by?(user.id)).to eql(false)
-    end
-
-    it "when user is a creator" do
-      subject.roles.creator.pluck(:user_id).each do |user_id|
-        expect(subject.reviewable_by?(user_id)).to eql(false)
-      end
-    end
+    let!(:plan) { build_plan(true, true, true) }
+    let!(:user) { create(:user) }
 
-    it "when user is a administrator" do
-      subject.roles.administrator.pluck(:user_id).each do |user_id|
-        expect(subject.reviewable_by?(user_id)).to eql(false)
-      end
+    before do
+      plan.feedback_requested = true
+      plan.save
+      create(:perm, :review_org_plans)
     end
 
-    it "when user is a editor" do
-      subject.roles.editor.pluck(:user_id).each do |user_id|
-        expect(subject.reviewable_by?(user_id)).to eql(false)
-      end
-    end
+    subject { plan }
 
-    it "when user is a commenter" do
-      # Commenters should only be able to review if they are also a reviewer
-      subject.roles.commenter.each do |role|
-        expect(subject.reviewable_by?(role.user.id)).to eql(role.reviewer?)
-      end
+    it "when user is not a reviewer" do
+      expect(subject.reviewable_by?(user.id)).to eql(false)
     end
 
     it "when user is a reviewer" do
-      subject.roles.reviewer.pluck(:user_id).each do |user_id|
-        expect(subject.reviewable_by?(user_id)).to eql(true)
-      end
+      user.org = plan.owner.org
+      user.save
+      user.perms << Perm.review_plans
+      expect(subject.owner.org).to eql(user.org)
+      expect(user.can_review_plans?).to eql(true)
+      expect(plan.feedback_requested?).to eql(true)
+      expect(subject.reviewable_by?(user.id)).to eql(true)
     end
 
   end
@@ -1033,7 +1054,7 @@
 
   describe "#owner" do
 
-    let!(:plan) { build_plan(true, true, true, true) }
+    let!(:plan) { build_plan(true, true, true) }
 
     subject { plan }
 
@@ -1121,22 +1142,17 @@
     end
 
     it "is shared if the plan has an administrator" do
-      plan = build_plan(true, false, false, false)
+      plan = build_plan(true, false, false)
       expect(plan.shared?).to eql(true)
     end
 
     it "is shared if the plan has an editor" do
-      plan = build_plan(false, true, false, false)
+      plan = build_plan(false, true, false)
       expect(plan.shared?).to eql(true)
     end
 
     it "is shared if the plan has an commenter" do
-      plan = build_plan(false, false, true, false)
-      expect(plan.shared?).to eql(true)
-    end
-
-    it "is shared if the plan has an reviewer" do
-      plan = build_plan(false, false, false, true)
+      plan = build_plan(false, false, true)
       expect(plan.shared?).to eql(true)
     end
 
@@ -1144,7 +1160,7 @@
 
   describe "#owner_and_coowners" do
 
-    let!(:plan) { build_plan(true, true, true, true) }
+    let!(:plan) { build_plan(true, true, true) }
 
     subject { plan }
 
@@ -1176,19 +1192,10 @@
       end
     end
 
-    it "does not include the reviewer" do
-      # Only if the reviewer is not also an administrator or creator
-      subject.roles.reviewer.each do |role|
-        if !role.creator? && !role.administrator?
-          expect(subject.owner_and_coowners).to_not include(role.user)
-        end
-      end
-    end
-
   end
 
   describe ".authors" do
-    let!(:plan) { build_plan(true, true, true, true) }
+    let!(:plan) { build_plan(true, true, true) }
 
     subject { plan }
 
@@ -1215,15 +1222,6 @@
         end
       end
     end
-
-    it "does not include the reviewer" do
-      # Only if the reviewer is not also an editor, administrator or creator
-      subject.roles.reviewer.each do |role|
-        if !role.creator? && !role.administrator? && !role.editor?
-          expect(subject.authors).to_not include(role.user)
-        end
-      end
-    end
   end
 
   describe "#num_answered_questions" do
diff --git a/spec/models/role_spec.rb b/spec/models/role_spec.rb
index c96a04c2ed..bdc35f7499 100644
--- a/spec/models/role_spec.rb
+++ b/spec/models/role_spec.rb
@@ -29,7 +29,7 @@
 
   describe ".deactivate!" do
     before do
-      @plan = build_plan(true, true, true, true)
+      @plan = build_plan(true, true, true)
     end
 
     subject { @plan }
@@ -60,25 +60,19 @@
         expect(@role.active).to eql(false)
       end
 
-      it "reviewer is no longer active" do
-        @role = subject.roles.reviewer.first
-        @role.deactivate!
-        expect(@role.active).to eql(false)
-      end
-
     end
 
     context "Deactivation calls Plan.deactivate! if Plan.authors is empty" do
 
       it "plan has no other authors" do
-        plan = build_plan(false, false, false, false)
+        plan = build_plan(false, false, false)
         role = plan.roles.creator.first
         role.plan.expects(:deactivate!).times(1)
         role.deactivate!
       end
 
       it "plan has another author" do
-        plan = build_plan(true, false, false, false)
+        plan = build_plan(true, false, false)
         role = plan.roles.creator.first
         role.plan.expects(:deactivate!).times(0)
         role.deactivate!
diff --git a/spec/support/helpers/roles_helper.rb b/spec/support/helpers/roles_helper.rb
index 19176e9a4e..1705cc647e 100644
--- a/spec/support/helpers/roles_helper.rb
+++ b/spec/support/helpers/roles_helper.rb
@@ -1,6 +1,6 @@
 module RolesHelper
 
-  def build_plan(administrator = false, editor = false, commenter = false, reviewer = false)
+  def build_plan(administrator = false, editor = false, commenter = false)
     org = create(:org)
     creator = create(:user, org: org)
     plan = create(:plan, answers: 2, guidance_groups: 2)
@@ -15,9 +15,6 @@ def build_plan(administrator = false, editor = false, commenter = false, reviewe
     if commenter
       create(:role, :commenter, :active, plan: plan, user: create(:user, org: org))
     end
-    if reviewer
-      create(:role, :reviewer, :active, plan: plan, user: create(:user, org: org))
-    end
     plan
   end