From 4d06c80bfeafbb3339db90ef42e28932b5d93e3b Mon Sep 17 00:00:00 2001
From: Ari Timonen <arit@fastmail.com>
Date: Mon, 30 Mar 2020 02:16:34 +0300
Subject: [PATCH] Fix up/down functions

---
 app/controllers/categories_controller.rb |  4 ++--
 app/models/concerns/extra.rb             | 19 +++++++++----------
 2 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb
index 20f47eb..3940090 100644
--- a/app/controllers/categories_controller.rb
+++ b/app/controllers/categories_controller.rb
@@ -46,13 +46,13 @@ class CategoriesController < ApplicationController
 
   def up
     raise AccessError unless @category.can_update? cuser
-    @category.move_up(["domain = ?", @category.domain], "sort")
+    @category.move_up(Category.where(domain: @category.domain), 'sort')
     redirect_to :categories
   end
 
   def down
     raise AccessError unless @category.can_update? cuser
-    @category.move_down(["domain = ?", @category.domain], "sort")
+    @category.move_down(Category.where(domain: @category.domain), 'sort')
     redirect_to :categories
   end
 
diff --git a/app/models/concerns/extra.rb b/app/models/concerns/extra.rb
index 6b46adb..b10d692 100644
--- a/app/models/concerns/extra.rb
+++ b/app/models/concerns/extra.rb
@@ -29,27 +29,26 @@ module Extra
       Sanitize.clean(text.to_s).bbcode_to_html.gsub(/\n|\r\n/, "<br>").html_safe
     end
 
-    def move_up(scope, column = "position")
+    def move_up(objects, column = "position")
       n = 0
-      objects = self.class.all(conditions: scope, order: column)
-      objects.each do |item|
+      # the objects need to be assigned before loop or the order is not right
+      (objects = objects.order(column)).each_with_index do |item, i|
         if item.id == id and n > 0
           old_position = item[column]
-          item.update_attribute(column, objects.fetch(n-1)[column])
-          objects.fetch(n-1).update_attribute(column, old_position)
+          item.update_attribute(column, objects[i-1][column])
+          objects[i-1].update_attribute(column, old_position)
         end
         n = n + 1
       end
     end
 
-    def move_down(scope, column = "position")
+    def move_down(objects, column = "position")
       n = 0
-      objects = self.class.all(conditions: scope, order: column)
-      objects.each do |item|
+      (objects = objects.order(column)).each_with_index do |item, i|
         if item.id == id and n < (objects.length-1)
           old_position = item[column]
-          item.update_attribute(column, objects.fetch(n+1)[column])
-          objects.fetch(n+1).update_attribute(column, old_position)
+          item.update_attribute(column, objects[n+1][column])
+          objects[n+1].update_attribute(column, old_position)
         end
         n = n + 1
       end