From a96253929e817e48366a4d48d7e0a8b988d774e2 Mon Sep 17 00:00:00 2001 From: Sergey Nartimov Date: Sun, 28 Oct 2012 13:40:39 +0300 Subject: [PATCH] use methods with memoization instead of temporary instance vars --- app/controllers/apps_controller.rb | 13 +++---------- app/models/app.rb | 8 ++++++++ app/views/apps/index.html.haml | 4 ++-- spec/controllers/apps_controller_spec.rb | 11 ----------- spec/views/apps/index.html.haml_spec.rb | 2 -- 5 files changed, 13 insertions(+), 25 deletions(-) diff --git a/app/controllers/apps_controller.rb b/app/controllers/apps_controller.rb index 0fcc9f1..93ae780 100644 --- a/app/controllers/apps_controller.rb +++ b/app/controllers/apps_controller.rb @@ -52,20 +52,13 @@ class AppsController < InheritedResources::Base protected def collection - @unresolved_counts, @problem_counts = {}, {} @apps ||= begin apps = end_of_association_chain.all - # Cache counts for unresolved errs and problems - apps.each do |app| - @unresolved_counts[app.id] ||= app.problems.unresolved.count - @problem_counts[app.id] ||= app.problems.count - end - # Sort apps by number of unresolved errs, then problem counts. - apps.sort do |a,b| - (@unresolved_counts[b.id] <=> @unresolved_counts[a.id]).nonzero? || - (@problem_counts[b.id] <=> @problem_counts[a.id]).nonzero? || + apps.sort do |a, b| + (b.unresolved_count <=> a.unresolved_count).nonzero? || + (b.problem_count <=> a.problem_count).nonzero? || a.name <=> b.name end end diff --git a/app/models/app.rb b/app/models/app.rb index 573c03a..fc5095b 100644 --- a/app/models/app.rb +++ b/app/models/app.rb @@ -169,6 +169,14 @@ class App end end + def unresolved_count + @unresolved_count ||= problems.unresolved.count + end + + def problem_count + @problem_count ||= problems.count + end + protected def store_cached_attributes_on_problems diff --git a/app/views/apps/index.html.haml b/app/views/apps/index.html.haml index c99692f..29efcda 100644 --- a/app/views/apps/index.html.haml +++ b/app/views/apps/index.html.haml @@ -47,8 +47,8 @@ - revision = app.deploys.last.short_revision = link_to( app.last_deploy_at.to_s(:micro) << (revision.present? ? " (#{revision})" : ""), app_deploys_path(app)) %td.count - - if @problem_counts[app.id] > 0 - - unresolved = @unresolved_counts[app.id] + - if app.problem_count > 0 + - unresolved = app.unresolved_count = link_to unresolved, app_path(app), :class => (unresolved == 0 ? "resolved" : nil) - if @apps.none? %tr diff --git a/spec/controllers/apps_controller_spec.rb b/spec/controllers/apps_controller_spec.rb index d4684c1..f242885 100644 --- a/spec/controllers/apps_controller_spec.rb +++ b/spec/controllers/apps_controller_spec.rb @@ -31,17 +31,6 @@ describe AppsController do assigns(:apps).should_not include(unwatched_app) end end - - context 'when there is only one app' do - it 'sets unresolved_counts and problem_counts variables' do - sign_in Fabricate(:admin) - app = Fabricate(:app) - get :index - - assigns(:unresolved_counts).should == {app.id => 0} - assigns(:problem_counts).should == {app.id => 0} - end - end end describe "GET /apps/:id" do diff --git a/spec/views/apps/index.html.haml_spec.rb b/spec/views/apps/index.html.haml_spec.rb index 60359da..e5c595d 100644 --- a/spec/views/apps/index.html.haml_spec.rb +++ b/spec/views/apps/index.html.haml_spec.rb @@ -4,8 +4,6 @@ describe "apps/index.html.haml" do before do app = stub_model(App, :deploys => [stub_model(Deploy, :created_at => Time.now, :revision => "123456789abcdef")]) assign :apps, [app] - assign :problem_counts, {app.id => 0} - assign :unresolved_counts, {app.id => 0} controller.stub(:current_user) { stub_model(User) } end -- libgit2 0.21.2