jro_apps/redmine Commit - r14953:64afa24a7f72

Replaces acts_as_list with an implementation that handles #position= (#12909)....

Jean-Philippe Lang -

r14953:64afa24a7f72

parent child

Context file:

r14953:64afa24a7f72

Collapse all files

db/migrate/20160416072926_remove_position_defaults.rb created 644 +13 0

@@ -0,0 +1,13
	1	class RemovePositionDefaults < ActiveRecord::Migration
	2	def up
	3	[Board, CustomField, Enumeration, IssueStatus, Role, Tracker].each do \|klass\|
	4	change_column klass.table_name, :position, :integer, :default => nil
	5	end
	6	end
	7
	8	def down
	9	[Board, CustomField, Enumeration, IssueStatus, Role, Tracker].each do \|klass\|
	10	change_column klass.table_name, :position, :integer, :default => 1
	11	end
	12	end
	13	end

lib/redmine/acts/positioned.rb created 644 +134 0

@@ -0,0 +1,134
	1	# Redmine - project management software
	2	# Copyright (C) 2006-2016 Jean-Philippe Lang
	3	#
	4	# This program is free software; you can redistribute it and/or
	5	# modify it under the terms of the GNU General Public License
	6	# as published by the Free Software Foundation; either version 2
	7	# of the License, or (at your option) any later version.
	8	#
	9	# This program is distributed in the hope that it will be useful,
	10	# but WITHOUT ANY WARRANTY; without even the implied warranty of
	11	# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
	12	# GNU General Public License for more details.
	13	#
	14	# You should have received a copy of the GNU General Public License
	15	# along with this program; if not, write to the Free Software
	16	# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
	17
	18	module Redmine
	19	module Acts
	20	module Positioned
	21	def self.included(base)
	22	base.extend ClassMethods
	23	end
	24
	25	# This extension provides the capabilities for reordering objects in a list.
	26	# The class needs to have a +position+ column defined as an integer on the
	27	# mapped database table.
	28	module ClassMethods
	29	# Configuration options are:
	30	#
	31	# * +scope+ - restricts what is to be considered a list. Must be a symbol
	32	# or an array of symbols
	33	def acts_as_positioned(options = {})
	34	class_attribute :positioned_options
	35	self.positioned_options = {:scope => Array(options[:scope])}
	36
	37	send :include, Redmine::Acts::Positioned::InstanceMethods
	38
	39	before_save :set_default_position
	40	after_save :update_position
	41	after_destroy :remove_position
	42	end
	43	end
	44
	45	module InstanceMethods
	46	def self.included(base)
	47	base.extend ClassMethods
	48	end
	49
	50	# Move to the given position
	51	# For compatibility with the previous way of sorting items
	52	def move_to=(pos)
	53	case pos.to_s
	54	when 'highest'
	55	self.position = 1
	56	when 'higher'
	57	self.position -= 1 if position > 1
	58	when 'lower'
	59	self.position += 1
	60	when 'lowest'
	61	self.position = nil
	62	set_default_position
	63	end
	64	end
	65
	66	private
	67
	68	def position_scope
	69	build_position_scope {\|c\| send(c)}
	70	end
	71
	72	def position_scope_was
	73	build_position_scope {\|c\| send("#{c}_was")}
	74	end
	75
	76	def build_position_scope
	77	condition_hash = self.class.positioned_options[:scope].inject({}) do \|h, column\|
	78	h[column] = yield(column)
	79	h
	80	end
	81	self.class.where(condition_hash)
	82	end
	83
	84	def set_default_position
	85	if position.nil?
	86	self.position = position_scope.maximum(:position).to_i + (new_record? ? 1 : 0)
	87	end
	88	end
	89
	90	def update_position
	91	if !new_record? && position_scope_changed?
	92	remove_position
	93	insert_position
	94	elsif position_changed?
	95	if position_was.nil?
	96	insert_position
	97	else
	98	shift_positions
	99	end
	100	end
	101	end
	102
	103	def insert_position
	104	position_scope.where("position >= ? AND id <> ?", position, id).update_all("position = position + 1")
	105	end
	106
	107	def remove_position
	108	position_scope_was.where("position >= ? AND id <> ?", position_was, id).update_all("position = position - 1")
	109	end
	110
	111	def position_scope_changed?
	112	(changed & self.class.positioned_options[:scope].map(&:to_s)).any?
	113	end
	114
	115	def shift_positions
	116	offset = position_was <=> position
	117	min, max = [position, position_was].sort
	118	r = position_scope.where("id <> ? AND position BETWEEN ? AND ?", id, min, max).update_all("position = position + #{offset}")
	119	if r != max - min
	120	reset_positions_in_list
	121	end
	122	end
	123
	124	def reset_positions_in_list
	125	position_scope.reorder(:position, :id).pluck(:id).each_with_index do \|record_id, p\|
	126	self.class.where(:id => record_id).update_all(:position => p+1)
	127	end
	128	end
	129	end
	130	end
	131	end
	132	end
	133
	134	ActiveRecord::Base.send :include, Redmine::Acts::Positioned

test/unit/lib/redmine/acts/positioned_with_scope_test.rb created 644 +53 0

@@ -0,0 +1,53
	1	# Redmine - project management software
	2	# Copyright (C) 2006-2016 Jean-Philippe Lang
	3	#
	4	# This program is free software; you can redistribute it and/or
	5	# modify it under the terms of the GNU General Public License
	6	# as published by the Free Software Foundation; either version 2
	7	# of the License, or (at your option) any later version.
	8	#
	9	# This program is distributed in the hope that it will be useful,
	10	# but WITHOUT ANY WARRANTY; without even the implied warranty of
	11	# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
	12	# GNU General Public License for more details.
	13	#
	14	# You should have received a copy of the GNU General Public License
	15	# along with this program; if not, write to the Free Software
	16	# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
	17
	18	require File.expand_path('../../../../../test_helper', __FILE__)
	19
	20	class Redmine::Acts::PositionedWithScopeTest < ActiveSupport::TestCase
	21	fixtures :projects, :boards
	22
	23	def test_create_should_default_to_last_position
	24	b = Board.generate!(:project_id => 1)
	25	assert_equal 3, b.reload.position
	26
	27	b = Board.generate!(:project_id => 3)
	28	assert_equal 1, b.reload.position
	29	end
	30
	31	def test_create_should_insert_at_given_position
	32	b = Board.generate!(:project_id => 1, :position => 2)
	33
	34	assert_equal 2, b.reload.position
	35	assert_equal [1, 3, 1, 2], Board.order(:id).pluck(:position)
	36	end
	37
	38	def test_destroy_should_remove_position
	39	b = Board.generate!(:project_id => 1, :position => 2)
	40	b.destroy
	41
	42	assert_equal [1, 2, 1], Board.order(:id).pluck(:position)
	43	end
	44
	45	def test_update_should_update_positions
	46	b = Board.generate!(:project_id => 1)
	47	assert_equal 3, b.position
	48
	49	b.position = 2
	50	b.save!
	51	assert_equal [1, 3, 1, 2], Board.order(:id).pluck(:position)
	52	end
	53	end

test/unit/lib/redmine/acts/positioned_without_scope_test.rb created 644 +55 0

@@ -0,0 +1,55
	1	# Redmine - project management software
	2	# Copyright (C) 2006-2016 Jean-Philippe Lang
	3	#
	4	# This program is free software; you can redistribute it and/or
	5	# modify it under the terms of the GNU General Public License
	6	# as published by the Free Software Foundation; either version 2
	7	# of the License, or (at your option) any later version.
	8	#
	9	# This program is distributed in the hope that it will be useful,
	10	# but WITHOUT ANY WARRANTY; without even the implied warranty of
	11	# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
	12	# GNU General Public License for more details.
	13	#
	14	# You should have received a copy of the GNU General Public License
	15	# along with this program; if not, write to the Free Software
	16	# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
	17
	18	require File.expand_path('../../../../../test_helper', __FILE__)
	19
	20	class Redmine::Acts::PositionedWithoutScopeTest < ActiveSupport::TestCase
	21	fixtures :trackers, :issue_statuses
	22
	23	def test_create_should_default_to_last_position
	24	t = Tracker.generate
	25	t.save!
	26
	27	assert_equal 4, t.reload.position
	28	end
	29
	30	def test_create_should_insert_at_given_position
	31	t = Tracker.generate
	32	t.position = 2
	33	t.save!
	34
	35	assert_equal 2, t.reload.position
	36	assert_equal [1, 3, 4, 2], Tracker.order(:id).pluck(:position)
	37	end
	38
	39	def test_destroy_should_remove_position
	40	t = Tracker.generate!
	41	Tracker.generate!
	42	t.destroy
	43
	44	assert_equal [1, 2, 3, 4], Tracker.order(:id).pluck(:position)
	45	end
	46
	47	def test_update_should_update_positions
	48	t = Tracker.generate!
	49	assert_equal 4, t.position
	50
	51	t.position = 2
	52	t.save!
	53	assert_equal [1, 3, 4, 2], Tracker.order(:id).pluck(:position)
	54	end
	55	end

app/models/board.rb +1 -1

               has_many :messages, lambda {order("#{Message.table_name}.created_on DESC")}, :dependent => :destroy
               belongs_to :last_message, :class_name => 'Message'
               acts_as_tree :dependent => :nullify
-              acts_as_list :scope => '(project_id = #{project_id} AND parent_id #{parent_id ? "= #{parent_id}" : "IS NULL"})'
+              acts_as_positioned :scope => [:project_id, :parent_id]
               acts_as_watchable
               validates_presence_of :name, :description

app/models/custom_field.rb +1 -1

                        :dependent => :delete_all
               has_many :custom_values, :dependent => :delete_all
               has_and_belongs_to_many :roles, :join_table => "#{table_name_prefix}custom_fields_roles#{table_name_suffix}", :foreign_key => "custom_field_id"
-              acts_as_list :scope => 'type = \'#{self.class}\''
+              acts_as_positioned
               serialize :possible_values
               store :format_store

app/models/enumeration.rb +23 -18

               belongs_to :project
-              acts_as_list :scope => 'type = \'#{type}\' AND #{parent_id ? "parent_id = #{parent_id}" : "parent_id IS NULL"}'
+              acts_as_positioned :scope => :parent_id
               acts_as_customizable
               acts_as_tree
                 return new == previous
               end
-              # Overrides acts_as_list reset_positions_in_list so that enumeration overrides
+              private
-              # get the same position as the overriden enumeration
-              def reset_positions_in_list
-                acts_as_list_class.where(scope_condition).reorder("#{position_column} ASC, id ASC").each_with_index do |item, i|
-                  acts_as_list_class.where("id = :id OR parent_id = :id", :id => item.id).
-                    update_all({position_column => (i + 1)})
-                end
-              end
-            private
               def check_integrity
                 raise "Cannot delete enumeration" if self.in_use?
               end
-              # Overrides acts_as_list add_to_list_bottom so that enumeration overrides
+              # Overrides Redmine::Acts::Positioned#set_default_position so that enumeration overrides
               # get the same position as the overriden enumeration
-              def add_to_list_bottom
+              def set_default_position
-                if parent
+                if position.nil? && parent
-                  self[position_column] = parent.position
+                  self.position = parent.position
-                else
+                end
-                  super
+                super
+              end
+              # Overrides Redmine::Acts::Positioned#update_position so that overrides get the same
+              # position as the overriden enumeration
+              def update_position
+                super
+                if position_changed?
+                  self.class.where.not(:parent_id => nil).update_all(
+                    "position = coalesce((
+                      select position
+                      from (select id, position from enumerations) as parent
+                      where parent_id = parent.id), 1)"
+                  )
                 end
               end
-              # Overrides acts_as_list remove_from_list so that enumeration overrides
+              # Overrides Redmine::Acts::Positioned#remove_position so that enumeration overrides
               # get the same position as the overriden enumeration
-              def remove_from_list
+              def remove_position
                 if parent_id.blank?
                   super
                 end

app/models/issue_status.rb +1 -1

               before_destroy :check_integrity
               has_many :workflows, :class_name => 'WorkflowTransition', :foreign_key => "old_status_id"
               has_many :workflow_transitions_as_new_status, :class_name => 'WorkflowTransition', :foreign_key => "new_status_id"
-              acts_as_list
+              acts_as_positioned
               after_update :handle_is_closed_change
               before_destroy :delete_workflow_rules

app/models/role.rb +3 -3

               has_many :member_roles, :dependent => :destroy
               has_many :members, :through => :member_roles
-              acts_as_list
+              acts_as_positioned :scope => :builtin
               serialize :permissions, ::Role::PermissionsAttributeCoder
               attr_protected :builtin
               def self.find_or_create_system_role(builtin, name)
                 role = where(:builtin => builtin).first
                 if role.nil?
-                  role = create(:name => name, :position => 0) do |r|
+                  role = create(:name => name) do |r|
                     r.builtin = builtin
                   end
-                  raise "Unable to create the #{name} role." if role.new_record?
+                  raise "Unable to create the #{name} role (#{role.errors.full_messages.join(',')})." if role.new_record?
                 end
                 role
               end

app/models/tracker.rb +1 -1

               has_and_belongs_to_many :projects
               has_and_belongs_to_many :custom_fields, :class_name => 'IssueCustomField', :join_table => "#{table_name_prefix}custom_fields_trackers#{table_name_suffix}", :association_foreign_key => 'custom_field_id'
-              acts_as_list
+              acts_as_positioned
               attr_protected :fields_bits

lib/redmine.rb +2 0

               # Redcarpet is not available
             end
+            require 'redmine/acts/positioned'
             require 'redmine/scm/base'
             require 'redmine/access_control'
             require 'redmine/access_keys'

test/fixtures/custom_fields.yml +1 0

                 SGXDqWzDp2prc2Tigqw2NTTDuQ==
               - Other value
               field_format: list
+              position: 1

test/fixtures/enumerations.yml +2 0

               type: Enumeration
               is_default: true
               active: true
+              position: 1
             enumerations_013:
               name: Another Enumeration
               id: 13
               type: Enumeration
               active: true
+              position: 2
             enumerations_014:
               name: Inactive Activity
               id: 14

test/fixtures/roles.yml +2 -2

                 - :browse_repository
                 - :view_changesets
-              position: 4
+              position: 1
             roles_005:
               name: Anonymous
               id: 5
                 - :browse_repository
                 - :view_changesets
-              position: 5
+              position: 1

test/functional/roles_controller_test.rb +1 -1

               def test_move_lowest
                 put :update, :id => 2, :role => {:move_to => 'lowest'}
                 assert_redirected_to '/roles'
-                assert_equal Role.count, Role.find(2).position
+                assert_equal Role.givable.count, Role.find(2).position
               end
             end

test/object_helpers.rb +6 -1

                 status
               end
-              def Tracker.generate!(attributes={})
+              def Tracker.generate(attributes={})
                 @generated_tracker_name ||= 'Tracker 0'
                 @generated_tracker_name.succ!
                 tracker = Tracker.new(attributes)
                 tracker.name = @generated_tracker_name.dup if tracker.name.blank?
                 tracker.default_status ||= IssueStatus.order('position').first || IssueStatus.generate!
                 yield tracker if block_given?
+                tracker
+              end
+              def Tracker.generate!(attributes={}, &block)
+                tracker = Tracker.generate(attributes, &block)
                 tracker.save!
                 tracker
               end

test/unit/enumeration_test.rb +1 0

                 b = IssuePriority.create!(:name => 'B')
                 override = IssuePriority.create!(:name => 'BB', :parent_id => b.id)
                 b.move_to = 'higher'
+                b.save!
                 assert_equal [2, 1, 1], [a, b, override].map(&:reload).map(&:position)
               end

test/unit/issue_priority_test.rb +1 -19

@@ -55,25 +55,6 class IssuePriorityTest < ActiveSupport::TestCase
55	assert_equal [1, 2, 3], priorities.map(&:position)	55	assert_equal [1, 2, 3], priorities.map(&:position)
56	end	56	end
57		57
58	def test_reset_positions_in_list_should_set_sequential_positions
59	IssuePriority.delete_all
60
61	priorities = [1, 2, 3].map {\|i\| IssuePriority.create!(:name => "P#{i}")}
62	priorities[0].update_attribute :position, 4
63	priorities[1].update_attribute :position, 2
64	priorities[2].update_attribute :position, 7
65	assert_equal [4, 2, 7], priorities.map(&:reload).map(&:position)
66
67	priorities[0].reset_positions_in_list
68	assert_equal [2, 1, 3], priorities.map(&:reload).map(&:position)
69	end
70
71	def test_moving_in_list_should_reset_positions
72	priority = IssuePriority.first
73	priority.expects(:reset_positions_in_list).once
74	priority.move_to = 'higher'
75	end
76
77	def test_clear_position_names_should_set_position_names_to_nil	58	def test_clear_position_names_should_set_position_names_to_nil
78	IssuePriority.clear_position_names	59	IssuePriority.clear_position_names
79	assert IssuePriority.all.all? {\|priority\| priority.position_name.nil?}	60	assert IssuePriority.all.all? {\|priority\| priority.position_name.nil?}
@@ -102,6 +83,7 class IssuePriorityTest < ActiveSupport::TestCase
102	def test_moving_a_priority_should_update_position_names	83	def test_moving_a_priority_should_update_position_names
103	prio = IssuePriority.first	84	prio = IssuePriority.first
104	prio.move_to = 'lowest'	85	prio.move_to = 'lowest'
		86	prio.save!
105	prio.reload	87	prio.reload
106	assert_equal 'highest', prio.position_name	88	assert_equal 'highest', prio.position_name
107	end	89	end

General Comments 0

Write
Preview

You need to be logged in to leave comments. Login now

No TODOs yet

	Site-wide shortcuts
/	Use quick search box
g h	Goto home page
g g	Goto my private gists page
g G	Goto my public gists page
g 0-9	Goto bookmarked items from 0-9
n r	New repository page
n g	New gist page

	Repositories
g s	Goto summary page
g c	Goto changelog page
g f	Goto files page
g F	Goto files page with file search activated
g p	Goto pull requests page
g o	Goto repository settings
g O	Goto repository access permissions settings
t s	Toggle sidebar on some pages