Merge pull request #2413 from gnclmorais/reduce-members-loaded-on-new-event

Reduce members loaded on new event

Author: gnclmorais

app/controllers/application_controller.rb

@@ -105,15 +105,15 @@ def redirect_path
   end
 
   def authenticate_admin!
-    redirect_to root_path, notice: "You can't be here" unless logged_in? && current_user.has_role?(:admin)
+    redirect_to root_path, notice: "You can't be here" unless logged_in? && current_user.is_admin?
   end
 
   def authenticate_admin_or_organiser!
     redirect_to root_path, notice: "You can't be here" unless manager?
   end
 
   def manager?
-    logged_in? && (current_user.is_admin? || current_user.has_role?(:organiser, :any))
+    logged_in? && current_user.manager?
   end
 
   helper_method :manager?

app/models/member.rb

@@ -51,10 +51,20 @@ class Member < ApplicationRecord
 
   scope :with_skill, ->(skill_name) { tagged_with(skill_name) }
 
+  scope :admin, -> { with_role(:admin) }
+  scope :organiser, -> { with_role(:organiser, :any) }
+  scope :manager, -> { admin.or(organiser).distinct }
+
+  scope :sort_alphabetically, -> { order(:name, :surname) }
+
   acts_as_taggable_on :skills
 
   attr_accessor :attendance, :newsletter
 
+  def manager?
+    is_admin? || has_role?(:organiser, :any)
+  end
+
   def banned?
     bans.active.present? || bans.permanent.present?
   end

app/views/admin/events/_form.html.haml

@@ -1,3 +1,7 @@
+:ruby
+  all_sponsors = Sponsor.all
+  all_managers = Member.manager.sort_alphabetically
+
 = simple_form_for [:admin, @event] do |f|
   .row
     .col-12
@@ -34,15 +38,15 @@
       %p
         %strong Please add sponsors only to either Standard level OR Gold/Silver/Bronze levels.
     .col-12
-      = f.association :sponsors, label: 'Standard sponsors', input_html: { data: { placeholder: 'Select standard sponsors' }}, collection: Sponsor.all
+      = f.association :sponsors, label: 'Standard sponsors', input_html: { data: { placeholder: 'Select standard sponsors' }}, collection: all_sponsors
     .col-12.col-md-6.col-lg-4
-      = f.association :bronze_sponsors, input_html: { data: { placeholder: 'Select bronze sponsors' }}, collection: Sponsor.all
+      = f.association :bronze_sponsors, input_html: { data: { placeholder: 'Select bronze sponsors' }}, collection: all_sponsors
     .col-12.col-md-6.col-lg-4
-      = f.association :silver_sponsors, input_html: { data: { placeholder: 'Select silver sponsors' }}, collection: Sponsor.all
+      = f.association :silver_sponsors, input_html: { data: { placeholder: 'Select silver sponsors' }}, collection: all_sponsors
     .col-12.col-md-6.col-lg-4
-      = f.association :gold_sponsors, input_html: { data: { placeholder: 'Select gold sponsors' }}, collection: Sponsor.all
+      = f.association :gold_sponsors, input_html: { data: { placeholder: 'Select gold sponsors' }}, collection: all_sponsors
     .col-12
-      = f.input :organisers, collection: Member.all, value_method: :id, label_method: :full_name, selected: @event.organisers.map(&:id), input_html: { multiple: true }
+      = f.input :organisers, collection: all_managers, value_method: :id, label_method: :full_name, selected: @event.organisers.pluck(&:id), input_html: { multiple: true }
     .col-12
       = f.input :announce_only, as: :boolean, hint: 'Events where invitations are not handled via our application'
     .col-12

spec/models/member_spec.rb

@@ -164,6 +164,50 @@
         expect(Member.find_members_by_name('').size).to eq(0)
       end
     end
+  end
+
+  describe '.admin' do
+    it 'returns members with admin role' do
+      admin_member = Fabricate(:member)
+      admin_member.add_role :admin
+      non_admin = Fabricate(:member)
+
+      expect(described_class.admin).to include(admin_member)
+      expect(described_class.admin).not_to include(non_admin)
+    end
+  end
+
+  describe '.organiser' do
+    it 'returns members with organiser role' do
+      chapter = Fabricate(:chapter)
+      organiser = Fabricate(:member)
+      organiser.add_role :organiser, chapter
+      non_organiser = Fabricate(:member)
 
+      expect(described_class.organiser).to include(organiser)
+      expect(described_class.organiser).not_to include(non_organiser)
+    end
+  end
+
+  describe '.manager' do
+    it 'returns both admins and organisers' do
+      admin_member = Fabricate(:member)
+      admin_member.add_role :admin
+
+      chapter = Fabricate(:chapter)
+      organiser = Fabricate(:member)
+      organiser.add_role :organiser, chapter
+      # Add another role to test for duplicates
+      organiser.add_role :admin
+
+      non_manager = Fabricate(:member)
+
+      managers = described_class.manager
+
+      expect(managers).to include(admin_member, organiser)
+      expect(managers).not_to include(non_manager)
+
+      expect(managers.size).to eq(managers.distinct.size)
+    end
   end
 end