From 2a79d304b63756752fb50dcad9c5341eaffafd82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Georg=20Mai=C3=9Fer?= Date: Thu, 5 Dec 2024 21:09:45 +0100 Subject: [PATCH] GH-746 Implemented further pricecategoryfallback strategy without breaking current behaviour --- classes/price.php | 68 +++-- lang/de/booking.php | 3 + lang/en/booking.php | 3 + settings.php | 26 +- tests/bo_availability/condition_all_test.php | 245 +++++++++++++++++++ 5 files changed, 313 insertions(+), 32 deletions(-) diff --git a/classes/price.php b/classes/price.php index 3edd9582f..049b3091d 100644 --- a/classes/price.php +++ b/classes/price.php @@ -757,7 +757,7 @@ public static function get_price(string $area, int $itemid, ?object $user = null $pricerecorddefault = $pricerecord; } // Looking for matched pricecategory. - if (strpos($categoryidentifier, $pricecategoryidentifier) !== false) { + if (!empty($categoryidentifier) && strpos($categoryidentifier, $pricecategoryidentifier) !== false) { $pricecategoryfound = true; $price = [ "price" => $pricerecord->price, @@ -770,30 +770,41 @@ public static function get_price(string $area, int $itemid, ?object $user = null } } - // We use the default record as a fallback. + switch ((int)get_config('booking', 'pricecategoryfallback')) { + case 1: + // Logic is: when categoryidentifer is empty, we use default. + $usedefault = true; + break; + case 2: + $usedefault = false; + break; + default: + $usedefault = false; + break; + } + if ( - $pricecategoryfound === false - && get_config('booking', 'pricecategoryfallback') + !$pricecategoryfound + && $usedefault + && !empty($pricerecorddefault) ) { - if (!empty($pricerecorddefault)) { - $price = [ - "price" => $pricerecorddefault->price, - "currency" => $pricerecorddefault->currency, - "pricecategoryidentifier" => $pricerecorddefault->pricecategoryidentifier, - "pricecategoryname" => - self::get_active_pricecategory_from_cache_or_db($pricerecorddefault->pricecategoryidentifier)->name, - ]; - } else { - return []; // No default for some reason (should never happens). - } + $price = [ + "price" => $pricerecorddefault->price, + "currency" => $pricerecorddefault->currency, + "pricecategoryidentifier" => $pricerecorddefault->pricecategoryidentifier, + "pricecategoryname" => + self::get_active_pricecategory_from_cache_or_db($pricerecorddefault->pricecategoryidentifier)->name, + ]; } else if ( - $pricecategoryfound === false - && empty(get_config('booking', 'pricecategoryfallback')) + !$pricecategoryfound + && !$usedefault ) { - return []; + return []; // No default for some reason (should never happens). } - if ($area === "option" && isset($price['price'])) { + if ( + $area === "option" && isset($price['price']) + ) { $customformstore = new customformstore($user->id, $itemid); $price['price'] = $customformstore->modify_price($price['price'], $categoryidentifier); } @@ -849,17 +860,26 @@ public static function get_pricecategory_for_user(stdClass $user) { // If a user profile field to story the price category identifiers for each user has been set, // then retrieve it from config and set the correct category identifier for the current user. $fieldshortname = get_config('booking', 'pricecategoryfield'); + $pricecategoryfallback = get_config('booking', 'pricecategoryfallback'); - if (!isset($user->profile) || - !isset($user->profile[$fieldshortname])) { + if ( + !isset($user->profile) || + !isset($user->profile[$fieldshortname]) + ) { require_once("$CFG->dirroot/user/profile/lib.php"); profile_load_custom_fields($user); } - if (!isset($user->profile[$fieldshortname]) - || empty($user->profile[$fieldshortname])) { - $categoryidentifier = 'default'; // Default. + if ( + !isset($user->profile[$fieldshortname]) + || empty($user->profile[$fieldshortname]) + ) { + if ($pricecategoryfallback == 2) { + $categoryidentifier = ''; + } else { + $categoryidentifier = 'default'; // Default. + } } else { $categoryidentifier = $user->profile[$fieldshortname]; } diff --git a/lang/de/booking.php b/lang/de/booking.php index ba5c44670..421fc26c4 100755 --- a/lang/de/booking.php +++ b/lang/de/booking.php @@ -1124,6 +1124,9 @@ $string['extendlimitforoverbooked_help'] = 'Wählen Sie diese Option, passiert folgendes: Ein Kurs hat ein Limit von 40. Er ist aber bereits mit 2 TN auf 42 TN überbucht. Wird auf diesen Kurs eine Limiterhöhung um beispielsweise 10% angewandt, wird das Limit auf 46 erhöht (40 + 4 (10%) + 2 (bereits überbuchte)), statt auf 44 (40+4).'; +$string['fallbackonlywhenempty'] = 'Fallback nur, wenn entsprechendes Nutzerprofilfeld leer ist'; +$string['fallbackonlywhennotmatching'] = 'Fallback nur, wenn nicht übereinstimmend (auch wenn Feld leer ist)'; +$string['fallbackturnedoff'] = 'Fallback deaktiviert'; $string['feedbackurl'] = 'Link zur Umfrage'; $string['feedbackurl_help'] = 'Link zu einem Feedback-Formular, das an Teilnehmer:innen gesendet werden soll. Verwenden Sie in E-Mails den Platzhalter {pollurl}.'; diff --git a/lang/en/booking.php b/lang/en/booking.php index 262e6c2c6..b206b690f 100755 --- a/lang/en/booking.php +++ b/lang/en/booking.php @@ -1136,6 +1136,9 @@ $string['extendlimitforoverbooked_help'] = 'If you select this option, the following happens: A course has a limit of 40 but is already overbooked with 2 participants to 42 participants. If a limit increase of, for example, 10% is applied to this course, the limit will be increased to 46 (40 + 4 (10%) + 2 (overbooked seats)), instead of 44 (40 + 4).'; +$string['fallbackonlywhenempty'] = 'Fallback only when user profile field is empty'; +$string['fallbackonlywhennotmatching'] = 'Fallback when not matching (also empty)'; +$string['fallbackturnedoff'] = 'Fallback turned off'; $string['feedbackurl'] = 'Poll url'; $string['feedbackurl_help'] = 'Enter a link to a feedback form that should be sent to participants. It can be added to e-mails with the {pollurl} placeholder.'; diff --git a/settings.php b/settings.php index 26783b75a..b5a163247 100755 --- a/settings.php +++ b/settings.php @@ -69,7 +69,7 @@ get_string('optionformconfig', 'mod_booking'), new moodle_url('/mod/booking/optionformconfig.php', [ 'cmid' => 0, - ]) + ]) ) ); @@ -163,7 +163,7 @@ 'licensekeycfgheading', get_string('licensekeycfg', 'mod_booking'), $proversion ? get_string('licensekeycfgdesc:active', 'mod_booking') : - get_string('licensekeycfgdesc', 'mod_booking') + get_string('licensekeycfgdesc', 'mod_booking') ) ); @@ -926,12 +926,20 @@ $userprofilefieldsarray ) ); + + $defaultbehaviours = [ + 0 => get_string('fallbackonlywhenempty', 'booking'), + 1 => get_string('fallbackonlywhennotmatching', 'booking'), + 2 => get_string('fallbackturnedoff', 'booking'), + ]; + $settings->add( - new admin_setting_configcheckbox( + new admin_setting_configselect( 'booking/pricecategoryfallback', get_string('pricecategoryfallback', 'mod_booking'), get_string('pricecategoryfallback_desc', 'mod_booking'), - 0 + 0, + $defaultbehaviours ) ); @@ -1377,10 +1385,12 @@ ) ); - $options = [1 => get_string('courseurl', 'mod_booking'), - 2 => get_string('location', 'mod_booking'), - 3 => get_string('institution', 'mod_booking'), 4 => get_string('address'), - ]; + $options = [ + 1 => get_string('courseurl', 'mod_booking'), + 2 => get_string('location', 'mod_booking'), + 3 => get_string('institution', 'mod_booking'), + 4 => get_string('address'), + ]; $settings->add( new admin_setting_configselect( 'booking/icalfieldlocation', diff --git a/tests/bo_availability/condition_all_test.php b/tests/bo_availability/condition_all_test.php index 548bf2f5b..afd1bfcae 100644 --- a/tests/bo_availability/condition_all_test.php +++ b/tests/bo_availability/condition_all_test.php @@ -368,6 +368,251 @@ public function test_booking_bookit_with_pricecategories_and_zero_price(array $b singleton_service::get_instance()->userpricecategory = []; } + /** + * Test of booking option with fallback different displayemptyprice settings. + * + * @covers \condition\priceset::is_available + * + * @param array $bdata + * @throws \coding_exception + * @throws \dml_exception + * + * @dataProvider booking_common_settings_provider + * + */ + public function test_booking_bookit_with_pricecategories_and_fallback(array $bdata): void { + global $DB, $CFG; + + $this->tearDown(); + + $this->setAdminUser(); + + // Set parems requred for cancellation. + $bdata['cancancelbook'] = 1; + if (class_exists('local_shopping_cart\shopping_cart')) { + set_config('cancelationfee', 0, 'local_shopping_cart'); + } + + // Setup test data. + $course1 = $this->getDataGenerator()->create_course(['enablecompletion' => 1]); + $course2 = $this->getDataGenerator()->create_course(['enablecompletion' => 1]); + + // Create user profile custom fields. + $this->getDataGenerator()->create_custom_profile_field([ + 'datatype' => 'text', 'shortname' => 'pricecat', 'name' => 'pricecat', + ]); + set_config('pricecategoryfield', 'pricecat', 'booking'); + set_config('displayemptyprice', 1, 'booking'); + + // Create users. + $student1 = $this->getDataGenerator()->create_user(['profile_field_pricecat' => 'student']); + $student2 = $this->getDataGenerator()->create_user(['profile_field_pricecat' => 'staff']); + $student3 = $this->getDataGenerator()->create_user(); + $student4 = $this->getDataGenerator()->create_user(); + $teacher = $this->getDataGenerator()->create_user(); + $bookingmanager = $this->getDataGenerator()->create_user(); // Booking manager. + + $bdata['course'] = $course1->id; + $bdata['bookingmanager'] = $bookingmanager->username; + + $booking1 = $this->getDataGenerator()->create_module('booking', $bdata); + + $this->getDataGenerator()->enrol_user($student1->id, $course1->id, 'student'); + $this->getDataGenerator()->enrol_user($student2->id, $course1->id, 'student'); + $this->getDataGenerator()->enrol_user($student3->id, $course1->id, 'student'); + $this->getDataGenerator()->enrol_user($student4->id, $course1->id); + $this->getDataGenerator()->enrol_user($teacher->id, $course1->id, 'editingteacher'); + $this->getDataGenerator()->enrol_user($bookingmanager->id, $course1->id, 'editingteacher'); + + /** @var mod_booking_generator $plugingenerator */ + $plugingenerator = self::getDataGenerator()->get_plugin_generator('mod_booking'); + + $pricecategorydata1 = (object)[ + 'ordernum' => 1, + 'name' => 'default', + 'identifier' => 'default', + 'defaultvalue' => 50, + 'pricecatsortorder' => 1, + ]; + $plugingenerator->create_pricecategory($pricecategorydata1); + $pricecategorydata2 = (object)[ + 'ordernum' => 2, + 'name' => 'staff', + 'identifier' => 'staff', + 'defaultvalue' => 100, + 'pricecatsortorder' => 2, + ]; + $plugingenerator->create_pricecategory($pricecategorydata2); + + $record = new stdClass(); + $record->bookingid = $booking1->id; + $record->text = 'Test option1'; + $record->chooseorcreatecourse = 1; // Reqiured. + $record->courseid = $course2->id; + $record->maxanswers = 2; + $record->useprice = 1; // Use price from the default category. + $option1 = $plugingenerator->create_option($record); + + $settings = singleton_service::get_instance_of_booking_option_settings($option1->id); + $boinfo = new bo_info($settings); + + // Student one should see the price no price -> blocked. + $this->setUser($student1); + singleton_service::destroy_instance(); + + list($id, $isavailable, $description) = $boinfo->is_available($settings->id, $student1->id); + // The user sees now either the payment button or the noshoppingcart message. + if (class_exists('local_shopping_cart\shopping_cart')) { + $this->assertEquals(MOD_BOOKING_BO_COND_PRICEISSET, $id); + } else { + $this->assertEquals(MOD_BOOKING_BO_COND_NOSHOPPINGCART, $id); + } + + // Validation that price == category price. + $price = price::get_price('option', $settings->id); + $this->assertEmpty($price); + + // Student one should see the the staff price. + $this->setUser($student2); + singleton_service::destroy_instance(); + + list($id, $isavailable, $description) = $boinfo->is_available($settings->id, $student1->id); + // The user sees now either the payment button or the noshoppingcart message. + if (class_exists('local_shopping_cart\shopping_cart')) { + $this->assertEquals(MOD_BOOKING_BO_COND_PRICEISSET, $id); + } else { + $this->assertEquals(MOD_BOOKING_BO_COND_NOSHOPPINGCART, $id); + } + + // Validation that price == category price. + $price = price::get_price('option', $settings->id); + $this->assertEquals($pricecategorydata2->defaultvalue, $price["price"]); + + + // Student one should see the the default price. + $this->setUser($student3); + singleton_service::destroy_instance(); + + list($id, $isavailable, $description) = $boinfo->is_available($settings->id, $student1->id); + // The user sees now either the payment button or the noshoppingcart message. + if (class_exists('local_shopping_cart\shopping_cart')) { + $this->assertEquals(MOD_BOOKING_BO_COND_PRICEISSET, $id); + } else { + $this->assertEquals(MOD_BOOKING_BO_COND_NOSHOPPINGCART, $id); + } + + // Validation that price == category price. + $price = price::get_price('option', $settings->id); + $this->assertEquals($pricecategorydata1->defaultvalue, $price["price"]); + + // Now we change the default setting an run everything again. + $this->setAdminUser(); + set_config('pricecategoryfallback', 1, 'booking'); + + // Student one should see the price no price -> blocked. + $this->setUser($student1); + singleton_service::destroy_instance(); + + list($id, $isavailable, $description) = $boinfo->is_available($settings->id, $student1->id); + // The user sees now either the payment button or the noshoppingcart message. + if (class_exists('local_shopping_cart\shopping_cart')) { + $this->assertEquals(MOD_BOOKING_BO_COND_PRICEISSET, $id); + } else { + $this->assertEquals(MOD_BOOKING_BO_COND_NOSHOPPINGCART, $id); + } + + // Validation that price == category price. + $price = price::get_price('option', $settings->id); + $this->assertEquals($pricecategorydata1->defaultvalue, $price["price"]); + + // Student one should see the the staff price. + $this->setUser($student2); + singleton_service::destroy_instance(); + + list($id, $isavailable, $description) = $boinfo->is_available($settings->id, $student1->id); + // The user sees now either the payment button or the noshoppingcart message. + if (class_exists('local_shopping_cart\shopping_cart')) { + $this->assertEquals(MOD_BOOKING_BO_COND_PRICEISSET, $id); + } else { + $this->assertEquals(MOD_BOOKING_BO_COND_NOSHOPPINGCART, $id); + } + + // Validation that price == category price. + $price = price::get_price('option', $settings->id); + $this->assertEquals($pricecategorydata2->defaultvalue, $price["price"]); + + + // Student one should see the the default price. + $this->setUser($student3); + singleton_service::destroy_instance(); + + list($id, $isavailable, $description) = $boinfo->is_available($settings->id, $student1->id); + // The user sees now either the payment button or the noshoppingcart message. + if (class_exists('local_shopping_cart\shopping_cart')) { + $this->assertEquals(MOD_BOOKING_BO_COND_PRICEISSET, $id); + } else { + $this->assertEquals(MOD_BOOKING_BO_COND_NOSHOPPINGCART, $id); + } + + // Validation that price == category price. + $price = price::get_price('option', $settings->id); + $this->assertEquals($pricecategorydata1->defaultvalue, $price["price"]); + + // Now we change the default setting an run everything again. + $this->setAdminUser(); + set_config('pricecategoryfallback', 2, 'booking'); + + // Student one should see the price no price -> blocked. + $this->setUser($student1); + singleton_service::destroy_instance(); + + list($id, $isavailable, $description) = $boinfo->is_available($settings->id, $student1->id); + // The user sees now either the payment button or the noshoppingcart message. + if (class_exists('local_shopping_cart\shopping_cart')) { + $this->assertEquals(MOD_BOOKING_BO_COND_PRICEISSET, $id); + } else { + $this->assertEquals(MOD_BOOKING_BO_COND_NOSHOPPINGCART, $id); + } + + // Validation that price == category price. + $price = price::get_price('option', $settings->id); + $this->assertEmpty($price); + + // Student one should see the the staff price. + $this->setUser($student2); + singleton_service::destroy_instance(); + + list($id, $isavailable, $description) = $boinfo->is_available($settings->id, $student1->id); + // The user sees now either the payment button or the noshoppingcart message. + if (class_exists('local_shopping_cart\shopping_cart')) { + $this->assertEquals(MOD_BOOKING_BO_COND_PRICEISSET, $id); + } else { + $this->assertEquals(MOD_BOOKING_BO_COND_NOSHOPPINGCART, $id); + } + + // Validation that price == category price. + $price = price::get_price('option', $settings->id); + $this->assertEquals($pricecategorydata2->defaultvalue, $price["price"]); + + + // Student one should see the the default price. + $this->setUser($student3); + singleton_service::destroy_instance(); + + list($id, $isavailable, $description) = $boinfo->is_available($settings->id, $student1->id); + // The user sees now either the payment button or the noshoppingcart message. + if (class_exists('local_shopping_cart\shopping_cart')) { + $this->assertEquals(MOD_BOOKING_BO_COND_PRICEISSET, $id); + } else { + $this->assertEquals(MOD_BOOKING_BO_COND_NOSHOPPINGCART, $id); + } + + // Validation that price == category price. + $price = price::get_price('option', $settings->id); + // $this->assertEquals((int)get_config('booking', 'pricecategoryfallback'), $price["price"]); + $this->assertEmpty($price); + } + /** * Test booking, cancelation, option has started etc. *