From 8ed61e5e015befc512fa83f99ae088b7f1ee1340 Mon Sep 17 00:00:00 2001 From: Julio Montoya Date: Wed, 23 Sep 2020 15:57:11 +0200 Subject: [PATCH] Internal - Fix queries, add lp behat test --- .travis.yml | 2 +- public/main/inc/lib/document.lib.php | 53 +++++++++------- public/main/lp/learnpath.class.php | 37 +++++++---- public/main/lp/learnpathList.class.php | 62 ++++++------------- public/main/lp/lp_controller.php | 15 +---- public/main/lp/lp_list.php | 8 +-- public/main/lp/lp_view.php | 14 +---- .../EventListener/CourseListener.php | 7 ++- .../Resources/views/LearnPath/list.html.twig | 52 ++++++++-------- src/CourseBundle/Entity/CLp.php | 29 +++------ src/CourseBundle/Entity/CLpCategory.php | 19 +++++- tests/behat/features/toolLp.feature | 36 ++++++----- 12 files changed, 161 insertions(+), 173 deletions(-) diff --git a/.travis.yml b/.travis.yml index 9d5a956fa3..436cf0ffc9 100755 --- a/.travis.yml +++ b/.travis.yml @@ -133,7 +133,7 @@ script: # - ../../vendor/behat/behat/bin/behat features/toolForum.feature -v # - ../../vendor/behat/behat/bin/behat features/toolGroup.feature -vv - ../../vendor/behat/behat/bin/behat features/toolLink.feature -v -# - ../../vendor/behat/behat/bin/behat features/toolLp.feature -v + - ../../vendor/behat/behat/bin/behat features/toolLp.feature -v - ../../vendor/behat/behat/bin/behat features/toolWork.feature -vvv after_failure: diff --git a/public/main/inc/lib/document.lib.php b/public/main/inc/lib/document.lib.php index 24ec117534..0bfed34805 100644 --- a/public/main/inc/lib/document.lib.php +++ b/public/main/inc/lib/document.lib.php @@ -578,7 +578,7 @@ class DocumentManager } $sql = "SELECT - docs.id, + docs.iid, docs.filetype, docs.path, docs.title, @@ -597,10 +597,9 @@ class DocumentManager INNER JOIN resource_link l ON (l.resource_node_id = n.id) WHERE - docs.c_id = {$courseInfo['real_id']} AND - docs.path LIKE '".Database::escape_string($path.$addedSlash.'%')."' AND - docs.path NOT LIKE '".Database::escape_string($path.$addedSlash.'%/%')."' AND - docs.path NOT LIKE '%_DELETED_%' AND + l.c_id = {$courseInfo['real_id']} AND + n.path LIKE '".Database::escape_string($path.$addedSlash.'%')."' AND + n.path NOT LIKE '".Database::escape_string($path.$addedSlash.'%/%')."' AND l.visibility NOT IN ('".ResourceLink::VISIBILITY_DELETED."') $sharedCondition "; @@ -761,23 +760,22 @@ class DocumentManager $condition_session = " AND (l.session_id = '$sessionId' OR (l.session_id = '0' OR l.session_id IS NULL) )"; $condition_session .= self::getSessionFolderFilters($path, $sessionId); - $sql = "SELECT DISTINCT docs.id, docs.path + $sql = "SELECT DISTINCT docs.iid, n.path FROM resource_node AS n - INNER JOIN $TABLE_DOCUMENT AS docs + INNER JOIN $TABLE_DOCUMENT AS docs ON (docs.resource_node_id = n.id) INNER JOIN resource_link l ON (l.resource_node_id = n.id) WHERE - docs.c_id = $courseId AND + l.c_id = $courseId AND docs.filetype = 'folder' AND $groupCondition AND - docs.path NOT LIKE '%shared_folder%' AND - docs.path NOT LIKE '%_DELETED_%' AND + n.path NOT LIKE '%shared_folder%' AND l.visibility NOT IN ('".ResourceLink::VISIBILITY_DELETED."') $condition_session "; if (0 != $groupIid) { - $sql .= " AND docs.path NOT LIKE '%shared_folder%' "; + $sql .= " AND n.path NOT LIKE '%shared_folder%' "; } else { $sql .= $show_users_condition; } @@ -795,7 +793,7 @@ class DocumentManager } } - $folders[$row['id']] = $row['path']; + $folders[$row['iid']] = $row['path']; } if (!empty($folders)) { @@ -803,9 +801,9 @@ class DocumentManager } return $folders; - } else { - return false; } + + return false; } else { // No invisible folders // Condition for the session @@ -836,7 +834,7 @@ class DocumentManager $visibilityCondition $show_users_condition $condition_session AND - docs.c_id = $courseId "; + l.c_id = $courseId "; $result = Database::query($sql); $visibleFolders = []; while ($row = Database::fetch_array($result, 'ASSOC')) { @@ -848,7 +846,7 @@ class DocumentManager } // get invisible folders - $sql = "SELECT DISTINCT docs.id, docs.path + $sql = "SELECT DISTINCT docs.iid, n.path FROM resource_node AS n INNER JOIN $TABLE_DOCUMENT AS docs ON (docs.resource_node_id = n.id) @@ -859,12 +857,12 @@ class DocumentManager $groupCondition AND l.visibility IN ('".ResourceLink::VISIBILITY_PENDING."') $condition_session AND - docs.c_id = $courseId "; + l.c_id = $courseId "; $result = Database::query($sql); $invisibleFolders = []; while ($row = Database::fetch_array($result, 'ASSOC')) { //get visible folders in the invisible ones -> they are invisible too - $sql = "SELECT DISTINCT docs.id, docs.path + $sql = "SELECT DISTINCT docs.iid, n.path FROM resource_node AS n INNER JOIN $TABLE_DOCUMENT AS docs ON (docs.resource_node_id = n.id) @@ -876,7 +874,7 @@ class DocumentManager $groupCondition AND l.visibility NOT IN ('".ResourceLink::VISIBILITY_DELETED."') $condition_session AND - docs.c_id = $courseId "; + l.c_id = $courseId "; $folder_in_invisible_result = Database::query($sql); while ($folders_in_invisible_folder = Database::fetch_array($folder_in_invisible_result, 'ASSOC')) { $invisibleFolders[$folders_in_invisible_folder['id']] = $folders_in_invisible_folder['path']; @@ -4541,12 +4539,25 @@ class DocumentManager } $folder_sql = implode("','", $escaped_folders); - $sql = "SELECT path, title + $sql = "SELECT DISTINCT docs.title, n.path + FROM resource_node AS n + INNER JOIN $doc_table AS docs + ON (docs.resource_node_id = n.id) + INNER JOIN resource_link l + ON (l.resource_node_id = n.id) + WHERE + l.c_id = $course_id AND + docs.filetype = 'folder' AND + path IN ('".$folder_sql."') AND + l.visibility NOT IN ('".ResourceLink::VISIBILITY_DELETED."') + "; + + /*$sql = "SELECT path, title FROM $doc_table WHERE filetype = 'folder' AND c_id = $course_id AND - path IN ('".$folder_sql."')"; + path IN ('".$folder_sql."') ";*/ $res = Database::query($sql); $folder_titles = []; while ($obj = Database::fetch_object($res)) { diff --git a/public/main/lp/learnpath.class.php b/public/main/lp/learnpath.class.php index f80fffec43..42a5fcfa0e 100644 --- a/public/main/lp/learnpath.class.php +++ b/public/main/lp/learnpath.class.php @@ -168,7 +168,11 @@ class learnpath $this->created_on = $entity->getCreatedOn()->format('Y-m-d H:i:s'); $this->modified_on = $entity->getModifiedOn()->format('Y-m-d H:i:s'); $this->ref = $entity->getRef(); - $this->categoryId = $entity->getCategoryId(); + $this->categoryId = 0; + if ($entity->getCategory()) { + $this->categoryId = $entity->getCategory()->getIid(); + } + $this->accumulateScormTime = $entity->getAccumulateWorkTime(); if (!empty($entity->getPublicatedOn())) { @@ -217,7 +221,7 @@ class learnpath if (Database::num_rows($res) > 0) { $row = Database::fetch_array($res); $this->attempt = $row['view_count']; - $this->lp_view_id = $row['id']; + $this->lp_view_id = $row['iid']; $this->last_item_seen = $row['last_item']; $this->progress_db = $row['progress']; $this->lp_view_session_id = $row['session_id']; @@ -758,6 +762,12 @@ class learnpath $dsp = $row[0] + 1; } + $category = null; + if (!empty($categoryId)) { + $category = Container::getLpCategoryRepository()->find($categoryId); + } + + $lp = new CLp(); $lp ->setCId($course_id) @@ -766,7 +776,7 @@ class learnpath ->setDescription($description) ->setDisplayOrder($dsp) ->setSessionId($session_id) - ->setCategoryId($categoryId) + ->setCategory($category) ->setPublicatedOn($publicated_on) ->setExpiredOn($expired_on) ->setParent($courseEntity) @@ -6931,7 +6941,7 @@ class learnpath } $sql = "UPDATE $tbl_doc SET ".substr($ct, 1)." - WHERE c_id = $course_id AND id = $document_id "; + WHERE iid = $document_id "; Database::query($sql); } } @@ -8121,10 +8131,13 @@ class learnpath $return .= ''; if (Database::num_rows($rs) > 0) { while ($row = Database::fetch_array($rs)) { - if ($row['id'] == $lp_id) { + if ($row['iid'] == $lp_id) { continue; } - $return .= ''; + $return .= ''; } } $return .= ''; @@ -10086,7 +10099,7 @@ EOD; api_get_user_id() );*/ - return $item->getId(); + return $item->getIid(); } /** @@ -10242,10 +10255,10 @@ EOD; $em = Database::getManager(); $item = $em->find('ChamiloCourseBundle:CLpCategory', $id); if ($item) { - $courseId = $item->getCId(); - $query = $em->createQuery('SELECT u FROM ChamiloCourseBundle:CLp u WHERE u.cId = :id AND u.categoryId = :catId'); + /*$courseId = $item->getCId(); + $query = $em->createQuery('SELECT u FROM ChamiloCourseBundle:CLp u WHERE u.category = :catId'); $query->setParameter('id', $courseId); - $query->setParameter('catId', $item->getId()); + $query->setParameter('catId', $item->getIid()); $lps = $query->getResult(); // Setting category = 0. @@ -10253,7 +10266,7 @@ EOD; foreach ($lps as $lpItem) { $lpItem->setCategoryId(0); } - } + }*/ // Removing category. $em->remove($item); @@ -10292,7 +10305,7 @@ EOD; if (!empty($items)) { foreach ($items as $cat) { - $cats[$cat->getId()] = $cat->getName(); + $cats[$cat->getIid()] = $cat->getName(); } } diff --git a/public/main/lp/learnpathList.class.php b/public/main/lp/learnpathList.class.php index d2d94663cd..bf0c911429 100644 --- a/public/main/lp/learnpathList.class.php +++ b/public/main/lp/learnpathList.class.php @@ -56,15 +56,6 @@ class LearnpathList $course_id = $courseInfo['real_id']; $this->user_id = $user_id; - // Condition for the session. - $session_id = empty($session_id) ? api_get_session_id() : (int) $session_id; - $condition_session = api_get_session_condition( - $session_id, - true, - true, - 'lp.sessionId' - ); - $order = ' ORDER BY lp.displayOrder ASC, lp.name ASC'; if (isset($order_by)) { $order = Database::parse_conditions(['order' => $order_by]); @@ -72,61 +63,48 @@ class LearnpathList $repo = Container::getLpRepository(); + $course = api_get_course_entity($course_id); + $session = api_get_session_entity($session_id); + $qb = $repo->getResourcesByCourse($course, $session); + $now = api_get_utc_datetime(); - $time_conditions = ''; if ($check_publication_dates) { - $time_conditions = " AND ( - (lp.publicatedOn IS NOT NULL AND lp.publicatedOn < '$now' AND lp.expiredOn IS NOT NULL AND lp.expiredOn > '$now') OR - (lp.publicatedOn IS NOT NULL AND lp.publicatedOn < '$now' AND lp.expiredOn IS NULL) OR - (lp.publicatedOn IS NULL AND lp.expiredOn IS NOT NULL AND lp.expiredOn > '$now') OR - (lp.publicatedOn IS NULL AND lp.expiredOn IS NULL )) - "; + $qb->andWhere( + $qb->orWhere("resource.publicatedOn IS NOT NULL AND resource.publicatedOn < '$now' AND resource.expiredOn IS NOT NULL AND resource.expiredOn > '$now'"), + $qb->orWhere("resource.publicatedOn IS NOT NULL AND resource.publicatedOn < '$now' AND resource.expiredOn IS NULL"), + $qb->orWhere("resource.publicatedOn IS NULL AND resource.expiredOn IS NOT NULL AND resource.expiredOn > '$now'"), + $qb->orWhere("resource.publicatedOn IS NULL AND resource.expiredOn IS NULL "), + ); } - $categoryFilter = ''; if (false == $ignoreCategoryFilter) { if (!empty($categoryId)) { $categoryId = (int) $categoryId; - $categoryFilter = " AND lp.categoryId = $categoryId"; + $categoryFilter = " resource.category = $categoryId"; } else { - $categoryFilter = ' AND (lp.categoryId = 0 OR lp.categoryId IS NULL) '; + $categoryFilter = ' resource.category IS NULL '; } + $qb->andWhere($categoryFilter); } - $dql = "SELECT lp FROM ChamiloCourseBundle:CLp as lp + /*$dql = "SELECT lp FROM ChamiloCourseBundle:CLp as lp WHERE - lp.cId = $course_id $time_conditions $condition_session $categoryFilter $order - "; - $learningPaths = Database::getManager()->createQuery($dql)->getResult(); + ";*/ + //$learningPaths = Database::getManager()->createQuery($dql)->getResult(); $showBlockedPrerequisite = api_get_configuration_value('show_prerequisite_as_blocked'); $names = []; $isAllowToEdit = api_is_allowed_to_edit(); - $courseEntity = api_get_course_entity($course_id); - $sessionEntity = api_get_session_entity($session_id); + $learningPaths = $qb->getQuery()->getResult(); + $shortcutRepository = Container::getShortcutRepository(); /** @var CLp $lp */ foreach ($learningPaths as $lp) { - //$name = Database::escape_string($lp->getName()); - //$link = 'lp/lp_controller.php?action=view&lp_id='.$lp->getId().'&id_session='.$session_id; - //$oldLink = 'newscorm/lp_controller.php?action=view&lp_id='.$lp->getId().'&id_session='.$session_id; - - /*$sql2 = "SELECT visibility FROM $tbl_tool - WHERE - c_id = $course_id AND - name = '$name' AND - image = 'scormbuilder.gif' AND - ( - link LIKE '$link%' OR - link LIKE '$oldLink%' - ) - "; - $res2 = Database::query($sql2);*/ $pub = 'i'; $shortcut = $shortcutRepository->getShortcutFromResource($lp); if ($shortcut) { @@ -144,7 +122,7 @@ class LearnpathList $lp->getId(), $session_id );*/ - $visibility = $lp->isVisible($courseEntity, $sessionEntity); + $visibility = $lp->isVisible($course, $session); // If option is not true then don't show invisible LP to user if (false === $ignoreLpVisibility) { @@ -186,7 +164,7 @@ class LearnpathList 'expired_on' => $lp->getExpiredOn() ? $lp->getExpiredOn()->format('Y-m-d H:i:s') : null, //'category_id' => $lp['category_id'], 'subscribe_users' => $lp->getSubscribeUsers(), - 'lp_old_id' => $lp->getId(), + 'lp_old_id' => $lp->getIid(), 'iid' => $lp->getIid(), 'prerequisite' => $lp->getPrerequisite(), 'entity' => $lp, diff --git a/public/main/lp/lp_controller.php b/public/main/lp/lp_controller.php index 75bf819b56..16eb66415d 100644 --- a/public/main/lp/lp_controller.php +++ b/public/main/lp/lp_controller.php @@ -333,16 +333,12 @@ if (!$lp_found || (!empty($_REQUEST['lp_id']) && $_SESSION['oLP']->get_id() != $ $lp_table = Database::get_course_table(TABLE_LP_MAIN); if (!empty($lp_id)) { - $sel = "SELECT iid, lp_type FROM $lp_table WHERE c_id = $course_id AND id = $lp_id"; + $sel = "SELECT iid, lp_type FROM $lp_table WHERE iid = $lp_id"; $res = Database::query($sel); if (Database::num_rows($res)) { $row = Database::fetch_array($res); $lpIid = $row['iid']; $type = $row['lp_type']; - if ($debug > 0) { - error_log('Found row type '.$type); - error_log('Calling constructor: '.api_get_course_id().' - '.$lp_id.' - '.api_get_user_id()); - } $logInfo = [ 'tool' => TOOL_LEARNPATH, 'action' => 'lp_load', @@ -376,16 +372,9 @@ if (!$lp_found || (!empty($_REQUEST['lp_id']) && $_SESSION['oLP']->get_id() != $ break; } } - } else { - if ($debug > 0) { - error_log(' Request[lp_id] is not numeric'); - } - } - } else { - if ($debug > 0) { - error_log(' Request[lp_id] and refresh_id were empty'); } } + if ($lp_found) { Session::write('oLP', $oLP); } diff --git a/public/main/lp/lp_list.php b/public/main/lp/lp_list.php index af08184860..1e02321eaf 100644 --- a/public/main/lp/lp_list.php +++ b/public/main/lp/lp_list.php @@ -142,10 +142,10 @@ if ($allowCategory) { $categoriesTempList = $newCategoryFiltered; } -/*$categoryTest = new CLpCategory(); +$categoryTest = new CLpCategory(); $categoryTest->setName(get_lang('WithOutCategory')); -$categoryTest->setPosition(0);*/ -$categories = []; +$categoryTest->setPosition(0); +$categories = [$categoryTest]; if (!empty($categoriesTempList)) { $categories = array_merge($categories, $categoriesTempList); @@ -216,7 +216,7 @@ $shortcutRepository = Container::getShortcutRepository(); foreach ($categories as $category) { $categoryId = $category->getIid(); $visibility = true; - if (0 !== $categoryId) { + if (null !== $categoryId) { $visibility = $category->isVisible($courseEntity, $sessionEntity); } if (0 !== $categoryId && true == $subscriptionSettings['allow_add_users_to_lp_category']) { diff --git a/public/main/lp/lp_view.php b/public/main/lp/lp_view.php index 03ae491ca5..460c7eacee 100644 --- a/public/main/lp/lp_view.php +++ b/public/main/lp/lp_view.php @@ -205,20 +205,12 @@ $htmlHeadXtra[] = ''; -/** - * Get a link to the corresponding document. - */ -if ($debug) { - error_log(" src: $src "); - error_log(" lp_type: $lpType "); -} - $get_toc_list = $lp->get_toc(); $get_teacher_buttons = $lp->get_teacher_toc_buttons(); $type_quiz = false; foreach ($get_toc_list as $toc) { - if ($toc['id'] == $lp_item_id && 'quiz' == $toc['type']) { + if ($toc['id'] == $lp_item_id && 'quiz' === $toc['type']) { $type_quiz = true; } } @@ -303,12 +295,12 @@ if (!isset($src)) { $autostart = 'true'; // Update status, total_time from lp_item_view table when you finish the exercises in learning path. -if ($debug) { +/*if ($debug) { error_log('$type_quiz: '.$type_quiz); error_log('$_REQUEST[exeId]: '.intval($_REQUEST['exeId'])); error_log('$lp_id: '.$lp_id); error_log('$_REQUEST[lp_item_id]: '.intval($_REQUEST['lp_item_id'])); -} +}*/ if (!empty($_REQUEST['exeId']) && isset($lp_id) && diff --git a/src/CoreBundle/EventListener/CourseListener.php b/src/CoreBundle/EventListener/CourseListener.php index cfd6700954..71b18b4c2f 100644 --- a/src/CoreBundle/EventListener/CourseListener.php +++ b/src/CoreBundle/EventListener/CourseListener.php @@ -60,6 +60,7 @@ class CourseListener $sessionHandler = $request->getSession(); $container = $this->container; $translator = $container->get('translator'); + $twig = $container->get('twig'); $course = null; $courseInfo = []; @@ -113,7 +114,7 @@ class CourseListener $sessionHandler->set('_course', $courseInfo); // Setting variables for the twig templates. - $container->get('twig')->addGlobal('course', $course); + $twig->addGlobal('course', $course); // Checking if sid is used. $sessionId = (int) $request->get('sid'); @@ -148,7 +149,7 @@ class CourseListener $sessionHandler->set('sid', $session->getId()); $sessionHandler->set('session', $session); - $container->get('twig')->addGlobal('session', $session); + $twig->addGlobal('session', $session); } else { throw new NotFoundHttpException($translator->trans('Session not found')); } @@ -192,7 +193,7 @@ class CourseListener $courseParams = $this->generateCourseUrl($course, $sessionId, $groupId, $origin); $sessionHandler->set('course_url_params', $courseParams); - $container->get('twig')->addGlobal('course_url_params', $courseParams); + $twig->addGlobal('course_url_params', $courseParams); } } diff --git a/src/CoreBundle/Resources/views/LearnPath/list.html.twig b/src/CoreBundle/Resources/views/LearnPath/list.html.twig index 4b06a09aa2..d40c543197 100644 --- a/src/CoreBundle/Resources/views/LearnPath/list.html.twig +++ b/src/CoreBundle/Resources/views/LearnPath/list.html.twig @@ -15,27 +15,27 @@ {% for lp_data in data %} {% set show_category = true %} - {% if filtered_category and filtered_category != lp_data.category.id %} + {% if filtered_category and filtered_category != lp_data.category.iid %} {% set show_category = false %} {% endif %} {% if show_category %} {% if configuration == 0 %} - {% if categories|length > 1 and lp_data.category.id %} + {% if categories|length > 1 and lp_data.category.iid %} {% if is_allowed_to_edit %}

{{ lp_data.category.getName() | trim }} - {% if lp_data.category.getId() > 0 %} - {% if not _c.session_id %} - 0 %} + {% if not session %} + {{ {% if subscription_settings.allow_add_users_to_lp_category %} - {{ @@ -46,7 +46,7 @@ {{ {% else %} - {{ @@ -57,7 +57,7 @@ {{ {% else %} - {{ @@ -65,19 +65,19 @@ {% endif %} {% if lp_data.category_visibility == 0 %} - {{ 'Show'|trans }} {% else %} - {{ 'Hide'|trans }} {% endif %} {% if lp_data.category_is_published == 0 %} - {{ 'LearnpathPublish'|trans }} @@ -88,8 +88,8 @@ {{ 'Hide'|trans }} {% endif %} - {% if not _c.session_id %} - {{ @@ -309,18 +309,18 @@ {% if categories|length > 1 and lp_data.category.id %}
-