Issue#dcc2983 - Fix for preventing adding a Contributor via Contributor

app/controllers/paginable/plans_controller.rb

@@ -58,16 +58,17 @@ def org_admin
     end
     # rubocop:enable Metrics/AbcSize
 
-    # GET /paginable/plans/org_admin/:page
-    def org_admin_other_user
+    # GET /paginable/users/:id/plans
+    def index
       @user = User.find(params[:id])
       authorize @user
       raise Pundit::NotAuthorizedError unless current_user.present? && current_user.can_org_admin? && @user.present?
 
       paginable_renderise(
-        partial: 'org_admin_other_user',
+        partial: 'index',
         scope: Plan.active(@user),
-        query_params: { sort_field: 'plans.updated_at', sort_direction: :desc }
+        query_params: { sort_field: 'plans.updated_at', sort_direction: :desc },
+        format: :json
       )
     end
   end

app/controllers/super_admin/orgs_controller.rb

@@ -153,7 +153,8 @@ def org_params
       params.require(:org).permit(:name, :abbreviation, :logo, :managed,
                                   :contact_email, :contact_name,
                                   :remove_logo, :feedback_enabled, :feedback_msg,
-                                  :org_id, :org_name, :org_crosswalk)
+                                  :org_id, :org_name, :org_crosswalk,
+                                  :funder, :institution, :organisation)
     end
 
     def merge_params

app/models/concerns/exportable_plan.rb

@@ -174,7 +174,7 @@ def show_section_for_csv(csv, phase, section, headings, unanswered, hash)
       answer_text = ''
       if answer.present?
         answer_text += answer.question_options.pluck(:text).join(', ') if answer.question_options.any?
-        answer_text += answer.text if answer.answered?
+        answer_text += answer.text if answer.answered? && answer.text.present?
       elsif unanswered
         answer_text += _('Not Answered')
       end

app/models/contributor.rb

@@ -130,10 +130,20 @@ def merge(other)
 
   private
 
+  # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity
   def name_or_email_presence
-    return true unless name.blank? && email.blank?
+    errors.add(:name, _("can't be blank.")) if name.blank? && Rails.configuration.x.application.require_contributor_name
+    if email.blank? && Rails.configuration.x.application.require_contributor_email
+      errors.add(:email,
+                 _("can't be blank."))
+    end
+
+    if name.blank? && email.blank? && errors.size.zero?
+      errors.add(:name, _("can't be blank if no email is provided."))
+      errors.add(:email, _("can't be blank if no name is provided."))
+    end
 
-    errors.add(:name, _("can't be blank if no email is provided"))
-    errors.add(:email, _("can't be blank if no name is provided"))
+    errors.size.zero?
   end
+  # rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity
 end

app/models/org.rb

@@ -94,8 +94,6 @@ class Org < ApplicationRecord
   validates :name, presence: { message: PRESENCE_MESSAGE },
                    uniqueness: { message: UNIQUENESS_MESSAGE }
 
-  validates :abbreviation, presence: { message: PRESENCE_MESSAGE }
-
   validates :is_other, inclusion: { in: BOOLEAN_VALUES,
                                     message: PRESENCE_MESSAGE }
 
@@ -290,10 +288,12 @@ def org_admin_plans
 
     if Rails.configuration.x.plans.org_admins_read_all
       Plan.includes(:template, :phases, :roles, :users).where(id: combined_plan_ids)
+          .where(roles: { active: true })
     else
       Plan.includes(:template, :phases, :roles, :users).where(id: combined_plan_ids)
           .where.not(visibility: Plan.visibilities[:privately_visible])
           .where.not(visibility: Plan.visibilities[:is_test])
+          .where(roles: { active: true })
     end
   end
   # rubocop:enable Metrics/AbcSize

app/views/org_admin/users/edit.html.erb

@@ -75,9 +75,9 @@
 <div class="row">
   <div class="col-md-12">
     <%= paginable_renderise(
-      partial: '/paginable/plans/org_admin_other_user',
+      partial: '/paginable/plans/index',
       controller: 'paginable/plans',
-      action: 'org_admin_other_user',
+      action: 'index',
       scope: @plans,
       query_params: { sort_field: 'plans.updated_at', sort_direction: 'desc' }) %>
   </div>

app/views/org_admin/users/plans.html.erb

@@ -7,9 +7,9 @@
 <div class="row">
   <div class="col-md-12">
     <%= paginable_renderise(
-      partial: '/paginable/plans/org_admin_other_user',
+      partial: '/paginable/plans/index',
       controller: 'paginable/plans',
-      action: 'org_admin_other_user',
+      action: 'index',
       remote: true,
       scope: @plans,
       query_params: { sort_field: 'plans.updated_at', sort_direction: 'desc' }) %>

app/views/orgs/_profile_form.html.erb

@@ -25,7 +25,7 @@
     <div class="row">
       <div class="form-group col-xs-8">
         <%= f.label :abbreviation, _('Organisation abbreviated name'), class: "control-label" %>
-        <%= f.text_field :abbreviation, id: "org_abbreviation", class: "form-control", "aria-required": true %>
+        <%= f.text_field :abbreviation, id: "org_abbreviation", class: "form-control" %>
       </div>
     </div>
   <% end %>

app/views/paginable/plans/_org_admin_other_user.html.erb → app/views/paginable/plans/_index.html.erb

@@ -21,8 +21,8 @@
             <% end %>
           </td>
           <td><%= plan.template.title %></td>
-          <td><%= plan.owner&.org&.name %></td>
-          <td><%= plan.owner&.name(false) %></td>
+          <td><%= plan.owner.org.name if plan.owner.present? %></td>
+          <td><%= plan.owner.name(false) if plan.owner.present? %></td>
           <td><%= l(plan.updated_at.to_date, formats: :short) %></td>
           <td class="plan-visibility">
             <%= plan.visibility === 'is_test' ? _('Test') : sanitize(display_visibility(plan.visibility)) %>

app/views/shared/org_selectors/_external_only.html.erb

@@ -13,8 +13,8 @@ presenter = OrgSelectionPresenter.new(orgs: [default_org],
 placeholder = _("Begin typing to see a list of suggestions.")
 %>
 
-<%= form.label :org_name, label %>
-<%= form.text_field :org_name, class: "form-control autocomplete",
+<%= form.label :name, label %>
+<%= form.text_field :name, class: "form-control autocomplete",
                             placeholder: placeholder,
                             value: presenter.name,
                             spellcheck: true,

app/views/super_admin/users/edit.html.erb

@@ -109,9 +109,9 @@
 <div class="row">
   <div class="col-md-12">
     <%= paginable_renderise(
-      partial: '/paginable/plans/org_admin_other_user',
+      partial: '/paginable/plans/index',
       controller: 'paginable/plans',
-      action: 'org_admin_other_user',
+      action: 'index',
       scope: @plans,
       query_params: { sort_field: 'plans.updated_at', sort_direction: 'desc' }) %>
   </div>

config/initializers/_dmproadmap.rb

@@ -118,6 +118,10 @@ class Application < Rails::Application
     # Setting to display phone number in contributor form
     config.x.application.display_contributor_phone_number = false
 
+    # Setting require contributor requirement of contributor name and email
+    config.x.application.require_contributor_name = false
+    config.x.application.require_contributor_email = false
+
     # ------------------- #
     # SHIBBOLETH SETTINGS #
     # ------------------- #

config/routes.rb

@@ -210,17 +210,17 @@
 
       get 'org_admin/:page', action: :org_admin, on: :collection, as: :org_admin
 
-      get 'org_admin_other_user/:page', action: :org_admin_other_user,
-                                        on: :collection, as: :org_admin_other_user
-
       # Paginable actions for contributors
       resources :contributors, only: %i[index] do
         get 'index/:page', action: :index, on: :collection, as: :index
       end
     end
+
     # Paginable actions for users
-    resources :users, only: [] do
-      get 'index/:page', action: :index, on: :collection, as: :index
+    resources :users, only: %i[index] do
+      member do
+        resources :plans, only: %(index)
+      end
     end
     # Paginable actions for themes
     resources :themes, only: [] do

spec/models/org_spec.rb

@@ -12,8 +12,6 @@
         .with_message('must be unique')
     }
 
-    it { is_expected.to validate_presence_of(:abbreviation) }
-
     it { is_expected.to allow_values(true, false).for(:is_other) }
 
     it { is_expected.not_to allow_value(nil).for(:is_other) }
@@ -401,6 +399,7 @@
         @perm = build(:perm)
         @perm.name = 'grant_permissions'
         user.perms << @perm
+        plan.add_user!(user.id, :reviewer)
         plan.privately_visible!
       end
 
@@ -413,6 +412,7 @@
         @perm = build(:perm)
         @perm.name = 'grant_permissions'
         user.perms << @perm
+        plan.add_user!(user.id, :reviewer)
         plan.publicly_visible!
       end