swap priorities instead of increasing/decreasing the priorities, fixes #3031

Closes #3031

Merge request studip/studip!2035

app/controllers/course/topics.php

@@ -31,6 +31,7 @@ class Course_TopicsController extends AuthenticatedController
     public function index_action()
     {
         $this->topics = CourseTopic::findBySeminar_id(Context::getId());
+        $this->topic_links = $this->createLinksForTopics($this->topics);
         $this->cancelled_dates_locked = LockRules::Check(Context::getId(), 'cancelled_dates');
     }
 
@@ -107,26 +108,25 @@ class Course_TopicsController extends AuthenticatedController
         $this->redirect($this->indexURL(['open' => $topic->id]));
     }
 
-    public function move_up_action(CourseTopic $topic)
+    public function swap_action(CourseTopic $a, CourseTopic $b)
     {
         if (!Request::isPost()) {
             throw new MethodNotAllowedException();
         }
 
-        $topic->increasePriority();
-
-        $this->redirect($this->indexURL(['open' => $topic->id]));
-    }
-
-    public function move_down_action(CourseTopic $topic)
-    {
-        if (!Request::isPost()) {
-            throw new MethodNotAllowedException();
+        if (
+            $a->seminar_id !== Context::getId()
+            || $b->seminar_id !== Context::getId()
+        ) {
+            throw new Exception(_('Eines oder mehrere Themen gehören nicht zur ausgewählten Veranstaltung.'));
         }
 
-        $topic->decreasePriority();
+        [$a->priority, $b->priority] = [$b->priority, $a->priority];
 
-        $this->redirect($this->indexURL(['open' => $topic->id]));
+        $a->store();
+        $b->store();
+
+        $this->redirect($this->indexURL(['open' => $a->id]));
     }
 
     public function allow_public_action()
@@ -263,4 +263,30 @@ class Course_TopicsController extends AuthenticatedController
             );
         }
     }
+
+    private function createLinksForTopics(array $topics): array
+    {
+        $links = array_combine(
+            array_column($topics, 'id'),
+            array_fill(0, count($topics), ['previous' => null, 'next' => null])
+        );
+
+        $last = null;
+        foreach ($topics as $topic) {
+            if ($last !== null) {
+                $links[$topic->id]['previous'] = $last;
+            }
+            $last = $topic;
+        }
+
+        $next = null;
+        foreach (array_reverse($topics) as $topic) {
+            if ($next !== null) {
+                $links[$topic->id]['next'] = $next;
+            }
+            $next = $topic;
+        }
+
+        return $links;
+    }
 }

app/views/course/topics/index.php

+<?php
+/**
+ * @var CourseTopic[] $topics
+ * @var Course_TopicsController $controller
+ * @var array<array{next: ?CourseTopic, previous: ?CourseTopic}> $topic_links
+ */
+?>
 <? if (count($topics) > 0) : ?>
 <table class="default withdetails">
     <colgroup>
@@ -102,13 +109,13 @@
                             <? endif ?>
 
                             <span class="button-group">
-                            <? if ($key > 0) : ?>
-                                <form action="<?= $controller->move_up($topic) ?>" method="post" style="display: inline;">
+                            <? if ($topic_links[$topic->id]['previous']) : ?>
+                                <form action="<?= $controller->swap($topic, $topic_links[$topic->id]['previous']) ?>" method="post" style="display: inline;">
                                     <?= Studip\Button::createMoveUp(_('nach oben verschieben')) ?>
                                 </form>
                             <? endif ?>
-                            <? if ($key < count($topics) - 1) : ?>
-                                <form action="<?=$controller->move_down($topic)?>" method="post" style="display: inline;">
+                            <? if ($topic_links[$topic->id]['next']) : ?>
+                                <form action="<?= $controller->swap($topic, $topic_links[$topic->id]['next']) ?>" method="post" style="display: inline;">
                                     <?= Studip\Button::createMoveDown(_('nach unten verschieben')) ?>
                                 </form>
                             <? endif ?>

lib/models/CourseTopic.class.php

@@ -197,6 +197,7 @@ class CourseTopic extends SimpleORMap
      * mean higher priority.
      *
      * @return boolean
+     * @todo Deprecated, remove for Stud.IP 6.0
      */
     public function increasePriority()
     {
@@ -227,6 +228,8 @@ class CourseTopic extends SimpleORMap
      * Decreases the priority of this topic. Meaning the topic will be sorted further down.
      * Be aware that this actually increases the priority property since higher numbers
      * mean lower priority.
+     *
+     * @todo Deprecated, remove for Stud.IP 6.0
      */
     public function decreasePriority()
     {