- Test stabilization: Overview prio rearrangement fails in the case that an Overview with the same prio as the one changed to exists.

- Migrated Overview MiniTest to RSpec.

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

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

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

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

test/unit/overview_test.rb

@@ -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