From a1090a1b7456c4c4c679b32958e7154d8bb8a7fc Mon Sep 17 00:00:00 2001 From: johnnymo87 Date: Sat, 11 Apr 2015 18:14:53 +0000 Subject: [PATCH 01/21] implements miniprofiler, with superadmin restriction commented out --- Gemfile | 2 ++ Gemfile.lock | 11 +++++++++++ app/controllers/application_controller.rb | 9 ++++++++- 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index 6f5a99f09..f72773cfc 100644 --- a/Gemfile +++ b/Gemfile @@ -88,3 +88,5 @@ gem 'url_validator' gem 'rails_autolink' gem "paranoia", "~> 2.0" +gem 'rack-mini-profiler' +gem 'flamegraph' diff --git a/Gemfile.lock b/Gemfile.lock index 1ad76e58b..97fbbe1c9 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -138,8 +138,13 @@ GEM factory_girl_rails (4.5.0) factory_girl (~> 4.5.0) railties (>= 3.0.0) + fast_stack (0.1.0) + rake + rake-compiler fattr (2.2.2) ffi (1.9.6) + flamegraph (0.1.0) + fast_stack flay (2.6.1) ruby_parser (~> 3.0) sexp_processor (~> 4.0) @@ -231,6 +236,8 @@ GEM phantomjs (1.9.8.0) procto (0.0.2) rack (1.6.0) + rack-mini-profiler (0.9.3) + rack (>= 1.1.3) rack-protection (1.5.3) rack rack-test (0.6.3) @@ -277,6 +284,8 @@ GEM rainbow (2.0.0) raindrops (0.13.0) rake (10.4.2) + rake-compiler (0.9.5) + rake redcard (1.1.0) redcarpet (3.2.2) reek (1.6.6) @@ -407,6 +416,7 @@ DEPENDENCIES devise_invitable (~> 1.3.0) execjs factory_girl_rails + flamegraph font-awesome-rails geocoder gmaps4rails (= 2.1.2) @@ -420,6 +430,7 @@ DEPENDENCIES newrelic_rpm paranoia (~> 2.0) pg + rack-mini-profiler rack_session_access railroady rails (~> 4.2.0) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index dc8ebd329..f9328583d 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -2,7 +2,10 @@ class ApplicationController < ActionController::Base protect_from_forgery - before_filter :store_location, :assign_footer_page_links + before_filter :store_location, + :assign_footer_page_links, + :miniprofiler + include CustomErrors # To prevent infinite redirect loops, only requests from white listed @@ -86,6 +89,10 @@ def superadmin? current_user.try :superadmin? end + def miniprofiler + # Rack::MiniProfiler.authorize_request if superadmin? + end + def assign_footer_page_links @footer_page_links = Page.visible_links end From 035d8aa8e707cc81596c2b9987df8c226d9a4bca Mon Sep 17 00:00:00 2001 From: johnnymo87 Date: Sat, 11 Apr 2015 20:05:05 +0000 Subject: [PATCH 02/21] exposes recently_updated_and_has_owner via SQL --- app/controllers/organisations_controller.rb | 1 + app/services/queries/organisations.rb | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index 2215e3c97..c28a7f74b 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -137,6 +137,7 @@ def self.build params private def build_map_markers(organisations) + Queries::Organisations.xyz(organisations) ::MapMarkerJson.build(organisations) do |org, marker| marker.lat org.latitude marker.lng org.longitude diff --git a/app/services/queries/organisations.rb b/app/services/queries/organisations.rb index 646fb0839..3c343298a 100644 --- a/app/services/queries/organisations.rb +++ b/app/services/queries/organisations.rb @@ -20,5 +20,17 @@ def self.search_by_keyword_and_category(parsed_params) end end + def self.xyz(organisations) + recently_updated = "organisations.updated_at > now() - interval '1 year'" + has_owner = "organisations.id IN (SELECT users.organisation_id FROM users)" + condition = + "CASE WHEN #{recently_updated} AND #{has_owner} THEN TRUE ELSE FALSE END" + x=organisations + .select("organisations.*, (#{condition}) as recently_updated_and_has_owner") + debugger + x + # .joins('LEFT OUTER JOIN users on users.organisation_id = organisations.id') + end + end end From 236f153c5a894c63ef22847296280e6fda725be5 Mon Sep 17 00:00:00 2001 From: johnnymo87 Date: Sat, 11 Apr 2015 20:12:34 +0000 Subject: [PATCH 03/21] applies new calculated column to building the org data --- app/controllers/organisations_controller.rb | 5 ++--- app/models/organisation.rb | 10 +++++----- app/services/map_marker_json.rb | 2 +- app/services/queries/organisations.rb | 5 +---- 4 files changed, 9 insertions(+), 13 deletions(-) diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index c28a7f74b..535426618 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -137,7 +137,6 @@ def self.build params private def build_map_markers(organisations) - Queries::Organisations.xyz(organisations) ::MapMarkerJson.build(organisations) do |org, marker| marker.lat org.latitude marker.lng org.longitude @@ -147,8 +146,8 @@ def build_map_markers(organisations) partial: 'shared/custom_marker', locals: { attrs: org.gmaps4rails_marker_attrs } ), - index: org.not_updated_recently_or_has_no_owner? ? -1 : 1, - type: org.not_updated_recently_or_has_no_owner? ? 'small_org' : 'large_org' + index: org.recently_updated_and_has_owner ? 1 : -1, + type: org.recently_updated_and_has_owner ? 'large_org' : 'small_org' ) end end diff --git a/app/models/organisation.rb b/app/models/organisation.rb index 7a6ab9d10..7688ff502 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -113,14 +113,14 @@ def self.contains_name?(key) end def gmaps4rails_marker_attrs - if not_updated_recently_or_has_no_owner? - ['https://maps.gstatic.com/intl/en_ALL/mapfiles/markers2/measle.png', - 'data-id' => id, - class: 'measle'] - else + if recently_updated_and_has_owner ['http://mt.googleapis.com/vt/icon/name=icons/spotlight/spotlight-poi.png', 'data-id' => id, class: 'marker'] + else + ['https://maps.gstatic.com/intl/en_ALL/mapfiles/markers2/measle.png', + 'data-id' => id, + class: 'measle'] end end diff --git a/app/services/map_marker_json.rb b/app/services/map_marker_json.rb index 2de808c22..53eeb1519 100644 --- a/app/services/map_marker_json.rb +++ b/app/services/map_marker_json.rb @@ -1,6 +1,6 @@ module MapMarkerJson def self.build(organisations) - Gmaps4rails.build_markers(organisations) do |org, marker| + Gmaps4rails.build_markers(Queries::Organisations.xyz(organisations)) do |org, marker| yield org, marker end.select do |marker| marker[:lat].present? && marker[:lng].present? diff --git a/app/services/queries/organisations.rb b/app/services/queries/organisations.rb index 3c343298a..4818e2618 100644 --- a/app/services/queries/organisations.rb +++ b/app/services/queries/organisations.rb @@ -25,11 +25,8 @@ def self.xyz(organisations) has_owner = "organisations.id IN (SELECT users.organisation_id FROM users)" condition = "CASE WHEN #{recently_updated} AND #{has_owner} THEN TRUE ELSE FALSE END" - x=organisations + organisations .select("organisations.*, (#{condition}) as recently_updated_and_has_owner") - debugger - x - # .joins('LEFT OUTER JOIN users on users.organisation_id = organisations.id') end end From 29285729a560bd0252555e3cc4cfdeb29e7f7340 Mon Sep 17 00:00:00 2001 From: johnnymo87 Date: Sat, 11 Apr 2015 20:31:00 +0000 Subject: [PATCH 04/21] left outer joins users table rather than querying it for organisation_ids each time --- app/services/queries/organisations.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/services/queries/organisations.rb b/app/services/queries/organisations.rb index 4818e2618..f75f1066c 100644 --- a/app/services/queries/organisations.rb +++ b/app/services/queries/organisations.rb @@ -22,10 +22,11 @@ def self.search_by_keyword_and_category(parsed_params) def self.xyz(organisations) recently_updated = "organisations.updated_at > now() - interval '1 year'" - has_owner = "organisations.id IN (SELECT users.organisation_id FROM users)" + has_owner = "organisations.id IN (users.organisation_id)" condition = "CASE WHEN #{recently_updated} AND #{has_owner} THEN TRUE ELSE FALSE END" organisations + .joins('LEFT OUTER JOIN users on users.organisation_id = organisations.id') .select("organisations.*, (#{condition}) as recently_updated_and_has_owner") end From 9e6d2af420bda862397f8ecc3860cf49584440df Mon Sep 17 00:00:00 2001 From: johnnymo87 Date: Sat, 11 Apr 2015 18:14:53 +0000 Subject: [PATCH 05/21] implements miniprofiler, with superadmin restriction commented out --- Gemfile | 2 ++ Gemfile.lock | 11 +++++++++++ app/controllers/application_controller.rb | 9 ++++++++- 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index 6f5a99f09..f72773cfc 100644 --- a/Gemfile +++ b/Gemfile @@ -88,3 +88,5 @@ gem 'url_validator' gem 'rails_autolink' gem "paranoia", "~> 2.0" +gem 'rack-mini-profiler' +gem 'flamegraph' diff --git a/Gemfile.lock b/Gemfile.lock index 1ad76e58b..97fbbe1c9 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -138,8 +138,13 @@ GEM factory_girl_rails (4.5.0) factory_girl (~> 4.5.0) railties (>= 3.0.0) + fast_stack (0.1.0) + rake + rake-compiler fattr (2.2.2) ffi (1.9.6) + flamegraph (0.1.0) + fast_stack flay (2.6.1) ruby_parser (~> 3.0) sexp_processor (~> 4.0) @@ -231,6 +236,8 @@ GEM phantomjs (1.9.8.0) procto (0.0.2) rack (1.6.0) + rack-mini-profiler (0.9.3) + rack (>= 1.1.3) rack-protection (1.5.3) rack rack-test (0.6.3) @@ -277,6 +284,8 @@ GEM rainbow (2.0.0) raindrops (0.13.0) rake (10.4.2) + rake-compiler (0.9.5) + rake redcard (1.1.0) redcarpet (3.2.2) reek (1.6.6) @@ -407,6 +416,7 @@ DEPENDENCIES devise_invitable (~> 1.3.0) execjs factory_girl_rails + flamegraph font-awesome-rails geocoder gmaps4rails (= 2.1.2) @@ -420,6 +430,7 @@ DEPENDENCIES newrelic_rpm paranoia (~> 2.0) pg + rack-mini-profiler rack_session_access railroady rails (~> 4.2.0) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index dc8ebd329..f9328583d 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -2,7 +2,10 @@ class ApplicationController < ActionController::Base protect_from_forgery - before_filter :store_location, :assign_footer_page_links + before_filter :store_location, + :assign_footer_page_links, + :miniprofiler + include CustomErrors # To prevent infinite redirect loops, only requests from white listed @@ -86,6 +89,10 @@ def superadmin? current_user.try :superadmin? end + def miniprofiler + # Rack::MiniProfiler.authorize_request if superadmin? + end + def assign_footer_page_links @footer_page_links = Page.visible_links end From 258998ee75e5942ad547fb090e1227493481b70c Mon Sep 17 00:00:00 2001 From: johnnymo87 Date: Sat, 11 Apr 2015 20:05:05 +0000 Subject: [PATCH 06/21] exposes recently_updated_and_has_owner via SQL --- app/controllers/organisations_controller.rb | 1 + app/services/queries/organisations.rb | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index 6648f2265..ad7534838 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -127,6 +127,7 @@ def self.build params private def build_map_markers(organisations) + Queries::Organisations.xyz(organisations) ::MapMarkerJson.build(organisations) do |org, marker| marker.lat org.latitude marker.lng org.longitude diff --git a/app/services/queries/organisations.rb b/app/services/queries/organisations.rb index 646fb0839..3c343298a 100644 --- a/app/services/queries/organisations.rb +++ b/app/services/queries/organisations.rb @@ -20,5 +20,17 @@ def self.search_by_keyword_and_category(parsed_params) end end + def self.xyz(organisations) + recently_updated = "organisations.updated_at > now() - interval '1 year'" + has_owner = "organisations.id IN (SELECT users.organisation_id FROM users)" + condition = + "CASE WHEN #{recently_updated} AND #{has_owner} THEN TRUE ELSE FALSE END" + x=organisations + .select("organisations.*, (#{condition}) as recently_updated_and_has_owner") + debugger + x + # .joins('LEFT OUTER JOIN users on users.organisation_id = organisations.id') + end + end end From 887269ca3765d04e730defe2cb81455b4e93728a Mon Sep 17 00:00:00 2001 From: johnnymo87 Date: Sat, 11 Apr 2015 20:12:34 +0000 Subject: [PATCH 07/21] applies new calculated column to building the org data --- app/controllers/organisations_controller.rb | 5 ++--- app/models/organisation.rb | 10 +++++----- app/services/map_marker_json.rb | 2 +- app/services/queries/organisations.rb | 5 +---- 4 files changed, 9 insertions(+), 13 deletions(-) diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index ad7534838..f7fbc9b54 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -127,7 +127,6 @@ def self.build params private def build_map_markers(organisations) - Queries::Organisations.xyz(organisations) ::MapMarkerJson.build(organisations) do |org, marker| marker.lat org.latitude marker.lng org.longitude @@ -137,8 +136,8 @@ def build_map_markers(organisations) partial: 'shared/custom_marker', locals: { attrs: org.gmaps4rails_marker_attrs } ), - index: org.not_updated_recently_or_has_no_owner? ? -1 : 1, - type: org.not_updated_recently_or_has_no_owner? ? 'small_org' : 'large_org' + index: org.recently_updated_and_has_owner ? 1 : -1, + type: org.recently_updated_and_has_owner ? 'large_org' : 'small_org' ) end end diff --git a/app/models/organisation.rb b/app/models/organisation.rb index 01709fe8a..67bbe7345 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -89,14 +89,14 @@ def self.filter_by_categories(category_ids) end def gmaps4rails_marker_attrs - if not_updated_recently_or_has_no_owner? - ['https://maps.gstatic.com/intl/en_ALL/mapfiles/markers2/measle.png', - 'data-id' => id, - class: 'measle'] - else + if recently_updated_and_has_owner ['http://mt.googleapis.com/vt/icon/name=icons/spotlight/spotlight-poi.png', 'data-id' => id, class: 'marker'] + else + ['https://maps.gstatic.com/intl/en_ALL/mapfiles/markers2/measle.png', + 'data-id' => id, + class: 'measle'] end end diff --git a/app/services/map_marker_json.rb b/app/services/map_marker_json.rb index 2de808c22..53eeb1519 100644 --- a/app/services/map_marker_json.rb +++ b/app/services/map_marker_json.rb @@ -1,6 +1,6 @@ module MapMarkerJson def self.build(organisations) - Gmaps4rails.build_markers(organisations) do |org, marker| + Gmaps4rails.build_markers(Queries::Organisations.xyz(organisations)) do |org, marker| yield org, marker end.select do |marker| marker[:lat].present? && marker[:lng].present? diff --git a/app/services/queries/organisations.rb b/app/services/queries/organisations.rb index 3c343298a..4818e2618 100644 --- a/app/services/queries/organisations.rb +++ b/app/services/queries/organisations.rb @@ -25,11 +25,8 @@ def self.xyz(organisations) has_owner = "organisations.id IN (SELECT users.organisation_id FROM users)" condition = "CASE WHEN #{recently_updated} AND #{has_owner} THEN TRUE ELSE FALSE END" - x=organisations + organisations .select("organisations.*, (#{condition}) as recently_updated_and_has_owner") - debugger - x - # .joins('LEFT OUTER JOIN users on users.organisation_id = organisations.id') end end From d6d650bbe8e34dd1e6337c3af0ff88b7302f856d Mon Sep 17 00:00:00 2001 From: johnnymo87 Date: Sat, 11 Apr 2015 20:31:00 +0000 Subject: [PATCH 08/21] left outer joins users table rather than querying it for organisation_ids each time --- app/services/queries/organisations.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/services/queries/organisations.rb b/app/services/queries/organisations.rb index 4818e2618..f75f1066c 100644 --- a/app/services/queries/organisations.rb +++ b/app/services/queries/organisations.rb @@ -22,10 +22,11 @@ def self.search_by_keyword_and_category(parsed_params) def self.xyz(organisations) recently_updated = "organisations.updated_at > now() - interval '1 year'" - has_owner = "organisations.id IN (SELECT users.organisation_id FROM users)" + has_owner = "organisations.id IN (users.organisation_id)" condition = "CASE WHEN #{recently_updated} AND #{has_owner} THEN TRUE ELSE FALSE END" organisations + .joins('LEFT OUTER JOIN users on users.organisation_id = organisations.id') .select("organisations.*, (#{condition}) as recently_updated_and_has_owner") end From 468c6ec70ed3304e45b36fd757f7b7310a10b2dd Mon Sep 17 00:00:00 2001 From: mtc2013 Date: Thu, 16 Apr 2015 14:09:51 -0400 Subject: [PATCH 09/21] Change org spec to use computed fields --- spec/models/organisation_spec.rb | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/spec/models/organisation_spec.rb b/spec/models/organisation_spec.rb index ee65e22e9..fee9d0f9f 100644 --- a/spec/models/organisation_spec.rb +++ b/spec/models/organisation_spec.rb @@ -59,9 +59,15 @@ end describe "#gmaps4rails_marker_attrs" do + + def build_org_with_computed_fields_and_updated_at org, updated_at = nil + org.update_attributes(updated_at: updated_at) unless updated_at.nil? + Queries::Organisations.xyz(Organisation.where(id: org.id)).first + end context 'no user' do it 'returns small icon when no associated user' do - expect(@org1.gmaps4rails_marker_attrs).to eq(["https://maps.gstatic.com/intl/en_ALL/mapfiles/markers2/measle.png", {"data-id"=>@org1.id, :class=>"measle"}]) + expect(build_org_with_computed_fields_and_updated_at(@org1).gmaps4rails_marker_attrs).to eq(["https://maps.gstatic.com/intl/en_ALL/mapfiles/markers2/measle.png", + {"data-id"=>@org1.id, :class=>"measle"}]) end end @@ -72,25 +78,23 @@ @org1.users << [usr] @org1.save! end - after(:each) do - allow(Time).to receive(:now).and_call_original - end it 'returns large icon when there is an associated user' do - expect(@org1.gmaps4rails_marker_attrs).to eq( ["http://mt.googleapis.com/vt/icon/name=icons/spotlight/spotlight-poi.png", {"data-id"=>@org1.id,:class=>"marker"}]) + expect(build_org_with_computed_fields_and_updated_at(@org1).gmaps4rails_marker_attrs).to eq( ["http://mt.googleapis.com/vt/icon/name=icons/spotlight/spotlight-poi.png", + {"data-id"=>@org1.id,:class=>"marker"}]) end [365, 366, 500].each do |days| it "returns small icon when update is #{days} days old" do - future_time = Time.at(Time.now + days.day) - allow(Time).to receive(:now){future_time} - expect(@org1.gmaps4rails_marker_attrs).to eq(["https://maps.gstatic.com/intl/en_ALL/mapfiles/markers2/measle.png", {"data-id"=>@org1.id, :class=>"measle"}]) + past_time = Time.at(Time.now - days.day) + expect(build_org_with_computed_fields_and_updated_at(@org1, past_time).gmaps4rails_marker_attrs).to eq(["https://maps.gstatic.com/intl/en_ALL/mapfiles/markers2/measle.png", + {"data-id"=>@org1.id, :class=>"measle"}]) end end [ 2, 100, 200, 364].each do |days| it "returns large icon when update is only #{days} days old" do - future_time = Time.at(Time.now + days.day) - allow(Time).to receive(:now){future_time} - expect(@org1.gmaps4rails_marker_attrs).to eq( ["http://mt.googleapis.com/vt/icon/name=icons/spotlight/spotlight-poi.png", {"data-id"=>@org1.id, :class=>"marker"} ]) + past_time = Time.at(Time.now - days.day) + expect(build_org_with_computed_fields_and_updated_at(@org1, past_time).gmaps4rails_marker_attrs).to eq( ["http://mt.googleapis.com/vt/icon/name=icons/spotlight/spotlight-poi.png", + {"data-id"=>@org1.id, :class=>"marker"} ]) end end end From 121b7e5685ca22e0cb67b097986490aedbf38483 Mon Sep 17 00:00:00 2001 From: johnnymo87 Date: Sun, 19 Apr 2015 16:43:52 +0000 Subject: [PATCH 10/21] adds generous 5 second pad to spec so that query has time to run --- spec/models/organisation_spec.rb | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/spec/models/organisation_spec.rb b/spec/models/organisation_spec.rb index fee9d0f9f..4cd8e6139 100644 --- a/spec/models/organisation_spec.rb +++ b/spec/models/organisation_spec.rb @@ -83,11 +83,18 @@ def build_org_with_computed_fields_and_updated_at org, updated_at = nil {"data-id"=>@org1.id,:class=>"marker"}]) end - [365, 366, 500].each do |days| + [ 365, 366, 500 ].each do |days| it "returns small icon when update is #{days} days old" do - past_time = Time.at(Time.now - days.day) - expect(build_org_with_computed_fields_and_updated_at(@org1, past_time).gmaps4rails_marker_attrs).to eq(["https://maps.gstatic.com/intl/en_ALL/mapfiles/markers2/measle.png", - {"data-id"=>@org1.id, :class=>"measle"}]) + # adds generous 5 second pad for query to run + past_time = Time.current.advance(days: -days).advance(seconds: -5) + expect( + build_org_with_computed_fields_and_updated_at( + @org1, past_time + ).gmaps4rails_marker_attrs + ).to eq([ + "https://maps.gstatic.com/intl/en_ALL/mapfiles/markers2/measle.png", + {"data-id"=>@org1.id, :class=>"measle"} + ]) end end [ 2, 100, 200, 364].each do |days| From b64bf960daa98bdcdd9c359ee57973c63ccda72d Mon Sep 17 00:00:00 2001 From: johnnymo87 Date: Sun, 19 Apr 2015 17:40:35 +0000 Subject: [PATCH 11/21] injects time into SQL from ruby --- app/services/queries/organisations.rb | 6 +++++- features/home_page/map.feature | 2 +- features/support/env.rb | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/services/queries/organisations.rb b/app/services/queries/organisations.rb index f75f1066c..b9e3b189c 100644 --- a/app/services/queries/organisations.rb +++ b/app/services/queries/organisations.rb @@ -20,8 +20,12 @@ def self.search_by_keyword_and_category(parsed_params) end end + FORMAT = '%Y-%m-%d %H:%M:%S.%N' + def self.xyz(organisations) - recently_updated = "organisations.updated_at > now() - interval '1 year'" + one_year_ago = Time.current.advance(years: -1) + recently_updated = "organisations.updated_at > '#{one_year_ago.strftime(FORMAT)}'" + # recently_updated = "organisations.updated_at > #{one_year_ago.strftime}" has_owner = "organisations.id IN (users.organisation_id)" condition = "CASE WHEN #{recently_updated} AND #{has_owner} THEN TRUE ELSE FALSE END" diff --git a/features/home_page/map.feature b/features/home_page/map.feature index 287b29448..bb77f4139 100644 --- a/features/home_page/map.feature +++ b/features/home_page/map.feature @@ -34,7 +34,7 @@ Feature: Map of local charities Given I travel "" days into the future And I visit the home page Then the organisation "Youth UK" should have a icon - Examples: + Examples: |days | size | | 2 | large| |100 | large| diff --git a/features/support/env.rb b/features/support/env.rb index 9da48836b..dacf9bf87 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -25,7 +25,7 @@ # prefer to use XPath just remove this line and adjust any selectors in your # steps to use the XPath syntax. Capybara.default_selector = :css -Capybara.default_wait_time = 20 +Capybara.default_wait_time = 3 Capybara.javascript_driver = :webkit From 32a57d69f93d38b6661a2622b6dc7abb87913e1b Mon Sep 17 00:00:00 2001 From: mtc2013 Date: Sun, 19 Apr 2015 14:17:28 -0400 Subject: [PATCH 12/21] Change to Timecop. Fix issues with leap year testing cases --- Gemfile | 2 +- Gemfile.lock | 5 ++--- features/home_page/map.feature | 15 +++++++-------- features/step_definitions/basic_steps.rb | 6 ++++-- features/support/env.rb | 4 ++-- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/Gemfile b/Gemfile index f72773cfc..38725d006 100644 --- a/Gemfile +++ b/Gemfile @@ -12,7 +12,7 @@ gem 'heroku-api' # for Heroku deployment - as described in Ap. A of ELLS book group :development, :test do - gem 'delorean' + gem 'timecop' gem 'metric_fu' gem 'database_cleaner', '1.0.1' gem 'launchy' diff --git a/Gemfile.lock b/Gemfile.lock index 97fbbe1c9..a46da065a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -115,8 +115,6 @@ GEM database_cleaner (1.0.1) debug_inspector (0.0.2) debugger-linecache (1.2.0) - delorean (2.1.0) - chronic devise (3.4.1) bcrypt (~> 3.0) orm_adapter (~> 0.1) @@ -361,6 +359,7 @@ GEM thor (0.19.1) thread_safe (0.3.4) tilt (1.4.1) + timecop (0.7.3) twitter-bootstrap-rails (2.2.8) actionpack (>= 3.1) execjs @@ -411,7 +410,6 @@ DEPENDENCIES cucumber-rails cucumber-rails-training-wheels database_cleaner (= 1.0.1) - delorean devise (~> 3.4.0) devise_invitable (~> 1.3.0) execjs @@ -444,6 +442,7 @@ DEPENDENCIES simplecov sinatra-base therubyracer + timecop twitter-bootstrap-rails (= 2.2.8) uglifier (= 2.5.3) underscore-rails diff --git a/features/home_page/map.feature b/features/home_page/map.feature index bb77f4139..c1d4b5a77 100644 --- a/features/home_page/map.feature +++ b/features/home_page/map.feature @@ -31,18 +31,17 @@ Feature: Map of local charities @time_travel @javascript Scenario Outline: Organisation map has small icon for organisations updated more than 365 days ago - Given I travel "" days into the future + Given I travel a year plus "" days into the future And I visit the home page Then the organisation "Youth UK" should have a icon Examples: |days | size | - | 2 | large| - |100 | large| - |200 | large| - |364 | large| - |365 | small| - |366 | small| - |500 | small| + | -10 | large| + | -1 | large| + | 0 | small| + | 1 | small| + | 10 | small| + |100 | small| @javascript Scenario: Organisation map has small icon for organisation with no users diff --git a/features/step_definitions/basic_steps.rb b/features/step_definitions/basic_steps.rb index d435b3b9c..fd9bb4e5f 100644 --- a/features/step_definitions/basic_steps.rb +++ b/features/step_definitions/basic_steps.rb @@ -2,8 +2,10 @@ require 'uri-handler' include ApplicationHelper -Then(/^I travel "(.*?)" days into the future$/) do |days| - Delorean.time_travel_to "#{days} days from now" +Then(/^I travel a year plus "(.*?)" days into the future$/) do |days| + Timecop.travel( + Time.current.advance(years: 1, days: days.to_i) + ) end Then(/^I should see the "(.*?)" image linked to "(.*?)"$/) do |image_alt, link| within("a[href='#{link}']") do diff --git a/features/support/env.rb b/features/support/env.rb index dacf9bf87..ebf872001 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -13,7 +13,7 @@ require 'rack_session_access/capybara' require 'factory_girl_rails' require 'aruba/cucumber' -require 'delorean' +require 'timecop' Dir['../../spec/factories/*.rb'].each {|file| require_relative file } # https://github.com/jnicklas/capybara/commit/4f805d5a1c42da29ed32ab0371e24add2dc08af1 @@ -94,5 +94,5 @@ end After('@time_travel') do - Delorean.back_to_1985 + Timecop.return end From c1723e8c40a5edb4756e8a3bbc98549924db4899 Mon Sep 17 00:00:00 2001 From: mtc2013 Date: Sun, 19 Apr 2015 17:38:14 -0400 Subject: [PATCH 13/21] Make org controller deal with relation and accompanying spec --- app/controllers/organisations_controller.rb | 5 +++-- spec/controllers/organisations_controller_spec.rb | 9 +++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index f7fbc9b54..e15d44c04 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -27,7 +27,8 @@ def index # GET /organisations/1 # GET /organisations/1.json def show - @organisation = Organisation.find(params[:id]) + org_relation = Organisation.where(id: params[:id]) + @organisation = org_relation.first @pending_org_admin = current_user.pending_org_admin? @organisation if current_user @editable = current_user.can_edit?(@organisation) if current_user @deletable = current_user.can_delete?(@organisation) if current_user @@ -35,7 +36,7 @@ def show @grabbable = current_user ? current_user.can_request_org_admin?(@organisation) : true @can_propose_edits = current_user.present? && !@editable # @next_path = current_user ? organisation_user_path(@organisation.id, current_user.id) : new_user_session_path - @markers = build_map_markers([@organisation]) + @markers = build_map_markers(org_relation) end # GET /organisations/new diff --git a/spec/controllers/organisations_controller_spec.rb b/spec/controllers/organisations_controller_spec.rb index d64b8abf4..c7648e27f 100644 --- a/spec/controllers/organisations_controller_spec.rb +++ b/spec/controllers/organisations_controller_spec.rb @@ -15,8 +15,9 @@ def double_organisation(stubs={}) describe "#build_map_markers" do render_views - let(:org) { create :organisation } - subject { JSON.parse(controller.send(:build_map_markers, org)).first } + let!(:org) { create :organisation } + let(:org_relation){Organisation.all} + subject { JSON.parse(controller.send(:build_map_markers, org_relation)).first } it { expect(subject['lat']).to eq org.latitude } it { expect(subject['lng']).to eq org.longitude } it { expect(subject['infowindow']).to include org.id.to_s } @@ -24,7 +25,7 @@ def double_organisation(stubs={}) it { expect(subject['infowindow']).to include org.description } context 'markers without coords omitted' do let!(:org) { create :organisation, address: '150 pinner rd', latitude: nil, longitude: nil } - it { expect(JSON.parse(controller.send(:build_map_markers, org))).to be_empty } + it { expect(JSON.parse(controller.send(:build_map_markers, org_relation))).to be_empty } end end @@ -138,7 +139,7 @@ def double_organisation(stubs={}) before(:each) do @user = double("User") allow(@user).to receive(:pending_org_admin?) - allow(Organisation).to receive(:find).with('37') { real_org} + allow(Organisation).to receive(:where).with(id: '37') { Organisation.where(id: '37')} allow(@user).to receive(:can_edit?) allow(@user).to receive(:can_delete?) allow(@user).to receive(:can_create_volunteer_ops?) From c32409688b32761b8b35e099b861f8955453eac3 Mon Sep 17 00:00:00 2001 From: mtc2013 Date: Sun, 19 Apr 2015 17:45:33 -0400 Subject: [PATCH 14/21] Make controller use real query and real id in spec --- spec/controllers/organisations_controller_spec.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/spec/controllers/organisations_controller_spec.rb b/spec/controllers/organisations_controller_spec.rb index c7648e27f..4fa9f8fa0 100644 --- a/spec/controllers/organisations_controller_spec.rb +++ b/spec/controllers/organisations_controller_spec.rb @@ -139,7 +139,6 @@ def double_organisation(stubs={}) before(:each) do @user = double("User") allow(@user).to receive(:pending_org_admin?) - allow(Organisation).to receive(:where).with(id: '37') { Organisation.where(id: '37')} allow(@user).to receive(:can_edit?) allow(@user).to receive(:can_delete?) allow(@user).to receive(:can_create_volunteer_ops?) @@ -148,7 +147,7 @@ def double_organisation(stubs={}) end it 'should use a two_column layout' do - get :show, :id => '37' + get :show, :id => real_org.id.to_s expect(response).to render_template 'layouts/two_columns' end @@ -157,7 +156,7 @@ def double_organisation(stubs={}) @org = real_org expect(controller).to receive(:build_map_markers).and_return(markers) expect(Organisation).to receive(:find).with('37') { @org } - get :show, :id => '37' + get :show, :id => real_org.id.to_s expect(assigns(:organisation)).to be(real_org) expect(assigns(:markers)).to eq(markers) end @@ -165,19 +164,19 @@ def double_organisation(stubs={}) context "editable flag is assigned to match user permission" do it "user with permission leads to editable flag true" do expect(@user).to receive(:can_edit?).with(real_org).and_return(true) - get :show, :id => 37 + get :show, id: real_org.id.to_s expect(assigns(:editable)).to be(true) end it "user without permission leads to editable flag false" do expect(@user).to receive(:can_edit?).with(real_org).and_return(true) - get :show, :id => 37 + get :show, id: real_org.id.to_s expect(assigns(:editable)).to be(true) end it 'when not signed in editable flag is nil' do allow(controller).to receive(:current_user).and_return(nil) - get :show, :id => 37 + get :show, id: real_org.id.to_s expect(assigns(:editable)).to be_nil end end From d715c9bd930e9229d01a5e06a32e7ac4a6843525 Mon Sep 17 00:00:00 2001 From: mtc2013 Date: Sun, 19 Apr 2015 17:50:39 -0400 Subject: [PATCH 15/21] Make show method use real query in spec --- spec/controllers/organisations_controller_spec.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/spec/controllers/organisations_controller_spec.rb b/spec/controllers/organisations_controller_spec.rb index 4fa9f8fa0..dbfab907a 100644 --- a/spec/controllers/organisations_controller_spec.rb +++ b/spec/controllers/organisations_controller_spec.rb @@ -155,9 +155,8 @@ def double_organisation(stubs={}) markers='my markers' @org = real_org expect(controller).to receive(:build_map_markers).and_return(markers) - expect(Organisation).to receive(:find).with('37') { @org } get :show, :id => real_org.id.to_s - expect(assigns(:organisation)).to be(real_org) + expect(assigns(:organisation)).to eq(real_org) expect(assigns(:markers)).to eq(markers) end @@ -186,19 +185,19 @@ def double_organisation(stubs={}) allow(@user).to receive(:can_edit?) expect(@user).to receive(:can_request_org_admin?).with(real_org).and_return(true) allow(controller).to receive(:current_user).and_return(@user) - get :show, :id => 37 + get :show, :id => real_org.id.to_s expect(assigns(:grabbable)).to be(true) end it 'assigns grabbable to false when user cannot request org superadmin status' do allow(@user).to receive(:can_edit?) expect(@user).to receive(:can_request_org_admin?).with(real_org).and_return(false) allow(controller).to receive(:current_user).and_return(@user) - get :show, :id => 37 + get :show, :id => real_org.id.to_s expect(assigns(:grabbable)).to be(false) end it 'when not signed in grabbable flag is true' do allow(controller).to receive(:current_user).and_return(nil) - get :show, :id => 37 + get :show, :id => real_org.id.to_s expect(assigns(:grabbable)).to be true end end From 8ebd0c053f1a211b106ed72a1b2a6e49a99512a8 Mon Sep 17 00:00:00 2001 From: mtc2013 Date: Sun, 19 Apr 2015 17:57:56 -0400 Subject: [PATCH 16/21] Use where query in #edit --- app/controllers/organisations_controller.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index e15d44c04..a26f75bba 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -48,8 +48,9 @@ def new # GET /organisations/1/edit def edit - @organisation = Organisation.find(params[:id]) - @markers = build_map_markers([@organisation]) + org_relation = Organisation.where(id: params[:id]) + @organisation = org_relation.first + @markers = build_map_markers(org_relation) @categories_start_with = Category.first_category_name_in_each_type return false unless user_can_edit? @organisation #respond_to do |format| From d03337ed8ab624c694167dd006d2df37303de40b Mon Sep 17 00:00:00 2001 From: johnnymo87 Date: Mon, 20 Apr 2015 00:15:44 +0000 Subject: [PATCH 17/21] replaces join on users table with subselect in case statement --- app/services/queries/organisations.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/services/queries/organisations.rb b/app/services/queries/organisations.rb index b9e3b189c..2c6d32309 100644 --- a/app/services/queries/organisations.rb +++ b/app/services/queries/organisations.rb @@ -26,11 +26,10 @@ def self.xyz(organisations) one_year_ago = Time.current.advance(years: -1) recently_updated = "organisations.updated_at > '#{one_year_ago.strftime(FORMAT)}'" # recently_updated = "organisations.updated_at > #{one_year_ago.strftime}" - has_owner = "organisations.id IN (users.organisation_id)" + has_owner = "organisations.id IN (SELECT users.organisation_id FROM users)" condition = "CASE WHEN #{recently_updated} AND #{has_owner} THEN TRUE ELSE FALSE END" organisations - .joins('LEFT OUTER JOIN users on users.organisation_id = organisations.id') .select("organisations.*, (#{condition}) as recently_updated_and_has_owner") end From 8f9cf4f73f66f7aaca249562512d57e246e04d25 Mon Sep 17 00:00:00 2001 From: mtc2013 Date: Mon, 20 Apr 2015 18:32:17 -0400 Subject: [PATCH 18/21] Make service method deal with array of orgs by individuating each org, ie instance_eval, since the number is likely to be small Adjust controller back to old way Adjust specs to deal with this change Adjust feature for charity worker reminder to use new step for year with offset Adjust updated_recently method to use year instead of 365 days --- app/controllers/organisations_controller.rb | 10 ++++------ app/models/organisation.rb | 2 +- app/services/queries/organisations.rb | 10 ++++++++++ .../remind_charity_admin_to_update_page.feature | 4 ++-- spec/controllers/organisations_controller_spec.rb | 6 +++--- spec/controllers/volunteer_ops_controller_spec.rb | 4 ++-- 6 files changed, 22 insertions(+), 14 deletions(-) diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index a26f75bba..f7fbc9b54 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -27,8 +27,7 @@ def index # GET /organisations/1 # GET /organisations/1.json def show - org_relation = Organisation.where(id: params[:id]) - @organisation = org_relation.first + @organisation = Organisation.find(params[:id]) @pending_org_admin = current_user.pending_org_admin? @organisation if current_user @editable = current_user.can_edit?(@organisation) if current_user @deletable = current_user.can_delete?(@organisation) if current_user @@ -36,7 +35,7 @@ def show @grabbable = current_user ? current_user.can_request_org_admin?(@organisation) : true @can_propose_edits = current_user.present? && !@editable # @next_path = current_user ? organisation_user_path(@organisation.id, current_user.id) : new_user_session_path - @markers = build_map_markers(org_relation) + @markers = build_map_markers([@organisation]) end # GET /organisations/new @@ -48,9 +47,8 @@ def new # GET /organisations/1/edit def edit - org_relation = Organisation.where(id: params[:id]) - @organisation = org_relation.first - @markers = build_map_markers(org_relation) + @organisation = Organisation.find(params[:id]) + @markers = build_map_markers([@organisation]) @categories_start_with = Category.first_category_name_in_each_type return false unless user_can_edit? @organisation #respond_to do |format| diff --git a/app/models/organisation.rb b/app/models/organisation.rb index 67bbe7345..8c22b4a21 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -35,7 +35,7 @@ class Organisation < ActiveRecord::Base after_save :uninvite_users, if: ->{ email_changed? } def not_updated_recently? - updated_at < 365.day.ago + updated_at < 1.year.ago end def uninvite_users diff --git a/app/services/queries/organisations.rb b/app/services/queries/organisations.rb index 2c6d32309..09cca46b8 100644 --- a/app/services/queries/organisations.rb +++ b/app/services/queries/organisations.rb @@ -23,6 +23,16 @@ def self.search_by_keyword_and_category(parsed_params) FORMAT = '%Y-%m-%d %H:%M:%S.%N' def self.xyz(organisations) + if organisations.method(:select).arity == 0 + organisations.each do |o| + o.instance_eval %{ + def recently_updated_and_has_owner + !not_updated_recently? && !self.users.empty? + end + } + end + return organisations + end one_year_ago = Time.current.advance(years: -1) recently_updated = "organisations.updated_at > '#{one_year_ago.strftime(FORMAT)}'" # recently_updated = "organisations.updated_at > #{one_year_ago.strftime}" diff --git a/features/individual_charity_pages/remind_charity_admin_to_update_page.feature b/features/individual_charity_pages/remind_charity_admin_to_update_page.feature index 722970565..4a6e9b9bb 100644 --- a/features/individual_charity_pages/remind_charity_admin_to_update_page.feature +++ b/features/individual_charity_pages/remind_charity_admin_to_update_page.feature @@ -17,7 +17,7 @@ Feature: Charity worker is reminded annually to edit own charity profile @javascript @time_travel Scenario: Org owner is reminded to update details after a year - Given I travel "365" days into the future + Given I travel a year plus "0" days into the future And I visit the home page And I click "Login" When I sign in as "superadmin@friendly.org" with password "pppppppp" with javascript @@ -25,7 +25,7 @@ Feature: Charity worker is reminded annually to edit own charity profile @javascript @time_travel Scenario: Org owner is not reminded to update details prior to a year - Given I travel "364" days into the future + Given I travel a year plus "-1" days into the future And I visit the home page And I click "Login" When I sign in as "superadmin@friendly.org" with password "pppppppp" with javascript diff --git a/spec/controllers/organisations_controller_spec.rb b/spec/controllers/organisations_controller_spec.rb index dbfab907a..5aa3511d8 100644 --- a/spec/controllers/organisations_controller_spec.rb +++ b/spec/controllers/organisations_controller_spec.rb @@ -206,13 +206,13 @@ def double_organisation(stubs={}) context 'depends on can_create_volunteer_ops?' do it 'true' do expect(@user).to receive(:can_create_volunteer_ops?) { true } - get :show, :id => 37 + get :show, :id => real_org.id.to_s expect(assigns(:can_create_volunteer_op)).to be true end it 'false' do expect(@user).to receive(:can_create_volunteer_ops?) { false } - get :show, :id => 37 + get :show, :id => real_org.id.to_s expect(assigns(:can_create_volunteer_op)).to be false end end @@ -220,7 +220,7 @@ def double_organisation(stubs={}) it 'will not be called when current user is nil' do allow(controller).to receive_messages current_user: nil expect(@user).not_to receive :can_create_volunteer_ops? - get :show, :id => 37 + get :show, :id => real_org.id.to_s expect(assigns(:can_create_volunteer_op)).to be nil end end diff --git a/spec/controllers/volunteer_ops_controller_spec.rb b/spec/controllers/volunteer_ops_controller_spec.rb index d0847a759..3b9112b25 100644 --- a/spec/controllers/volunteer_ops_controller_spec.rb +++ b/spec/controllers/volunteer_ops_controller_spec.rb @@ -29,7 +29,7 @@ render_views let(:org) { create :organisation } let!(:op) { create :volunteer_op, organisation: org } - subject { JSON.parse(controller.send(:build_map_markers, org)).first } + subject { JSON.parse(controller.send(:build_map_markers, [org])).first } it { expect(subject['lat']).to eq org.latitude } it { expect(subject['lng']).to eq org.longitude } it { expect(subject['infowindow']).to include org.id.to_s } @@ -39,7 +39,7 @@ it { expect(subject['infowindow']).to include op.description } context 'markers without coords omitted' do let(:org) { create :organisation, address: "0 pinnner road", latitude: nil, longitude: nil } - it { expect(JSON.parse(controller.send(:build_map_markers, org))).to be_empty } + it { expect(JSON.parse(controller.send(:build_map_markers, [org]))).to be_empty } end end From ca0bc96538436983c88645b8e5c7161f6622cb18 Mon Sep 17 00:00:00 2001 From: mtc2013 Date: Tue, 21 Apr 2015 13:15:24 -0400 Subject: [PATCH 19/21] Change method from xyz to add_recently_updated_and_has_owner Change specs to accomodate name change Add specs to test both scope and array arguments to add_recently_updated_and_has_owner --- app/services/map_marker_json.rb | 2 +- app/services/queries/organisations.rb | 2 +- spec/models/organisation_spec.rb | 2 +- spec/services/queries/organisations_spec.rb | 30 +++++++++++++++++++++ 4 files changed, 33 insertions(+), 3 deletions(-) diff --git a/app/services/map_marker_json.rb b/app/services/map_marker_json.rb index 53eeb1519..be980308a 100644 --- a/app/services/map_marker_json.rb +++ b/app/services/map_marker_json.rb @@ -1,6 +1,6 @@ module MapMarkerJson def self.build(organisations) - Gmaps4rails.build_markers(Queries::Organisations.xyz(organisations)) do |org, marker| + Gmaps4rails.build_markers(Queries::Organisations.add_recently_updated_and_has_owner(organisations)) do |org, marker| yield org, marker end.select do |marker| marker[:lat].present? && marker[:lng].present? diff --git a/app/services/queries/organisations.rb b/app/services/queries/organisations.rb index 09cca46b8..642eca8d5 100644 --- a/app/services/queries/organisations.rb +++ b/app/services/queries/organisations.rb @@ -22,7 +22,7 @@ def self.search_by_keyword_and_category(parsed_params) FORMAT = '%Y-%m-%d %H:%M:%S.%N' - def self.xyz(organisations) + def self.add_recently_updated_and_has_owner(organisations) if organisations.method(:select).arity == 0 organisations.each do |o| o.instance_eval %{ diff --git a/spec/models/organisation_spec.rb b/spec/models/organisation_spec.rb index 4cd8e6139..98ef8e4c0 100644 --- a/spec/models/organisation_spec.rb +++ b/spec/models/organisation_spec.rb @@ -62,7 +62,7 @@ def build_org_with_computed_fields_and_updated_at org, updated_at = nil org.update_attributes(updated_at: updated_at) unless updated_at.nil? - Queries::Organisations.xyz(Organisation.where(id: org.id)).first + Queries::Organisations.add_recently_updated_and_has_owner(Organisation.where(id: org.id)).first end context 'no user' do it 'returns small icon when no associated user' do diff --git a/spec/services/queries/organisations_spec.rb b/spec/services/queries/organisations_spec.rb index 82b791c5c..38c466ba8 100644 --- a/spec/services/queries/organisations_spec.rb +++ b/spec/services/queries/organisations_spec.rb @@ -6,6 +6,8 @@ # not initialized! let(:category3) { create(:category, :charity_commission_id => 307) } + + let!(:org1) do create( :organisation, @@ -123,4 +125,32 @@ def run_spec it { expect(run_spec).to include org1, org2, org3 } end + context '#add_computed_recently_updated_and_has_owner' do + let(:user1){create(:user)} + let(:user2){create(:user, email: "blah@blah.org")} + before{org1.update_attributes(updated_at: 2.year.ago); org1.users << user1; org2.users << user2; } + context 'computes correctly when array passed in' do + it do + orgs = described_class.add_recently_updated_and_has_owner([org1, org2, org3]) + expect(orgs[0].recently_updated_and_has_owner).to be false + expect(orgs[1].recently_updated_and_has_owner).to be true + expect(orgs[2].recently_updated_and_has_owner).to be false + end + end + context 'computes correctly when scope passed in' do + it do + orgs = described_class.add_recently_updated_and_has_owner(Organisation.all) + orgs.each do |o| + case o.name + when org1.name + expect(o.recently_updated_and_has_owner).to be false + when org2.name + expect(o.recently_updated_and_has_owner).to be true + when org3.name + expect(o.recently_updated_and_has_owner).to be false + end + end + end + end + end end From 464d21b6a5d3cf2e151b2d565f9eecb186b5fe76 Mon Sep 17 00:00:00 2001 From: mtc2013 Date: Fri, 1 May 2015 15:10:36 -0400 Subject: [PATCH 20/21] Refactor individuation into private method; remove miniprofiler --- Gemfile | 4 +--- Gemfile.lock | 11 ----------- app/controllers/application_controller.rb | 7 +------ app/services/queries/organisations.rb | 20 ++++++++++++-------- 4 files changed, 14 insertions(+), 28 deletions(-) diff --git a/Gemfile b/Gemfile index 38725d006..8a21b9fbe 100644 --- a/Gemfile +++ b/Gemfile @@ -87,6 +87,4 @@ gem 'url_validator' gem 'rails_autolink' -gem "paranoia", "~> 2.0" -gem 'rack-mini-profiler' -gem 'flamegraph' +gem "paranoia", "~> 2.0" \ No newline at end of file diff --git a/Gemfile.lock b/Gemfile.lock index a46da065a..b2be99962 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -136,13 +136,8 @@ GEM factory_girl_rails (4.5.0) factory_girl (~> 4.5.0) railties (>= 3.0.0) - fast_stack (0.1.0) - rake - rake-compiler fattr (2.2.2) ffi (1.9.6) - flamegraph (0.1.0) - fast_stack flay (2.6.1) ruby_parser (~> 3.0) sexp_processor (~> 4.0) @@ -234,8 +229,6 @@ GEM phantomjs (1.9.8.0) procto (0.0.2) rack (1.6.0) - rack-mini-profiler (0.9.3) - rack (>= 1.1.3) rack-protection (1.5.3) rack rack-test (0.6.3) @@ -282,8 +275,6 @@ GEM rainbow (2.0.0) raindrops (0.13.0) rake (10.4.2) - rake-compiler (0.9.5) - rake redcard (1.1.0) redcarpet (3.2.2) reek (1.6.6) @@ -414,7 +405,6 @@ DEPENDENCIES devise_invitable (~> 1.3.0) execjs factory_girl_rails - flamegraph font-awesome-rails geocoder gmaps4rails (= 2.1.2) @@ -428,7 +418,6 @@ DEPENDENCIES newrelic_rpm paranoia (~> 2.0) pg - rack-mini-profiler rack_session_access railroady rails (~> 4.2.0) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index f9328583d..cc69be589 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -3,8 +3,7 @@ class ApplicationController < ActionController::Base protect_from_forgery before_filter :store_location, - :assign_footer_page_links, - :miniprofiler + :assign_footer_page_links include CustomErrors @@ -89,10 +88,6 @@ def superadmin? current_user.try :superadmin? end - def miniprofiler - # Rack::MiniProfiler.authorize_request if superadmin? - end - def assign_footer_page_links @footer_page_links = Page.visible_links end diff --git a/app/services/queries/organisations.rb b/app/services/queries/organisations.rb index 642eca8d5..957c4db38 100644 --- a/app/services/queries/organisations.rb +++ b/app/services/queries/organisations.rb @@ -24,14 +24,7 @@ def self.search_by_keyword_and_category(parsed_params) def self.add_recently_updated_and_has_owner(organisations) if organisations.method(:select).arity == 0 - organisations.each do |o| - o.instance_eval %{ - def recently_updated_and_has_owner - !not_updated_recently? && !self.users.empty? - end - } - end - return organisations + return add_recently_update_and_has_owner_to_enumerable(organisations) end one_year_ago = Time.current.advance(years: -1) recently_updated = "organisations.updated_at > '#{one_year_ago.strftime(FORMAT)}'" @@ -43,5 +36,16 @@ def recently_updated_and_has_owner .select("organisations.*, (#{condition}) as recently_updated_and_has_owner") end + private + def self.add_recently_update_and_has_owner_to_enumerable(organisations) + organisations.each do |o| + o.instance_eval %{ + def recently_updated_and_has_owner + !not_updated_recently? && !self.users.empty? + end + } + return organisations + end + end end end From 4bba4cc1533b031b4f28d451d8f0d3908d58a1c9 Mon Sep 17 00:00:00 2001 From: mtc2013 Date: Fri, 1 May 2015 15:54:24 -0400 Subject: [PATCH 21/21] Correct return statement after block, not in the middle of --- app/services/queries/organisations.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/queries/organisations.rb b/app/services/queries/organisations.rb index 957c4db38..7f82623d9 100644 --- a/app/services/queries/organisations.rb +++ b/app/services/queries/organisations.rb @@ -44,8 +44,8 @@ def recently_updated_and_has_owner !not_updated_recently? && !self.users.empty? end } - return organisations end + return organisations end end end