Browse Source

Maintenance: Force usage of 'reorder' instead of 'order'.

Martin Gruner 2 years ago
parent
commit
a69fc3a451

+ 46 - 0
.rubocop/cop/zammad/active_record_reorder.rb

@@ -0,0 +1,46 @@
+# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+module RuboCop
+  module Cop
+    module Zammad
+      #
+      #
+      # @example
+      #   # bad
+      #   Ticket.where(state: open_states).order(...)
+      #
+      #   # good
+      #   Ticket.where(state: open_states).reorder(...)
+      #   default_scope { order(:prio, :id) }
+      #   scope :sorted, -> { order(position: :asc) }
+
+      class ActiveRecordReorder < Base
+        extend AutoCorrector
+
+        def_node_matcher :active_record_order_call?, <<-PATTERN
+          $(send _ {:order} _ ...)
+        PATTERN
+
+        def_node_matcher :default_scope_call?, <<-PATTERN
+          $(send _ {:default_scope} ...)
+        PATTERN
+
+        def_node_matcher :scope_call?, <<-PATTERN
+          $(send _ {:scope} sym ...)
+        PATTERN
+
+        MSG = "Prefer 'reorder' over 'order' to prevent issues with default ordering.".freeze
+
+        def on_send(node)
+          return if active_record_order_call?(node).nil?
+          return if default_scope_call?(node.parent&.children&.first)
+          return if scope_call?(node.parent&.parent)
+
+          add_offense(node) do |corrector|
+            corrector.replace(node.loc.selector, :reorder)
+          end
+        end
+      end
+    end
+  end
+end

+ 8 - 7
.rubocop/rubocop_zammad.rb

@@ -1,19 +1,20 @@
 # Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
 
-require_relative 'cop/zammad/exists_condition'
+require_relative 'cop/zammad/active_record_reorder'
+require_relative 'cop/zammad/correct_migration_timestamp'
 require_relative 'cop/zammad/detect_translatable_string'
 require_relative 'cop/zammad/enforce_in_modal'
+require_relative 'cop/zammad/exists_condition'
 require_relative 'cop/zammad/exists_date_time_precision'
 require_relative 'cop/zammad/exists_db_strategy'
 require_relative 'cop/zammad/exists_reset_column_information'
-require_relative 'cop/zammad/correct_migration_timestamp'
-require_relative 'cop/zammad/no_to_sym_on_string'
-require_relative 'cop/zammad/prefer_negated_if_over_unless'
-require_relative 'cop/zammad/update_copyright'
 require_relative 'cop/zammad/forbid_rand'
 require_relative 'cop/zammad/forbid_default_scope'
 require_relative 'cop/zammad/forbid_translatable_marker'
+require_relative 'cop/zammad/migration_scheduler_last_run'
+require_relative 'cop/zammad/no_to_sym_on_string'
+require_relative 'cop/zammad/prefer_negated_if_over_unless'
+require_relative 'cop/zammad/timezone_default'
 require_relative 'cop/zammad/to_forbid_over_not_to_permit'
 require_relative 'cop/zammad/trigger_from_commit_hooks'
-require_relative 'cop/zammad/timezone_default'
-require_relative 'cop/zammad/migration_scheduler_last_run'
+require_relative 'cop/zammad/update_copyright'

+ 1 - 1
app/controllers/application_controller/renders_models.rb

@@ -118,7 +118,7 @@ module ApplicationController::RendersModels
     order_by   = sql_helper.get_order_by(params, 'ASC')
     order_sql  = sql_helper.get_order(sort_by, order_by)
 
-    generic_objects = object.order(Arel.sql(order_sql)).offset(pagination.offset).limit(pagination.limit)
+    generic_objects = object.reorder(Arel.sql(order_sql)).offset(pagination.offset).limit(pagination.limit)
 
     if response_expand?
       list = []

+ 1 - 1
app/controllers/calendars_controller.rb

@@ -6,7 +6,7 @@ class CalendarsController < ApplicationController
   def init
     assets = {}
     record_ids = []
-    Calendar.all.order(:name, :created_at).each do |calendar|
+    Calendar.all.reorder(:name, :created_at).each do |calendar|
       record_ids.push calendar.id
       assets = calendar.assets(assets)
     end

+ 1 - 1
app/controllers/channels_email_controller.rb

@@ -11,7 +11,7 @@ class ChannelsEmailController < ApplicationController
     not_used_email_address_ids = []
     accounts_fixed = []
     assets = {}
-    Channel.order(:id).each do |channel|
+    Channel.reorder(:id).each do |channel|
       if system_online_service && channel.preferences && channel.preferences['online_service_disable']
         email_addresses = EmailAddress.where(channel_id: channel.id)
         email_addresses.each do |email_address|

+ 1 - 1
app/controllers/channels_facebook_controller.rb

@@ -9,7 +9,7 @@ class ChannelsFacebookController < ApplicationController
       assets = external_credential.assets(assets)
     end
     channel_ids = []
-    Channel.where(area: 'Facebook::Account').order(:id).each do |channel|
+    Channel.where(area: 'Facebook::Account').reorder(:id).each do |channel|
       assets = channel.assets(assets)
       channel_ids.push channel.id
     end

+ 1 - 1
app/controllers/channels_google_controller.rb

@@ -14,7 +14,7 @@ class ChannelsGoogleController < ApplicationController
     end
 
     channel_ids = []
-    Channel.where(area: 'Google::Account').order(:id).each do |channel|
+    Channel.where(area: 'Google::Account').reorder(:id).each do |channel|
       assets = channel.assets(assets)
       channel_ids.push channel.id
     end

+ 1 - 1
app/controllers/channels_microsoft365_controller.rb

@@ -14,7 +14,7 @@ class ChannelsMicrosoft365Controller < ApplicationController
     end
 
     channel_ids = []
-    Channel.where(area: 'Microsoft365::Account').order(:id).each do |channel|
+    Channel.where(area: 'Microsoft365::Account').reorder(:id).each do |channel|
       assets = channel.assets(assets)
       channel_ids.push channel.id
     end

+ 1 - 1
app/controllers/channels_telegram_controller.rb

@@ -7,7 +7,7 @@ class ChannelsTelegramController < ApplicationController
   def index
     assets = {}
     channel_ids = []
-    Channel.where(area: 'Telegram::Bot').order(:id).each do |channel|
+    Channel.where(area: 'Telegram::Bot').reorder(:id).each do |channel|
       assets = channel.assets(assets)
       channel_ids.push channel.id
     end

+ 1 - 1
app/controllers/channels_twitter_controller.rb

@@ -68,7 +68,7 @@ class ChannelsTwitterController < ApplicationController
       external_credential_ids.push external_credential.id
     end
     channel_ids = []
-    Channel.where(area: 'Twitter::Account').order(:id).each do |channel|
+    Channel.where(area: 'Twitter::Account').reorder(:id).each do |channel|
       assets = channel.assets(assets)
       channel_ids.push channel.id
     end

Some files were not shown because too many files changed in this diff