Merge pull request #5979 from christianbeeznest/rna-22253

Course: Fix category list actions, reorder on moveUp, export CSV error - refs BT#22253

public/main/admin/course_category.php

@@ -22,7 +22,7 @@ $parentInfo = [];
 if (!empty($categoryId)) {
     $categoryInfo = $parentInfo = CourseCategory::getCategoryById($categoryId);
 }
-$parentId = $parentInfo ? $parentInfo['id'] : null;
+$parentId = $parentInfo ? $parentInfo['parent_id'] : null;
 
 switch ($action) {
     case 'delete':
@@ -37,31 +37,38 @@ switch ($action) {
         }
         if (!empty($categoryId)) {
             $categoryRepo->delete($categoryRepo->find($categoryId));
+            CourseCategory::reorganizeTreePos($parentId);
             Display::addFlash(Display::return_message(get_lang('Deleted')));
         }
         header('Location: '.api_get_self().'?category='.Security::remove_XSS($category));
         exit;
-        break;
     case 'export':
         $courses = CourseCategory::getCoursesInCategory($categoryId);
-        if (!empty($courses)) {
+        if ($courses && count($courses) > 0) {
             $name = api_get_local_time().'_'.$categoryInfo['code'];
             $courseList = [];
+
+            /* @var \Chamilo\CoreBundle\Entity\Course $course */
             foreach ($courses as $course) {
-                $courseList[] = $course->getTitle();
+                $courseList[] = [$course->getTitle()];
-            }
-            Export::arrayToCsv($courseList, $name);
             }
 
-        header('Location: '.api_get_self());
+            $header = [get_lang('Course Title')];
+            Export::arrayToCsvSimple($courseList, $name, false, $header);
+        } else {
+            Display::addFlash(Display::return_message(get_lang('No courses found for this category'), 'warning'));
+            header('Location: '.api_get_self().'?category='.Security::remove_XSS($categoryId));
             exit;
+        }
         break;
     case 'moveUp':
-        CourseCategory::moveNodeUp($categoryId, $_GET['tree_pos'], $category);
+        if (CourseCategory::moveNodeUp($categoryId, $_GET['tree_pos'], $parentId)) {
             Display::addFlash(Display::return_message(get_lang('Update successful')));
+        } else {
+            Display::addFlash(Display::return_message(get_lang('Cannot move category up'), 'error'));
+        }
         header('Location: '.api_get_self().'?category='.Security::remove_XSS($category));
         exit;
-        break;
     case 'add':
         if (isset($_POST['formSent']) && $_POST['formSent']) {
             $categoryEntity = CourseCategory::add(
@@ -127,7 +134,7 @@ if ('add' === $action || 'edit' === $action) {
 
     $form_title = 'add' === $action ? get_lang('Add category') : get_lang('Edit this category');
     if (!empty($categoryInfo)) {
-        $form_title .= ' '.get_lang('Into').' '.$categoryInfo['name'];
+        $form_title .= ' '.get_lang('Into').' '.$categoryInfo['title'];
     }
     $url = api_get_self().'?action='.Security::remove_XSS($action).'&id='.Security::remove_XSS($categoryId);
     $form = new FormValidator('course_category', 'post', $url);
@@ -223,7 +230,7 @@ if ('add' === $action || 'edit' === $action) {
     }
     echo Display::toolbarAction('categories', [$actions]);
     if (!empty($parentInfo)) {
-        echo Display::page_subheader($parentInfo['name'].' ('.$parentInfo['code'].')');
+        echo Display::page_subheader($parentInfo['title'].' ('.$parentInfo['code'].')');
     }
     echo CourseCategory::listCategories($categoryInfo);
 }

public/main/inc/lib/course_category.lib.php

@@ -240,52 +240,55 @@ class CourseCategory
      *
      * @return bool
      */
-    public static function moveNodeUp($code, $tree_pos, $parent_id)
+    public static function moveNodeUp($categoryId, $treePos, $parentId): bool
     {
         $table = Database::get_main_table(TABLE_MAIN_CATEGORY);
-        $code = Database::escape_string($code);
+        $categoryId = (int) $categoryId;
-        $tree_pos = (int) $tree_pos;
+        $treePos = (int) $treePos;
-        $parent_id = Database::escape_string($parent_id);
 
-        $parentIdCondition = " AND (parent_id IS NULL OR parent_id = '' )";
+        $parentIdCondition = "parent_id IS NULL";
-        if (!empty($parent_id)) {
+        if (!empty($parentId)) {
-            $parentIdCondition = " AND parent_id = '$parent_id' ";
+            $parentIdCondition = "parent_id = '".Database::escape_string($parentId)."'";
         }
 
-        $sql = "SELECT code,tree_pos
+        self::reorganizeTreePos($parentId);
+
+        $sql = "SELECT id, tree_pos
             FROM $table
-                WHERE
+            WHERE $parentIdCondition AND tree_pos < $treePos
-                    tree_pos < $tree_pos
-                    $parentIdCondition
             ORDER BY tree_pos DESC
-                LIMIT 0,1";
+            LIMIT 1";
 
         $result = Database::query($sql);
-        if (!$row = Database::fetch_array($result)) {
+        $previousCategory = Database::fetch_array($result);
-            $sql = "SELECT code, tree_pos
+
-                    FROM $table
+        if (!$previousCategory) {
-                    WHERE
-                        tree_pos > $tree_pos
-                        $parentIdCondition
-                    ORDER BY tree_pos DESC
-                    LIMIT 0,1";
-            $result2 = Database::query($sql);
-            if (!$row = Database::fetch_array($result2)) {
             return false;
         }
+
+        Database::query("UPDATE $table SET tree_pos = {$previousCategory['tree_pos']} WHERE id = $categoryId");
+        Database::query("UPDATE $table SET tree_pos = $treePos WHERE id = {$previousCategory['id']}");
+
+        return true;
     }
 
-        $sql = "UPDATE $table
+    public static function reorganizeTreePos($parentId): void
-                SET tree_pos ='".$row['tree_pos']."'
+    {
-                WHERE code='$code'";
+        $table = Database::get_main_table(TABLE_MAIN_CATEGORY);
-        Database::query($sql);
 
-        $sql = "UPDATE $table
+        $parentIdCondition = "parent_id IS NULL";
-                SET tree_pos = '$tree_pos'
+        if (!empty($parentId)) {
-                WHERE code= '".$row['code']."'";
+            $parentIdCondition = "parent_id = '".Database::escape_string($parentId)."'";
-        Database::query($sql);
+        }
 
-        return true;
+        $sql = "SELECT id FROM $table WHERE $parentIdCondition ORDER BY tree_pos";
+        $result = Database::query($sql);
+
+        $newTreePos = 1;
+        while ($row = Database::fetch_array($result)) {
+            Database::query("UPDATE $table SET tree_pos = $newTreePos WHERE id = {$row['id']}");
+            $newTreePos++;
+        }
     }
 
     /**

public/main/inc/lib/export.lib.inc.php

@@ -55,7 +55,7 @@ class Export
      *
      * @return string|void Returns the file path if $writeOnly is true, otherwise sends the file for download and exits.
      */
-    public static function arrayToCsvSimple(array $data, string $filename = 'export', bool $writeOnly = false)
+    public static function arrayToCsvSimple(array $data, string $filename = 'export', bool $writeOnly = false, array $header = [])
     {
         $file = api_get_path(SYS_ARCHIVE_PATH) . uniqid('') . '.csv';
 
@@ -65,16 +65,12 @@ class Export
             throw new \RuntimeException("Unable to create or open the file: $file");
         }
 
-        if (is_array($data)) {
+        if (!empty($header)) {
-            foreach ($data as $row) {
+            fputcsv($handle, $header, ';');
-                $line = '';
-                if (is_array($row)) {
-                    foreach ($row as $value) {
-                        $line .= '"' . str_replace('"', '""', (string)$value) . '";';
-                    }
-                }
-                fwrite($handle, rtrim($line, ';') . "\n");
         }
+
+        foreach ($data as $row) {
+            fputcsv($handle, (array)$row, ';');
         }
 
         fclose($handle);