diff --git a/app/models/overview.rb b/app/models/overview.rb index 7f731f059..8740f7b87 100644 --- a/app/models/overview.rb +++ b/app/models/overview.rb @@ -22,16 +22,40 @@ class Overview < ApplicationModel private def rearrangement + # rearrange only in case of changed prio return true if !changes['prio'] - prio = 0 - Overview.all.order(prio: :asc, updated_at: :desc).pluck(:id).each do |overview_id| - prio += 1 + + previous_ordered_ids = self.class.all.order( + prio: :asc, + updated_at: :desc + ).pluck(:id) + + rearranged_prio = 0 + previous_ordered_ids.each do |overview_id| + + # don't process currently updated overview next if id == overview_id - Overview.without_callback(:update, :before, :rearrangement) do - overview = Overview.find(overview_id) - next if overview.prio == prio - overview.prio = prio - overview.save! + + rearranged_prio += 1 + + # increase rearranged prio by one to avoid a collition + # with the changed prio of current instance + if rearranged_prio == prio + rearranged_prio += 1 + end + + # don't start rearrange logic for overviews that alredy get rearranged + self.class.without_callback(:update, :before, :rearrangement) do + # fetch and update overview only if prio needs to change + overview = self.class.where( + id: overview_id + ).where.not( + prio: rearranged_prio + ).take + + next if overview.blank? + + overview.update!(prio: rearranged_prio) end end end diff --git a/spec/factories/overview.rb b/spec/factories/overview.rb index 71de26af2..551559d9e 100644 --- a/spec/factories/overview.rb +++ b/spec/factories/overview.rb @@ -1,9 +1,14 @@ +FactoryBot.define do + sequence :test_factory_name do |n| + "Test Overview #{n}" + end +end + FactoryBot.define do factory :overview do - name 'My Factory Tickets' - link 'my_factory_tickets' - prio 1100 + name { generate(:test_factory_name) } + prio 1 role_ids { [ Role.find_by(name: 'Customer').id, Role.find_by(name: 'Agent').id, Role.find_by(name: 'Admin').id ] } out_of_office true condition do diff --git a/spec/models/overview_spec.rb b/spec/models/overview_spec.rb new file mode 100644 index 000000000..eb14c81e9 --- /dev/null +++ b/spec/models/overview_spec.rb @@ -0,0 +1,75 @@ +require 'rails_helper' + +RSpec.describe Overview do + + context 'link generation' do + + it 'generates from name' do + overview = create(:overview, name: 'Not Shown Admin 2') + expect(overview.link).to eq('not_shown_admin_2') + end + + it 'ensures uniquenes' do + overview1, overview2, overview3 = create_list(:overview, 3, name: 'Übersicht') + + expect(overview1.link).not_to eq(overview2.link) + expect(overview1.link).not_to eq(overview3.link) + expect(overview2.link).not_to eq(overview3.link) + end + + context 'given link' do + + it 'keeps on create' do + overview = create(:overview, name: 'Übersicht', link: 'my_overview') + expect(overview.link).to eq('my_overview') + end + + it 'keeps on update' do + overview = create(:overview, name: 'Übersicht') + overview.update!(link: 'my_overview_2') + expect(overview.link).to eq('my_overview_2') + end + end + + context 'URL save' do + + it 'handles umlauts' do + overview = create(:overview, name: 'Übersicht') + expect(overview.link).to eq('ubersicht') + end + + it 'handles spaces' do + overview = create(:overview, name: " Meine Übersicht \n") + expect(overview.link).to eq('meine_ubersicht') + end + + it 'handles special chars' do + overview = create(:overview, name: 'Д дФ ф') + expect(overview.link).to match(/^\d{1,3}$/) + end + + it 'removes special char fallback if possible' do + overview = create(:overview, name: ' Д дФ ф abc ') + expect(overview.link).to eq('abc') + end + end + end + + describe '#rearrangement' do + + it 'rearranges prio of other overviews on prio change' do + + overview1 = create(:overview, prio: 1) + overview2 = create(:overview, prio: 2) + overview3 = create(:overview, prio: 3) + + overview2.update!(prio: 3) + + overviews = described_class.all.order(prio: :asc).pluck(:id) + + expect(overviews.first).to eq(overview1.id) + expect(overviews.second).to eq(overview3.id) + expect(overviews.third).to eq(overview2.id) + end + end +end diff --git a/spec/models/ticket/overviews_spec.rb b/spec/models/ticket/overviews_spec.rb index 2ba80203d..f8ae6119c 100644 --- a/spec/models/ticket/overviews_spec.rb +++ b/spec/models/ticket/overviews_spec.rb @@ -27,7 +27,7 @@ RSpec.describe Ticket::Overviews do overview = create(:overview, condition: condition) result = Ticket::Overviews.index(user) - result = result.select { |x| x[:overview][:name] == 'My Factory Tickets' } + result = result.select { |x| x[:overview][:name] == overview.name } expect(result.count).to be == 1 expect(result[0][:count]).to be == 2 diff --git a/test/unit/overview_test.rb b/test/unit/overview_test.rb deleted file mode 100644 index 384576ca7..000000000 --- a/test/unit/overview_test.rb +++ /dev/null @@ -1,388 +0,0 @@ - -require 'test_helper' - -class OverviewTest < ActiveSupport::TestCase - - test 'overview link' do - UserInfo.current_user_id = 1 - roles = Role.where(name: 'Agent') - - overview = Overview.create!( - name: 'Not Shown Admin 2', - roles: roles, - condition: { - 'ticket.state_id' => { - operator: 'is', - value: [1, 2, 3], - }, - }, - order: { - by: 'created_at', - direction: 'DESC', - }, - view: { - d: %w[title customer state created_at], - s: %w[number title customer state created_at], - m: %w[number title customer state created_at], - view_mode_default: 's', - }, - ) - assert_equal(overview.link, 'not_shown_admin_2') - overview.destroy! - - overview = Overview.create!( - name: 'My assigned Tickets 2', - roles: roles, - condition: { - 'ticket.state_id' => { - operator: 'is', - value: [1, 2, 3], - }, - }, - order: { - by: 'created_at', - direction: 'DESC', - }, - view: { - d: %w[title customer state created_at], - s: %w[number title customer state created_at], - m: %w[number title customer state created_at], - view_mode_default: 's', - }, - ) - assert_equal(overview.link, 'my_assigned_tickets_2') - overview.destroy! - - overview = Overview.create!( - name: 'Übersicht', - roles: roles, - condition: { - 'ticket.state_id' => { - operator: 'is', - value: [1, 2, 3], - }, - }, - order: { - by: 'created_at', - direction: 'DESC', - }, - view: { - d: %w[title customer state created_at], - s: %w[number title customer state created_at], - m: %w[number title customer state created_at], - view_mode_default: 's', - }, - ) - assert_equal(overview.link, 'ubersicht') - overview.destroy! - - overview = Overview.create!( - name: " Übersicht \n", - roles: roles, - condition: { - 'ticket.state_id' => { - operator: 'is', - value: [1, 2, 3], - }, - }, - order: { - by: 'created_at', - direction: 'DESC', - }, - view: { - d: %w[title customer state created_at], - s: %w[number title customer state created_at], - m: %w[number title customer state created_at], - view_mode_default: 's', - }, - ) - assert_equal(overview.link, 'ubersicht') - overview.destroy! - - overview1 = Overview.create!( - name: 'Meine Übersicht', - roles: roles, - condition: { - 'ticket.state_id' => { - operator: 'is', - value: [1, 2, 3], - }, - }, - order: { - by: 'created_at', - direction: 'DESC', - }, - view: { - d: %w[title customer state created_at], - s: %w[number title customer state created_at], - m: %w[number title customer state created_at], - view_mode_default: 's', - }, - ) - assert_equal(overview1.link, 'meine_ubersicht') - overview2 = Overview.create!( - name: 'Meine Übersicht', - roles: roles, - condition: { - 'ticket.state_id' => { - operator: 'is', - value: [1, 2, 3], - }, - }, - order: { - by: 'created_at', - direction: 'DESC', - }, - view: { - d: %w[title customer state created_at], - s: %w[number title customer state created_at], - m: %w[number title customer state created_at], - view_mode_default: 's', - }, - ) - assert(overview2.link.start_with?('meine_ubersicht')) - assert_not_equal(overview1.link, overview2.link) - overview1.destroy! - overview2.destroy! - - overview = Overview.create!( - name: 'Д дФ ф', - roles: roles, - condition: { - 'ticket.state_id' => { - operator: 'is', - value: [1, 2, 3], - }, - }, - order: { - by: 'created_at', - direction: 'DESC', - }, - view: { - d: %w[title customer state created_at], - s: %w[number title customer state created_at], - m: %w[number title customer state created_at], - view_mode_default: 's', - }, - ) - assert_match(/^\d{1,3}$/, overview.link) - overview.destroy! - - overview = Overview.create!( - name: ' Д дФ ф abc ', - roles: roles, - condition: { - 'ticket.state_id' => { - operator: 'is', - value: [1, 2, 3], - }, - }, - order: { - by: 'created_at', - direction: 'DESC', - }, - view: { - d: %w[title customer state created_at], - s: %w[number title customer state created_at], - m: %w[number title customer state created_at], - view_mode_default: 's', - }, - ) - assert_equal(overview.link, 'abc') - overview.destroy! - - overview = Overview.create!( - name: 'Übersicht', - link: 'my_overview', - roles: roles, - condition: { - 'ticket.state_id' => { - operator: 'is', - value: [1, 2, 3], - }, - }, - order: { - by: 'created_at', - direction: 'DESC', - }, - view: { - d: %w[title customer state created_at], - s: %w[number title customer state created_at], - m: %w[number title customer state created_at], - view_mode_default: 's', - }, - ) - assert_equal(overview.link, 'my_overview') - - overview.name = 'Übersicht2' - overview.link = 'my_overview2' - overview.save! - - assert_equal(overview.link, 'my_overview2') - - overview.destroy! - end - - test 'same url' do - UserInfo.current_user_id = 1 - - roles = Role.where(name: 'Agent') - - overview1 = Overview.create!( - name: 'My own assigned Tickets', - roles: roles, - condition: { - 'ticket.state_id' => { - operator: 'is', - value: [1, 2, 3], - }, - }, - order: { - by: 'created_at', - direction: 'DESC', - }, - view: { - d: %w[title customer state created_at], - s: %w[number title customer state created_at], - m: %w[number title customer state created_at], - view_mode_default: 's', - }, - ) - assert_equal(overview1.link, 'my_own_assigned_tickets') - - overview2 = Overview.create!( - name: 'My own assigned Tickets', - roles: roles, - condition: { - 'ticket.state_id' => { - operator: 'is', - value: [1, 2, 3], - }, - }, - order: { - by: 'created_at', - direction: 'DESC', - }, - view: { - d: %w[title customer state created_at], - s: %w[number title customer state created_at], - m: %w[number title customer state created_at], - view_mode_default: 's', - }, - ) - assert_equal(overview2.link, 'my_own_assigned_tickets_1') - - overview3 = Overview.create!( - name: 'My own assigned Tickets', - roles: roles, - condition: { - 'ticket.state_id' => { - operator: 'is', - value: [1, 2, 3], - }, - }, - order: { - by: 'created_at', - direction: 'DESC', - }, - view: { - d: %w[title customer state created_at], - s: %w[number title customer state created_at], - m: %w[number title customer state created_at], - view_mode_default: 's', - }, - ) - assert_equal(overview3.link, 'my_own_assigned_tickets_2') - - overview1.destroy! - overview2.destroy! - overview3.destroy! - end - - test 'priority rearrangement' do - UserInfo.current_user_id = 1 - - roles = Role.where(name: 'Agent') - - overview1 = Overview.create!( - name: 'Overview1', - link: 'my_overview', - roles: roles, - condition: { - 'ticket.state_id' => { - operator: 'is', - value: [1, 2, 3], - }, - }, - order: { - by: 'created_at', - direction: 'DESC', - }, - view: { - d: %w[title customer state created_at], - s: %w[number title customer state created_at], - m: %w[number title customer state created_at], - view_mode_default: 's', - }, - prio: 1, - ) - - overview2 = Overview.create!( - name: 'Overview2', - link: 'my_overview', - roles: roles, - condition: { - 'ticket.state_id' => { - operator: 'is', - value: [1, 2, 3], - }, - }, - order: { - by: 'created_at', - direction: 'DESC', - }, - view: { - d: %w[title customer state created_at], - s: %w[number title customer state created_at], - m: %w[number title customer state created_at], - view_mode_default: 's', - }, - prio: 2, - ) - - overview3 = Overview.create!( - name: 'Overview3', - link: 'my_overview', - roles: roles, - condition: { - 'ticket.state_id' => { - operator: 'is', - value: [1, 2, 3], - }, - }, - order: { - by: 'created_at', - direction: 'DESC', - }, - view: { - d: %w[title customer state created_at], - s: %w[number title customer state created_at], - m: %w[number title customer state created_at], - view_mode_default: 's', - }, - prio: 3, - ) - - overview2.prio = 3 - overview2.save! - - overviews = Overview.all.order(prio: :asc).pluck(:id) - assert_equal(overview1.id, overviews[0]) - assert_equal(overview3.id, overviews[1]) - assert_equal(overview2.id, overviews[2]) - - overview1.destroy! - overview2.destroy! - overview3.destroy! - end -end