Redmine

If the value of the issue_id is "1,2,3" in the filter, the query executes successfully.
If the value of the issue_id is "1 2 3" or "1 2 3" or "1, 2, 3", the query fails with message "•Issue is invalid"

app/models/query.rb

line 417, replace the "," as "\D" can use any non-numeric characters as separator.

/\A[+-]?\d+(\D[+-]?\d+)*\z/
digit separated by one non-digit

/\A[+-]?\d+(\D*\d+)*\z/
digit separated by any non-digit, easier for users.

Marius BALTEANU wrote:

It is the expected behaviour and I'm not sure that we should support more delimiters.

Hi Marius,

Currently the regexp is quite strict, only support digits seperated by only comma, no other character is allowed, it's not a bug.
But according to my survey, most of the users will not want to input the Issue ID one by one manually.
Users will copy the IDs from tables such as excel, the data would be "1 2 3 ", the delimiter should be "tab", and if copied from csv, may be the delimiter would be space.

So if we loose the restrict, users may have better experience.

Here is my patch, case check added for integer.

  def validate_query_filters
    filters.each_key do |field|
      if values_for(field)
        case type_for(field)
        when :integer
          case operator_for(field)
          when "=" 
            #add_filter_error(field, :invalid) if values_for(field).detect {|v| v.present? && !v.match(/\A[+-]?\d+(\D[+-]?\d+)*\z/) }
            add_filter_error(field, :invalid) if values_for(field).detect {|v| v.present? && !v.match(/\A[+-]?\d+(\D*\d+)*\z/) }
          else
            add_filter_error(field, :invalid) if values_for(field).detect {|v| v.present? && !v.match(/\A[+-]?\d+(,[+-]?\d+)*\z/) }
          end
        when :float
          add_filter_error(field, :invalid) if values_for(field).detect {|v| v.present? && !v.match(/\A[+-]?\d+(\.\d*)?\z/) }
        when :date, :date_past
          case operator_for(field)
          when "=", ">=", "<=", "><" 
            add_filter_error(field, :invalid) if values_for(field).detect {|v|
              v.present? && (!v.match(/\A\d{4}-\d{2}-\d{2}(T\d{2}((:)?\d{2}){0,2}(Z|\d{2}:?\d{2})?)?\z/) || parse_date(v).nil?)
            }
          when ">t-", "<t-", "t-", ">t+", "<t+", "t+", "><t+", "><t-" 
            add_filter_error(field, :invalid) if values_for(field).detect {|v| v.present? && !v.match(/^\d+$/) }
          end
        end
      end

      add_filter_error(field, :blank) unless
          # filter requires one or more values
          (values_for(field) and !values_for(field).first.blank?) or
          # filter doesn't require any value
          ["o", "c", "!*", "*", "t", "ld", "w", "lw", "l2w", "m", "lm", "y", "*o", "!o"].include? operator_for(field)
    end if filters
  end