improved query patch

time_list_controller now removes unwanted filters from the filter_list
make time_bookings belong_to project
added missing dependency to time_log => time_bookings (missed "delete_all")
added some translations
added migration #8
added project_patch
cleaned the code

app/controllers/time_list_controller.rb

@@ -14,26 +14,16 @@ def index
     # overwrite the initial column_names cause if no columns are specified, the Query class uses default values
     # which depend on issues
     @query.column_names = @query.column_names || [:id, :time_log_id, :time_entry_id, :comments, :user, :project]
-    @query.filters.delete("status_id")
-    @query.filters.delete("tracker_id")
 
-    @query.group_by=nil
+    # temporarily limit the available filters for the view!
+    @query.available_filters.delete_if { |key,value| !key.to_s.start_with?('tt_') }
 
     sort_init(@query.sort_criteria.empty? ? [['id', 'desc']] : @query.sort_criteria)
     sort_update(@query.sortable_columns)
 
     if @query.valid?
       @limit = per_page_option
 
-      #@issue_count = @query.issue_count
-      #@issue_pages = Paginator.new self, @issue_count, @limit, params['page']
-      #@offset ||= @issue_pages.current.offset
-      #@issues = @query.issues(:include => [:assigned_to, :tracker, :priority, :category, :fixed_version],
-      #                        :order => sort_clause,
-      #                        :offset => @offset,
-      #                        :limit => @limit)
-      #@issue_count_by_group = @query.issue_count_by_group
-
       @booking_count = @query.booking_count
       @booking_pages = Paginator.new self, @booking_count, @limit, params['page']
       @offset ||= @booking_pages.current.offset

app/models/time_booking.rb

@@ -1,11 +1,11 @@
 class TimeBooking < ActiveRecord::Base
   unloadable
 
-  attr_accessible :started_on, :stopped_at, :time_entry_id, :time_log_id, :virtual
+  attr_accessible :started_on, :stopped_at, :time_entry_id, :time_log_id, :virtual, :project
+  belongs_to :project
   belongs_to :time_log
   belongs_to :time_entry, :dependent => :delete
   has_one :virtual_comment, :dependent => :delete
-  #has_one :project, :through => :time_entry
 
   validates_presence_of :time_log_id
   validates :time_entry_id, :presence => true, :unless => Proc.new { |tb| tb.virtual }
@@ -17,7 +17,7 @@ def initialize(args = {}, options = {})
       # without issue_id, create an virtual booking!
       if args[:issue].nil?
         # create a virtual booking
-        super({:virtual => true, :time_log_id => args[:time_log_id], :started_on => args[:started_on], :stopped_at => args[:stopped_at]})
+        super({:virtual => true, :time_log_id => args[:time_log_id], :project_id => args[:project_id], :started_on => args[:started_on], :stopped_at => args[:stopped_at]})
         self.save
         # this part looks not very Rails like yet... should refactor it if any time to
         vcomment = VirtualComment.where(:time_booking_id => self.id).first_or_create
@@ -36,7 +36,7 @@ def initialize(args = {}, options = {})
           # due to the mass-assignment security, we have to set the user_id extra
           time_entry.user_id = args[:user_id]
           time_entry.save
-          super({:time_entry_id => time_entry.id, :time_log_id => args[:time_log_id], :started_on => args[:started_on], :stopped_at => args[:stopped_at]})
+          super({:time_entry_id => time_entry.id, :time_log_id => args[:time_log_id], :started_on => args[:started_on], :stopped_at => args[:stopped_at, :project => args[:issue].project]})
         end
       end
     end
@@ -72,15 +72,15 @@ def comments
     end
   end
 
-  def project
-    if self.virtual
-      pid = self.time_log.project_id
-      Project.find(pid) unless pid.nil?
-      #self.time_log.project
-    else
-      self.time_entry.project
-    end
-  end
+  #def project
+  #  if self.virtual
+  #    pid = self.time_log.project_id
+  #    Project.find(pid) unless pid.nil?
+  #    #self.time_log.project
+  #  else
+  #    self.time_entry.project
+  #  end
+  #end
 
   def user
     self.time_log.user

app/models/time_log.rb

@@ -4,7 +4,7 @@ class TimeLog < ActiveRecord::Base
   attr_accessible :user_id, :started_on, :stopped_at, :project_id, :comments, :issue_id, :spent_time
   attr_accessor :issue_id, :spent_time
   belongs_to :user
-  has_many :time_bookings
+  has_many :time_bookings, :dependent => :delete_all
   has_many :time_entries, :through => :time_bookings
 
 #  belongs_to :project
@@ -17,7 +17,7 @@ def initialize(arguments = nil, *args)
   # method returns true if all works well, false otherwise
   def add_booking(args = {})
     # TODO not set activity_id default value to 1 / only for testing because redmine requires activity_id
-    default_args = {:started_on => self.started_on, :stopped_at => self.stopped_at, :comments => self.comments, :activity_id => 1, :issue => nil, :spent_time => nil, :virtual => false}
+    default_args = {:started_on => self.started_on, :stopped_at => self.stopped_at, :comments => self.comments, :activity_id => 1, :issue => nil, :spent_time => nil, :virtual => false, :project_id => self.project_id}
     args = default_args.merge(args)
 
     # basic calculations are always the same

config/locales/en.yml

@@ -1,5 +1,17 @@
 # English strings go here for Rails i18n
 en:
+  #the first three entries are temporarily
+  field_tt_booking_id: "Time booking"
+  field_tt_time_entry_id: "Time entry"
+  field_tt_time_log_id: "Time log"
+
+  # regular translations:
+  field_tt_comments: "Comments"
+  field_tt_due_date: "Due date"
+  field_tt_project: "Project"
+  field_tt_issue: "Issue"
+  field_tt_start_date: "Start date"
+  field_tt_user: "User"
   list_time_trackers: "list"
   no_time_tracker: "No time tracker"
   no_time_logs: "No time logs"

db/migrate/008_add_projectid_timebooking.rb

@@ -0,0 +1,16 @@
+class AddProjectidTimebooking < ActiveRecord::Migration
+  def up
+    add_column :time_bookings, :project_id, :integer
+    TimeBooking.all.each do |tb|
+      if tb.virtual
+        tb.update_attribute(:project_id, tb.time_log.project_id)
+      else
+        tb.update_attribute(:project_id, tb.time_entry.project_id)
+      end
+    end
+  end
+
+  def down
+    remove_column :time_bookings, :project_id
+  end
+end

init.rb

@@ -7,6 +7,9 @@
 require 'user_patch'
 User.send(:include, UserPatch)
 
+require 'project_patch'
+Project.send(:include, ProjectPatch)
+
 require 'menu_patch'
 Redmine::MenuManager::MenuHelper.send(:include, MenuPatch)
 

lib/project_patch.rb

@@ -0,0 +1,15 @@
+module ProjectPatch
+  def self.included(base)
+    base.send(:extend, ClassMethods)
+    base.send(:include, InstanceMethods)
+    base.class_eval do
+      has_many :time_bookings, :dependent => :delete_all
+    end
+  end
+
+  module ClassMethods
+  end
+
+  module InstanceMethods
+  end
+end

lib/query_patch.rb

@@ -13,13 +13,13 @@ def self.included(base)
     base.send(:extend, ClassMethods)
     base.send(:include, InstanceMethods)
     base.class_eval do
-      alias_method_chain :available_filters, :pj
+      alias_method_chain :available_filters, :time_tracker
 
-      base.add_available_column(QueryColumn.new(:id, :sortable => "#{TimeBooking.table_name}.id", :caption => "bookings"))
-      base.add_available_column(QueryColumn.new(:time_log_id, :sortable => "#{TimeLog.table_name}.id", :caption => "logs"))
-      base.add_available_column(QueryColumn.new(:time_entry_id, :sortable => "#{TimeEntry.table_name}.id", :caption => "entries"))
-      base.add_available_column(QueryColumn.new(:comments, :sortable => "#{TimeEntry.table_name}.comments", :caption => "comments"))
-      base.add_available_column(QueryColumn.new(:user, :sortable => "#{User.table_name}.login", :caption => "user"))
+      base.add_available_column(QueryColumn.new(:id, :sortable => "#{TimeBooking.table_name}.id", :caption => :field_tt_booking_id))
+      base.add_available_column(QueryColumn.new(:time_log_id, :sortable => "#{TimeLog.table_name}.id", :caption => :field_tt_time_log_id))
+      base.add_available_column(QueryColumn.new(:time_entry_id, :sortable => "#{TimeEntry.table_name}.id", :caption => :field_tt_time_entry_id))
+      base.add_available_column(QueryColumn.new(:comments, :sortable => "#{TimeEntry.table_name}.comments", :caption => :field_tt_comments))
+      base.add_available_column(QueryColumn.new(:user, :sortable => "#{User.table_name}.login", :caption => :field_tt_user))
     end
   end
 
@@ -29,16 +29,31 @@ module ClassMethods
   # TODO refactor the instance methods!!
   module InstanceMethods
 
-    def available_filters_with_pj
-      @available_filters = available_filters_without_pj
-      @available_filters['pj'] = @available_filters["project_id"]
+    def available_filters_with_time_tracker
+
+      # speedup for recursive calls, so we only calc the content for the query once!
+      return @available_filters if @available_filters
+
+      @available_filters = available_filters_without_time_tracker
+
+      # use raw Query as template to get the content for two complex fields without copying the source
+      tq = Query.new
+
+      @available_filters['tt_project'] = tq.available_filters_without_time_tracker["project_id"].clone       # :oder => 1
+      @available_filters['tt_start_date'] = { :type => :date, :order => 2 }
+      @available_filters['tt_due_date'] = { :type => :date, :order => 3 }
+      @available_filters['tt_issue'] = { :type => :list, :order => 4, :values => Issue.all.collect{|s| [s.subject, s.id.to_s] } }
+      @available_filters['tt_user'] = tq.available_filters_without_time_tracker["author_id"].clone           # :oder => 5
+      @available_filters['tt_comments'] = { :type => :text, :order => 6 }
       @available_filters
     end
 
     # Returns the bookings count
     def booking_count
       # TODO refactor includes
-      TimeBooking.count(:include => [:time_log, :time_entry], :conditions => statement)
+      # for some reason including the :project will result in an "ambiguous column" - error if we try to group by "project"
+      #TimeBooking.count(:include => [:project, :virtual_comment, :time_entry, :time_log => :user], :conditions => statement)
+      TimeBooking.count(:include => [:virtual_comment, :time_entry, :time_log => :user], :conditions => statement)
     rescue ::ActiveRecord::StatementInvalid => e
       raise StatementInvalid.new(e.message)
     end
@@ -49,7 +64,7 @@ def booking_count_by_group
       if grouped?
         begin
           # Rails3 will raise an (unexpected) RecordNotFound if there's only a nil group value
-          r = TimeBooking.count(:group => group_by_statement, :include => [:time_log, :time_entry], :conditions => statement)
+          r = TimeBooking.count(:group => group_by_statement, :include => [:virtual_comment, :time_entry, :time_log => :user], :conditions => statement)
         rescue ActiveRecord::RecordNotFound
           r = {nil => booking_count}
         end
@@ -69,38 +84,52 @@ def bookings(options={})
       order_option = [group_by_sort_order, options[:order]].reject { |s| s.blank? }.join(',')
       order_option = nil if order_option.blank?
 
-      # TODO figure out what benefits are coming with the joins and how to use them correctly
-      #joins = (order_option && order_option.include?('authors')) ? "LEFT OUTER JOIN users authors ON authors.id = #{Issue.table_name}.author_id" : nil
-      #joins = "LEFT JOIN #{TimeBooking.table_name} ON #{TimeEntry.table_name}.id = #{TimeBooking.table_name}.time_entry_id"
-      #joins = nil
-      joins = "LEFT OUTER JOIN #{Project.table_name} ON
-              (
-              #{TimeBooking.table_name}.virtual = 't' AND #{Project.table_name}.id = #{TimeLog.table_name}.project_id
-              OR
-              #{TimeBooking.table_name}.virtual = 'f' AND #{Project.table_name}.id = #{TimeEntry.table_name}.project_id
-              )"
-
-      #TimeBooking.scoped(:conditions => options[:conditions]).find :all, :include => ([:time_entry => :project, :time_log => [:user, :project]] + (options[:include] || [])).uniq,
-      TimeBooking.scoped(:conditions => options[:conditions]).find :all, :include => ([:time_entry, :time_log => :user] + (options[:include] || [])).uniq,
-                                                                   :conditions => statement,
-                                                                   :order => order_option,
-                                                                   #:joins => [:time_log, :time_entry],
-                                                                   :joins => joins,
-                                                                   :limit => options[:limit],
-                                                                   :offset => options[:offset]
+      TimeBooking.scoped(:conditions => options[:conditions]).
+          includes(([:project, :virtual_comment, :time_entry, :time_log => :user] + (options[:include] || [])).uniq).
+          where(statement).
+          order(order_option).
+          limit(options[:limit]).
+          offset(options[:offset])
 
     rescue ::ActiveRecord::StatementInvalid => e
       raise StatementInvalid.new(e.message)
     end
 
     # sql statements for where clauses have to be in an "sql_for_#{filed-name}_field" method
-    def sql_for_pj_field(field, operator, value)
+    # so we have to implement some where-clauses for every new filter here
+
+    def sql_for_tt_project_field(field, operator, value)
       if value.delete('mine')
         value += User.current.memberships.map(&:project_id).map(&:to_s)
       end
+      if operator == "="
+        "( #{TimeBooking.table_name}.project_id IN (" + value.collect { |val| "'#{connection.quote_string(val)}'" }.join(",") + ") )"
+      else
+        "( #{TimeBooking.table_name}.project_id NOT IN (" + value.collect { |val| "'#{connection.quote_string(val)}'" }.join(",") + ") OR #{TimeBooking.table_name}.project_id IS NULL )"
+      end
+    end
+
+    def sql_for_tt_start_date_field(field, operator, value)
+      # stub
+    end
+
+    def sql_for_tt_due_date_field(field, operator, value)
+      # stub
+    end
+
+    def sql_for_tt_issue_field(field, operator, value)
+      # stub
+    end
+
+    def sql_for_tt_user_field(field, operator, value)
+      if value.delete('me')
+        value += User.current.id.to_s.to_a
+      end
+      "( #{User.table_name}.id #{operator == "=" ? 'IN' : 'NOT IN'} (" + value.collect { |val| "'#{connection.quote_string(val)}'" }.join(",") + ") )"
+    end
 
-      "(#{TimeBooking.table_name}.virtual = 't' AND #{TimeLog.table_name}.project_id IN (" + value.collect { |val| "'#{connection.quote_string(val)}'" }.join(",") + ") )" +
-          " OR ( #{TimeBooking.table_name}.virtual = 'f' AND  #{TimeEntry.table_name}.project_id IN (" + value.collect { |val| "'#{connection.quote_string(val)}'" }.join(",") + ") )"
+    def sql_for_tt_comments_field(field, operator, value)
+      # stub
     end
   end
 end