MDL-69166 core_payment: improvements to api, small fixes
authorMarina Glancy <marina@moodle.com>
Wed, 30 Sep 2020 17:21:30 +0000 (19:21 +0200)
committerShamim Rezaie <shamim@moodle.com>
Tue, 27 Oct 2020 03:40:49 +0000 (14:40 +1100)
19 files changed:
enrol/fee/classes/payment/provider.php
enrol/fee/classes/plugin.php
enrol/fee/templates/payment_region.mustache
payment/amd/build/gateways_modal.min.js
payment/amd/build/gateways_modal.min.js.map
payment/amd/build/repository.min.js
payment/amd/build/repository.min.js.map
payment/amd/src/gateways_modal.js
payment/amd/src/repository.js
payment/classes/external/get_gateways_for_currency.php
payment/classes/helper.php
payment/classes/local/callback/provider.php
payment/gateway/paypal/amd/build/gateways_modal.min.js
payment/gateway/paypal/amd/build/gateways_modal.min.js.map
payment/gateway/paypal/amd/src/gateways_modal.js
payment/gateway/paypal/classes/external/get_config_for_js.php
payment/gateway/paypal/classes/external/transaction_complete.php
payment/gateway/paypal/classes/paypal_helper.php
payment/templates/gateway.mustache

index 8dfd227..2a6d43b 100644 (file)
@@ -55,9 +55,10 @@ class provider implements \core_payment\local\callback\provider {
      * Callback function that delivers what the user paid for to them.
      *
      * @param int $instanceid The enrolment instance id
+     * @param int $paymentid payment id as inserted into the 'payments' table, if needed for reference
      * @return bool Whether successful or not
      */
-    public static function deliver_order(int $instanceid): bool {
+    public static function deliver_order(int $instanceid, int $paymentid): bool {
         global $DB, $USER;
 
         $instance = $DB->get_record('enrol', ['enrol' => 'fee', 'id' => $instanceid], '*', MUST_EXIST);
index e777703..aa5ed3c 100644 (file)
@@ -202,16 +202,9 @@ class enrol_fee_plugin extends enrol_plugin {
             echo '<p>'.get_string('nocost', 'enrol_fee').'</p>';
         } else {
 
-            $locale = get_string('localecldr', 'langconfig');
-            $fmt = NumberFormatter::create($locale, NumberFormatter::CURRENCY);
-            $localisedcost = numfmt_format_currency($fmt, $cost, $instance->currency);
-
             $data = [
                 'isguestuser' => isguestuser(),
-                'cost' => $localisedcost,
-                'currency' => $instance->currency,
-                'accountid' => $instance->customint1,
-                'amount' => $cost,
+                'cost' => \core_payment\helper::get_cost_as_string($instance->cost, $instance->currency),
                 'instanceid' => $instance->id,
                 'description' => get_string('purchasedescription', 'enrol_fee',
                     format_string($course->fullname, true, ['context' => $context])),
index 458e76d..649ab5a 100644 (file)
@@ -41,7 +41,6 @@
         "cost": "$108.50",
         "amount": 108.50,
         "currency": "AUD",
-        "accountid": 1,
         "instanceid": 11,
         "description": "Enrolment in course Introduction to algorithms",
         "isguestuser": false
@@ -63,9 +62,6 @@
             class="btn btn-secondary"
             type="button"
             id="gateways-modal-trigger-{{ uniqid }}"
-            data-amount="{{amount}}"
-            data-currency="{{currency}}"
-            data-accountid="{{accountid}}"
             data-component="enrol_fee"
             data-componentid="{{instanceid}}"
             data-description={{# quote }}{{description}}{{/ quote }}
index ab28fe4..4f1e815 100644 (file)
Binary files a/payment/amd/build/gateways_modal.min.js and b/payment/amd/build/gateways_modal.min.js differ
index 86be958..ede802b 100644 (file)
Binary files a/payment/amd/build/gateways_modal.min.js.map and b/payment/amd/build/gateways_modal.min.js.map differ
index f65181d..707571d 100644 (file)
Binary files a/payment/amd/build/repository.min.js and b/payment/amd/build/repository.min.js differ
index a5b47c4..9995107 100644 (file)
Binary files a/payment/amd/build/repository.min.js.map and b/payment/amd/build/repository.min.js.map differ
index 6112b2a..643371a 100644 (file)
@@ -93,12 +93,6 @@ const show = async(rootNode, {
         if (gateway) {
             processPayment(
                 gateway,
-                {
-                    value: parseFloat(rootNode.dataset.amount),
-                    currency: rootNode.dataset.currency,
-                    surcharge: parseInt((rootElement.querySelector(Selectors.values.gateway) || {dataset: {surcharge: 0}})
-                        .dataset.surcharge),
-                },
                 rootNode.dataset.component,
                 rootNode.dataset.componentid,
                 rootNode.dataset.description,
@@ -128,13 +122,11 @@ const show = async(rootNode, {
     // Re-calculate the cost when gateway is changed.
     rootElement.addEventListener('change', e => {
         if (e.target.matches(Selectors.elements.gateways)) {
-            updateCostRegion(rootElement, parseFloat(rootNode.dataset.amount), rootNode.dataset.currency);
+            updateCostRegion(rootElement);
         }
     });
 
-    const currency = rootNode.dataset.currency;
-    const accountid = rootNode.dataset.accountid;
-    const gateways = await getGatewaysSupportingCurrency(currency, accountid);
+    const gateways = await getGatewaysSupportingCurrency(rootNode.dataset.component, rootNode.dataset.componentid);
     const context = {
         gateways
     };
@@ -142,7 +134,7 @@ const show = async(rootNode, {
     const {html, js} = await Templates.renderForPromise('core_payment/gateways', context);
     Templates.replaceNodeContents(rootElement.querySelector(Selectors.regions.gatewaysContainer), html, js);
     selectSingleGateway(rootElement);
-    await updateCostRegion(rootElement, parseFloat(rootNode.dataset.amount), rootNode.dataset.currency);
+    await updateCostRegion(rootElement);
 };
 
 /**
@@ -166,13 +158,11 @@ const selectSingleGateway = root => {
  * @param {string} currency The currency part of cost in the 3-letter ISO-4217 format
  * @returns {Promise<void>}
  */
-const updateCostRegion = async(root, amount, currency) => {
-    const locale = await updateCostRegion.locale; // This only takes a bit the first time.
+const updateCostRegion = async(root) => {
     const surcharge = parseInt((root.querySelector(Selectors.values.gateway) || {dataset: {surcharge: 0}}).dataset.surcharge);
-    amount += amount * surcharge / 100;
-    const localisedCost = amount.toLocaleString(locale, {style: "currency", currency: currency});
+    const cost = root.querySelector(Selectors.values.gateway).dataset.cost;
 
-    const {html, js} = await Templates.renderForPromise('core_payment/fee_breakdown', {fee: localisedCost, surcharge});
+    const {html, js} = await Templates.renderForPromise('core_payment/fee_breakdown', {fee: cost, surcharge});
     Templates.replaceNodeContents(root.querySelector(Selectors.regions.costContainer), html, js);
 };
 updateCostRegion.locale = getString("localecldr", "langconfig");
@@ -181,21 +171,15 @@ updateCostRegion.locale = getString("localecldr", "langconfig");
  * Process payment using the selected gateway.
  *
  * @param {string} gateway The gateway to be used for payment
- * @param {Object} amount - Amount of payment
- * @param {number} amount.value The numerical part of the amount
- * @param {string} amount.currency The currency part of the amount in the three-character ISO-4217 format
- * @param {number} amount.surcharge The surcharge percentage that should be added to the amount
  * @param {string} component Name of the component that the componentid belongs to
  * @param {number} componentid An internal identifier that is used by the component
  * @param {string} description Description of the payment
  * @param {processPaymentCallback} callback The callback function to call when processing is finished
  * @returns {Promise<void>}
  */
-const processPayment = async(gateway, {value, currency, surcharge = 0}, component, componentid, description, callback) => {
+const processPayment = async(gateway, component, componentid, description, callback) => {
     const paymentMethod = await import(`pg_${gateway}/gateways_modal`);
-
-    value += value * surcharge / 100;
-    paymentMethod.process(value, currency, component, componentid, description, callback);
+    paymentMethod.process(component, componentid, description, callback);
 };
 
 /**
index 64a968a..5199c33 100644 (file)
@@ -27,16 +27,16 @@ import Ajax from 'core/ajax';
 /**
  * Returns the list of gateways that can process payments in the given currency.
  *
- * @param {string} currency The currency in the three-character ISO-4217 format
- * @param {int} accountid
+ * @param {String} component
+ * @param {Integer} componentid
  * @returns {Promise<{shortname: string, name: string, description: String}[]>}
  */
-export const getGatewaysSupportingCurrency = (currency, accountid) => {
+export const getGatewaysSupportingCurrency = (component, componentid) => {
     const request = {
         methodname: 'core_payment_get_gateways_for_currency',
         args: {
-            currency,
-            accountid
+            component,
+            componentid
         }
     };
     return Ajax.call([request])[0];
index e17cd66..30df0b9 100644 (file)
@@ -24,6 +24,7 @@
 
 namespace core_payment\external;
 
+use core_payment\helper;
 use external_api;
 use external_function_parameters;
 use external_value;
@@ -43,34 +44,40 @@ class get_gateways_for_currency extends external_api {
      */
     public static function execute_parameters(): external_function_parameters {
         return new external_function_parameters(
-                ['currency' => new external_value(PARAM_ALPHA, 'Currency code'),
-                    'accountid' => new external_value(PARAM_INT, 'Account id')]
+                ['component' => new external_value(PARAM_COMPONENT, 'Component'),
+                    'componentid' => new external_value(PARAM_INT, 'Component id')]
         );
     }
 
     /**
      * Returns the list of gateways that can process payments in the given currency.
      *
-     * @param string $currency The currency in the three-character ISO-4217 format.
-     * @param int $accountid
+     * @param string $component
+     * @param int $componentid
      * @return \stdClass[]
      */
-    public static function execute(string $currency, int $accountid): array {
+    public static function execute(string $component, int $componentid): array {
 
         $params = external_api::validate_parameters(self::execute_parameters(), [
-            'currency' => $currency,
-            'accountid' => $accountid,
+            'component' => $component,
+            'componentid' => $componentid,
         ]);
 
         $list = [];
-        $gateways = \core_payment\helper::get_gateways_for_currency($params['currency'], $params['accountid']);
+        $gateways = \core_payment\helper::get_gateways_for_currency($params['component'], $params['componentid']);
+        [
+            'amount' => $amount,
+            'currency' => $currency
+        ] = \core_payment\helper::get_cost($params['component'], $params['componentid']);
 
         foreach ($gateways as $gateway) {
+            $surcharge = \core_payment\helper::get_gateway_surcharge($gateway);
             $list[] = (object)[
                 'shortname' => $gateway,
                 'name' => get_string('gatewayname', 'pg_' . $gateway),
                 'description' => get_string('gatewaydescription', 'pg_' . $gateway),
-                'surcharge' => \core_payment\helper::get_gateway_surcharge($gateway),
+                'surcharge' => $surcharge,
+                'cost' => helper::get_cost_as_string(helper::get_cost_with_surcharge($amount, $surcharge, $currency), $currency),
             ];
         }
 
@@ -89,6 +96,8 @@ class get_gateways_for_currency extends external_api {
                     'name' => new external_value(PARAM_TEXT, 'Human readable name of the gateway'),
                     'description' => new external_value(PARAM_TEXT, 'description of the gateway'),
                     'surcharge' => new external_value(PARAM_INT, 'percentage of surcharge when using the gateway'),
+                    'cost' => new external_value(PARAM_TEXT,
+                        'Cost in human-readable form (amount plus surcharge with currency sign)'),
                 ])
         );
     }
index 5ba6921..9e21115 100644 (file)
@@ -47,7 +47,7 @@ class helper {
             /** @var \pg_paypal\gateway $classname */
             $classname = '\pg_' . $plugin . '\gateway';
 
-            $currencies += $classname::get_supported_currencies();
+            $currencies += component_class_callback($classname, 'get_supported_currencies', [], []);
         }
 
         $currencies = array_unique($currencies);
@@ -58,13 +58,18 @@ class helper {
     /**
      * Returns the list of gateways that can process payments in the given currency.
      *
-     * @param string $currency The currency in the three-character ISO-4217 format.
-     * @param int $accountid
+     * @param string $component
+     * @param int $componentid
      * @return string[]
      */
-    public static function get_gateways_for_currency(string $currency, int $accountid): array {
+    public static function get_gateways_for_currency(string $component, int $componentid): array {
         $gateways = [];
 
+        [
+            'amount' => $amount,
+            'currency' => $currency,
+            'accountid' => $accountid,
+        ] = self::get_cost($component, $componentid);
         $account = new account($accountid);
         if (!$account->get('id') || !$account->get('enabled')) {
             return $gateways;
@@ -77,7 +82,7 @@ class helper {
             /** @var gateway $classname */
             $classname = '\pg_' . $plugin . '\gateway';
 
-            $currencies = $classname::get_supported_currencies();
+            $currencies = component_class_callback($classname, 'get_supported_currencies', [], []);
             if (in_array($currency, $currencies)) {
                 $gateways[] = $plugin;
             }
@@ -86,33 +91,60 @@ class helper {
         return $gateways;
     }
 
+    /**
+     * Calculates the cost with the surcharge
+     *
+     * @param float $amount amount in the currency units
+     * @param float $surcharge surcharge in percents
+     * @param string $currency currency, used for calculating the number of fractional digits
+     * @return float
+     */
+    public static function get_cost_with_surcharge(float $amount, float $surcharge, string $currency): float {
+        return round($amount + $amount * $surcharge / 100, 2); // TODO number of digits depends on currency.
+    }
+
+    /**
+     * Returns human-readable amount with fixed number of fractional digits and currency indicator
+     *
+     * @param float $amount
+     * @param string $currency
+     * @return string
+     * @throws \coding_exception
+     */
+    public static function get_cost_as_string(float $amount, string $currency): string {
+        if (class_exists('NumberFormatter') && function_exists('numfmt_format_currency')) {
+            $locale = get_string('localecldr', 'langconfig');
+            $fmt = \NumberFormatter::create($locale, \NumberFormatter::CURRENCY);
+            $localisedcost = numfmt_format_currency($fmt, $amount, $currency);
+        } else {
+            $localisedcost = sprintf("%.2f %s", $amount, $currency); // TODO number of digits depends on currency.
+        }
+
+        return $localisedcost;
+    }
+
     /**
      * Returns the percentage of surcharge that is applied when using a gateway
      *
      * @param string $gateway Name of the gateway
-     * @return int
+     * @return float
      */
-    public static function get_gateway_surcharge(string $gateway): int {
-        return get_config('pg_' . $gateway, 'surcharge') ?: 0;
+    public static function get_gateway_surcharge(string $gateway): float {
+        return (float)get_config('pg_' . $gateway, 'surcharge');
     }
 
     /**
      * Returns the attributes to place on a pay button.
      *
-     * @param float $amount Amount of payment
-     * @param string $currency Currency of payment
      * @param string $component Name of the component that the componentid belongs to
      * @param int $componentid An internal identifier that is used by the component
      * @param string $description Description of the payment
      * @return array
      */
-    public static function gateways_modal_link_params(float $amount, string $currency, string $component, int $componentid,
-            string $description): array {
+    public static function gateways_modal_link_params(string $component, int $componentid, string $description): array {
         return [
             'id' => 'gateways-modal-trigger',
             'role' => 'button',
-            'data-amount' => $amount,
-            'data-currency' => $currency,
             'data-component' => $component,
             'data-componentid' => $componentid,
             'data-description' => $description,
@@ -163,13 +195,15 @@ class helper {
     /**
      * Delivers what the user paid for.
      *
+     * @uses \core_payment\local\callback\provider::deliver_order()
+     *
      * @param string $component Name of the component that the componentid belongs to
      * @param int $componentid An internal identifier that is used by the component
+     * @param int $paymentid payment id as inserted into the 'payments' table, if needed for reference
      * @return bool Whether successful or not
-     * @throws \moodle_exception
      */
-    public static function deliver_order(string $component, int $componentid): bool {
-        $result = component_class_callback("$component\\payment\\provider", 'deliver_order', [$componentid]);
+    public static function deliver_order(string $component, int $componentid, int $paymentid): bool {
+        $result = component_class_callback("$component\\payment\\provider", 'deliver_order', [$componentid, $paymentid]);
 
         if ($result === null) {
             throw new \moodle_exception('callbacknotimplemented', 'core_payment', '', $component);
@@ -182,6 +216,7 @@ class helper {
      * Stores essential information about the payment and returns the "id" field of the payment record in DB.
      * Each payment gateway may then store the additional information their way.
      *
+     * @param int $accountid Account id
      * @param string $component Name of the component that the componentid belongs to
      * @param int $componentid An internal identifier that is used by the component
      * @param int $userid Id of the user who is paying
@@ -190,7 +225,7 @@ class helper {
      * @param string $gateway The gateway that is used for the payment
      * @return int
      */
-    public static function save_payment(string $component, int $componentid, int $userid, float $amount, string $currency,
+    public static function save_payment(int $accountid, string $component, int $componentid, int $userid, float $amount, string $currency,
             string $gateway): int {
         global $DB;
 
@@ -201,6 +236,7 @@ class helper {
         $record->amount = $amount;
         $record->currency = $currency;
         $record->gateway = $gateway;
+        $record->accountid = $accountid;
         $record->timecreated = $record->timemodified = time();
 
         $id = $DB->insert_record('payments', $record);
index b94c7bf..1e247ce 100644 (file)
@@ -43,8 +43,9 @@ interface provider {
     public static function get_cost(int $identifier): array;
 
     /**
-     * @param int $identifier An identifier that is known to the plugin
+     * @param int $componentid An identifier that is known to the plugin
+     * @param int $paymentid payment id as inserted into the 'payments' table, if needed for reference
      * @return bool Whether successful or not
      */
-    public static function deliver_order(int $identifier): bool;
+    public static function deliver_order(int $componentid, int $paymentid): bool;
 }
index 9107c30..1993d1f 100644 (file)
Binary files a/payment/gateway/paypal/amd/build/gateways_modal.min.js and b/payment/gateway/paypal/amd/build/gateways_modal.min.js differ
index 5075bf4..4c8f59d 100644 (file)
Binary files a/payment/gateway/paypal/amd/build/gateways_modal.min.js.map and b/payment/gateway/paypal/amd/build/gateways_modal.min.js.map differ
index 206d365..eab6336 100644 (file)
@@ -45,15 +45,13 @@ const showModalWithPlaceholder = async() => {
 /**
  * Process the payment.
  *
- * @param {double} amount Amount of payment
- * @param {string} currency The currency in the three-character ISO-4217 format
  * @param {string} component Name of the component that the componentid belongs to
  * @param {number} componentid An internal identifier that is used by the component
  * @param {string} description Description of the payment
  * @param {processCallback} callback The callback function to call when processing is finished
  * @returns {Promise<void>}
  */
-export const process = async(amount, currency, component, componentid, description, callback) => {
+export const process = async(component, componentid, description, callback) => {
 
     const [
         modal,
@@ -62,6 +60,8 @@ export const process = async(amount, currency, component, componentid, descripti
         showModalWithPlaceholder(),
         Repository.getConfigForJs(component, componentid),
     ]);
+    const currency = paypalConfig.currency;
+    const amount = paypalConfig.cost; // Cost with surcharge.
 
     modal.getRoot().on(ModalEvents.hidden, () => {
         // Destroy when hidden.
index f15b481..1347842 100644 (file)
@@ -62,10 +62,14 @@ class get_config_for_js extends external_api {
         ]);
 
         $config = helper::get_gateway_configuration($component, $componentid, 'paypal');
+        $cost = helper::get_cost($component, $componentid);
+        $surcharge = helper::get_gateway_surcharge('paypal');
 
         return [
             'clientid' => $config['clientid'],
             'brandname' => $config['brandname'],
+            'cost' => helper::get_cost_with_surcharge($cost['amount'], $surcharge, $cost['currency']),
+            'currency' => $cost['currency'],
         ];
     }
 
@@ -78,6 +82,8 @@ class get_config_for_js extends external_api {
         return new external_single_structure([
             'clientid' => new external_value(PARAM_TEXT, 'PayPal client ID'),
             'brandname' => new external_value(PARAM_TEXT, 'Brand name'),
+            'cost' => new external_value(PARAM_FLOAT, 'Cost with gateway surcharge'),
+            'currency' => new external_value(PARAM_TEXT, 'Currency'),
         ]);
     }
 }
index 0e2a0e3..56016a8 100644 (file)
@@ -80,9 +80,8 @@ class transaction_complete extends external_api {
         ] = payment_helper::get_cost($component, $componentid);
 
         // Add surcharge if there is any.
-        if ($config->surcharge) {
-            $amount += $amount * $config->surcharge / 100;
-        }
+        $surcharge = helper::get_gateway_surcharge('paypal');
+        $amount = helper::get_cost_with_surcharge($amount, $surcharge, $currency);
 
         $paypalhelper = new paypal_helper($config->clientid, $config->secret, $sandbox);
         $orderdetails = $paypalhelper->get_order_details($orderid);
@@ -100,10 +99,8 @@ class transaction_complete extends external_api {
                         $success = true;
                         // Everything is correct. Let's give them what they paid for.
                         try {
-                            payment_helper::deliver_order($component, $componentid);
-
-                            $paymentid = payment_helper::save_payment($component, $componentid, (int) $USER->id, $amount,
-                                    $currency, 'paypal');
+                            $paymentid = payment_helper::save_payment((int)$accountid, $component, $componentid, (int) $USER->id,
+                                $amount, $currency, 'paypal');
 
                             // Store PayPal extra information.
                             $record = new \stdClass();
@@ -111,6 +108,8 @@ class transaction_complete extends external_api {
                             $record->pp_orderid = $orderid;
 
                             $DB->insert_record('pg_paypal', $record);
+
+                            payment_helper::deliver_order($component, $componentid, $paymentid);
                         } catch (\Exception $e) {
                             debugging('Exception while trying to process payment: ' . $e->getMessage(), DEBUG_DEVELOPER);
                             $success = false;
@@ -148,7 +147,7 @@ class transaction_complete extends external_api {
     public static function execute_returns() {
         return new external_function_parameters([
             'success' => new external_value(PARAM_BOOL, 'Whether everything was successful or not.'),
-            'message' => new external_value(PARAM_TEXT, 'Message (usually the error message).', VALUE_OPTIONAL),
+            'message' => new external_value(PARAM_RAW, 'Message (usually the error message).'),
         ]);
     }
 }
index f74984e..ff3409a 100644 (file)
@@ -162,10 +162,8 @@ class paypal_helper {
             ],
         ];
 
-        $command = '{}';
-
         $curl = new curl();
-        $result = $curl->get($location, $command, $options);
+        $result = $curl->get($location, [], $options);
 
         return json_decode($result, true);
     }
index 9e2c41a..bc03694 100644 (file)
@@ -42,7 +42,7 @@
 
 }}
 <div class="custom-control custom-radio {{shortname}}">
-    <input class="custom-control-input" type="radio" name="payby" id="id-payby-{{uniqid}}-{{shortname}}" data-surcharge="{{surcharge}}" value="{{shortname}}" {{#checked}} checked="checked" {{/checked}} />
+    <input class="custom-control-input" type="radio" name="payby" id="id-payby-{{uniqid}}-{{shortname}}" data-cost="{{cost}}" data-surcharge="{{surcharge}}" value="{{shortname}}" {{#checked}} checked="checked" {{/checked}} />
     <label class="custom-control-label bg-light border p-3 my-3" for="id-payby-{{uniqid}}-{{shortname}}">
         <p class="h3">{{name}}</p>
         <p class="content mb-2">{{description}}</p>