From 2c68bb0305a64ad6f23d87aff8d2792be5c9c8cf Mon Sep 17 00:00:00 2001 From: Raphael Date: Wed, 19 Jan 2011 18:06:35 -0800 Subject: [PATCH] Remove n-query problem from contact_list --- app/controllers/aspects_controller.rb | 6 ++++-- app/helpers/aspects_helper.rb | 8 ++++---- app/views/aspects/edit.html.haml | 2 +- app/views/layouts/_header.html.haml | 2 +- app/views/people/_share_with_pane.html.haml | 4 ++-- .../requests/_manage_aspect_contacts.haml | 2 +- app/views/shared/_contact_list.html.haml | 2 +- spec/controllers/aspects_controller_spec.rb | 19 +++++++++++++++++-- 8 files changed, 31 insertions(+), 14 deletions(-) diff --git a/app/controllers/aspects_controller.rb b/app/controllers/aspects_controller.rb index 75d579bfb..597891c76 100644 --- a/app/controllers/aspects_controller.rb +++ b/app/controllers/aspects_controller.rb @@ -78,22 +78,24 @@ class AspectsController < ApplicationController end def edit - @aspect = current_user.aspects.where(:id => params[:id]).first + @aspect = current_user.aspects.where(:id => params[:id]).includes(:contacts => {:person => :profile}).first @contacts = current_user.contacts.includes(:person => :profile).where(:pending => false) unless @aspect render :file => "#{Rails.root}/public/404.html", :layout => false, :status => 404 else @aspect_ids = [@aspect.id] - @aspect_contacts_count = @aspect.contacts.count + @aspect_contacts_count = @aspect.contacts.length render :layout => false end end def manage + Rails.logger.info("Controller time") @aspect = :manage @contacts = current_user.contacts.includes(:person => :profile).where(:pending => false) @remote_requests = Request.where(:recipient_id => current_user.person.id).includes(:sender => :profile) @aspects = @all_aspects.includes(:contacts => {:person => :profile}) + Rails.logger.info("VIEW TIME!!!!!!") end def update diff --git a/app/helpers/aspects_helper.rb b/app/helpers/aspects_helper.rb index d99912fa6..7deffb791 100644 --- a/app/helpers/aspects_helper.rb +++ b/app/helpers/aspects_helper.rb @@ -38,11 +38,11 @@ module AspectsHelper :class => 'added button' end - def aspect_membership_button(aspect_id, contact, person) - if contact.nil? || !contact.aspect_ids.include?(aspect_id) - add_to_aspect_button(aspect_id, person.id) + def aspect_membership_button(aspect, contact, person) + if contact.nil? || !aspect.contacts.include?(contact) + add_to_aspect_button(aspect.id, person.id) else - remove_from_aspect_button(aspect_id, person.id) + remove_from_aspect_button(aspect.id, person.id) end end diff --git a/app/views/aspects/edit.html.haml b/app/views/aspects/edit.html.haml index d12d91c4a..bb63cf99a 100644 --- a/app/views/aspects/edit.html.haml +++ b/app/views/aspects/edit.html.haml @@ -26,7 +26,7 @@ = button_to t('.remove_aspect'), @aspect, :method => "delete", :confirm => t('.confirm_remove_aspect'), :class => 'button' - if @contacts.count > 0 - = render 'shared/contact_list', :aspect_id => @aspect.id, :contacts => @contacts + = render 'shared/contact_list', :aspect => @aspect, :contacts => @contacts #aspect_edit_controls = link_to t('.rename'), '#' diff --git a/app/views/layouts/_header.html.haml b/app/views/layouts/_header.html.haml index b8903cdb5..62ee1c754 100644 --- a/app/views/layouts/_header.html.haml +++ b/app/views/layouts/_header.html.haml @@ -46,7 +46,7 @@ - for aspect in @all_aspects %li{:data=>{:guid=>aspect.id}, :class => ("selected" if @object_aspect_ids.include?(aspect.id))} - = link_for_aspect(aspect, :class => 'aspect_selector', :title => "#{aspect.contacts.count} contacts") + = link_for_aspect(aspect, :class => 'aspect_selector', :title => "#{aspect.aspect_memberships.length} contacts") %li = link_to '+', '#add_aspect_pane', :class => "add_aspect_button", :title => t('aspects.manage.add_a_new_aspect'), :rel => 'facebox' diff --git a/app/views/people/_share_with_pane.html.haml b/app/views/people/_share_with_pane.html.haml index 01885e4a6..74c5ced8b 100644 --- a/app/views/people/_share_with_pane.html.haml +++ b/app/views/people/_share_with_pane.html.haml @@ -9,12 +9,12 @@ %span.name = link_to aspect.name, aspect .right - = aspect_membership_button(aspect.id, contact, person) + = aspect_membership_button(aspect, contact, person) - for aspect in aspects_without_person %li{:data=>{:guid=>aspect.id}} %span.name = link_to aspect.name, aspect .right - = aspect_membership_button(aspect.id, contact, person) + = aspect_membership_button(aspect, contact, person) diff --git a/app/views/requests/_manage_aspect_contacts.haml b/app/views/requests/_manage_aspect_contacts.haml index b92145e16..5edeb5a05 100644 --- a/app/views/requests/_manage_aspect_contacts.haml +++ b/app/views/requests/_manage_aspect_contacts.haml @@ -9,7 +9,7 @@ %i= aspect.name .span-6.append-1.last %h3= t('.existing') - = render 'shared/contact_list', :aspect_id => aspect.id, :contacts => contacts, :manage => defined?(manage) + = render 'shared/contact_list', :aspect => aspect, :contacts => contacts, :manage => defined?(manage) .span-7.last = render 'shared/add_contact', :aspect_id => aspect.id diff --git a/app/views/shared/_contact_list.html.haml b/app/views/shared/_contact_list.html.haml index 4cc16027c..a0d6a1f3e 100644 --- a/app/views/shared/_contact_list.html.haml +++ b/app/views/shared/_contact_list.html.haml @@ -14,5 +14,5 @@ .description = contact.person.diaspora_handle .right - = aspect_membership_button(aspect_id, contact, contact.person) + = aspect_membership_button(aspect, contact, contact.person) diff --git a/spec/controllers/aspects_controller_spec.rb b/spec/controllers/aspects_controller_spec.rb index 19bcae232..1ddea2459 100644 --- a/spec/controllers/aspects_controller_spec.rb +++ b/spec/controllers/aspects_controller_spec.rb @@ -88,14 +88,16 @@ describe AspectsController do connect_users(@user, @aspect0, user, aspect) post = @user.post(:status_message, :message => "hello#{n}", :to => @aspect1.id) @posts << post - user.comment "yo#{post.message}", :on => post + 8.times do |n| + user.comment "yo#{post.message}", :on => post + end end end it 'takes time' do Benchmark.realtime{ get :index - }.should < 1.5 + }.should < 0.8 end end end @@ -141,6 +143,19 @@ describe AspectsController do get :manage response.should be_success end + it "performs reasonably" do + require 'benchmark' + 8.times do |n| + aspect = @user.aspects.create(:name => "aspect#{n}") + 8.times do |o| + person = Factory(:person) + @user.activate_contact(person, aspect) + end + end + Benchmark.realtime{ + get :manage + }.should < 2 + end it "assigns aspect to manage" do get :manage assigns(:aspect).should == :manage