MDL-69166 core_payment: addressing integration review

moodle · Oct 27, 2020 · efc576f · efc576f
1 parent 90fbc58
commit efc576f
Show file tree

Hide file tree

Showing 15 changed files with 207 additions and 124 deletions.
diff --git a/enrol/fee/tests/behat/fee.feature b/enrol/fee/tests/behat/fee.feature
@@ -14,7 +14,6 @@ Feature: Signing up for a course with a fee enrolment method
       | user     | course | role           |
       | teacher1 | C1     | editingteacher |
       | manager1 | C1     | manager        |
-    And payment plugin "paypal" is enabled
     And the following "core_payment > payment accounts" exist:
       | name           | gateways |
       | Account1       | paypal   |
@@ -31,16 +30,15 @@ Feature: Signing up for a course with a fee enrolment method
     And I select "Fee" from the "Add method" singleselect
     And I set the following fields to these values:
       | Payment account | Account1 |
-      | Enrol cost      | 10       |
+      | Enrolment fee   | 123.45   |
       | Currency        | Euro     |
     And I press "Add method"
     And I log out
     And I log in as "student1"
     And I am on course index
     And I follow "Course 1"
-    And I should see "This course requires a payment for entry."
-    #And I should see "Cost: EUR 10.00" # TODO for some reason behat does not "see" this text.
-    And I press "Pay enrolment fee"
-    And I should see "PayPal" in the "Select Payment Type" "dialogue"
-    And I click on "Cancel" "button" in the "Select Payment Type" "dialogue"
-    And I log out
+    Then I should see "This course requires a payment for entry."
+    And I should see "123.45"
+    And I press "Select payment type"
+    And I should see "PayPal" in the "Select payment type" "dialogue"
+    And I click on "Cancel" "button" in the "Select payment type" "dialogue"
diff --git a/lang/en/payment.php b/lang/en/payment.php
@@ -23,9 +23,14 @@
  */
 
 $string['accountarchived'] = 'Archived';
-$string['accountdeleteconfirm'] = 'If this account has previous payments, they will be archived, otherwise all other data will be permanently deleted. Are you sure you want to continue?';
+$string['accountdeleteconfirm'] = 'If this account has previous payments, it will be archived, otherwise its configuration data will be permanently deleted. Are you sure you want to continue?';
+$string['accountconfignote'] = 'Payment gateways for this account will be configured separately';
+$string['accountidnumber'] = 'ID number';
+$string['accountidnumber_help'] = 'The ID number is only used when matching the account against external systems and is not displayed anywhere on the site. If the account has an official code name it may be entered, otherwise the field can be left blank.';
 $string['accountname'] = 'Account name';
+$string['accountname_help'] = 'How this account will be identified for teachers or managers who set up payments (for example in the course enrolment plugin)';
 $string['accountnotavailable'] = 'Not available';
+$string['paymentaccountsexplained'] = 'Create one or multiple payment accounts for this site. Each account includes configuration for available payment gateways. The person who configures payments on the site (for example, payment for the course enrolment) will be able to chose from the available accounts.';
 $string['callbacknotimplemented'] = 'The callback is not implemented for component {$a}.';
 $string['createaccount'] = 'Create payment account';
 $string['deleteorarchive'] = 'Delete or archive';

diff --git a/payment/accounts.php b/payment/accounts.php
@@ -36,7 +36,7 @@
 
 $accounts = \core_payment\helper::get_payment_accounts_to_manage(context_system::instance(), $showarchived);
 $table = new html_table();
-$table->head = [get_string('accountname', 'payment'), get_string('type_pg', 'plugin'), ''];
+$table->head = [get_string('accountname', 'payment'), get_string('type_pg_plural', 'plugin'), ''];
 $table->colclasses = ['', '', 'mdl-right'];
 $table->data = [];
 foreach ($accounts as $account) {
@@ -74,6 +74,16 @@
     $table->data[] = [$name, join(', ', $gateways), $OUTPUT->render($menu)];
 }
 
+echo html_writer::div(get_string('paymentaccountsexplained', 'payment'), 'pb-2');
+
+if (has_capability('moodle/site:config', context_system::instance())) {
+    // For administrators add a link to "Manage payment gateways" page.
+    $link = html_writer::link(new moodle_url('/admin/settings.php', ['section' => 'managepaymentgateways']),
+        get_string('type_pgmanage', 'plugin'));
+    $text = get_string('gotomanageplugins', 'payment', $link);
+    echo html_writer::div($text, 'pb-2');
+}
+
 echo html_writer::table($table);
 
 $PAGE->requires->event_handler('[data-action=delete]', 'click', 'M.util.show_confirm_dialog',
@@ -84,12 +94,4 @@
 
 echo $OUTPUT->single_button(new moodle_url('/payment/manage_account.php'), get_string('createaccount', 'payment'), 'get');
 
-if (has_capability('moodle/site:config', context_system::instance())) {
-    // For administrators add a link to "Manage payment gateways" page.
-    $link = html_writer::link(new moodle_url('/admin/settings.php', ['section' => 'managepaymentgateways']),
-        get_string('type_pgmanage', 'plugin'));
-    $text = get_string('gotomanageplugins', 'payment', $link);
-    echo html_writer::div($text, 'pt-3');
-}
-
 echo $OUTPUT->footer();
diff --git a/payment/classes/account_gateway.php b/payment/classes/account_gateway.php
@@ -26,8 +26,6 @@
 
 use core\persistent;
 
-defined('MOODLE_INTERNAL') || die();
-
 /**
  * Class account_gateway
  *
@@ -46,14 +44,13 @@ class account_gateway extends persistent {
      *
      * @return array
      */
-    protected static function define_properties() : array {
+    protected static function define_properties(): array {
         return array(
             'accountid' => [
                 'type' => PARAM_INT,
             ],
             'gateway' => [
                 'type' => PARAM_COMPONENT,
-                // TODO select with options?
             ],
             'enabled' => [
                 'type' => PARAM_BOOL,
@@ -72,7 +69,6 @@ protected static function define_properties() : array {
      * Return the gateway name ready for display
      *
      * @return string
-     * @throws \coding_exception
      */
     public function get_display_name(): string {
         return get_string('pluginname', 'pg_' . $this->get('gateway'));
@@ -82,8 +78,6 @@ public function get_display_name(): string {
      * Gateway management url
      *
      * @return \moodle_url
-     * @throws \coding_exception
-     * @throws \moodle_exception
      */
     public function get_edit_url(): \moodle_url {
         $params = $this->get('id') ? ['id' => $this->get('id')] :
@@ -95,7 +89,6 @@ public function get_edit_url(): \moodle_url {
      * Get corresponding account
      *
      * @return account
-     * @throws \coding_exception
      */
     public function get_account(): account {
         return new account($this->get('accountid'));
@@ -105,7 +98,6 @@ public function get_account(): account {
      * Parse configuration from the json-encoded stored value
      *
      * @return array
-     * @throws \coding_exception
      */
     public function get_configuration(): array {
         $config = @json_decode($this->get('config'), true);

diff --git a/payment/classes/form/account.php b/payment/classes/form/account.php
@@ -26,8 +26,6 @@
 
 use core\form\persistent;
 
-defined('MOODLE_INTERNAL') || die();
-
 /**
  * Class account
  *
@@ -50,14 +48,18 @@ public function definition() {
         $mform->addElement('hidden', 'contextid');
 
         $mform->addElement('text', 'name', get_string('accountname', 'payment'), 'maxlength="255"');
+        $mform->addHelpButton('name', 'accountname', 'payment');
         $mform->setType('name', PARAM_TEXT);
         $mform->addRule('name', get_string('required'), 'required', null, 'client');
         $mform->addRule('name', get_string('maximumchars', '', 255), 'maxlength', 255, 'server');
 
-        $mform->addElement('text', 'idnumber', get_string('idnumber'), 'maxlength="100"');
+        $mform->addElement('text', 'idnumber', get_string('accountidnumber', 'payment'), 'maxlength="100"');
+        $mform->addHelpButton('idnumber', 'accountidnumber', 'payment');
         $mform->setType('idnumber', PARAM_RAW_TRIMMED);
         $mform->addRule('idnumber', get_string('maximumchars', '', 100), 'maxlength', 100, 'server');
 
+        $mform->addElement('static', 'staticinfo', '', get_string('accountconfignote', 'payment'));
+
         $mform->addElement('advcheckbox', 'enabled', get_string('enable'));
         $this->add_action_buttons();
     }

diff --git a/payment/classes/form/account_gateway.php b/payment/classes/form/account_gateway.php
@@ -25,9 +25,6 @@
 namespace core_payment\form;
 
 use core\form\persistent;
-use core_payment\gateway;
-
-defined('MOODLE_INTERNAL') || die();
 
 /**
  * Class account_gateway

diff --git a/payment/classes/gateway.php b/payment/classes/gateway.php
@@ -24,8 +24,6 @@
 
 namespace core_payment;
 
-defined('MOODLE_INTERNAL') || die();
-
 /**
  * Base class for payment gateways.
  *

diff --git a/payment/classes/helper.php b/payment/classes/helper.php
@@ -28,8 +28,6 @@
 use core_payment\event\account_deleted;
 use core_payment\event\account_updated;
 
-defined('MOODLE_INTERNAL') || die();
-
 /**
  * Helper class for the payment subsystem.
  *

diff --git a/payment/gateway/paypal/db/install.php b/payment/gateway/paypal/db/install.php
@@ -0,0 +1,29 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * pg_paypal installer script.
+ *
+ * @package    pg_paypal
+ * @copyright  2020 Marina Glancy
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+function xmldb_pg_paypal_install() {
+    // Enable the Paypal payment gateway on installation. It still needs to be configured and enabled for accounts.
+    $order = (!empty($CFG->pg_plugins_sortorder)) ? explode(',', $CFG->pg_plugins_sortorder) : [];
+    set_config('pg_plugins_sortorder', join(',', array_merge($order, ['paypal'])));
+}
diff --git a/payment/tests/behat/accounts.feature b/payment/tests/behat/accounts.feature
@@ -6,7 +6,7 @@ Feature: Manage payment accounts
     When I log in as "admin"
     And I navigate to "Payments > Payment accounts" in site administration
     And I follow "Manage payment gateways"
-    And I click on "Enable" "link" in the "PayPal" "table_row"
+    Then "Australian Dollar" "text" should exist in the "PayPal" "table_row"
     And I follow "Payment accounts"
     And I press "Create payment account"
     And I set the field "Account name" to "TestAccount"
@@ -22,8 +22,7 @@ Feature: Manage payment accounts
 
   @javascript
   Scenario: Configuring gateways on payment accounts
-    Given payment plugin "paypal" is enabled
-    And the following "core_payment > payment accounts" exist:
+    Given the following "core_payment > payment accounts" exist:
       | name           |
       | Account1       |
       | Account2       |
@@ -44,8 +43,7 @@ Feature: Manage payment accounts
 
   @javascript
   Scenario: Deleting payment accounts
-    Given payment plugin "paypal" is enabled
-    And the following "core_payment > payment accounts" exist:
+    Given the following "core_payment > payment accounts" exist:
       | name           |
       | Account1       |
       | Account2       |
@@ -60,8 +58,7 @@ Feature: Manage payment accounts
 
   @javascript
   Scenario: Archiving and restoring accounts
-    Given payment plugin "paypal" is enabled
-    And the following "users" exist:
+    Given the following "users" exist:
       | username |
       | user1    |
     And the following "core_payment > payment accounts" exist:

diff --git a/payment/tests/behat/behat_payment.php b/payment/tests/behat/behat_payment.php
diff --git a/payment/tests/generator/behat_core_payment_generator.php b/payment/tests/generator/behat_core_payment_generator.php
@@ -23,8 +23,6 @@
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 
-defined('MOODLE_INTERNAL') || die();
-
 
 /**
  * Behat data generator for core_payment.
@@ -36,6 +34,11 @@
  */
 class behat_core_payment_generator extends behat_generator_base {
 
+    /**
+     * Get a list of the entities that can be created.
+     *
+     * @return array entity name => information about how to generate.
+     */
     protected function get_creatable_entities(): array {
         return [
             'payment accounts' => [

diff --git a/payment/tests/generator/lib.php b/payment/tests/generator/lib.php
@@ -14,8 +14,6 @@
 // You should have received a copy of the GNU General Public License
 // along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
 
-defined('MOODLE_INTERNAL') || die();
-
 /**
  * Quiz module test data generator class
  *
@@ -26,6 +24,7 @@
  */
 class core_payment_generator extends component_generator_base {
 
+    /** @var int */
     protected $accountcounter = 0;
 
     /**
@@ -34,7 +33,7 @@ class core_payment_generator extends component_generator_base {
      * @param array $data account data (name, idnumber, enabled) and additionally field 'gateways' that can include
      *    a list of gateways that should be mock-enabled for this account.
      */
-    public function create_payment_account(array $data): \core_payment\account {
+    public function create_payment_account(array $data = []): \core_payment\account {
         $this->accountcounter++;
         $gateways = [];
         if (!empty($data['gateways'])) {
@@ -70,12 +69,13 @@ public function create_payment(array $data): int {
             $data['gateway'] = reset($gateways);
         }
 
-        $id = $DB->insert_record('payments', $data + [
-            'component' => 'testcomponent',
-            'paymentarea' => 'teatarea',
-            'componentid' => 0,
-            'currency' => 'AUD'
-        ]);
+        $id = $DB->insert_record('payments', $data +
+            [
+                'component' => 'testcomponent',
+                'paymentarea' => 'teatarea',
+                'componentid' => 0,
+                'currency' => 'AUD',
+            ]);
         return $id;
     }