diff options
Diffstat (limited to 'app/models/concerns')
| -rw-r--r-- | app/models/concerns/issuable.rb | 34 | ||||
| -rw-r--r-- | app/models/concerns/relative_positioning.rb | 90 |
2 files changed, 95 insertions, 29 deletions
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 3cf4c67d7e7..91f4eb13ecc 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -144,7 +144,8 @@ module Issuable when 'milestone_due_desc' then order_milestone_due_desc when 'downvotes_desc' then order_downvotes_desc when 'upvotes_desc' then order_upvotes_desc - when 'priority' then order_labels_priority(excluded_labels: excluded_labels) + when 'label_priority' then order_labels_priority(excluded_labels: excluded_labels) + when 'priority' then order_due_date_and_labels_priority(excluded_labels: excluded_labels) when 'position_asc' then order_position_asc else order_by(method) @@ -154,7 +155,28 @@ module Issuable sorted.order(id: :desc) end - def order_labels_priority(excluded_labels: []) + def order_due_date_and_labels_priority(excluded_labels: []) + # The order_ methods also modify the query in other ways: + # + # - For milestones, we add a JOIN. + # - For label priority, we change the SELECT, and add a GROUP BY.# + # + # After doing those, we need to reorder to the order we want. The existing + # ORDER BYs won't work because: + # + # 1. We need milestone due date first. + # 2. We can't ORDER BY a column that isn't in the GROUP BY and doesn't + # have an aggregate function applied, so we do a useless MIN() instead. + # + milestones_due_date = 'MIN(milestones.due_date)' + + order_milestone_due_asc. + order_labels_priority(excluded_labels: excluded_labels, extra_select_columns: [milestones_due_date]). + reorder(Gitlab::Database.nulls_last_order(milestones_due_date, 'ASC'), + Gitlab::Database.nulls_last_order('highest_priority', 'ASC')) + end + + def order_labels_priority(excluded_labels: [], extra_select_columns: []) params = { target_type: name, target_column: "#{table_name}.id", @@ -164,7 +186,12 @@ module Issuable highest_priority = highest_label_priority(params).to_sql - select("#{table_name}.*, (#{highest_priority}) AS highest_priority"). + select_columns = [ + "#{table_name}.*", + "(#{highest_priority}) AS highest_priority" + ] + extra_select_columns + + select(select_columns.join(', ')). group(arel_table[:id]). reorder(Gitlab::Database.nulls_last_order('highest_priority', 'ASC')) end @@ -234,6 +261,7 @@ module Issuable user: user.hook_attrs, project: project.hook_attrs, object_attributes: hook_attrs, + labels: labels.map(&:hook_attrs), # DEPRECATED repository: project.hook_attrs.slice(:name, :url, :description, :homepage) } diff --git a/app/models/concerns/relative_positioning.rb b/app/models/concerns/relative_positioning.rb index 603f2dd7e5d..f1d8532a6d6 100644 --- a/app/models/concerns/relative_positioning.rb +++ b/app/models/concerns/relative_positioning.rb @@ -2,16 +2,14 @@ module RelativePositioning extend ActiveSupport::Concern MIN_POSITION = 0 + START_POSITION = Gitlab::Database::MAX_INT_VALUE / 2 MAX_POSITION = Gitlab::Database::MAX_INT_VALUE + IDEAL_DISTANCE = 500 included do after_save :save_positionable_neighbours end - def min_relative_position - self.class.in_projects(project.id).minimum(:relative_position) - end - def max_relative_position self.class.in_projects(project.id).maximum(:relative_position) end @@ -26,7 +24,7 @@ module RelativePositioning maximum(:relative_position) end - prev_pos || MIN_POSITION + prev_pos end def next_relative_position @@ -39,55 +37,95 @@ module RelativePositioning minimum(:relative_position) end - next_pos || MAX_POSITION + next_pos end def move_between(before, after) return move_after(before) unless after return move_before(after) unless before + # If there is no place to insert an issue we need to create one by moving the before issue closer + # to its predecessor. This process will recursively move all the predecessors until we have a place + if (after.relative_position - before.relative_position) < 2 + before.move_before + @positionable_neighbours = [before] + end + + self.relative_position = position_between(before.relative_position, after.relative_position) + end + + def move_after(before = self) pos_before = before.relative_position + pos_after = before.next_relative_position + + if before.shift_after? + issue_to_move = self.class.in_projects(project.id).find_by!(relative_position: pos_after) + issue_to_move.move_after + @positionable_neighbours = [issue_to_move] + + pos_after = issue_to_move.relative_position + end + + self.relative_position = position_between(pos_before, pos_after) + end + + def move_before(after = self) pos_after = after.relative_position + pos_before = after.prev_relative_position - if pos_after && (pos_before == pos_after) - self.relative_position = pos_before - before.move_before(self) - after.move_after(self) + if after.shift_before? + issue_to_move = self.class.in_projects(project.id).find_by!(relative_position: pos_before) + issue_to_move.move_before + @positionable_neighbours = [issue_to_move] - @positionable_neighbours = [before, after] - else - self.relative_position = position_between(pos_before, pos_after) + pos_before = issue_to_move.relative_position end + + self.relative_position = position_between(pos_before, pos_after) end - def move_before(after) - self.relative_position = position_between(after.prev_relative_position, after.relative_position) + def move_to_end + self.relative_position = position_between(max_relative_position || START_POSITION, MAX_POSITION) end - def move_after(before) - self.relative_position = position_between(before.relative_position, before.next_relative_position) + # Indicates if there is an issue that should be shifted to free the place + def shift_after? + next_pos = next_relative_position + next_pos && (next_pos - relative_position) == 1 end - def move_to_end - self.relative_position = position_between(max_relative_position, MAX_POSITION) + # Indicates if there is an issue that should be shifted to free the place + def shift_before? + prev_pos = prev_relative_position + prev_pos && (relative_position - prev_pos) == 1 end private # This method takes two integer values (positions) and - # calculates some random position between them. The range is huge as - # the maximum integer value is 2147483647. Ideally, the calculated value would be - # exactly between those terminating values, but this will introduce possibility of a race condition - # so two or more issues can get the same value, we want to avoid that and we also want to avoid - # using a lock here. If we have two issues with distance more than one thousand, we are OK. - # Given the huge range of possible values that integer can fit we shoud never face a problem. + # calculates the position between them. The range is huge as + # the maximum integer value is 2147483647. We are incrementing position by IDEAL_DISTANCE * 2 every time + # when we have enough space. If distance is less then IDEAL_DISTANCE we are calculating an average number def position_between(pos_before, pos_after) pos_before ||= MIN_POSITION pos_after ||= MAX_POSITION pos_before, pos_after = [pos_before, pos_after].sort - rand(pos_before.next..pos_after.pred) + halfway = (pos_after + pos_before) / 2 + distance_to_halfway = pos_after - halfway + + if distance_to_halfway < IDEAL_DISTANCE + halfway + else + if pos_before == MIN_POSITION + pos_after - IDEAL_DISTANCE + elsif pos_after == MAX_POSITION + pos_before + IDEAL_DISTANCE + else + halfway + end + end end def save_positionable_neighbours |