Redmine 4.1.7

app/models/attachment.rb

@@ -29,7 +29,8 @@ class Attachment < ActiveRecord::Base
   validates_length_of :filename, :maximum => 255
   validates_length_of :disk_filename, :maximum => 255
   validates_length_of :description, :maximum => 255
-  validate :validate_max_file_size, :validate_file_extension
+  validate :validate_max_file_size
+  validate :validate_file_extension, :if => :filename_changed?
 
   acts_as_event :title => :filename,
                 :url => Proc.new {|o| {:controller => 'attachments', :action => 'show', :id => o.id, :filename => o.filename}}
@@ -76,11 +77,9 @@ class Attachment < ActiveRecord::Base
   end
 
   def validate_file_extension
-    if @temp_file
-      extension = File.extname(filename)
-      unless self.class.valid_extension?(extension)
-        errors.add(:base, l(:error_attachment_extension_not_allowed, :extension => extension))
-      end
+    extension = File.extname(filename)
+    unless self.class.valid_extension?(extension)
+      errors.add(:base, l(:error_attachment_extension_not_allowed, :extension => extension))
     end
   end
 
@@ -436,15 +435,15 @@ class Attachment < ActiveRecord::Base
   private
 
   def reuse_existing_file_if_possible
-    original_diskfile = nil
+    original_diskfile = diskfile
+    original_filename = disk_filename
     reused = with_lock do
       if existing = Attachment
                       .where(digest: self.digest, filesize: self.filesize)
-                      .where('id <> ? and disk_filename <> ?',
-                             self.id, self.disk_filename)
-                      .first
+                      .where.not(disk_filename: original_filename)
+                      .order(:id)
+                      .last
         existing.with_lock do
-          original_diskfile = self.diskfile
           existing_diskfile = existing.diskfile
           if File.readable?(original_diskfile) &&
             File.readable?(existing_diskfile) &&
@@ -455,7 +454,7 @@ class Attachment < ActiveRecord::Base
         end
       end
     end
-    if reused
+    if reused && Attachment.where(disk_filename: original_filename).none?
       File.delete(original_diskfile)
     end
   rescue ActiveRecord::StatementInvalid, ActiveRecord::RecordNotFound

app/models/issue.rb

@@ -201,6 +201,11 @@ class Issue < ActiveRecord::Base
     user_tracker_permission?(user, :delete_issues)
   end
 
+  # Overrides Redmine::Acts::Attachable::InstanceMethods#attachments_deletable?
+  def attachments_deletable?(user=User.current)
+    attributes_editable?(user)
+  end
+
   def initialize(attributes=nil, *args)
     super
     if new_record?
@@ -471,7 +476,6 @@ class Issue < ActiveRecord::Base
     'custom_field_values',
     'custom_fields',
     'lock_version',
-    'notes',
     :if => lambda {|issue, user| issue.new_record? || issue.attributes_editable?(user)})
   safe_attributes(
     'notes',
@@ -722,7 +726,7 @@ class Issue < ActiveRecord::Base
       errors.add :start_date, :earlier_than_minimum_start_date, :date => format_date(soonest_start)
     end
 
-    if fixed_version
+    if project && fixed_version
       if !assignable_versions.include?(fixed_version)
         errors.add :fixed_version_id, :inclusion
       elsif reopening? && fixed_version.closed?
@@ -737,7 +741,7 @@ class Issue < ActiveRecord::Base
       end
     end
 
-    if assigned_to_id_changed? && assigned_to_id.present?
+    if project && assigned_to_id_changed? && assigned_to_id.present?
       unless assignable_users.include?(assigned_to)
         errors.add :assigned_to_id, :invalid
       end
@@ -937,6 +941,8 @@ class Issue < ActiveRecord::Base
 
   # Users the issue can be assigned to
   def assignable_users
+    return [] if project.nil?
+
     users = project.assignable_users(tracker).to_a
     users << author if author && author.active?
     if assigned_to_id_was.present? && assignee = Principal.find_by_id(assigned_to_id_was)
@@ -948,6 +954,7 @@ class Issue < ActiveRecord::Base
   # Versions that the issue can be assigned to
   def assignable_versions
     return @assignable_versions if @assignable_versions
+    return [] if project.nil?
 
     versions = project.shared_versions.open.to_a
     if fixed_version
@@ -1704,12 +1711,12 @@ class Issue < ActiveRecord::Base
           if children.any?
             child_with_total_estimated_hours = children.select {|c| c.total_estimated_hours.to_f > 0.0}
             if child_with_total_estimated_hours.any?
-              average = child_with_total_estimated_hours.map(&:total_estimated_hours).sum.to_f / child_with_total_estimated_hours.count
+              average = child_with_total_estimated_hours.map(&:total_estimated_hours).sum.to_d / child_with_total_estimated_hours.count
             else
-              average = 1.0
+              average = 1.0.to_d
             end
             done = children.map {|c|
-              estimated = c.total_estimated_hours.to_f
+              estimated = (c.total_estimated_hours || 0.0).to_d
               estimated = average unless estimated > 0.0
               ratio = c.closed? ? 100 : (c.done_ratio || 0)
               estimated * ratio
@@ -1734,8 +1741,8 @@ class Issue < ActiveRecord::Base
       # a different project and that is not systemwide shared
       Issue.joins(:project, :fixed_version).
         where("#{Issue.table_name}.fixed_version_id IS NOT NULL" +
-          " AND #{Issue.table_name}.project_id <> #{Version.table_name}.project_id" +
-          " AND #{Version.table_name}.sharing <> 'system'").
+          " AND #{Issue.table_name}.project_id <> #{::Version.table_name}.project_id" +
+          " AND #{::Version.table_name}.sharing <> 'system'").
         where(conditions).each do |issue|
         next if issue.project.nil? || issue.fixed_version.nil?
         unless issue.project.shared_versions.include?(issue.fixed_version)
@@ -1756,7 +1763,7 @@ class Issue < ActiveRecord::Base
 
   # Callback on file attachment
   def attachment_added(attachment)
-    if current_journal && !attachment.new_record?
+    if current_journal && !attachment.new_record? && !copy?
       current_journal.journalize_attachment(attachment, :added)
     end
   end

app/models/issue_query.rb

@@ -513,7 +513,8 @@ class IssueQuery < Query
 
   def sql_for_fixed_version_status_field(field, operator, value)
     where = sql_for_field(field, operator, value, Version.table_name, "status")
-    version_ids = versions(:conditions => [where]).map(&:id)
+    version_id_scope = project ? project.shared_versions : Version.visible
+    version_ids = version_id_scope.where(where).pluck(:id)
 
     nl = operator == "!" ? "#{Issue.table_name}.fixed_version_id IS NULL OR" : ''
     "(#{nl} #{sql_for_field("fixed_version_id", "=", version_ids, Issue.table_name, "fixed_version_id")})"
@@ -521,7 +522,8 @@ class IssueQuery < Query
 
   def sql_for_fixed_version_due_date_field(field, operator, value)
     where = sql_for_field(field, operator, value, Version.table_name, "effective_date")
-    version_ids = versions(:conditions => [where]).map(&:id)
+    version_id_scope = project ? project.shared_versions : Version.visible
+    version_ids = version_id_scope.where(where).pluck(:id)
 
     nl = operator == "!*" ? "#{Issue.table_name}.fixed_version_id IS NULL OR" : ''
     "(#{nl} #{sql_for_field("fixed_version_id", "=", version_ids, Issue.table_name, "fixed_version_id")})"

app/models/mail_handler.rb

@@ -225,9 +225,8 @@ class MailHandler < ActionMailer::Base
 
     # check permission
     unless handler_options[:no_permission_check]
-      unless user.allowed_to?(:add_issue_notes, issue.project) ||
-               user.allowed_to?(:edit_issues, issue.project)
-        raise UnauthorizedAction, "not allowed to add notes on issues to project [#{project.name}]"
+      unless issue.notes_addable?
+        raise UnauthorizedAction, "not allowed to add notes on issues to project [#{issue.project.name}]"
       end
     end
 
@@ -276,7 +275,7 @@ class MailHandler < ActionMailer::Base
     end
 
     unless handler_options[:no_permission_check]
-      raise UnauthorizedAction, "not allowed to add messages to project [#{project.name}]" unless user.allowed_to?(:add_messages, message.project)
+      raise UnauthorizedAction, "not allowed to add messages to project [#{message.project.name}]" unless user.allowed_to?(:add_messages, message.project)
     end
 
     if !message.locked?

app/models/project.rb

@@ -883,6 +883,17 @@ class Project < ActiveRecord::Base
     end
   end
 
+  # Overrides Redmine::Acts::Customizable::InstanceMethods#validate_custom_field_values
+  # so that custom values that are not editable are not validated (eg. a custom field that
+  # is marked as required should not trigger a validation error if the user is not allowed
+  # to edit this field).
+  def validate_custom_field_values
+    user = User.current
+    if new_record? || custom_field_values_changed?
+      editable_custom_field_values(user).each(&:validate_value)
+    end
+  end
+
   # Returns the custom_field_values that can be edited by the given user
   def editable_custom_field_values(user=nil)
     visible_custom_field_values(user)

app/models/project_query.rb

@@ -70,7 +70,7 @@ class ProjectQuery < Query
   def available_columns
     return @available_columns if @available_columns
     @available_columns = self.class.available_columns.dup
-    @available_columns += ProjectCustomField.visible.
+    @available_columns += project_custom_fields.visible.
                             map {|cf| QueryCustomFieldColumn.new(cf) }
     @available_columns
   end

app/models/query.rb

@@ -51,7 +51,7 @@ class QueryColumn
 
   # Returns true if the column is sortable, otherwise false
   def sortable?
-    !@sortable.nil?
+    @sortable.present?
   end
 
   def sortable
@@ -501,7 +501,7 @@ class Query < ActiveRecord::Base
       if has_filter?(field) || !filter.remote
         options[:values] = filter.values
         if options[:values] && values_for(field)
-          missing = Array(values_for(field)).select(&:present?) - options[:values].map(&:last)
+          missing = Array(values_for(field)).select(&:present?) - options[:values].map{|v| v[1]}
           if missing.any? && respond_to?(method = "find_#{field}_filter_values")
             options[:values] += send(method, missing)
           end
@@ -609,13 +609,18 @@ class Query < ActiveRecord::Base
     if project
       project.rolled_up_custom_fields
     else
-      IssueCustomField.all
+      IssueCustomField.sorted
     end
   end
 
   # Returns a scope of project custom fields that are available as columns or filters
   def project_custom_fields
-    ProjectCustomField.all
+    ProjectCustomField.sorted
+  end
+
+  # Returns a scope of time entry custom fields that are available as columns or filters
+  def time_entry_custom_fields
+    TimeEntryCustomField.sorted
   end
 
   # Returns a scope of project statuses that are available as columns or filters
@@ -866,10 +871,10 @@ class Query < ActiveRecord::Base
 
   def project_statement
     project_clauses = []
-    active_subprojects_ids = []
+    subprojects_ids = []
 
-    active_subprojects_ids = project.descendants.active.map(&:id) if project
-    if active_subprojects_ids.any?
+    subprojects_ids = project.descendants.where.not(status: Project::STATUS_ARCHIVED).ids if project
+    if subprojects_ids.any?
       if has_filter?("subproject_id")
         case operator_for("subproject_id")
         when '='
@@ -878,7 +883,7 @@ class Query < ActiveRecord::Base
           project_clauses << "#{Project.table_name}.id IN (%s)" % ids.join(',')
         when '!'
           # exclude the selected subprojects
-          ids = [project.id] + active_subprojects_ids - values_for("subproject_id").map(&:to_i)
+          ids = [project.id] + subprojects_ids - values_for("subproject_id").map(&:to_i)
           project_clauses << "#{Project.table_name}.id IN (%s)" % ids.join(',')
         when '!*'
           # main project only

app/models/repository.rb

@@ -461,6 +461,10 @@ class Repository < ActiveRecord::Base
     scope
   end
 
+  def valid_name?(name)
+    scm.valid_name?(name)
+  end
+
   protected
 
   # Validates repository url based against an optional regular expression

app/models/setting.rb

@@ -100,7 +100,8 @@ class Setting < ActiveRecord::Base
     v = read_attribute(:value)
     # Unserialize serialized settings
     if available_settings[name]['serialized'] && v.is_a?(String)
-      v = YAML::load(v)
+      # YAML.load works as YAML.safe_load if Psych >= 4.0 is installed
+      v = YAML.respond_to?(:unsafe_load) ? YAML.unsafe_load(v) : YAML.load(v)
       v = force_utf8_strings(v)
     end
     v = v.to_sym if available_settings[name]['format'] == 'symbol' && !v.blank?

app/models/time_entry.rb

@@ -113,6 +113,13 @@ class TimeEntry < ActiveRecord::Base
             self.project_id = issue.project_id
           end
           @invalid_issue_id = nil
+        elsif user.allowed_to?(:log_time, issue.project) && issue.assigned_to_id_changed? && issue.previous_assignee == User.current
+          current_assignee = issue.assigned_to
+          issue.assigned_to = issue.previous_assignee
+          unless issue.visible?(user)
+            @invalid_issue_id = issue_id
+          end
+          issue.assigned_to = current_assignee
         else
           @invalid_issue_id = issue_id
         end
@@ -122,7 +129,14 @@ class TimeEntry < ActiveRecord::Base
       else
         @invalid_user_id = nil
       end
+
+      # Delete assigned custom fields not visible by the user
+      editable_custom_field_ids = editable_custom_field_values(user).map {|v| v.custom_field_id.to_s}
+      self.custom_field_values.delete_if do |c|
+        !editable_custom_field_ids.include?(c.custom_field.id.to_s)
+      end
     end
+
     attrs
   end
 
@@ -193,7 +207,7 @@ class TimeEntry < ActiveRecord::Base
 
   # Returns the custom_field_values that can be edited by the given user
   def editable_custom_field_values(user=nil)
-    visible_custom_field_values
+    visible_custom_field_values(user)
   end
 
   # Returns the custom fields that can be edited by the given user

app/models/time_entry_query.rb

@@ -89,7 +89,7 @@ class TimeEntryQuery < Query
     activities = (project ? project.activities : TimeEntryActivity.shared)
     add_available_filter(
       "activity_id",
-      :type => :list, :values => activities.map {|a| [a.name, a.id.to_s]}
+      :type => :list, :values => activities.map {|a| [a.name, (a.parent_id || a.id).to_s]}
     )
     add_available_filter(
       "project.status",
@@ -101,7 +101,7 @@ class TimeEntryQuery < Query
     add_available_filter "comments", :type => :text
     add_available_filter "hours", :type => :float
 
-    add_custom_fields_filters(TimeEntryCustomField)
+    add_custom_fields_filters(time_entry_custom_fields)
     add_associations_custom_fields_filters :project
     add_custom_fields_filters(issue_custom_fields, :issue)
     add_associations_custom_fields_filters :user
@@ -110,11 +110,11 @@ class TimeEntryQuery < Query
   def available_columns
     return @available_columns if @available_columns
     @available_columns = self.class.available_columns.dup
-    @available_columns += TimeEntryCustomField.visible.
+    @available_columns += time_entry_custom_fields.visible.
                             map {|cf| QueryCustomFieldColumn.new(cf) }
     @available_columns += issue_custom_fields.visible.
                             map {|cf| QueryAssociationCustomFieldColumn.new(:issue, cf, :totalable => false) }
-    @available_columns += ProjectCustomField.visible.
+    @available_columns += project_custom_fields.visible.
                             map {|cf| QueryAssociationCustomFieldColumn.new(:project, cf) }
     @available_columns
   end

app/models/token.rb

@@ -115,11 +115,13 @@ class Token < ActiveRecord::Base
     return nil unless action.present? && /\A[a-z0-9]+\z/i.match?(key)
 
     token = Token.find_by(:action => action, :value => key)
-    if token && (token.action == action) && (token.value == key) && token.user
-      if validity_days.nil? || (token.created_on > validity_days.days.ago)
-        token
-      end
-    end
+    return unless token
+    return unless token.action == action
+    return unless ActiveSupport::SecurityUtils.secure_compare(token.value.to_s, key)
+    return unless token.user
+    return unless validity_days.nil? || (token.created_on > validity_days.days.ago)
+
+    token
   end
 
   def self.generate_token_value

app/models/wiki_page.rb

@@ -155,11 +155,7 @@ class WikiPage < ActiveRecord::Base
   end
 
   def content_for_version(version=nil)
-    if content
-      result = content.versions.find_by_version(version.to_i) if version
-      result ||= content
-      result
-    end
+    (content && version) ? content.versions.find_by_version(version.to_i) : content
   end
 
   def diff(version_to=nil, version_from=nil)