Browse Source

Fixes #3647 - Error 500 / zammad {"error":" 123456 is out of range for ActiveModel::Type::Integer with limit 4 bytes"}

Mantas Masalskis 3 years ago
parent
commit
92822b6713

+ 2 - 2
app/assets/javascripts/app/controllers/_ui_element/object_manager_attribute.coffee

@@ -279,7 +279,7 @@ class App.UiElement.object_manager_attribute extends App.UiElement.ApplicationUi
       params: params
     )
     configureAttributes = [
-      { name: 'data_option::min', display: 'Minimal', tag: 'integer', null: false, default: 0, min: 1 },
+      { name: 'data_option::min', display: 'Minimal', tag: 'integer', null: false, default: 0, min: -2147483647, max: 2147483647 },
     ]
     integerMin = new App.ControllerForm(
       model:
@@ -288,7 +288,7 @@ class App.UiElement.object_manager_attribute extends App.UiElement.ApplicationUi
       params: params
     )
     configureAttributes = [
-      { name: 'data_option::max', display: 'Maximal', tag: 'integer', null: false, default: 999999999, min: 2 },
+      { name: 'data_option::max', display: 'Maximal', tag: 'integer', null: false, min: -2147483647, max: 2147483647, default: 999999999 },
     ]
     integerMax = new App.ControllerForm(
       model:

+ 20 - 3
app/models/object_manager/attribute.rb

@@ -22,6 +22,10 @@ class ObjectManager::Attribute < ApplicationModel
     active
   ].freeze
 
+  VALIDATE_INTEGER_MIN    = -2_147_483_647
+  VALIDATE_INTEGER_MAX    = 2_147_483_647
+  VALIDATE_INTEGER_REGEXP = %r{^-?\d+$}.freeze
+
   self.table_name = 'object_manager_attributes'
 
   belongs_to :object_lookup, optional: true
@@ -897,10 +901,23 @@ is certain attribute used by triggers, overviews or schedulers
       [{ failed:  !local_data_option[:maxlength].to_s.match?(%r{^\d+$}),
          message: 'must have integer for :maxlength' }]
     when 'integer'
-      [{ failed:  !local_data_option[:min].to_s.match?(%r{^\d+$}),
+      min = local_data_option[:min]
+      max = local_data_option[:max]
+
+      [{ failed:  !VALIDATE_INTEGER_REGEXP.match?(min.to_s),
          message: 'must have integer for :min' },
-       { failed:  !local_data_option[:max].to_s.match?(%r{^\d+$}),
-         message: 'must have integer for :max' }]
+       { failed:  !VALIDATE_INTEGER_REGEXP.match?(max.to_s),
+         message: 'must have integer for :max' },
+       { failed:  !(min.is_a?(Integer) && min >= VALIDATE_INTEGER_MIN),
+         message: 'min must be higher than -2147483648' },
+       { failed:  !(min.is_a?(Integer) && min <= VALIDATE_INTEGER_MAX),
+         message: 'min must be lower than 2147483648' },
+       { failed:  !(max.is_a?(Integer) && max >= VALIDATE_INTEGER_MIN),
+         message: 'max must be higher than -2147483648' },
+       { failed:  !(max.is_a?(Integer) && max <= VALIDATE_INTEGER_MAX),
+         message: 'max must be lower than 2147483648' },
+       { failed:  !(max.is_a?(Integer) && min.is_a?(Integer) && min <= max),
+         message: 'min must be lower than max' }]
     when %r{^((tree_)?select|checkbox)$}
       [{ failed:  !local_data_option.key?(:default),
          message: 'must have value for :default' },

+ 18 - 0
db/migrate/20210717101728_issue_3647_custom_object_attribute_integer.rb

@@ -0,0 +1,18 @@
+# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
+
+class Issue3647CustomObjectAttributeInteger < ActiveRecord::Migration[6.0]
+  def up
+    return if !Setting.exists?(name: 'system_init_done')
+
+    %i[min max].each do |attr|
+      ObjectManager::Attribute
+        .where(data_type: 'integer', editable: true)
+        .each do |attribute|
+          next if attribute.data_option[attr] <= 2_147_483_647
+
+          attribute.data_option[attr] = 2_147_483_647
+          attribute.save!(validate: false)
+        end
+    end
+  end
+end

+ 34 - 0
spec/db/migrate/issue_3647_custom_object_attribute_integer_spec.rb

@@ -0,0 +1,34 @@
+# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
+
+require 'rails_helper'
+
+RSpec.describe Issue3647CustomObjectAttributeInteger, type: :db_migration do
+  let(:integer_valid) { create(:object_manager_attribute_integer) }
+  let(:integer_max_over_max) do
+    object = build(:object_manager_attribute_integer, data_option: { default: 0, min: 0, max: 9_999_999_999 })
+    object.save(validate: false)
+    object
+  end
+  let(:integer_min_over_max) do
+    object = build(:object_manager_attribute_integer, data_option: { default: 0, min: 9_999_999_999, max: 99_999_999_999 })
+    object.save(validate: false)
+    object
+  end
+
+  it 'leaves valid integer intact' do
+    expect { migrate }
+      .not_to change { integer_valid.data_option[:max] }
+  end
+
+  it 'lowers max if it is too big' do
+    expect { migrate }
+      .to change { integer_max_over_max.reload.data_option[:max] }
+      .to 2_147_483_647
+  end
+
+  it 'lowers min if it is too big' do
+    expect { migrate }
+      .to change { integer_min_over_max.reload.data_option[:min] }
+      .to 2_147_483_647
+  end
+end

+ 1 - 1
spec/support/authenticated_as.rb

@@ -3,7 +3,7 @@
 module ZammadAuthenticatedAsHelper
   # parse authenticated_as params for request and system test helpers
   #
-  # @param input [Any] any to parse, see bellow for options
+  # @param input [Any] any to parse, see below for options
   # @param return_type [Symbol] :credentials or :user
   def authenticated_as_get_user(input, return_type:)
     case input

+ 105 - 0
spec/system/system/object_manager_spec.rb

@@ -113,4 +113,109 @@ RSpec.describe 'Admin Panel > Objects', type: :system do
 
     expect(ObjectManager::Attribute.last.data_option['options']).to eq(expected_data_options)
   end
+
+  # https://github.com/zammad/zammad/issues/3647
+  context 'when setting Min/Max values for integer' do
+    before do
+      page.find('.js-new').click
+
+      in_modal disappears: false do
+        fill_in 'Name', with: 'integer1'
+        fill_in 'Display', with: 'Integer1'
+        page.find('select[name=data_type]').select('Integer')
+      end
+    end
+
+    it 'verifies max value does not go above limit' do
+      in_modal disappears: false do
+        fill_in 'Maximal', with: '999999999999'
+
+        page.find('.js-submit').click
+
+        expect(page).to have_text 'Data option max must be lower than 2147483648'
+      end
+    end
+
+    it 'verifies max value does not go below limit' do
+      in_modal disappears: false do
+        fill_in 'Maximal', with: '-999999999999'
+
+        page.find('.js-submit').click
+
+        expect(page).to have_text 'Data option max must be higher than -2147483648'
+      end
+    end
+
+    it 'verifies max value can be set' do
+      in_modal do
+        fill_in 'Maximal', with: '128'
+
+        page.find('.js-submit').click
+      end
+
+      expect(page).to have_text 'Integer1'
+    end
+
+    it 'verifies max value can be set to a negative value' do
+      in_modal do
+        fill_in 'Minimal', with: '-256'
+        fill_in 'Maximal', with: '-128'
+
+        page.find('.js-submit').click
+      end
+
+      expect(page).to have_text 'Integer1'
+    end
+
+    it 'verifies min value does not go above limit' do
+      in_modal disappears: false do
+        fill_in 'Minimal', with: '999999999999'
+
+        page.find('.js-submit').click
+
+        expect(page).to have_text 'Data option min must be lower than 2147483648'
+      end
+    end
+
+    it 'verifies min value does not go below limit' do
+      in_modal disappears: false do
+        fill_in 'Minimal', with: '-999999999999'
+
+        page.find('.js-submit').click
+
+        expect(page).to have_text 'Data option min must be higher than -2147483648'
+      end
+    end
+
+    it 'verifies min value can be set' do
+      in_modal do
+        fill_in 'Minimal', with: '128'
+
+        page.find('.js-submit').click
+      end
+
+      expect(page).to have_text 'Integer1'
+    end
+
+    it 'verifies min value can be set to a negative value' do
+      in_modal do
+        fill_in 'Minimal', with: '-128'
+
+        page.find('.js-submit').click
+      end
+
+      expect(page).to have_text 'Integer1'
+    end
+
+    it 'verifies min value must be lower than max' do
+      in_modal disappears: false do
+        fill_in 'Minimal', with: '128'
+        fill_in 'Maximal', with: '-128'
+
+        page.find('.js-submit').click
+
+        expect(page).to have_text 'Data option min must be lower than max'
+      end
+    end
+  end
 end