new fixes

- Fix free cut calculations. - Add static cashes for discount calculation. - Fix deleted or hidden category balance error. - Skip confirm and transaction record for free instances. - Replace deprecated function mb_convert_encoding().
fmido88 · Aug 6, 2024 · 1e9e47c · 1e9e47c
1 parent e4d3926
commit 1e9e47c
Show file tree

Hide file tree

Showing 12 changed files with 125 additions and 39 deletions.
diff --git a/classes/category/operations.php b/classes/category/operations.php
@@ -256,19 +256,21 @@ private function single_cut($amount, $id) {
             $remain = 0;
         } else {
             $nonrefundable = $nonrefundable - $amount + $refundable;
+            $newfree = $free - $amount + $refundable;
+
             $this->nonrefundable = $this->nonrefundable - $amount + $refundable;
 
             $refundable = 0;
             $this->refundable -= $refundable;
 
-            $free = $this->details[$id]->free ?? 0;
             if ($nonrefundable >= 0) {
                 $remain = 0;
             } else {
                 $remain = abs($nonrefundable);
                 $nonrefundable = 0;
             }
-            $newfree = max($free - $remain, 0);
+
+            $newfree = max($newfree, 0);
             $freecut = max($free - $newfree, 0);
             $this->free -= $freecut;
             $this->freecut += $freecut;

diff --git a/classes/output/wallet_balance.php b/classes/output/wallet_balance.php
@@ -113,9 +113,14 @@ public function export_for_template(renderer_base $output) {
 
         $balancedetails = [];
         foreach ($details['catbalance'] as $id => $obj) {
-            $category = core_course_category::get($id);
+            $category = core_course_category::get($id, IGNORE_MISSING, true);
+
             $balancedetails[$id] = new stdClass;
-            $balancedetails[$id]->name = $category->get_nested_name(false);
+            if (empty($category)) {
+                $balancedetails[$id]->name = get_string('unknowncategory');
+            } else {
+                $balancedetails[$id]->name = $category->get_nested_name(false);
+            }
             $balancedetails[$id]->refundable = number_format($obj->refundable, 2) ?? 0;
             $balancedetails[$id]->nonrefundable = number_format($obj->nonrefundable, 2) ?? 0;
             $total = $obj->balance ?? $balancedetails[$id]->refundable + $balancedetails[$id]->nonrefundable;

diff --git a/classes/pages.php b/classes/pages.php
@@ -290,19 +290,22 @@ public static function get_offers_content() {
         $free = '';
         $withoffers = '';
 
-        $dom = new DOMDocument();
-        $injected = new DOMDocument();
+        $dom = new DOMDocument("1.0", "UTF-8");
+        $injected = new DOMDocument("1.0", "UTF-8");
         libxml_use_internal_errors(true);
 
         foreach ($courses as $course) {
 
-            $coursebox = mb_convert_encoding($renderer->course_info_box($course), 'HTML-ENTITIES', "UTF-8");
-
+            $coursebox = $renderer->course_info_box($course);
+            $coursebox = mb_encode_numericentity($coursebox, [0x80, 0x10FFFF, 0, ~0], 'UTF-8');
             $dom->loadHTML($coursebox);
 
             $fragment = $dom->createDocumentFragment();
             foreach ($course->offers as $offer) {
-                $injected->loadHTML(html_writer::div($offer, 'card-body'));
+                $offer = html_writer::div($offer, 'card-body');
+                $offer = mb_encode_numericentity($offer, [0x80, 0x10FFFF, 0, ~0], 'UTF-8');
+                $injected->loadHTML($offer);
+
                 $injectednode = $dom->importNode($injected->documentElement, true);
                 $fragment->appendChild($injectednode);
             }

diff --git a/classes/table/transactions.php b/classes/table/transactions.php
@@ -193,8 +193,15 @@ public function get_sql_sort() {
     protected function col_user($record) {
         if (!isset($this->users[$record->userid])) {
             $user = core_user::get_user($record->userid);
-            $user->fullname = fullname($user);
-            $user->url = new moodle_url('/user/profile.php', ['id' => $user->id]);
+            if (!$user) {
+                $user = new stdClass;
+                $user->fullname = get_string('usernotexist', 'enrol_wallet') . ' id:' . $record->userid;
+                $user->url = '#';
+            } else {
+                $user->fullname = fullname($user);
+                $user->url = new moodle_url('/user/profile.php', ['id' => $user->id]);
+            }
+
             $this->users[$record->userid] = $user;
         } else {
             $user = $this->users[$record->userid];

diff --git a/classes/util/balance.php b/classes/util/balance.php
@@ -289,7 +289,7 @@ private function set_balance_details() {
             'mainrefundable' => $record->refundable,
             'mainnonrefund'  => $record->nonrefundable,
             'mainbalance'    => $record->refundable + $record->nonrefundable,
-            'mainfree'       => $record->freegift ?? 0,
+            'mainfree'       => min($record->freegift ?? 0, $record->nonrefundable),
         ];
 
         // The id of the record to be saved in the cache.
@@ -306,9 +306,10 @@ private function set_balance_details() {
             $cats = json_decode($record->cat_balance);
             foreach ($cats as $id => $obj) {
                 $details['catbalance'][$id] = new \stdClass;
+
                 $details['catbalance'][$id]->refundable = $obj->refundable;
                 $details['catbalance'][$id]->nonrefundable = $obj->nonrefundable;
-                $details['catbalance'][$id]->free = $obj->free ?? 0;
+                $details['catbalance'][$id]->free = min($obj->free ?? 0, $obj->nonrefundable);
                 $details['catbalance'][$id]->balance = $obj->refundable + $obj->nonrefundable;
 
                 $details['total'] += $obj->refundable + $obj->nonrefundable;

diff --git a/classes/util/balance_op.php b/classes/util/balance_op.php
@@ -198,6 +198,7 @@ protected function cut_from_main($amount) {
             } else {
                 $nonrefundable = $nonrefundable - $remain;
             }
+
             $newfree = max($free - $remain, 0);
             $this->freecut += $free - $newfree;
         }

diff --git a/classes/util/instance.php b/classes/util/instance.php
@@ -80,7 +80,7 @@ class instance {
      * The coupon helper class object
      * @var coupons
      */
-    private $couponutil;
+    public $couponutil;
     /**
      * The id of the user we need to calculate the discount for.
      * @var int
@@ -98,6 +98,11 @@ class instance {
      */
     private $behavior;
 
+    /**
+     * Caching instances.
+     * @var array
+     */
+    protected static $cached = [];
     /**
      * Create a new enrol wallet instance helper class.
      * store the cost after discount.
@@ -122,10 +127,23 @@ public function __construct($instanceorid, $userid = 0) {
         } else {
             $this->userid = $userid;
         }
+
+
         $this->behavior = (int)get_config('enrol_wallet', 'discount_behavior');
         $this->calculate_cost_after_discount();
+        $this->set_static_cache();
+    }
+
+    private function set_static_cache() {
+        $cache = new \stdClass;
+        $cache->costafter = $this->costafter;
+        $cache->discounts = $this->discounts;
+        self::$cached[$this->id . '-' . $this->userid] = $cache;
     }
 
+    public static function reset_static_cache() {
+        self::$cached = [];
+    }
     /**
      * Get the enrol wallet instance by id.
      * @param int $instanceid
@@ -318,11 +336,19 @@ private function calculate_cost_after_discount() {
             $this->costafter = null;
             return;
         }
+
         $cost = (float)$cost;
         if ($cost == 0) {
             $this->costafter = $cost;
             return;
         }
+
+        $cache = self::$cached[$this->id . '-' . $this->userid] ?? null;
+        if ($cache) {
+            $this->discounts = $cache->discounts;
+            $this->costafter = $cache->costafter;
+            return;
+        }
         $discounts = $this->calculate_discounts();
         $discount = 0;
 
@@ -335,6 +361,7 @@ private function calculate_cost_after_discount() {
         } else {
             $discount = $this->calculate_sequential_discount($discounts);
         }
+
         $discount = min(1, $discount);
         $this->costafter = $cost * (1 - $discount);
     }
@@ -367,11 +394,14 @@ private function calculate_sequential_discount($discounts, $percentage = false)
      */
     public function get_cost_after_discount($recalculate = false) {
         if ($recalculate) {
+            self::reset_static_cache();
             $this->calculate_cost_after_discount();
         }
+
         if (!is_null($this->costafter) && is_numeric($this->costafter)) {
             return (float)$this->costafter;
         }
+
         return null;
     }
 

diff --git a/confirm.php b/confirm.php
@@ -58,6 +58,7 @@
 $helper = new instance($instanceid);
 $wallet = new enrol_wallet_plugin;
 $instance = $helper->get_instance();
+$cost = $helper->get_cost_after_discount();
 $course = get_course($courseid);
 
 $canselfenrol = ($wallet->can_self_enrol($instance, false) === true);
@@ -88,7 +89,7 @@
     throw new \moodle_exception('coursehidden', '', $CFG->wwwroot . '/');
 }
 
-if ($confirm && confirm_sesskey()) {
+if ($cost <= 0.01 || ($confirm && confirm_sesskey())) {
     // Notice and warnings may cause double deduct to the balance.
     set_debugging(DEBUG_NONE, false);
 
@@ -104,24 +105,10 @@
 $PAGE->set_secondary_navigation(false);
 $PAGE->navbar->add(get_string('enrolmentoptions', 'enrol'));
 
-echo $OUTPUT->header();
-
-echo $OUTPUT->heading(format_string($wallet->get_instance_name($instance), true, ['context' => $context]));
-
-$courserenderer = $PAGE->get_renderer('core', 'course');
-echo $courserenderer->course_info_box($course);
-
-$cancelurl = new moodle_url('/enrol/index.php', ['id' => $course->id]);
-$cancelbutton = new single_button($cancelurl, get_string('cancel'));
-
-$params['confirm'] = true;
-$pageurl->param('confirm', true);
-$confirmbutton = new single_button($pageurl, get_string('confirm'));
-
 $balance = balance::create_from_instance($instance);
 $a = [
     'balance'  => $balance->get_valid_balance(),
-    'cost'     => $helper->get_cost_after_discount(),
+    'cost'     => $cost,
     'currency' => $instance->currency,
     'course'   => $course->fullname,
     'policy'   => '',
@@ -143,8 +130,22 @@
     $a['policy'] = $policy;
 }
 
+$cancelurl = new moodle_url('/enrol/index.php', ['id' => $course->id]);
+$cancelbutton = new single_button($cancelurl, get_string('cancel'));
+
+$params['confirm'] = true;
+$pageurl->param('confirm', true);
+$confirmbutton = new single_button($pageurl, get_string('confirm'));
+
 $confirmationmsg = get_string('confirm_enrol_confirm', 'enrol_wallet', $a);
 
+echo $OUTPUT->header();
+
+echo $OUTPUT->heading(format_string($wallet->get_instance_name($instance), true, ['context' => $context]));
+
+$courserenderer = $PAGE->get_renderer('core', 'course');
+echo $courserenderer->course_info_box($course);
+
 echo $OUTPUT->confirm($confirmationmsg, $confirmbutton, $cancelbutton);
 
 echo $OUTPUT->footer();
diff --git a/lib.php b/lib.php
@@ -414,7 +414,9 @@ public function enrol_self(stdClass $instance, $user = null, $charge = true) {
         $op = new balance_op($user->id, $helper->get_course_category());
         // Get the final cost after discount (if there is no discount it return the full cost).
         $costafter = $helper->get_cost_after_discount();
+        $helper->reset_static_cache();
 
+        $charge = $charge && ($costafter >= 0.01);
         if ($charge) {
             $canborrow = enrol_wallet_is_borrow_eligible($user->id);
             // Deduct fees from user's account.
@@ -808,6 +810,7 @@ public function can_self_enrol(stdClass $instance, $checkuserenrolment = true) {
 
             }
         }
+
         // All restrictions checked.
         if (!empty($return)) {
             // Display them all.
@@ -869,6 +872,7 @@ protected function get_helper($instance, $userid = 0) {
         }
         return $this->helper;
     }
+
     /**
      * Return information for enrolment instance containing list of parameters required
      * for enrolment, name of enrolment plugin etc.
@@ -1910,7 +1914,7 @@ public function get_possible_currencies($account = null) {
      * @return int id of new instance, null if can not be created
      */
     public function add_instance($course, $fields = null) {
-
+        helper::reset_static_cache();
         offers::parse_data($fields);
 
         // In the form we are representing 2 db columns with one field.
@@ -1936,7 +1940,7 @@ public function add_instance($course, $fields = null) {
      * @param stdClass $data modified instance fields
      */
     public function update_instance($instance, $data) {
-
+        helper::reset_static_cache();
         offers::parse_data($data);
 
         // Check first if expiry notify is sent by the edit form (not sent in case of bulk edit only).

diff --git a/tests/util/balance_op_test.php b/tests/util/balance_op_test.php
@@ -887,25 +887,26 @@ public function test_free_balance(): void {
         $op->credit(50, $op::C_DISCOUNT, 0, '', false);
         $op->credit(50);
 
-        $op = new balance_op($user5->id, $cat2);
+        $op = new balance_op($user5->id, $cat1);
         $this->assertEquals(150, $op->get_total_balance());
         $this->assertEquals(100, $op->get_main_balance());
-        $this->assertEquals(150, $op->get_valid_balance());
+        $this->assertEquals(100, $op->get_valid_balance());
         $this->assertEquals(50, $op->get_main_nonrefundable());
         $this->assertEquals(50, $op->get_main_refundable());
         $this->assertEquals(100, $op->get_total_nonrefundable());
         $this->assertEquals(100, $op->get_total_free());
-        $this->assertEquals(100, $op->get_valid_free());
+        $this->assertEquals(50, $op->get_valid_free());
 
-        $op = new balance_op($user5->id, $cat1);
+        $op = new balance_op($user5->id, $cat2);
         $this->assertEquals(150, $op->get_total_balance());
         $this->assertEquals(100, $op->get_main_balance());
-        $this->assertEquals(100, $op->get_valid_balance());
+        $this->assertEquals(150, $op->get_valid_balance());
+        $this->assertEquals(100, $op->get_valid_nonrefundable());
         $this->assertEquals(50, $op->get_main_nonrefundable());
         $this->assertEquals(50, $op->get_main_refundable());
         $this->assertEquals(100, $op->get_total_nonrefundable());
         $this->assertEquals(100, $op->get_total_free());
-        $this->assertEquals(50, $op->get_valid_free());
+        $this->assertEquals(100, $op->get_valid_free());
 
         $op = new balance_op($user5->id, $cat2);
         $sink = $this->redirectEvents();
@@ -938,6 +939,34 @@ public function test_free_balance(): void {
         $this->assertEquals(50, $op->get_main_free());
         $this->assertEquals(50, $op->get_total_free());
         $this->assertEquals(50, $op->get_valid_free());
+
+        $op = new balance_op($user7->id, $cat2);
+        $op->credit(100);
+        $op->credit(50, $op::C_DISCOUNT, 0, '', false);
+
+        $this->assertEquals(150, $op->get_total_balance());
+        $this->assertEquals(0, $op->get_main_balance());
+        $this->assertEquals(150, $op->get_valid_balance());
+        $this->assertEquals(0, $op->get_main_nonrefundable());
+        $this->assertEquals(0, $op->get_main_refundable());
+        $this->assertEquals(50, $op->get_total_nonrefundable());
+        $this->assertEquals(50, $op->get_total_free());
+        $this->assertEquals(50, $op->get_valid_free());
+
+        $sink = $this->redirectEvents();
+        $op->debit(120, $op::OTHER);
+        $events = $sink->get_events();
+        $sink->close();
+
+        foreach ($events as $key => $event) {
+            if ($event->eventname !== '\enrol_wallet\event\transactions_triggered') {
+                unset($events[$key]);
+            }
+        }
+
+        $this->assertEquals(1, count($events));
+        $debitevent = reset($events);
+        $this->assertEquals(20, $debitevent->other['freecut']);
     }
 
     /**

diff --git a/tests/util/instance_test.php b/tests/util/instance_test.php
@@ -198,6 +198,9 @@ public function test_repurchase_discount_and_function(): void {
         $wallet = new enrol_wallet_plugin;
 
         $this->assertEquals(60, $inst->get_cost_after_discount(true));
+        $op = new balance_op($user->id);
+        $this->assertTrue(is_enrolled($context, $user));
+        $this->assertEquals(350, $op->get_total_balance());
         // Enrol the user.
         $wallet->enrol_self($instance, $user);
         $this->assertTrue(is_enrolled($context, $user, '', true));