From 59d6dfaad60ae6f1b6439c2a87ec56dcfd5f5a89 Mon Sep 17 00:00:00 2001 From: Julio Montoya Date: Thu, 13 Dec 2018 10:42:43 +0100 Subject: [PATCH] Return a more precise message when user cannot be subscribed see BT#15114 --- main/admin/course_import.php | 2 +- main/admin/course_user_import.php | 58 +-- main/admin/course_user_import_by_email.php | 2 +- main/admin/ldap_import_students.php | 3 +- main/admin/subscribe_user2course.php | 2 +- main/admin/user_import.php | 8 +- main/admin/user_update_import.php | 2 +- main/auth/courses.php | 91 +++-- main/auth/courses_categories.php | 7 +- main/auth/courses_controller.php | 52 --- main/auth/external_login/functions.inc.php | 2 +- main/auth/external_login/newUser.php | 2 +- main/auth/inscription.php | 4 +- main/course_home/course_home.php | 20 +- main/cron/import_csv.php | 14 +- main/inc/lib/auth.lib.php | 70 ---- main/inc/lib/course.lib.php | 343 +++++++++--------- main/inc/lib/events.lib.php | 2 +- main/inc/lib/sessionmanager.lib.php | 10 +- main/inc/lib/usergroup.lib.php | 4 +- main/inc/lib/webservices/Rest.php | 2 +- main/user/subscribe_user.php | 19 +- main/user/user_import.php | 4 +- main/webservices/cm_webservice_course.php | 2 +- main/webservices/registration.soap.php | 4 +- main/webservices/webservice_course.php | 2 +- .../src/buy_course_plugin.class.php | 2 +- .../lib/register_course_widget.class.php | 2 +- .../features/course_user_registration.feature | 4 +- tests/scripts/users_no_course.php | 2 +- 30 files changed, 324 insertions(+), 417 deletions(-) diff --git a/main/admin/course_import.php b/main/admin/course_import.php index 330743c75a..cb5747cb19 100755 --- a/main/admin/course_import.php +++ b/main/admin/course_import.php @@ -133,7 +133,7 @@ function save_courses_data($courses) if (!empty($courseInfo)) { if (!empty($teacherList)) { foreach ($teacherList as $teacher) { - CourseManager::add_user_to_course( + CourseManager::subscribeUser( $teacher['user_id'], $courseInfo['code'], COURSEMANAGER diff --git a/main/admin/course_user_import.php b/main/admin/course_user_import.php index 8edbdde0f4..8fefbc3a82 100755 --- a/main/admin/course_user_import.php +++ b/main/admin/course_user_import.php @@ -31,11 +31,8 @@ function validate_data($users_courses) // 2.1 Check whethher code has been allready used by this CVS-file. if (!isset($coursecodes[$user_course['CourseCode']])) { // 2.1.1 Check whether course with this code exists in the system. - $course_table = Database::get_main_table(TABLE_MAIN_COURSE); - $sql = "SELECT * FROM $course_table - WHERE code = '".Database::escape_string($user_course['CourseCode'])."'"; - $res = Database::query($sql); - if (Database::num_rows($res) == 0) { + $courseInfo = api_get_course_info($user_course['CourseCode']); + if (empty($courseInfo)) { $user_course['error'] = get_lang('CodeDoesNotExists'); $errors[] = $user_course; } else { @@ -69,7 +66,6 @@ function validate_data($users_courses) */ function save_data($users_courses) { - $user_table = Database::get_main_table(TABLE_MAIN_USER); $course_user_table = Database::get_main_table(TABLE_MAIN_COURSE_USER); $csv_data = []; $inserted_in_course = []; @@ -78,7 +74,9 @@ function save_data($users_courses) foreach ($users_courses as $user_course) { if (!in_array($user_course['CourseCode'], array_keys($courseListCache))) { $courseInfo = api_get_course_info($user_course['CourseCode']); - $courseListCache[$user_course['CourseCode']] = $courseInfo; + if ($courseInfo) { + $courseListCache[$user_course['CourseCode']] = $courseInfo; + } } else { $courseInfo = $courseListCache[$user_course['CourseCode']]; } @@ -87,11 +85,12 @@ function save_data($users_courses) } foreach ($csv_data as $username => $csv_subscriptions) { - $sql = "SELECT * FROM $user_table u - WHERE u.username = '".Database::escape_string($username)."'"; - $res = Database::query($sql); - $obj = Database::fetch_object($res); - $user_id = $obj->user_id; + $userInfo = api_get_user_info_from_username($username); + if (empty($userInfo)) { + continue; + } + + $user_id = $userInfo['user_id']; $sql = "SELECT * FROM $course_user_table cu WHERE cu.user_id = $user_id AND cu.relation_type <> ".COURSE_RELATION_TYPE_RRHH." "; $res = Database::query($sql); @@ -103,23 +102,28 @@ function save_data($users_courses) $to_subscribe = array_diff(array_keys($csv_subscriptions), array_keys($db_subscriptions)); $to_unsubscribe = array_diff(array_keys($db_subscriptions), array_keys($csv_subscriptions)); - if ($_POST['subscribe']) { + if (isset($_POST['subscribe']) && $_POST['subscribe']) { foreach ($to_subscribe as $courseId) { $courseInfo = $courseListById[$courseId]; $courseCode = $courseInfo['code']; - - CourseManager::subscribe_user( - $user_id, - $courseCode, - $csv_subscriptions[$courseId] - ); - $inserted_in_course[$courseInfo['code']] = $courseInfo['title']; + $result = CourseManager::subscribeUser( + $user_id, + $courseCode, + $csv_subscriptions[$courseId] + ); + if ($result) { + $inserted_in_course[$courseInfo['code']] = $courseInfo['title']; + } } } - if ($_POST['unsubscribe']) { + if (isset($_POST['unsubscribe']) && $_POST['unsubscribe']) { foreach ($to_unsubscribe as $courseId) { - $courseInfo = $courseListById[$courseId]; + if (isset($courseListById[$courseId])) { + $courseInfo = $courseListById[$courseId]; + } else { + $courseInfo = api_get_course_info_by_id($courseId); + } $courseCode = $courseInfo['code']; CourseManager::unsubscribe_user($user_id, $courseCode); } @@ -138,7 +142,7 @@ function save_data($users_courses) */ function parse_csv_data($file) { - $courses = Import :: csvToArray($file); + $courses = Import::csvToArray($file); return $courses; } @@ -176,17 +180,13 @@ if ($form->validate()) { $inserted_in_course = save_data($users_courses); // Build the alert message in case there were visual codes subscribed to. if ($_POST['subscribe']) { - $warn = get_lang('UsersSubscribedToBecauseVisualCode').': '; + //$warn = get_lang('UsersSubscribedToBecauseVisualCode').': '; } else { $warn = get_lang('UsersUnsubscribedFromBecauseVisualCode').': '; } if (!empty($inserted_in_course)) { - $warn = $warn.' '.get_lang('FileImported'); - // The users have been inserted in more than one course. - foreach ($inserted_in_course as $code => $info) { - $warn .= ' '.$info.' ('.$code.') '; - } + $warn = get_lang('FileImported'); } else { $warn = get_lang('ErrorsWhenImportingFile'); } diff --git a/main/admin/course_user_import_by_email.php b/main/admin/course_user_import_by_email.php index 6464ef97a7..9522c315d4 100755 --- a/main/admin/course_user_import_by_email.php +++ b/main/admin/course_user_import_by_email.php @@ -104,7 +104,7 @@ function save_data($users_courses) $course_info = api_get_course_info($course_code); $inserted_in_course[$course_code] = $course_info['title']; - CourseManager::subscribe_user( + CourseManager::subscribeUser( $user_id, $course_code, $csv_subscriptions[$course_code] diff --git a/main/admin/ldap_import_students.php b/main/admin/ldap_import_students.php index 28b9810a73..b57043edbd 100755 --- a/main/admin/ldap_import_students.php +++ b/main/admin/ldap_import_students.php @@ -140,9 +140,10 @@ if (empty($annee) && empty($course)) { } if (!empty($_POST['course'])) { foreach ($UserList as $user_id) { - CourseManager::add_user_to_course($user_id, $_POST['course']); + CourseManager::subscribeUser($user_id, $_POST['course']); } header('Location: course_information.php?code='.Security::remove_XSS($_POST['course'])); + exit; } else { $message = get_lang('NoUserAdded'); Display::addFlash(Display::return_message($message, 'normal', false)); diff --git a/main/admin/subscribe_user2course.php b/main/admin/subscribe_user2course.php index d8b283bff5..81de085ed5 100755 --- a/main/admin/subscribe_user2course.php +++ b/main/admin/subscribe_user2course.php @@ -97,7 +97,7 @@ if (isset($_POST['form_sent']) && $_POST['form_sent']) { foreach ($users as $user_id) { $user = api_get_user_info($user_id); if ($user['status'] != DRH) { - CourseManager::subscribe_user($user_id, $course_code); + CourseManager::subscribeUser($user_id, $course_code); } else { $errorDrh = 1; } diff --git a/main/admin/user_import.php b/main/admin/user_import.php index 889de4e29d..d564519b93 100644 --- a/main/admin/user_import.php +++ b/main/admin/user_import.php @@ -213,9 +213,11 @@ function save_data($users) if (isset($user['Courses']) && is_array($user['Courses'])) { foreach ($user['Courses'] as $course) { if (CourseManager::course_exists($course)) { - CourseManager::subscribe_user($user_id, $course, $user['Status']); - $course_info = CourseManager::get_course_information($course); - $inserted_in_course[$course] = $course_info['title']; + $result = CourseManager::subscribeUser($user_id, $course, $user['Status']); + if ($result) { + $course_info = api_get_course_info($course); + $inserted_in_course[$course] = $course_info['title']; + } } } } diff --git a/main/admin/user_update_import.php b/main/admin/user_update_import.php index ae0728ddc9..bb302f6d1a 100644 --- a/main/admin/user_update_import.php +++ b/main/admin/user_update_import.php @@ -199,7 +199,7 @@ function updateUsers($users) if (is_array($user['Courses'])) { foreach ($user['Courses'] as $course) { if (CourseManager::course_exists($course)) { - CourseManager::subscribe_user($user_id, $course, $user['Status']); + CourseManager::subscribeUser($user_id, $course, $user['Status']); $course_info = CourseManager::get_course_information($course); $inserted_in_course[$course] = $course_info['title']; } diff --git a/main/auth/courses.php b/main/auth/courses.php index f23f5488e5..de0fcfaa55 100755 --- a/main/auth/courses.php +++ b/main/auth/courses.php @@ -29,9 +29,7 @@ if (api_get_setting('course_catalog_published') !== 'true') { api_block_anonymous_users(); } -$user_can_view_page = false; - -//For students +// For students if (api_get_setting('allow_students_to_browse_courses') === 'false') { $user_can_view_page = false; } else { @@ -57,6 +55,8 @@ $actions = [ 'search_tag', 'search_session', 'set_collapsable', + 'subscribe_course_validation', + 'subscribe_course', ]; $action = CoursesAndSessionsCatalog::is(CATALOG_SESSIONS) ? 'display_sessions' : 'display_courses'; @@ -127,14 +127,6 @@ if (isset($_POST['submit_edit_course_category']) && } } -// we are deleting a course category -if ($action == 'deletecoursecategory' && isset($_GET['id'])) { - if (!empty($_GET['sec_token']) && $ctok == $_GET['sec_token']) { - $get_id_cat = intval($_GET['id']); - $courseController->delete_course_category($get_id_cat); - } -} - // We are creating a new user defined course category (= Create Course Category). if (isset($_POST['create_course_category']) && isset($_POST['title_course_category']) && @@ -160,16 +152,6 @@ if (isset($_REQUEST['search_course'])) { } } -// Subscribe user to course -if (isset($_REQUEST['subscribe_course'])) { - if (!empty($_GET['sec_token']) && $ctok == $_GET['sec_token']) { - $courseController->subscribe_user( - $_GET['subscribe_course'], - $searchTerm, - $categoryCode - ); - } -} // We are unsubscribing from a course (=Unsubscribe from course). if (isset($_GET['unsubscribe'])) { @@ -190,14 +172,67 @@ if (isset($_POST['unsubscribe'])) { } switch ($action) { - case 'subscribe_user_with_password': - $courseController->subscribe_user( - isset($_POST['subscribe_user_with_password']) ? $_POST['subscribe_user_with_password'] : '', - $searchTerm, - isset($_POST['category_code']) ? $_POST['category_code'] : '' + case 'deletecoursecategory': + // we are deleting a course category + if (isset($_GET['id'])) { + if (Security::check_token('get')) { + $courseController->delete_course_category($_GET['id']); + header('Location: '.api_get_self()); + exit; + } + } + break; + case 'subscribe_course': + if (api_is_anonymous()) { + header('Location: '.api_get_path(WEB_CODE_PATH).'auth/inscription.php?c='.$courseCodeToSubscribe); + exit; + } + $courseCodeToSubscribe = isset($_GET['subscribe_course']) ? Security::remove_XSS($_GET['subscribe_course']) : ''; + if (Security::check_token('get')) { + CourseManager::autoSubscribeToCourse($courseCodeToSubscribe); + header('Location: '.api_get_self()); + exit; + } + break; + case 'subscribe_course_validation': + $courseCodeToSubscribe = isset($_GET['subscribe_course']) ? Security::remove_XSS($_GET['subscribe_course']) : ''; + $courseInfo = api_get_course_info($courseCodeToSubscribe); + if (empty($courseInfo)) { + header('Location: '.api_get_self()); + exit; + } + $message = get_lang('CourseRequiresPassword').' '; + $message .= $courseInfo['title'].' ('.$courseInfo['visual_code'].') '; + + $action = api_get_self(). + '?action=subscribe_course_validation&sec_token='.Security::getTokenFromSession().'&subscribe_course='.$courseCodeToSubscribe; + $form = new FormValidator( + 'subscribe_user_with_password', + 'post', + $action ); - header('Location: '.api_get_self()); - exit; + $form->addHeader($message); + $form->addElement('hidden', 'sec_token', Security::getTokenFromSession()); + $form->addElement('hidden', 'subscribe_user_with_password', $courseInfo['code']); + $form->addElement('text', 'course_registration_code'); + $form->addButtonSave(get_lang('SubmitRegistrationCode')); + $content = $form->returnForm(); + + if ($form->validate()) { + if (sha1($_POST['course_registration_code']) === $courseInfo['registration_code']) { + CourseManager::autoSubscribeToCourse($_POST['subscribe_user_with_password']); + header('Location: '.api_get_self()); + exit; + } else { + Display::addFlash(Display::return_message(get_lang('CourseRegistrationCodeIncorrect')), 'warning'); + header('Location: '.$action); + exit; + } + } + + $template = new Template(get_lang('Subscribe'), true, true, false, false, false); + $template->assign('content', $content); + $template->display_one_col_template(); break; case 'createcoursecategory': $courseController->categoryList(); diff --git a/main/auth/courses_categories.php b/main/auth/courses_categories.php index ea3200ffa9..02e6263466 100755 --- a/main/auth/courses_categories.php +++ b/main/auth/courses_categories.php @@ -503,9 +503,14 @@ function return_already_registered_label($in_status) function return_register_button($course, $stok, $code, $search_term) { $title = get_lang('Subscribe'); + $action = 'subscribe_course'; + if (!empty($course['registration_code'])) { + $action = 'subscribe_course_validation'; + } + $html = Display::url( Display::returnFontAwesomeIcon('check').' '.$title, - api_get_self().'?action=subscribe_course&sec_token='.$stok. + api_get_self().'?action='.$action.'&sec_token='.$stok. '&subscribe_course='.$course['code'].'&search_term='.$search_term.'&category_code='.$code, ['class' => 'btn btn-success btn-sm', 'title' => $title, 'aria-label' => $title] ); diff --git a/main/auth/courses_controller.php b/main/auth/courses_controller.php index 3ca9b85ac0..2eb5316b2f 100755 --- a/main/auth/courses_controller.php +++ b/main/auth/courses_controller.php @@ -225,58 +225,6 @@ class CoursesController $this->view->render(); } - /** - * Auto user subscription to a course. - */ - public function subscribe_user($course_code, $search_term, $category_code) - { - $courseInfo = api_get_course_info($course_code); - - if (empty($courseInfo)) { - return false; - } - - // The course must be open in order to access the auto subscription - if (in_array( - $courseInfo['visibility'], - [ - COURSE_VISIBILITY_CLOSED, - COURSE_VISIBILITY_REGISTERED, - COURSE_VISIBILITY_HIDDEN, - ] - ) - ) { - Display::addFlash( - Display::return_message( - get_lang('SubscribingNotAllowed'), - 'warning' - ) - ); - } else { - // Redirect to subscription - if (api_is_anonymous()) { - header('Location: '.api_get_path(WEB_CODE_PATH).'auth/inscription.php?c='.$course_code); - exit; - } - $result = $this->model->subscribe_user($course_code); - if (!$result) { - Display::addFlash( - Display::return_message( - get_lang('CourseRegistrationCodeIncorrect'), - 'warning' - ) - ); - } else { - Display::addFlash( - Display::return_message($result['message'], 'normal', false) - ); - if (isset($result['content'])) { - Display::addFlash($result['content']); - } - } - } - } - /** * Create a category * render to listing view. diff --git a/main/auth/external_login/functions.inc.php b/main/auth/external_login/functions.inc.php index 7015dc4969..e8821270ff 100755 --- a/main/auth/external_login/functions.inc.php +++ b/main/auth/external_login/functions.inc.php @@ -216,7 +216,7 @@ function external_update_user($new_user) $autoSubscribe = explode('|', $u['courses']); foreach ($autoSubscribe as $code) { if (CourseManager::course_exists($code)) { - CourseManager::subscribe_user($u['user_id'], $code); + CourseManager::subscribeUser($u['user_id'], $code, STUDENT); } } } diff --git a/main/auth/external_login/newUser.php b/main/auth/external_login/newUser.php index 81701327d4..dd468e6805 100755 --- a/main/auth/external_login/newUser.php +++ b/main/auth/external_login/newUser.php @@ -34,7 +34,7 @@ if ($user !== false && ($chamilo_uid = external_add_user($user)) !== false) { $autoSubscribe = explode('|', $user['courses']); foreach ($autoSubscribe as $code) { if (CourseManager::course_exists($code)) { - CourseManager::subscribe_user($_user['user_id'], $code); + CourseManager::subscribeUser($_user['user_id'], $code); } } } diff --git a/main/auth/inscription.php b/main/auth/inscription.php index a06e920c18..eee389914e 100755 --- a/main/auth/inscription.php +++ b/main/auth/inscription.php @@ -812,7 +812,7 @@ if ($form->validate()) { ] ) ) { - CourseManager::subscribe_user( + CourseManager::subscribeUser( $user_id, $course_info['code'] ); @@ -1116,7 +1116,7 @@ if ($form->validate()) { ] ) ) { - CourseManager::subscribe_user( + CourseManager::subscribeUser( $user_id, $course_info['code'] ); diff --git a/main/course_home/course_home.php b/main/course_home/course_home.php index 2226262f9b..46b6562382 100755 --- a/main/course_home/course_home.php +++ b/main/course_home/course_home.php @@ -132,7 +132,7 @@ $isSpecialCourse = CourseManager::isSpecialCourse($courseId); if ($isSpecialCourse) { if (isset($_GET['autoreg']) && $_GET['autoreg'] == 1) { - if (CourseManager::subscribe_user($user_id, $course_code, STUDENT)) { + if (CourseManager::subscribeUser($user_id, $course_code, STUDENT)) { Session::write('is_allowed_in_course', true); } } @@ -141,18 +141,14 @@ if ($isSpecialCourse) { if (isset($_GET['action']) && $_GET['action'] == 'subscribe') { if (Security::check_token('get')) { Security::clear_token(); - $auth = new Auth(); - $msg = $auth->subscribe_user($course_code); - if (CourseManager::is_user_subscribed_in_course($user_id, $course_code)) { - Session::write('is_allowed_in_course', true); - } - if (!empty($msg)) { - $show_message .= Display::return_message( - get_lang($msg['message']), - 'info', - false - ); + $result = CourseManager::autoSubscribeToCourse($course_code); + if ($result) { + if (CourseManager::is_user_subscribed_in_course($user_id, $course_code)) { + Session::write('is_allowed_in_course', true); + } } + header('Location: '.api_get_self()); + exit; } } diff --git a/main/cron/import_csv.php b/main/cron/import_csv.php index 991e3fdbf6..ed6647babb 100755 --- a/main/cron/import_csv.php +++ b/main/cron/import_csv.php @@ -2419,7 +2419,7 @@ class ImportCsv $userCourseCategory = $courseUserData['user_course_cat']; } - CourseManager::subscribe_user( + $result = CourseManager::subscribeUser( $userId, $courseInfo['code'], $status, @@ -2427,9 +2427,15 @@ class ImportCsv $userCourseCategory ); - $this->logger->addInfo( - "User $userId added to course ".$courseInfo['code']." with status '$status' with course category: '$userCourseCategory'" - ); + if ($result) { + $this->logger->addInfo( + "User $userId added to course ".$courseInfo['code']." with status '$status' with course category: '$userCourseCategory'" + ); + } else { + $this->logger->addInfo( + "User $userId was NOT ADDED to course ".$courseInfo['code']." with status '$status' with course category: '$userCourseCategory'" + ); + } } } diff --git a/main/inc/lib/auth.lib.php b/main/inc/lib/auth.lib.php index cde3d5aeaf..eb850eb0aa 100755 --- a/main/inc/lib/auth.lib.php +++ b/main/inc/lib/auth.lib.php @@ -414,74 +414,4 @@ class Auth return $result; } - - /** - * Subscribe the user to a given course. - * - * @param string $course_code Course code - * - * @return string Message about results - */ - public function subscribe_user($course_code) - { - $user_id = api_get_user_id(); - $all_course_information = CourseManager::get_course_information($course_code); - - if ($all_course_information['registration_code'] == '' || - ( - isset($_POST['course_registration_code']) && - $_POST['course_registration_code'] == $all_course_information['registration_code'] - ) - ) { - if (api_is_platform_admin()) { - $status_user_in_new_course = COURSEMANAGER; - } else { - $status_user_in_new_course = null; - } - if (CourseManager::add_user_to_course($user_id, $course_code, $status_user_in_new_course)) { - $send = api_get_course_setting('email_alert_to_teacher_on_new_user_in_course', $course_code); - if ($send == 1) { - CourseManager::email_to_tutor( - $user_id, - $all_course_information['real_id'], - $send_to_tutor_also = false - ); - } elseif ($send == 2) { - CourseManager::email_to_tutor( - $user_id, - $all_course_information['real_id'], - $send_to_tutor_also = true - ); - } - $url = Display::url($all_course_information['title'], api_get_course_url($course_code)); - $message = sprintf(get_lang('EnrollToCourseXSuccessful'), $url); - } else { - $message = get_lang('ErrorContactPlatformAdmin'); - } - - return ['message' => $message]; - } else { - if (isset($_POST['course_registration_code']) && - $_POST['course_registration_code'] != $all_course_information['registration_code'] - ) { - return false; - } - $message = get_lang('CourseRequiresPassword').'
'; - $message .= $all_course_information['title'].' ('.$all_course_information['visual_code'].') '; - - $action = api_get_path(WEB_CODE_PATH)."auth/courses.php?action=subscribe_user_with_password&sec_token=".Security::getTokenFromSession(); - $form = new FormValidator( - 'subscribe_user_with_password', - 'post', - $action - ); - $form->addElement('hidden', 'sec_token', Security::getTokenFromSession()); - $form->addElement('hidden', 'subscribe_user_with_password', $all_course_information['code']); - $form->addElement('text', 'course_registration_code'); - $form->addButtonSave(get_lang('SubmitRegistrationCode')); - $content = $form->returnForm(); - - return ['message' => $message, 'content' => $content]; - } - } } diff --git a/main/inc/lib/course.lib.php b/main/inc/lib/course.lib.php index d3c2604a7c..37ad91f4dd 100755 --- a/main/inc/lib/course.lib.php +++ b/main/inc/lib/course.lib.php @@ -371,19 +371,22 @@ class CourseManager * @param mixed user_id or an array with user ids * @param string course code * @param int session id + * + * @return bool + * * @assert ('', '') === false */ public static function unsubscribe_user($user_id, $course_code, $session_id = 0) { if (empty($user_id)) { - return; + return false; } if (!is_array($user_id)) { $user_id = [$user_id]; } if (count($user_id) == 0) { - return; + return false; } if (!empty($session_id)) { @@ -392,8 +395,11 @@ class CourseManager $session_id = api_get_session_id(); } - $userList = []; + if (empty($course_code)) { + return false; + } + $userList = []; // Cleaning the $user_id variable if (is_array($user_id)) { $new_user_id_list = []; @@ -404,7 +410,7 @@ class CourseManager $userList = $new_user_id_list; $user_ids = implode(',', $new_user_id_list); } else { - $user_ids = intval($user_id); + $user_ids = (int) $user_id; $userList[] = $user_id; } @@ -420,7 +426,6 @@ class CourseManager Database::query($sql); // Erase user student publications (works) in the course - by André Boivin - if (!empty($userList)) { require_once api_get_path(SYS_CODE_PATH).'work/work.lib.php'; foreach ($userList as $userId) { @@ -550,74 +555,100 @@ class CourseManager } } + /** + * @param string $courseCode + * @param int $status + * + * @return bool + */ + public static function autoSubscribeToCourse($courseCode, $status = STUDENT) + { + $courseInfo = api_get_course_info($courseCode); + + if (empty($courseInfo)) { + return false; + } + + if (in_array( + $courseInfo['visibility'], + [ + COURSE_VISIBILITY_CLOSED, + COURSE_VISIBILITY_REGISTERED, + COURSE_VISIBILITY_HIDDEN, + ] + ) + ) { + Display::addFlash( + Display::return_message( + get_lang('SubscribingNotAllowed'), + 'warning' + ) + ); + + return false; + } + + return self::subscribeUser(api_get_user_id(), $courseInfo['code'], $status); + } + /** * Subscribe a user to a course. No checks are performed here to see if * course subscription is allowed. * - * @param int $user_id - * @param string $course_code + * @param int $userId + * @param string $courseCode * @param int $status (STUDENT, COURSEMANAGER, COURSE_ADMIN, NORMAL_COURSE_MEMBER) - * @param int $session_id + * @param int $sessionId * @param int $userCourseCategoryId + * @param bool $checkTeacherPermission * * @return bool True on success, false on failure * - * @see add_user_to_course * @assert ('', '') === false */ - public static function subscribe_user( - $user_id, - $course_code, + public static function subscribeUser( + $userId, + $courseCode, $status = STUDENT, - $session_id = 0, - $userCourseCategoryId = 0 + $sessionId = 0, + $userCourseCategoryId = 0, + $checkTeacherPermission = true ) { - if ($user_id != strval(intval($user_id))) { - return false; //detected possible SQL injection - } + $userId = (int) $userId; + $status = (int) $status; - if (empty($user_id) || empty($course_code)) { + if (empty($userId) || empty($courseCode)) { return false; } - $courseInfo = api_get_course_info($course_code); + $courseInfo = api_get_course_info($courseCode); if (empty($courseInfo)) { + Display::addFlash(Display::return_message(get_lang('CourseDoesNotExist'), 'warning')); + return false; } - $courseId = $courseInfo['real_id']; - $courseCode = $courseInfo['code']; - $userCourseCategoryId = (int) $userCourseCategoryId; + $userInfo = api_get_user_info($userId); - $session_id = empty($session_id) ? api_get_session_id() : (int) $session_id; - $status = ($status == STUDENT || $status == COURSEMANAGER) ? $status : STUDENT; + if (empty($userInfo)) { + Display::addFlash(Display::return_message(get_lang('UserDoesNotExist'), 'warning')); - // A preliminary check whether the user has been already registered on the platform. - $sql = "SELECT status FROM ".Database::get_main_table(TABLE_MAIN_USER)." - WHERE user_id = $user_id"; - if (Database::num_rows(Database::query($sql)) == 0) { - return false; // The user has not been registered to the platform. + return false; } - // Check whether the user has not been already subscribed to the course. - $sql = "SELECT * FROM ".Database::get_main_table(TABLE_MAIN_COURSE_USER)." - WHERE - user_id = $user_id AND - relation_type <> ".COURSE_RELATION_TYPE_RRHH." AND - c_id = $courseId - "; - if (empty($session_id)) { - if (Database::num_rows(Database::query($sql)) > 0) { - // The user has been already subscribed to the course. - return false; - } - } + $courseId = $courseInfo['real_id']; + $courseCode = $courseInfo['code']; + $userCourseCategoryId = (int) $userCourseCategoryId; - if (!empty($session_id)) { + $sessionId = empty($sessionId) ? api_get_session_id() : (int) $sessionId; + $status = $status === STUDENT || $status === COURSEMANAGER ? $status : STUDENT; + $courseUserTable = Database::get_main_table(TABLE_MAIN_COURSE_USER); + + if (!empty($sessionId)) { SessionManager::subscribe_users_to_session_course( - [$user_id], - $session_id, + [$userId], + $sessionId, $courseCode ); @@ -629,9 +660,8 @@ class CourseManager api_get_utc_datetime(), api_get_user_id(), $courseId, - $session_id + $sessionId ); - $userInfo = api_get_user_info($user_id); Event::addEvent( LOG_SUBSCRIBE_USER_TO_COURSE, LOG_USER_OBJECT, @@ -639,19 +669,98 @@ class CourseManager api_get_utc_datetime(), api_get_user_id(), $courseId, - $session_id + $sessionId ); return true; } else { - $userAdded = self::add_user_to_course( - $user_id, - $courseCode, - $status, - $userCourseCategoryId - ); + // Check whether the user has not been already subscribed to the course. + $sql = "SELECT * FROM ".Database::get_main_table(TABLE_MAIN_COURSE_USER)." + WHERE + user_id = $userId AND + relation_type <> ".COURSE_RELATION_TYPE_RRHH." AND + c_id = $courseId + "; + if (Database::num_rows(Database::query($sql)) > 0) { + Display::addFlash(Display::return_message(get_lang('AlreadyRegisteredToCourse'), 'warning')); + + return false; // The user has been subscribed to the course. + } + + if ($checkTeacherPermission && !api_is_course_admin()) { + // Check in advance whether subscription is allowed or not for this course. + if ((int) $courseInfo['subscribe'] === SUBSCRIBE_NOT_ALLOWED) { + Display::addFlash(Display::return_message(get_lang('SubscriptionNotAllowed'), 'warning')); + + return false; // Subscription is not allowed for this course. + } + } + + if ($status === STUDENT) { + // Check if max students per course extra field is set + $extraFieldValue = new ExtraFieldValue('course'); + $value = $extraFieldValue->get_values_by_handler_and_field_variable($courseId, 'max_subscribed_students'); + if (!empty($value) && isset($value['value'])) { + $maxStudents = $value['value']; + if ($maxStudents !== '') { + $maxStudents = (int) $maxStudents; + $count = self::get_user_list_from_course_code( + $courseCode, + 0, + null, + null, + STUDENT, + true, + false + ); + + if ($count >= $maxStudents) { + Display::addFlash(Display::return_message(get_lang('MaxNumberSubscribedStudentsReached'), 'warning')); + + return false; + } + } + } + } + + $maxSort = api_max_sort_value('0', $userId); + $params = [ + 'c_id' => $courseId, + 'user_id' => $userId, + 'status' => $status, + 'sort' => $maxSort + 1, + 'relation_type' => 0, + 'user_course_cat' => (int) $userCourseCategoryId, + ]; + $insertId = Database::insert($courseUserTable, $params); + + if ($insertId) { + Display::addFlash( + Display::return_message( + sprintf( + get_lang('UserXAddedToCourseX'), + $userInfo['complete_name_with_username'], + $courseInfo['title'] + ) + ) + ); + + $send = api_get_course_setting('email_alert_to_teacher_on_new_user_in_course', $courseCode); + + if ($send == 1) { + self::email_to_tutor( + $userId, + $courseInfo['real_id'], + false + ); + } elseif ($send == 2) { + self::email_to_tutor( + $userId, + $courseInfo['real_id'], + true + ); + } - if ($userAdded) { // Add event to the system log Event::addEvent( LOG_SUBSCRIBE_USER_TO_COURSE, @@ -662,7 +771,6 @@ class CourseManager $courseId ); - $userInfo = api_get_user_info($user_id); Event::addEvent( LOG_SUBSCRIBE_USER_TO_COURSE, LOG_USER_OBJECT, @@ -740,119 +848,6 @@ class CourseManager } } - /** - * Subscribe a user $user_id to a course defined by $courseCode. - * - * @author Hugues Peeters - * @author Roan Embrechts - * - * @param int $user_id the id of the user - * @param string $courseCode the course code - * @param int $status (optional) The user's status in the course - * @param int $userCourseCategoryId The user category in which this subscription will be classified - * @param bool $checkTeacherPermission - * - * @return false|string true if subscription succeeds, boolean false otherwise - * @assert ('', '') === false - */ - public static function add_user_to_course( - $user_id, - $courseCode, - $status = STUDENT, - $userCourseCategoryId = 0, - $checkTeacherPermission = true - ) { - $debug = false; - $user_table = Database::get_main_table(TABLE_MAIN_USER); - $course_table = Database::get_main_table(TABLE_MAIN_COURSE); - $course_user_table = Database::get_main_table(TABLE_MAIN_COURSE_USER); - - $status = ($status == STUDENT || $status == COURSEMANAGER) ? $status : STUDENT; - if (empty($user_id) || empty($courseCode) || ($user_id != strval(intval($user_id)))) { - return false; - } - - $courseCode = Database::escape_string($courseCode); - $courseInfo = api_get_course_info($courseCode); - $courseId = $courseInfo['real_id']; - - // Check in advance whether the user has already been registered on the platform. - $sql = "SELECT status FROM $user_table WHERE user_id = $user_id "; - if (Database::num_rows(Database::query($sql)) == 0) { - if ($debug) { - error_log('The user has not been registered to the platform'); - } - - return false; // The user has not been registered to the platform. - } - - // Check whether the user has already been subscribed to this course. - $sql = "SELECT * FROM $course_user_table - WHERE - user_id = $user_id AND - relation_type <> ".COURSE_RELATION_TYPE_RRHH." AND - c_id = $courseId"; - if (Database::num_rows(Database::query($sql)) > 0) { - if ($debug) { - error_log('The user has been already subscribed to the course'); - } - - return false; // The user has been subscribed to the course. - } - - if ($checkTeacherPermission && !api_is_course_admin()) { - // Check in advance whether subscription is allowed or not for this course. - $sql = "SELECT code, visibility FROM $course_table - WHERE id = $courseId AND subscribe = '".SUBSCRIBE_NOT_ALLOWED."'"; - if (Database::num_rows(Database::query($sql)) > 0) { - if ($debug) { - error_log('Subscription is not allowed for this course'); - } - - return false; // Subscription is not allowed for this course. - } - } - - if ($status === STUDENT) { - // Check if max students per course extra field is set - $extraFieldValue = new ExtraFieldValue('course'); - $value = $extraFieldValue->get_values_by_handler_and_field_variable($courseId, 'max_subscribed_students'); - if (!empty($value) && isset($value['value'])) { - $maxStudents = $value['value']; - if ($maxStudents !== '') { - $maxStudents = (int) $maxStudents; - $count = self::get_user_list_from_course_code( - $courseCode, - 0, - null, - null, - STUDENT, - true, - false - ); - - if ($count >= $maxStudents) { - return false; - } - } - } - } - - // Ok, subscribe the user. - $max_sort = api_max_sort_value('0', $user_id); - $params = [ - 'c_id' => $courseId, - 'user_id' => $user_id, - 'status' => $status, - 'sort' => $max_sort + 1, - 'relation_type' => 0, - 'user_course_cat' => (int) $userCourseCategoryId, - ]; - $insertId = Database::insert($course_user_table, $params); - - return $insertId; - } - /** * Add the user $userId visibility to the course $courseCode in the catalogue. * @@ -2602,10 +2597,8 @@ class CourseManager */ public static function email_to_tutor($user_id, $courseId, $send_to_tutor_also = false) { - if ($user_id != strval(intval($user_id))) { - return false; - } - $courseId = intval($courseId); + $user_id = (int) $user_id; + $courseId = (int) $courseId; $information = api_get_course_info_by_id($courseId); $course_code = $information['code']; $student = api_get_user_info($user_id); @@ -2618,9 +2611,9 @@ class CourseManager //if ($send_to_tutor_also = true) // Proposed change: if ($send_to_tutor_also) { - $sql .= " AND is_tutor = 1"; + $sql .= ' AND is_tutor = 1'; } else { - $sql .= " AND status = 1"; + $sql .= ' AND status = 1'; } $result = Database::query($sql); diff --git a/main/inc/lib/events.lib.php b/main/inc/lib/events.lib.php index c7f57e3e37..691522f217 100644 --- a/main/inc/lib/events.lib.php +++ b/main/inc/lib/events.lib.php @@ -89,7 +89,7 @@ class Event $autoSubscribe = explode('|', $autoSubscribe); foreach ($autoSubscribe as $code) { if (CourseManager::course_exists($code)) { - CourseManager::subscribe_user($userId, $code); + CourseManager::subscribeUser($userId, $code); } } } diff --git a/main/inc/lib/sessionmanager.lib.php b/main/inc/lib/sessionmanager.lib.php index ccc5dac21c..954c6b5c91 100755 --- a/main/inc/lib/sessionmanager.lib.php +++ b/main/inc/lib/sessionmanager.lib.php @@ -2316,11 +2316,11 @@ class SessionManager return false; } - $session_id = intval($session_id); + $session_id = (int) $session_id; + $session_visibility = (int) $session_visibility; $course_code = Database::escape_string($course_code); $courseInfo = api_get_course_info($course_code); $courseId = $courseInfo['real_id']; - $session_visibility = intval($session_visibility); if ($removeUsersNotInList) { $currentUsers = self::getUsersByCourseSession($session_id, $courseInfo, 0); @@ -2344,7 +2344,7 @@ class SessionManager $nbr_users = 0; foreach ($user_list as $enreg_user) { - $enreg_user = intval($enreg_user); + $enreg_user = (int) $enreg_user; // Checking if user exists in session - course - user table. $sql = "SELECT count(user_id) as count FROM $tbl_session_rel_course_rel_user @@ -5505,7 +5505,7 @@ SQL; $userCourseCategory = $courseUserData['user_course_cat']; } - CourseManager::subscribe_user( + CourseManager::subscribeUser( $teacherToAdd, $course_code, COURSEMANAGER, @@ -5628,7 +5628,7 @@ SQL; $userCourseCategory = $courseUserData['user_course_cat']; } - CourseManager::subscribe_user( + CourseManager::subscribeUser( $teacherId, $course_code, COURSEMANAGER, diff --git a/main/inc/lib/usergroup.lib.php b/main/inc/lib/usergroup.lib.php index 318714ee0e..8c591a0627 100755 --- a/main/inc/lib/usergroup.lib.php +++ b/main/inc/lib/usergroup.lib.php @@ -881,7 +881,7 @@ class UserGroup extends Model if ($course_info) { if (!empty($user_list)) { foreach ($user_list as $user_id) { - CourseManager::subscribe_user( + CourseManager::subscribeUser( $user_id, $course_info['code'] ); @@ -1031,7 +1031,7 @@ class UserGroup extends Model if (!empty($course_list)) { foreach ($course_list as $course_id) { $course_info = api_get_course_info_by_id($course_id); - CourseManager::subscribe_user($user_id, $course_info['code']); + CourseManager::subscribeUser($user_id, $course_info['code']); } } $params = [ diff --git a/main/inc/lib/webservices/Rest.php b/main/inc/lib/webservices/Rest.php index 79261dc4d7..b5dfc88c2d 100644 --- a/main/inc/lib/webservices/Rest.php +++ b/main/inc/lib/webservices/Rest.php @@ -1289,7 +1289,7 @@ class Rest extends WebService if (!$course_code) { $course_code = CourseManager::get_course_code_from_course_id($course_id); } - if (CourseManager::subscribe_user($user_id, $course_code)) { + if (CourseManager::subscribeUser($user_id, $course_code)) { return [true]; } else { return [false]; diff --git a/main/user/subscribe_user.php b/main/user/subscribe_user.php index e25b277536..54f39171f4 100755 --- a/main/user/subscribe_user.php +++ b/main/user/subscribe_user.php @@ -60,10 +60,9 @@ $list_not_register_user = ''; if (isset($_REQUEST['register'])) { $userInfo = api_get_user_info($_REQUEST['user_id']); if ($userInfo) { - $message = $userInfo['complete_name_with_username'].' '.get_lang('AddedToCourse'); - if ($type === COURSEMANAGER) { if (!empty($sessionId)) { + $message = $userInfo['complete_name_with_username'].' '.get_lang('AddedToCourse'); SessionManager::set_coach_to_course_session( $_REQUEST['user_id'], $sessionId, @@ -71,25 +70,17 @@ if (isset($_REQUEST['register'])) { ); Display::addFlash(Display::return_message($message)); } else { - $result = CourseManager::subscribe_user( + CourseManager::subscribeUser( $_REQUEST['user_id'], $courseInfo['code'], COURSEMANAGER ); - if ($result) { - Display::addFlash(Display::return_message($message)); - } } } else { - $result = CourseManager::subscribe_user( + CourseManager::subscribeUser( $_REQUEST['user_id'], $courseInfo['code'] ); - if ($result) { - Display::addFlash(Display::return_message($message)); - } else { - Display::addFlash(Display::return_message(get_lang('MaxNumberSubscribedStudentsReached'), 'warning')); - } } } header('Location:'.api_get_path(WEB_CODE_PATH).'user/user.php?'.api_get_cidreq().'&type='.$type); @@ -116,7 +107,7 @@ if (isset($_POST['action'])) { $isSuscribe[] = $message; } } else { - $result = CourseManager::subscribe_user( + $result = CourseManager::subscribeUser( $user_id, $courseInfo['code'], COURSEMANAGER @@ -127,7 +118,7 @@ if (isset($_POST['action'])) { } } } else { - $result = CourseManager::subscribe_user( + $result = CourseManager::subscribeUser( $user_id, $courseInfo['code'] ); diff --git a/main/user/user_import.php b/main/user/user_import.php index cd2e1ceb2b..839e772f23 100755 --- a/main/user/user_import.php +++ b/main/user/user_import.php @@ -104,10 +104,10 @@ if ($form->validate()) { foreach ($clean_users as $userId) { $userInfo = api_get_user_info($userId); - CourseManager::subscribe_user($userId, $course_code, $userType, $session_id); + CourseManager::subscribeUser($userId, $course_code, $userType, $session_id); if (empty($session_id)) { //just to make sure - if (CourseManager :: is_user_subscribed_in_course($userId, $course_code)) { + if (CourseManager::is_user_subscribed_in_course($userId, $course_code)) { $user_to_show[] = $userInfo['complete_name']; } } else { diff --git a/main/webservices/cm_webservice_course.php b/main/webservices/cm_webservice_course.php index 2b8b7931be..e3775f2bac 100755 --- a/main/webservices/cm_webservice_course.php +++ b/main/webservices/cm_webservice_course.php @@ -696,7 +696,7 @@ class WSCMCourse extends WSCM return true; } else { // Subscribe user - if (CourseManager::subscribe_user($user_id, $course_code, $status)) { + if (CourseManager::subscribeUser($user_id, $course_code, $status)) { return true; } else { return new WSError(203, 'An error occured subscribing to this course'); diff --git a/main/webservices/registration.soap.php b/main/webservices/registration.soap.php index d4772fe606..954352c20a 100755 --- a/main/webservices/registration.soap.php +++ b/main/webservices/registration.soap.php @@ -4653,7 +4653,7 @@ function WSSubscribeUserToCourse($params) if ($debug) { error_log('WSSubscribeUserToCourse courseCode: '.$courseCode); } - $result = CourseManager::add_user_to_course($user_id, $courseCode, $status, false, false); + $result = CourseManager::subscribeUser($user_id, $courseCode, $status, 0, 0, false); if ($result) { $resultValue = 1; if ($debug) { @@ -4767,7 +4767,7 @@ function WSSubscribeUserToCourseSimple($params) if ($debug) { error_log('Try to register: user_id= '.$user_id.' to course: '.$course_data['code']); } - if (!CourseManager::add_user_to_course($user_id, $course_data['code'], $status, false, false)) { + if (!CourseManager::subscribeUser($user_id, $course_data['code'], $status, 0, false, false)) { $result = 'User was not registered possible reasons: User already registered to the course, Course visibility doesnt allow user subscriptions '; if ($debug) { diff --git a/main/webservices/webservice_course.php b/main/webservices/webservice_course.php index f9f05fca41..2637873e4b 100755 --- a/main/webservices/webservice_course.php +++ b/main/webservices/webservice_course.php @@ -713,7 +713,7 @@ class WSCourse extends WS return true; } else { // Subscribe user - if (CourseManager::subscribe_user( + if (CourseManager::subscribeUser( $user_id, $course_code, $status diff --git a/plugin/buycourses/src/buy_course_plugin.class.php b/plugin/buycourses/src/buy_course_plugin.class.php index b0c72bb51f..367970a880 100644 --- a/plugin/buycourses/src/buy_course_plugin.class.php +++ b/plugin/buycourses/src/buy_course_plugin.class.php @@ -879,7 +879,7 @@ class BuyCoursesPlugin extends Plugin switch ($sale['product_type']) { case self::PRODUCT_TYPE_COURSE: $course = api_get_course_info_by_id($sale['product_id']); - $saleIsCompleted = CourseManager::subscribe_user($sale['user_id'], $course['code']); + $saleIsCompleted = CourseManager::subscribeUser($sale['user_id'], $course['code']); break; case self::PRODUCT_TYPE_SESSION: SessionManager::subscribeUsersToSession( diff --git a/plugin/search_course/lib/register_course_widget.class.php b/plugin/search_course/lib/register_course_widget.class.php index 0218bd2d49..4a7e0cee82 100755 --- a/plugin/search_course/lib/register_course_widget.class.php +++ b/plugin/search_course/lib/register_course_widget.class.php @@ -109,7 +109,7 @@ class RegisterCourseWidget $user_id = $_user['user_id']; } - return (bool) CourseManager::add_user_to_course($user_id, $course_code); + return (bool) CourseManager::subscribeUser($user_id, $course_code); } /** diff --git a/tests/behat/features/course_user_registration.feature b/tests/behat/features/course_user_registration.feature index 17b9275e8a..afe078ae04 100644 --- a/tests/behat/features/course_user_registration.feature +++ b/tests/behat/features/course_user_registration.feature @@ -7,7 +7,7 @@ Feature: Subscribe users to the course Given I am on "/main/user/subscribe_user.php?keyword=amann&type=5&cidReq=TEMP" Then I should see "Aimee" Then I follow "Register" - Then I should see "Aimee Mann (amann) has been registered to your course" + Then I should see "User Aimee Mann (amann) has been registered to course TEMP" Scenario: Unsubscribe user "amann" the course "TEMP" Given I am on "/main/user/user.php?cidReq=TEMP" @@ -20,4 +20,4 @@ Feature: Subscribe users to the course Given I am on "/main/user/subscribe_user.php?keyword=acostea&type=5&cidReq=TEMP" Then I should see "Andrea" Then I follow "Register" - Then I should see "Andrea Costea (acostea) has been registered to your course" \ No newline at end of file + Then I should see "User Aimee Mann (amann) has been registered to course TEMP" \ No newline at end of file diff --git a/tests/scripts/users_no_course.php b/tests/scripts/users_no_course.php index afa9b225e3..85a8f1b4af 100644 --- a/tests/scripts/users_no_course.php +++ b/tests/scripts/users_no_course.php @@ -27,7 +27,7 @@ $students = Database::store_result($result); if (!empty($students)) { foreach ($students as $student) { var_dump($student['username'].'- '.$student['user_id']); - $result = CourseManager::add_user_to_course($student['user_id'], $courseCode); + $result = CourseManager::subscribeUser($student['user_id'], $courseCode); var_dump($result); echo '
'; }