Skip to content

Commit

Permalink
Bugfix: Several fixes for self-learning courses (#743)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ibernhardf
ibernhardf committed Dec 3, 2024
1 parent a0e3216 commit 1a7f66d
Show file tree
Hide file tree
Showing 8 changed files with 72 additions and 46 deletions.
4 changes: 2 additions & 2 deletions classes/bo_availability/conditions/optionhasstarted.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ public function is_available(booking_option_settings $settings, int $userid, boo
// The actual meaning of this field is "allow booking after option has started".
if ($bookingsettings->allowupdate == 1) {
$isavailable = true;
} else if (!empty($settings->duration)) {
// For booking options with a duration (self-learning courses) this condition may never block.
} else if (!empty($settings->selflearningcourse)) {
// For self-learning courses this condition may never block.
$isavailable = true;
} else if (!empty($settings->coursestarttime)) {
// In this case, we have to check if the booking option has already started.
Expand Down
18 changes: 13 additions & 5 deletions classes/booking_option.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,8 @@ public function update_booked_users() {
// If $bookanyone is true, we do not check for enrolment.
$this->bookedusers = $bookanyone ? $allanswers : array_intersect_key($allanswers, $this->booking->canbookusers);

// TODO offer users with according caps to delete excluded users from booking option.
// phpcs:ignore moodle.Commenting.TodoComment.MissingInfoInline
// TODO: Offer users with according caps to delete excluded users from booking option.
$this->numberofanswers = count($this->bookedusers);
if (groups_get_activity_groupmode($this->booking->cm) == SEPARATEGROUPS &&
!has_capability('moodle/site:accessallgroups',
Expand Down Expand Up @@ -752,11 +753,11 @@ public function user_delete_response($userid, $cancelreservation = false,
// If a whole booking option was cancelled, we can use the new global booking rules...
// ...and react to the event bookingoption_cancelled instead.
if (!$bookingoptioncancel) {

if ($userid == $USER->id) {
// Participant cancelled the booking herself.
$msgparam = MOD_BOOKING_MSGPARAM_CANCELLED_BY_PARTICIPANT;

// phpcs:ignore moodle.Commenting.TodoComment.MissingInfoInline
// TODO: Trigger event.
} else {
// An admin user cancelled the booking.
Expand Down Expand Up @@ -1477,6 +1478,11 @@ public function enrol_user(
) {
global $DB;

if (!$isteacher) {
// Second check to make sure the user is really no teacher of this option.
$isteacher = booking_check_if_teacher($this->optionid, $userid);
}

$bookingsettings = singleton_service::get_instance_of_booking_settings_by_bookingid($this->bookingid);
if (!$manual) {
if (!$bookingsettings->autoenrol) {
Expand Down Expand Up @@ -1515,10 +1521,11 @@ public function enrol_user(
) {
// For self-learning courses, users will be enrolled from the time booked...
// ...until the duration of the self-learning course has passed #684.
// Use booking_check_if_teacher here instead of $isteacher to make this independent of the function call.
if (!empty($this->settings->selflearningcourse) && !$isteacher) {
$now = time();
$duration = $this->settings->duration ?? 0;
$end = $now + $duration;
$end = empty($duration) ? 0 : $now + $duration;
// Enrol using the default role from now until now + duration.
$enrol->enrol_user(
$instance,
Expand All @@ -1528,11 +1535,11 @@ public function enrol_user(
$end
);
} else if (empty($this->settings->semesterid) || $isteacher) {
// If a semester is set for the booking option...
// ...then we only want to enrol from semester startdate to semester enddate.
// Enrol using the default role.
$enrol->enrol_user($instance, $userid, ($roleid > 0 ? $roleid : $instance->roleid));
} else {
// If a semester is set for the booking option...
// ...then we only want to enrol from semester startdate to semester enddate.
if ($semesterobj = $DB->get_record('booking_semesters', ['id' => $this->settings->semesterid])) {
// Enrol using the default role from semester start until semester end.
$enrol->enrol_user(
Expand Down Expand Up @@ -2536,6 +2543,7 @@ public function return_array_of_sessions($bookingevent = null,
// 0 in a date id means it comes form normal course start & endtime.
// Therefore, there can't be these customfields.
if ($withcustomfields && $date->id !== 0) {
// phpcs:ignore moodle.Commenting.TodoComment.MissingInfoInline
// TODO: Can we cache this?
// Filter the matching customfields.
$returnsession['customfields'] = self::return_array_of_customfields(
Expand Down
6 changes: 4 additions & 2 deletions classes/booking_option_settings.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ class booking_option_settings {
/** @var int $sqlfilter defines if an element should be hidden via sql filter. hidden > 0 */
public $sqlfilter = 0;

/** @var int $attachedfiles The links on the attached files */
/** @var array $attachedfiles The links on the attached files */
public $attachedfiles = [];

/**
Expand Down Expand Up @@ -609,7 +609,9 @@ private function set_values(int $optionid, ?object $dbrecord = null) {
$this->electivecombinations = $dbrecord->electivecombinations;
}

// phpcs:ignore moodle.Commenting.TodoComment.MissingInfoInline
// TODO: This is a performance problem. We need to cache campaigns!
// phpcs:ignore moodle.Commenting.TodoComment.MissingInfoInline
// TODO: We need to cache get_all_campaigns too!
// Check if there are active campaigns.
// If yes, we need to apply the booking limit factor.
Expand Down Expand Up @@ -1069,7 +1071,7 @@ private function load_data_from_json(stdClass &$dbrecord) {
$this->boactions = $dbrecord->boactions ?? null;
$this->canceluntil = $dbrecord->canceluntil ?? 0;
$this->useprice = $dbrecord->useprice ?? null;
$this->selflearningcourse = $dbrecord->selflearningcourse ?? null;
$this->selflearningcourse = $dbrecord->selflearningcourse ?? 0;
$this->waitforconfirmation = $dbrecord->waitforconfirmation ?? 0;
$this->jsonobject = $dbrecord->jsonobject ?? null;
}
Expand Down
7 changes: 6 additions & 1 deletion classes/dates.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,12 @@ public static function set_data(stdClass &$defaultvalues) {
);
} else if (!empty($defaultvalues->id)) {
$settings = singleton_service::get_instance_of_booking_option_settings($defaultvalues->id);
$sessions = $settings->sessions;
// Make sure, no sessions are created for self-learning courses.
if (empty($settings->selflearningcourse)) {
$sessions = $settings->sessions;
} else {
$sessions = [];
}
$defaultvalues->datescounter = $datescounter;
}

Expand Down
8 changes: 5 additions & 3 deletions classes/option/fields/coursestarttime.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,12 @@ public static function prepare_save_field(
$returnvalue = null
): array {
if (!empty($formdata->selflearningcourse)) {
return parent::prepare_save_field($formdata, $newoption, $updateparam, $returnvalue);
} else {
return [];
/* For self-learning courses we store the sorting date (in field coursestarttime)
as both coursestarttime and courseendtime. */
$newoption->coursestarttime = $formdata->coursestarttime ?? 0;
$newoption->courseendtime = $formdata->coursestarttime ?? 0;
}
return [];
}

/**
Expand Down
23 changes: 12 additions & 11 deletions classes/output/bookingoption_description.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -363,17 +363,18 @@ public function __construct(

// Every date will be an array of datestring and customfields.
// But customfields will only be shown if we show booking option information inline.

$this->dates = $bookingoption->return_array_of_sessions(
$bookingevent,
$descriptionparam,
$withcustomfields,
$forbookeduser,
$ashtml
);

if (!empty($this->dates)) {
$this->datesexist = true;
// Make sure, that optiondates (sessions) are not stored for self-learning courses.
if (empty($settings->selflearningcourse)) {
$this->dates = $bookingoption->return_array_of_sessions(
$bookingevent,
$descriptionparam,
$withcustomfields,
$forbookeduser,
$ashtml
);
if (!empty($this->dates)) {
$this->datesexist = true;
}
}

$colteacher = new col_teacher($optionid, $settings);
Expand Down
38 changes: 20 additions & 18 deletions classes/output/col_coursestarttime.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,9 @@ public function __construct($optionid, $booking = null, $cmid = null, $collapsed
}

$settings = singleton_service::get_instance_of_booking_option_settings($optionid);
$this->optionid = $optionid;

// For self-learning courses, we do not show any optiondates (sessions).
if (!empty($settings->selflearningcourse)) {
$ba = singleton_service::get_instance_of_booking_answers($settings);
$buyforuser = price::return_user_to_buy_for();
Expand All @@ -89,16 +91,16 @@ public function __construct($optionid, $booking = null, $cmid = null, $collapsed

$this->selflearningcourse = true;
$this->duration = format_time($settings->duration);
}

$this->optionid = $optionid;
$this->datestrings = dates_handler::return_array_of_sessions_simple($optionid);
} else {
// No self-learning course.
$this->datestrings = dates_handler::return_array_of_sessions_simple($optionid);

$maxdates = get_config('booking', 'collapseshowsettings') ?? 2; // Hardcoded fallback on two.
$maxdates = get_config('booking', 'collapseshowsettings') ?? 2; // Hardcoded fallback on two.

// Show a collapse button for the dates.
if (!empty($this->datestrings) && count($this->datestrings) > $maxdates && $collapsed == true) {
$this->showcollapsebtn = true;
// Show a collapse button for the dates.
if (!empty($this->datestrings) && count($this->datestrings) > $maxdates && $collapsed == true) {
$this->showcollapsebtn = true;
}
}
}

Expand All @@ -112,17 +114,17 @@ public function __construct($optionid, $booking = null, $cmid = null, $collapsed
*/
public function export_for_template(renderer_base $output) {

if (!$this->datestrings) {
if (!empty($this->selflearningcourse)) {
$returnarr['selflearningcourse'] = $this->selflearningcourse;
$returnarr['duration'] = $this->duration;
if (!empty($this->timeremaining)) {
$returnarr['timeremaining'] = $this->timeremaining;
}
return $returnarr;
} else {
return [];
if (!empty($this->selflearningcourse)) {
$returnarr['selflearningcourse'] = $this->selflearningcourse;
$returnarr['duration'] = $this->duration;
if (!empty($this->timeremaining)) {
$returnarr['timeremaining'] = $this->timeremaining;
}
return $returnarr;
}

if (empty($this->datestrings)) {
return [];
}

$returnarr = [
Expand Down
14 changes: 10 additions & 4 deletions lib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1408,16 +1408,22 @@ function booking_extend_settings_navigation(settings_navigation $settings, navig
/**
* Check if logged in user is a teacher of the passed option.
* @param mixed|int $optionoroptionid optional option class or optionid
* @param int $userid optional userid, if none is provided, we use the logged-in $USER->id
* @return true if is assigned as teacher otherwise return false
*/
function booking_check_if_teacher($optionoroptionid = null) {
function booking_check_if_teacher($optionoroptionid = null, int $userid = 0) {
global $DB, $USER;

// If no userid is provided, we use the logged-in user.
if (empty($userid)) {
$userid = $USER->id;
}

if (empty($optionoroptionid)) {
// If we have no option, we check, if the teacher is a teacher of ANY option.
$user = $DB->get_records(
'booking_teachers',
['userid' => $USER->id]
['userid' => $userid]
);
if (empty($user)) {
return false;
Expand All @@ -1433,11 +1439,11 @@ function booking_check_if_teacher($optionoroptionid = null) {
return false;
}
$settings = singleton_service::get_instance_of_booking_option_settings($optionid);
if (in_array($USER->id, $settings->teacherids)) {
if (in_array($userid, $settings->teacherids)) {
return true;
} else if (
get_config('booking', 'responsiblecontactcanedit')
&& $settings->responsiblecontact == $USER->id
&& $settings->responsiblecontact == $userid
) {
return true;
} else {
Expand Down

0 comments on commit 1a7f66d

Please sign in to comment.