From 34ca6e7ed8c1b517327f7f14533f59f618f46503 Mon Sep 17 00:00:00 2001 From: Jennifer Konikowski Date: Thu, 20 Jun 2019 15:06:15 -0400 Subject: [PATCH] add averages to schools - fixes #20 - keeps test coverage at 100% - removes pagination from schools due to aggregate query function error --- app/controllers/courses_controller.rb | 2 +- app/controllers/schools_controller.rb | 4 +- app/models/course.rb | 1 + app/models/school.rb | 35 +++++++++- app/views/schools/_list.html.erb | 9 ++- app/views/schools/index.html.erb | 20 ++++++ app/views/users/_list.html.erb | 2 +- spec/controllers/schools_controller_spec.rb | 2 +- spec/models/school_spec.rb | 77 ++++++++++++++++++++- 9 files changed, 143 insertions(+), 9 deletions(-) diff --git a/app/controllers/courses_controller.rb b/app/controllers/courses_controller.rb index 87764b5a..29197264 100644 --- a/app/controllers/courses_controller.rb +++ b/app/controllers/courses_controller.rb @@ -9,7 +9,7 @@ class CoursesController < ApplicationController # GET /courses # GET /courses.json def index - courses = Course.all.with_averages + courses = Course.with_averages if params[:school_id].to_i.zero? (@filterrific = initialize_filterrific( courses, diff --git a/app/controllers/schools_controller.rb b/app/controllers/schools_controller.rb index 9b0e4a3d..c3264cdb 100644 --- a/app/controllers/schools_controller.rb +++ b/app/controllers/schools_controller.rb @@ -9,13 +9,13 @@ class SchoolsController < ApplicationController # GET /schools.json def index (@filterrific = initialize_filterrific( - School.all, + School.with_averages, params[:filterrific], select_options: { sorted_by: School.options_for_sorted_by } )) || return - @schools = @filterrific.find.page(params[:page]) + @schools = @filterrific.find end # GET /schools/1 diff --git a/app/models/course.rb b/app/models/course.rb index da0e7bb2..8841e943 100644 --- a/app/models/course.rb +++ b/app/models/course.rb @@ -94,6 +94,7 @@ class Course < ApplicationRecord .left_joins(:reviews, :school) .group('courses.id, schools.id') } + def full_number "#{department} #{number}" end diff --git a/app/models/school.rb b/app/models/school.rb index 5d14d308..93b9faa0 100644 --- a/app/models/school.rb +++ b/app/models/school.rb @@ -10,6 +10,9 @@ class School < ApplicationRecord available_filters: %i[ sorted_by search_query + with_avg_rating_gte + with_avg_difficulty_lte + with_avg_work_lte ] ) @@ -47,11 +50,38 @@ class School < ApplicationRecord order(Arel.sql("LOWER(schools.short_name) #{direction}")) when /^name_/ order(Arel.sql("LOWER(schools.name) #{direction}")) + when /^avg_rating_/ + order(Arel.sql("avg_rating #{direction}")) + when /^avg_difficulty_/ + order(Arel.sql("avg_difficulty #{direction}")) + when /^avg_work_/ + order(Arel.sql("avg_work #{direction}")) else raise(ArgumentError, "Invalid sort option: #{sort_option.inspect}") end } + scope :with_avg_rating_gte, ->(ref_num) { + having('avg(reviews.rating) >= ?', ref_num) + } + + scope :with_avg_difficulty_lte, ->(ref_num) { + having('avg(reviews.difficulty) <= ?', ref_num) + } + + scope :with_avg_work_lte, ->(ref_num) { + having('avg(reviews.work_required) <= ?', ref_num) + } + + scope :with_averages, -> { + select('schools.*, + avg(reviews.rating) as avg_rating, + avg(reviews.difficulty) as avg_difficulty, + avg(reviews.work_required) as avg_work') + .left_joins(courses: [:reviews]) + .group('schools.id') + } + def self.options_for_select schools = School.arel_table order(schools[:name].lower).pluck(:name, :id) @@ -62,7 +92,10 @@ def self.options_for_sorted_by ['Name (a-z)', 'name_asc'], ['Name (z-a)', 'name_desc'], ['Short Name (a-z)', 'short_name_asc'], - ['Short Name (z-a)', 'short_name_desc'] + ['Short Name (z-a)', 'short_name_desc'], + ['Average Rating (highest first)', 'avg_rating_desc'], + ['Average Work Required (lowest first)', 'avg_work_asc'], + ['Average Difficulty (lowest first)', 'avg_difficulty_asc'] ] end end diff --git a/app/views/schools/_list.html.erb b/app/views/schools/_list.html.erb index 75a89957..22eaa305 100644 --- a/app/views/schools/_list.html.erb +++ b/app/views/schools/_list.html.erb @@ -5,7 +5,7 @@ <% if schools.load.empty? %> No schools at this time! <% else %> - <%= page_entries_info schools, class: 'align-middle' %> + Displaying <%= pluralize(schools.size, 'school') %> <% end %>
@@ -19,6 +19,9 @@ <%= filterrific_sorting_link(@filterrific, :name) %> <%= filterrific_sorting_link(@filterrific, :short_name) %> + <%= filterrific_sorting_link(@filterrific, :avg_rating) %> + <%= filterrific_sorting_link(@filterrific, :avg_difficulty) %> + <%= filterrific_sorting_link(@filterrific, :avg_work) %> @@ -28,6 +31,9 @@ <%= link_to school.name, school %> <%= school.short_name %> + <%= school.avg_rating.round(1) %> + <%= school.avg_difficulty.round(1) %> + <%= school.avg_work.round(1) %> <% if can? :manage, School %> <%= link_to 'Edit', edit_school_path(school) %> <%= link_to 'Delete', school, method: :delete, data: { confirm: 'Are you sure?' } %> @@ -36,5 +42,4 @@ <% end %> - <%= paginate schools %>
diff --git a/app/views/schools/index.html.erb b/app/views/schools/index.html.erb index a51d15ad..c6335322 100644 --- a/app/views/schools/index.html.erb +++ b/app/views/schools/index.html.erb @@ -25,6 +25,26 @@ +
+
+
+ + <%= f.number_field(:with_avg_rating_gte) %> +
+
+
+
+ + <%= f.number_field(:with_avg_difficulty_lte) %> +
+
+
+
+ + <%= f.number_field(:with_avg_work_lte) %> +
+
+
<% end %> diff --git a/app/views/users/_list.html.erb b/app/views/users/_list.html.erb index ed66b527..11912229 100644 --- a/app/views/users/_list.html.erb +++ b/app/views/users/_list.html.erb @@ -5,7 +5,7 @@ <% if users.load.empty? %> No users at this time! <% else %> - <%= page_entries_info users, class: 'align-middle' %> + <%= page_entries_info users %> <% end %>
diff --git a/spec/controllers/schools_controller_spec.rb b/spec/controllers/schools_controller_spec.rb index 02ee1a61..23f7db2c 100644 --- a/spec/controllers/schools_controller_spec.rb +++ b/spec/controllers/schools_controller_spec.rb @@ -25,7 +25,7 @@ it 'assigns schools to @schools' do get :index - expect(assigns(:schools).size).to eq(3) + expect(assigns(:schools).length).to eq(3) end it 'renders the index template' do diff --git a/spec/models/school_spec.rb b/spec/models/school_spec.rb index 6a29f0bb..1f43880b 100644 --- a/spec/models/school_spec.rb +++ b/spec/models/school_spec.rb @@ -34,15 +34,87 @@ school1 = create(:school, name: 'Alphabets', short_name: 'abc') school2 = create(:school, name: 'Zebra', short_name: 'xyz') school3 = create(:school, name: 'Cows', short_name: 'Bovine') + course1 = create(:course, school: school1) + course2 = create(:course, school: school2) + course3 = create(:course, school: school3) + review1 = create(:review, course: course1, work_required: 1, difficulty: 5, rating: 10) + review2 = create(:review, course: course2, work_required: 5, difficulty: 10, rating: 1) + review3 = create(:review, course: course3, work_required: 10, difficulty: 1, rating: 5) aggregate_failures do expect(School.sorted_by('name_asc').first).to eq school1 expect(School.sorted_by('name_desc').first).to eq school2 expect(School.sorted_by('short_name_asc').first).to eq school1 expect(School.sorted_by('short_name_desc').first).to eq school2 + expect(School.with_averages.sorted_by('avg_work_desc').first).to eq school3 + expect(School.with_averages.sorted_by('avg_work_asc').first).to eq school1 + expect(School.with_averages.sorted_by('avg_rating_desc').first).to eq school1 + expect(School.with_averages.sorted_by('avg_rating_asc').first).to eq school2 + expect(School.with_averages.sorted_by('avg_difficulty_desc').first).to eq school2 + expect(School.with_averages.sorted_by('avg_difficulty_asc').first).to eq school3 expect { School.sorted_by('oh_no') }.to raise_error(ArgumentError, 'Invalid sort option: "oh_no"') end end end + + describe 'with_avg_rating_gte' do + it 'gets all reviews with query in review' do + school1 = create(:school) + school2 = create(:school) + school3 = create(:school) + course1 = create(:course, school: school1) + course2 = create(:course, school: school2) + course3 = create(:course, school: school3) + review1 = create(:review, rating: 10, course: course1) + review2 = create(:review, rating: 2, course: course2) + review3 = create(:review, rating: 5, course: course3) + aggregate_failures do + expect(School.with_averages.with_avg_rating_gte(3).length).to eq 2 + expect(School.with_averages.with_avg_rating_gte(3)).not_to include school2 + expect(School.with_averages.with_avg_rating_gte(3)).to include school1 + expect(School.with_averages.with_avg_rating_gte(3)).to include school3 + end + end + end + + describe 'with_avg_difficulty_lte' do + it 'gets all reviews with query in review' do + school1 = create(:school) + school2 = create(:school) + school3 = create(:school) + course1 = create(:course, school: school1) + course2 = create(:course, school: school2) + course3 = create(:course, school: school3) + review1 = create(:review, difficulty: 4, course: course1) + review2 = create(:review, difficulty: 10, course: course2) + review3 = create(:review, difficulty: 2, course: course3) + aggregate_failures do + expect(School.with_averages.with_avg_difficulty_lte(5).length).to eq 2 + expect(School.with_averages.with_avg_difficulty_lte(5)).not_to include school2 + expect(School.with_averages.with_avg_difficulty_lte(5)).to include school1 + expect(School.with_averages.with_avg_difficulty_lte(5)).to include school3 + end + end + end + + describe 'with_avg_work_lte' do + it 'gets all reviews with query in review' do + school1 = create(:school) + school2 = create(:school) + school3 = create(:school) + course1 = create(:course, school: school1) + course2 = create(:course, school: school2) + course3 = create(:course, school: school3) + review1 = create(:review, work_required: 10, course: course1) + review2 = create(:review, work_required: 5, course: course2) + review3 = create(:review, work_required: 15, course: course3) + aggregate_failures do + expect(School.with_averages.with_avg_work_lte(10).length).to eq 2 + expect(School.with_averages.with_avg_work_lte(10)).to include school1 + expect(School.with_averages.with_avg_work_lte(10)).to include school2 + expect(School.with_averages.with_avg_work_lte(10)).not_to include school3 + end + end + end end context 'methods' do it 'returns options for sorted by' do @@ -50,7 +122,10 @@ ['Name (a-z)', 'name_asc'], ['Name (z-a)', 'name_desc'], ['Short Name (a-z)', 'short_name_asc'], - ['Short Name (z-a)', 'short_name_desc'] + ['Short Name (z-a)', 'short_name_desc'], + ['Average Rating (highest first)', 'avg_rating_desc'], + ['Average Work Required (lowest first)', 'avg_work_asc'], + ['Average Difficulty (lowest first)', 'avg_difficulty_asc'] ] expect(School.options_for_sorted_by).to eq expected_options end