From 3ed8463e3ed06726fba4c763416af51e5e8d65b1 Mon Sep 17 00:00:00 2001 From: Ryan Lue Date: Thu, 18 Apr 2019 11:09:14 +0800 Subject: [PATCH] Refactoring: Clean up Ticket::Number & Ticket::Number::Increment classes --- app/models/ticket/number.rb | 15 ++-- app/models/ticket/number/base.rb | 27 +++++++ app/models/ticket/number/date.rb | 81 +++++-------------- app/models/ticket/number/increment.rb | 88 ++++++--------------- spec/models/ticket/number/date_spec.rb | 5 ++ spec/models/ticket/number/increment_spec.rb | 5 ++ 6 files changed, 87 insertions(+), 134 deletions(-) create mode 100644 app/models/ticket/number/base.rb diff --git a/app/models/ticket/number.rb b/app/models/ticket/number.rb index 66070816c..3e4db0cf8 100644 --- a/app/models/ticket/number.rb +++ b/app/models/ticket/number.rb @@ -16,13 +16,11 @@ returns =end def self.generate - - # generate number 49_999.times do number = adapter.generate - ticket = Ticket.find_by(number: number) - return number if !ticket + return number if !Ticket.exists?(number: number) end + raise "Can't generate new ticket number!" end @@ -42,12 +40,9 @@ returns adapter.check(string) end + # load backend based on config def self.adapter - - # load backend based on config - adapter_name = Setting.get('ticket_number') - raise 'Missing ticket_number setting option' if !adapter_name - - adapter_name.constantize + Setting.get('ticket_number')&.constantize || + raise('Missing ticket_number setting option') end end diff --git a/app/models/ticket/number/base.rb b/app/models/ticket/number/base.rb new file mode 100644 index 000000000..b94301808 --- /dev/null +++ b/app/models/ticket/number/base.rb @@ -0,0 +1,27 @@ +# Copyright (C) 2012-2019 Zammad Foundation, http://zammad-foundation.org/ +module Ticket::Number::Base + extend self + + private + + # The algorithm to calculate the checksum is derived from the one + # Deutsche Bundesbahn (german railway company) uses for calculation + # of the check digit of their vehikel numbering. + # The checksum is calculated by alternately multiplying the digits + # with 1 and 2 and adding the resulsts from left to right of the + # vehikel number. The modulus to 10 of this sum is substracted from + # 10. See: http://www.pruefziffernberechnung.de/F/Fahrzeugnummer.shtml + # (german) + def checksum(number) + chksum = 0 + mult = 1 + + number.to_s.split('').map(&:to_i).each do |digit| + chksum += digit * mult + mult = (mult % 3) + 1 + end + + chksum = 10 - (chksum % 10) + chksum.to_s[0] + end +end diff --git a/app/models/ticket/number/date.rb b/app/models/ticket/number/date.rb index 244609cb7..871b5d264 100644 --- a/app/models/ticket/number/date.rb +++ b/app/models/ticket/number/date.rb @@ -1,78 +1,30 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ module Ticket::Number::Date - module_function + extend Ticket::Number::Base - def generate + def self.generate + date = Time.zone.now.strftime('%F') - # get config - config = Setting.get('ticket_number_date') + counter = Ticket::Counter.create_with(content: '0') + .find_or_create_by(generator: 'Date') - t = Time.zone.now - date = t.strftime('%Y-%m-%d') - - # read counter - counter_increment = nil - Ticket::Counter.transaction do - counter = Ticket::Counter.where(generator: 'Date').lock(true).first - if !counter - counter = Ticket::Counter.new(generator: 'Date', content: '0') - end - - # increase counter - counter_increment, date_file = counter.content.to_s.split(';') - counter_increment = if date_file == date - counter_increment.to_i + 1 + counter.with_lock do + counter_increment = if counter.content.end_with?(date) + counter.content.split(';').first.to_i.next else 1 end - # store new counter value - counter.content = counter_increment.to_s + ';' + date - counter.save + counter.update(content: "#{counter_increment};#{date}") end - system_id = Setting.get('system_id') || '' - number = t.strftime('%Y%m%d') + system_id.to_s + format('%04d', counter_increment) + number = date.delete('-') + Setting.get('system_id').to_s + format('%04d', counter.content.split(';').first) + number += checksum(number) if config[:checksum] - # calculate a checksum - # The algorithm to calculate the checksum is derived from the one - # Deutsche Bundesbahn (german railway company) uses for calculation - # of the check digit of their vehikel numbering. - # The checksum is calculated by alternately multiplying the digits - # with 1 and 2 and adding the resulsts from left to right of the - # vehikel number. The modulus to 10 of this sum is substracted from - # 10. See: http://www.pruefziffernberechnung.de/F/Fahrzeugnummer.shtml - # (german) - - # fix for https://github.com/zammad/zammad/issues/413 - can be removed later - if config.class == FalseClass || config.class == TrueClass - config = { - checksum: config - } - end - - if config[:checksum] - chksum = 0 - mult = 1 - (1..number.length).each do |i| - digit = number.to_s[i, 1] - chksum = chksum + (mult * digit.to_i) - mult += 1 - if mult == 3 - mult = 1 - end - end - chksum %= 10 - chksum = 10 - chksum - if chksum == 10 - chksum = 1 - end - number += chksum.to_s - end number end - def check(string) + def self.check(string) return if string.blank? # get config @@ -100,4 +52,13 @@ module Ticket::Number::Date end ticket end + + def self.config + config = Setting.get('ticket_number_date') + return config if !config.in?([true, false]) + + # fix for https://github.com/zammad/zammad/issues/413 - can be removed later + { checksum: config } + end + private_class_method :config end diff --git a/app/models/ticket/number/increment.rb b/app/models/ticket/number/increment.rb index d104f6ed1..0db856caf 100644 --- a/app/models/ticket/number/increment.rb +++ b/app/models/ticket/number/increment.rb @@ -1,87 +1,40 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ module Ticket::Number::Increment - module_function + extend Ticket::Number::Base - def generate + def self.generate + counter = Ticket::Counter.create_with(content: '0') + .find_or_create_by(generator: 'Increment') - # get config - config = Setting.get('ticket_number_increment') - - # read counter - min_digs = config[:min_size] || 4 - counter_increment = nil - Ticket::Counter.transaction do - counter = Ticket::Counter.where(generator: 'Increment').lock(true).first - if !counter - counter = Ticket::Counter.new(generator: 'Increment', content: '0') - end - counter_increment = counter.content.to_i - - # increase counter - counter_increment += 1 - - # store new counter value - counter.content = counter_increment.to_s - counter.save + counter.with_lock do + counter.update(content: counter.content.to_i.next.to_s) end # fill up number counter - if config[:checksum] - min_digs = min_digs.to_i - 1 - end - fillup = Setting.get('system_id').to_s || '1' - 99.times do + head = (Setting.get('system_id') || 1).to_s + tail = counter.content - next if (fillup.length.to_i + counter_increment.to_s.length.to_i) >= min_digs.to_i + padding_length = (config[:min_size] || 4).to_i - head.length - tail.length + padding_length -= 1 if config[:checksum] + padding_length = 0 if padding_length.negative? + padding_length = 99 if padding_length > 99 - fillup = fillup + '0' - end - number = fillup.to_s + counter_increment.to_s + number = head + ('0' * padding_length) + tail + number += checksum(number) if config[:checksum] - # calculate a checksum - # The algorithm to calculate the checksum is derived from the one - # Deutsche Bundesbahn (german railway company) uses for calculation - # of the check digit of their vehikel numbering. - # The checksum is calculated by alternately multiplying the digits - # with 1 and 2 and adding the resulsts from left to right of the - # vehikel number. The modulus to 10 of this sum is substracted from - # 10. See: http://www.pruefziffernberechnung.de/F/Fahrzeugnummer.shtml - # (german) - if config[:checksum] - chksum = 0 - mult = 1 - (1..number.length).each do |i| - digit = number.to_s[i, 1] - chksum = chksum + (mult * digit.to_i) - mult += 1 - if mult == 3 - mult = 1 - end - end - chksum %= 10 - chksum = 10 - chksum - if chksum == 10 - chksum = 1 - end - number += chksum.to_s - end number end - def check(string) + def self.check(string) return if string.blank? # get config - system_id = Setting.get('system_id') || '' + system_id = Setting.get('ticket_number_ignore_system_id') ? '' : Setting.get('system_id').to_s ticket_hook = Setting.get('ticket_hook') - ticket_hook_divider = Setting.get('ticket_hook_divider') || '' + ticket_hook_divider = Setting.get('ticket_hook_divider').to_s ticket = nil - if Setting.get('ticket_number_ignore_system_id') == true - system_id = '' - end - # probe format # NOTE: we use `(?<=\W|^)` at the start of the regular expressions below # because `\b` fails when ticket_hook begins with a non-word character (like '#') @@ -89,12 +42,19 @@ module Ticket::Number::Increment ticket = Ticket.find_by(number: $1) break if ticket end + if !ticket string.scan(/(?<=\W|^)#{Regexp.quote(ticket_hook)}\s{0,2}(#{system_id}\d{2,48})\b/i) do ticket = Ticket.find_by(number: $1) break if ticket end end + ticket end + + def self.config + Setting.get('ticket_number_increment') + end + private_class_method :config end diff --git a/spec/models/ticket/number/date_spec.rb b/spec/models/ticket/number/date_spec.rb index f97cb04e5..6df58a7b9 100644 --- a/spec/models/ticket/number/date_spec.rb +++ b/spec/models/ticket/number/date_spec.rb @@ -6,6 +6,11 @@ RSpec.describe Ticket::Number::Date do before { travel_to(Time.zone.parse('1955-11-05')) } + it 'updates the "Date" Ticket::Counter' do + expect { number } + .to change { Ticket::Counter.find_by(generator: 'Date')&.content } + end + context 'with a "ticket_number_date" setting with checksum: false (default)' do context 'and a single-digit system_id' do before { Setting.set('system_id', 1) } diff --git a/spec/models/ticket/number/increment_spec.rb b/spec/models/ticket/number/increment_spec.rb index 899a4fd99..01a76f96f 100644 --- a/spec/models/ticket/number/increment_spec.rb +++ b/spec/models/ticket/number/increment_spec.rb @@ -6,6 +6,11 @@ RSpec.describe Ticket::Number::Increment do let(:system_id) { Setting.get('system_id') } let(:ticket_count) { Ticket::Counter.find_by(generator: 'Increment').content } + it 'updates the "Increment" Ticket::Counter' do + expect { number } + .to change { Ticket::Counter.find_by(generator: 'Increment').content } + end + context 'with a "ticket_number_increment" setting with...' do context 'min_size: 5' do before { Setting.set('ticket_number_increment', { min_size: 5 }) }