Redmine

Is it hard to commit to this patch?
I can improve the patch if there are any problems.

In TimeEntryActivity.default, all of the project's activities are already loaded from the database in the if statement. As such, the trailing find_by is redundant and should be replaced by Ruby-only filtering to avoid running another SQL query to load data we already have.

The trailing line of that method could thus be replaced by the following code with matching behavior to avoid the additional SQL query:

project.activities.detect { |activity| activity.parent_id == default_activity.id }

Also, it is not guaranteed that we have any default activity at all as admins could deselect the default flag on all activities. Thus would result in the super call to return nil which in turn would result in an exception in the final line of the overwritten default method. As such, we also have to check for default_activity.nil?. The following method should address both of my points:

def self.default(project=nil)
  default_activity = super()

  if default_activity.nil? || project.nil? || project.activities.blank? || project.activities.include?(default_activity)
    return default_activity
  end

  project.activities.detect { |activity| activity.parent_id == default_activity.id }
end

Thank you very much.