Refactor cached notes count code based on advice fro the ruby-nyc list. Thanks all!

Refactor out some finds to use automatically scopred finds on assication extensions.
Refactor out some error handling for very unusual cases.



git-svn-id: http://www.rousette.org.uk/svn/tracks-repos/trunk@486 a4c988fc-2ded-0310-b66e-134b36920a42

tracks/app/controllers/contexts_controller.rb

@@ -4,7 +4,7 @@ class ContextsController < ApplicationController
 
   before_filter :init, :except => [:create, :destroy, :order]
   before_filter :init_todos, :only => :show
-  before_filter :check_user_set_context, :only => [:update, :destroy]
+  before_filter :set_context_from_params, :only => [:update, :destroy]
   skip_before_filter :login_required, :only => [:index]
   prepend_before_filter :login_or_feed_token_required, :only => [:index]
   session :off, :only => :index, :if => Proc.new { |req| ['rss','atom','txt'].include?(req.parameters[:format]) }
@@ -91,9 +91,7 @@ class ContextsController < ApplicationController
   #
   def order
     params["list-contexts"].each_with_index do |id, position|
-      if check_user_matches_context_user(id)
-        Context.update(id, :position => position + 1)
-      end
+      @user.contexts.update(id, :position => position + 1)
     end
     render :nothing => true
   end
@@ -115,32 +113,8 @@ class ContextsController < ApplicationController
       end
     end
 
-    def check_user_set_context
+    def set_context_from_params
       @context = @user.contexts.find_by_params(params)
-      if @context.nil?
-        render :text => "Context not found.", :status => 404
-      end
-    end
-
-    def check_user_matches_context_user(id)
-       @context = Context.find_by_id_and_user_id(id, @user.id)
-       if @user == @context.user
-         return @context
-       else
-         @context = nil
-         notify :warning, "Project and session user mis-match: #{@context.user_id} and #{@user.id}!"
-         render :text => ''
-       end
-    end
-    
-    def check_user_return_item
-      item = Todo.find( params['id'] )
-      if @user == item.user
-        return item
-      else
-        notify :warning, "Item and session user mis-match: #{item.user.name} and #{@user.name}!"
-        render :text => ''
-      end
     end
      
     def init
@@ -154,7 +128,7 @@ class ContextsController < ApplicationController
     end
 
     def init_todos
-      check_user_set_context
+      set_context_from_params
       @done = @context.done_todos
       # @not_done_todos = @context.not_done_todos
       # TODO: Temporarily doing this search manually until I can work out a way

tracks/app/controllers/projects_controller.rb

@@ -2,7 +2,7 @@ class ProjectsController < ApplicationController
 
   helper :application, :todos, :notes
   before_filter :init, :except => [:create, :destroy, :order]
-  before_filter :check_user_set_project, :only => [:update, :destroy, :show]
+  before_filter :set_project_from_params, :only => [:update, :destroy, :show]
   before_filter :default_context_filter, :only => [:create,:update]
   skip_before_filter :login_required, :only => [:index]
   prepend_before_filter :login_or_feed_token_required, :only => [:index]
@@ -136,13 +136,7 @@ class ProjectsController < ApplicationController
         @active_projects = @projects.select{ |p| p.active? }
         @hidden_projects = @projects.select{ |p| p.hidden? }
         @completed_projects = @projects.select{ |p| p.completed? }
-        project_note_counts = Note.count(:group => 'project_id')
-        @projects.each do |project|
-          #define a singlton method "notes_count" on each project object
-          class << project; self end.send(:define_method, :notes_count) do
-            project_note_counts[project.id] || 0
-          end
-        end
+        @projects.cache_note_counts
         @new_project = @user.projects.build
         render
       end
@@ -171,21 +165,10 @@ class ProjectsController < ApplicationController
       end
     end
         
-    def check_user_set_project
+    def set_project_from_params
       @project = @user.projects.find_by_params(params)
-      render :text => 'Project not found', :status => 404 if @project.nil?
     end
-        
-    def check_user_return_item
-      item = Todo.find( params['id'] )
-      if @user == item.user
-        return item
-      else
-        notify :warning, "Item and session user mis-match: #{item.user.name} and #{@user.name}!"
-        render :text => ''
-      end
-    end
-     
+
     def init
       @source_view = params['_source_view'] || 'project'
       @projects = @user.projects

tracks/app/controllers/todos_controller.rb

@@ -3,6 +3,7 @@ class TodosController < ApplicationController
   helper :todos
 
   append_before_filter :init, :except => [ :destroy, :completed, :completed_archive, :check_deferred ]
+  append_before_filter :get_todo_from_params, :only => [ :edit, :toggle_check, :show, :update, :destroy ]
   skip_before_filter :login_required, :only => [:index]
   prepend_before_filter :login_or_feed_token_required, :only => [:index]
   session :off, :only => :index, :if => Proc.new { |req| is_feed_request(req) }
@@ -83,20 +84,17 @@ class TodosController < ApplicationController
   end
   
   def edit
-    @todo = check_user_return_todo
   end
   
   def show
-    item = check_user_return_todo
-    respond_to do |wants|
-       wants.xml { render :xml => item.to_xml( :root => 'todo', :except => :user_id ) }
-     end
+    respond_to do |format|
+      format.xml { render :xml => @todo.to_xml( :root => 'todo', :except => :user_id ) }
+    end
   end
 
   # Toggles the 'done' status of the action
   #
   def toggle_check
-    @todo = check_user_return_todo
     @todo.toggle_completion!
     @saved = @todo.save
     respond_to do |format|
@@ -122,7 +120,6 @@ class TodosController < ApplicationController
   end
 
   def update
-    @todo = check_user_return_todo
     @todo.tag_with(params[:tag_list],@user)
     @original_item_context_id = @todo.context_id
     @original_item_project_id = @todo.project_id
@@ -173,7 +170,7 @@ class TodosController < ApplicationController
   end
     
   def destroy
-    @todo = check_user_return_todo
+    @todo = get_todo_from_params
     @context_id = @todo.context_id
     @project_id = @todo.project_id
     @saved = @todo.destroy
@@ -271,21 +268,8 @@ class TodosController < ApplicationController
   
   private
 
-    def check_user_return_todo
-      todo = Todo.find( params['id'].to_i )
-      if @user == todo.user
-        return todo
-      else
-        @error_message = 'Item and session user mis-match: #{todo.user.name} and #{@todo.name}!'
-        respond_to do |wants|
-          wants.html do
-            notify :error, @error_message, 8.0
-            render :action => "index"
-          end
-          wants.js { render :action => 'error' }
-          wants.xml { render :text => @error_message, :status => 403 }
-        end
-      end
+    def get_todo_from_params
+      @todo = @user.todos.find(params['id'])
     end
 
     def init

tracks/app/models/project.rb

@@ -33,6 +33,7 @@ class Project < ActiveRecord::Base
   end
   
   attr_protected :user
+  attr_accessor :cached_note_count
 
   def self.null_object
     NullProject.new
@@ -67,6 +68,10 @@ class Project < ActiveRecord::Base
     end
   end
   
+  def note_count
+    cached_note_count || notes.count
+  end
+  
   alias_method :original_default_context, :default_context
 
   def default_context

tracks/app/models/user.rb

@@ -56,6 +56,12 @@ class User < ActiveRecord::Base
                 return nil if position == 0 && offset < 0
                 projects.at( position + offset)
               end
+              def cache_note_counts
+                project_note_counts = Note.count(:group => 'project_id')
+                self.each do |project|
+                  project.cached_note_count = project_note_counts[project.id] || 0
+                end
+              end
             end
   has_many :todos,
            :order => 'completed_at DESC, todos.created_at DESC',