Refactor: Move Method

Moved the save_time_entry_from_params from the BulkTimeEntriesController to the
TimeEntry (patched via the plugin).

Also includes some additional fixes:

* Added some tests for the behavior
* Fixed a bug the method body, was using next but not inside a loop
* Fixed a bug in #allowed_project?, wasn't using :conditions

app/controllers/bulk_time_entries_controller.rb

@@ -35,8 +35,8 @@ def save
 
       render :update do |page|
         @time_entries.each_pair do |html_id, entry|
-          @time_entry = save_time_entry_from_params(entry)
-          unless @time_entry.save
+          @time_entry = TimeEntry.create_bulk_time_entry(entry)
+          unless @time_entry && @time_entry.save
             page.replace "entry_#{html_id}", :partial => 'time_entry', :object => @time_entry
           else
             time_entry_target = if @time_entry.issue
@@ -80,17 +80,7 @@ def load_allowed_projects
       Project.allowed_to_condition(User.current, :log_time))
   end
 
-  def save_time_entry_from_params(entry)
-    next unless BulkTimeEntriesController.allowed_project?(entry[:project_id])
-    time_entry = TimeEntry.new(entry)
-    time_entry.hours = nil if time_entry.hours.blank? or time_entry.hours <= 0
-    time_entry.project_id = entry[:project_id] # project_id is protected from mass assignment
-    time_entry.user = User.current
-    time_entry
-  end
-  helper_method :save_time_entry_from_params
-
   def self.allowed_project?(project_id)
-    return User.current.projects.find_by_id(project_id, Project.allowed_to_condition(User.current, :log_time))
+    return User.current.projects.find_by_id(project_id, :conditions => Project.allowed_to_condition(User.current, :log_time))
   end
 end

init.rb

@@ -14,3 +14,10 @@
     :caption => :bulk_time_entry_title, :if => Proc.new{User.current.allowed_to?(:log_time, nil, :global => true)} 
 end
 
+# Patches to the Redmine core.
+require 'dispatcher'
+
+Dispatcher.to_prepare :bulk_time_entry_plugin do
+  require_dependency 'time_entry'
+  TimeEntry.send(:include, BulkTimeEntryPlugin::Patches::TimeEntryPatch)
+end

lib/bulk_time_entry_plugin/patches/time_entry_patch.rb

@@ -0,0 +1,23 @@
+module BulkTimeEntryPlugin
+  module Patches
+    module TimeEntryPatch
+      def self.included(base)
+        base.extend ClassMethods
+      end
+
+      module ClassMethods
+
+        def create_bulk_time_entry(entry)
+          return false unless BulkTimeEntriesController.allowed_project?(entry[:project_id])
+          time_entry = TimeEntry.new(entry)
+          time_entry.hours = nil if time_entry.hours.blank? or time_entry.hours <= 0
+          time_entry.project_id = entry[:project_id] # project_id is protected from mass assignment
+          time_entry.user = User.current
+          time_entry
+        end
+
+      end
+
+    end
+  end
+end

test/unit/lib/bulk_time_entry_plugin/patches/time_entry_patch_test.rb

@@ -0,0 +1,40 @@
+require 'test_helper'
+
+class BulkTimeEntryPlugin::Patches::TimeEntryPatchTest < ActiveSupport::TestCase
+  context "TimeEntry#create_bulk_time_entry" do
+    setup do
+      User.current = @user = User.generate_with_protected!
+      @project = Project.generate!
+      @role = Role.generate!(:permissions => Redmine::AccessControl.permissions.collect(&:name))
+      Member.generate!(:project => @project, :roles => [@role], :user_id => @user.id)
+      @valid_params = {:project_id => @project.id, :hours => 5}
+    end
+
+    should "return false if the current user is not allowed to log time to the project" do
+      @role.update_attributes(:permissions => [])
+
+      assert_equal false, TimeEntry.create_bulk_time_entry(@valid_params)
+    end
+
+    context "saving a valid record" do
+      should "save a new Time Entry record"
+
+      should "set the project of the new record" do
+        @entry = TimeEntry.create_bulk_time_entry(@valid_params)
+        assert_equal @project, @entry.project
+      end
+
+      should "assign the current user" do
+        @entry = TimeEntry.create_bulk_time_entry(@valid_params)
+        assert_equal @user, @entry.user
+      end
+
+      should "set hours to nil if they are passed in as 0" do
+        @entry = TimeEntry.create_bulk_time_entry(@valid_params.merge(:hours => 0))
+        assert_equal @user, @entry.user
+      end
+
+    end
+
+  end
+end