Merge pull request #343 from Mate2xo/calendar_performance

Calendar performance

Pressing the 'next' of 'previous' button of the calendar
makes a new request to /missions.json with date params.
We use these params to avoid fetching unneeded records from the DB.

Also, the base HTML view that is initially rendered
has no use for Mission records, as it's just a placeholder for FullCalendar

Gemfile

@@ -52,6 +52,7 @@ gem 'recurring_select' # Events recurrence rules set helper
 gem 'thredded', '~> 0.16.16' # Forum engine
 
 group :development, :test do
+  gem 'bullet'
   gem 'factory_bot_rails', '~> 4.0'
   gem 'faker' # Generate fake data for the seed.rb and spec factories
   gem 'guard-rspec', require: false
@@ -61,7 +62,6 @@ end
 
 group :development do
   gem 'annotate', '~> 2.7', '>= 2.7.4'
-  gem 'bullet', group: 'development'
   gem 'letter_opener'
   gem 'solargraph' # LSP provinding app documention through IDE
   # Access an interactive console on exception pages or by calling 'console' anywhere in the code.

app/controllers/missions_controller.rb

@@ -8,24 +8,32 @@ class MissionsController < ApplicationController
   before_action :set_authorized_mission, only: %i[show edit update destroy]
 
   def index
-    @missions = Mission.includes(:members)
+    respond_to do |format|
+      format.html
+      format.json do
+        @missions = Mission.includes(:members, :enrollments)
+        if (filter = date_filtering_params)
+          @missions = @missions.where(start_date: filter[:from]..filter[:to])
+        end
+      end
+    end
   end
 
+  def show; end
+
   def new
     @mission = Mission.new
   end
 
+  def edit; end
+
   def create
     @mission = Mission.new(permitted_params)
     @mission.author = current_member
 
     generate(@mission)
   end
 
-  def show; end
-
-  def edit; end
-
   def update
     if update_transaction.success?
       flash[:notice] = translate 'activerecord.notices.messages.update_success'
@@ -74,12 +82,10 @@ def generate(mission)
 
   def update_transaction
     @update_transaction ||=
-      begin
-        Missions::UpdateTransaction.new.with_step_args(
-          transform_time_slots_in_time_params_for_enrollment: [regulated: @mission.regulated?],
-          update: [mission: @mission]
-        ).call(permitted_params)
-      end
+      Missions::UpdateTransaction.new.with_step_args(
+        transform_time_slots_in_time_params_for_enrollment: [regulated: @mission.regulated?],
+        update: [mission: @mission]
+      ).call(permitted_params)
   end
 
   def permitted_params
@@ -101,7 +107,11 @@ def base_params
   end
 
   def regulated_mission_params
-    enrollment_params = params.require(:mission).permit(enrollments_attributes: [:id, :_destroy, :member_id, time_slots: []])
+    enrollment_params = params.require(:mission)
+                              .permit(enrollments_attributes: [
+                                        :id, :_destroy, :member_id,
+                                        { time_slots: [] }
+                                      ])
     base_params.merge(enrollment_params)
   end
 
@@ -114,4 +124,15 @@ def standard_mission_params
   def set_authorized_mission
     @mission = authorize Mission.find(params[:id])
   end
+
+  def date_filtering_params
+    return unless params[:start].present? && params[:end].present?
+
+    start_date = Date.parse(params[:start])
+    end_date = Date.parse(params[:end])
+
+    { from: start_date, to: end_date }
+  rescue ArgumentError => _e
+    nil
+  end
 end

bin/setup

@@ -20,7 +20,7 @@ chdir APP_ROOT do
   system('bundle check') || system!('bundle install')
 
   # Install JavaScript dependencies if using Yarn
-  # system('bin/yarn')
+  system('bin/yarn')
 
   # puts "\n== Copying sample files =="
   # unless File.exist?('config/database.yml')

config/environments/development.rb

@@ -63,7 +63,12 @@
   # routes, locales, etc. This feature depends on the listen gem.
   config.file_watcher = ActiveSupport::EventedFileUpdateChecker
 
-  Bullet.enable = true
-  Bullet.add_footer = true
-  Bullet.bullet_logger = true
+  config.after_initialize do
+    Bullet.enable = true
+    Bullet.alert = true
+    Bullet.bullet_logger = false
+    Bullet.console = true
+    Bullet.rails_logger = true
+    Bullet.add_footer = true
+  end
 end

config/environments/test.rb

@@ -46,4 +46,10 @@
 
   # Raises error for missing translations
   config.action_view.raise_on_missing_translations = true
+
+  config.after_initialize do
+    Bullet.enable = true
+    Bullet.bullet_logger = false
+    Bullet.raise = false # raise an error if n+1 query occurs
+  end
 end

spec/controllers/missions_controller_spec.rb

@@ -2,33 +2,33 @@
 
 require 'rails_helper'
 
+# TODO: Move these tests into the matching request spec
 RSpec.describe MissionsController, type: :controller do
-  let(:member) { create :member }
-  let(:super_admin) { create :member, :super_admin }
-  let(:mission) { create :mission }
+  let(:member) { create(:member) }
+  let(:super_admin) { create(:member, :super_admin) }
+  let(:mission) { create(:mission) }
   let(:valid_attributes) { attributes_for(:mission) }
 
   before { sign_in super_admin }
 
-  describe 'GET index' do
-    before { get :index }
-
-    it 'assigns @mission' do expect(assigns(:missions)).to include(mission) end
-    it { expect(response).to have_http_status(:success) }
-  end
-
   describe 'GET show' do
     before { get :show, params: { id: mission.id } }
 
     it { expect(response).to have_http_status :success }
-    it 'assigns @mission' do expect(assigns(:mission)).to eq(mission) end
+
+    it 'assigns @mission' do
+      expect(assigns(:mission)).to eq(mission)
+    end
   end
 
   describe 'GET edit' do
     before { get :edit, params: { id: mission.id } }
 
     it { expect(response).to have_http_status :success }
-    it 'assigns @mission' do expect(assigns(:mission)).to eq(mission) end
+
+    it 'assigns @mission' do
+      expect(assigns(:mission)).to eq(mission)
+    end
   end
 
   describe 'PUT update' do
@@ -38,7 +38,10 @@
         mission.reload
       end
 
-      it 'assigns @mission' do expect(assigns(:mission)).to eq(mission) end
+      it 'assigns @mission' do
+        expect(assigns(:mission)).to eq(mission)
+      end
+
       it { expect(response).to render_template(:show) }
 
       %i[
@@ -85,7 +88,8 @@ def invalid_request(invalid_attribute)
     end
 
     before do
-      mission_params['recurrence_rule'] = '{"interval":1, "until":null, "count":null, "validations":{ "day":[2,3,5,6] }, "rule_type":"IceCube::WeeklyRule", "week_start":1 }'
+      mission_params['recurrence_rule'] =
+        '{"interval":1, "until":null, "count":null, "validations":{ "day":[2,3,5,6] }, "rule_type":"IceCube::WeeklyRule", "week_start":1 }'
       mission_params['recurrence_end_date'] = 1.week.from_now
     end