Merge branch 'MDL-70911-master' of git://github.com/sarjona/moodle

abgreeve · Mar 10, 2021 · 644f327 · 644f327
2 parents 9e1da41 + 49c1d41
commit 644f327
Show file tree

Hide file tree

Showing 14 changed files with 271 additions and 61 deletions.
diff --git a/admin/settings/badges.php b/admin/settings/badges.php
@@ -57,20 +57,14 @@
             new lang_string('badgesalt_desc', 'badges'),
             'badges' . $SITE->timecreated, PARAM_ALPHANUM));
 
-    $backpacks = badges_get_site_backpacks();
-    $choices = array();
-    $defaultchoice = 0;
-    foreach ($backpacks as $backpack) {
-        $choices[$backpack->id] = $backpack->backpackweburl;
-        if ($backpack->backpackweburl == BADGRIO_BACKPACKWEBURL) {
-            $defaultchoice = $backpack->id;
-        }
-    }
-
     $globalsettings->add(new admin_setting_configcheckbox('badges_allowcoursebadges',
             new lang_string('allowcoursebadges', 'badges'),
             new lang_string('allowcoursebadges_desc', 'badges'), 1));
 
+    $globalsettings->add(new admin_setting_configcheckbox('badges_allowexternalbackpack',
+            new lang_string('allowexternalbackpack', 'badges'),
+            new lang_string('allowexternalbackpack_desc', 'badges'), 1));
+
     $ADMIN->add('badges', $globalsettings);
 
     $ADMIN->add('badges',
@@ -97,19 +91,6 @@
             array('moodle/badges:createbadge'), empty($CFG->enablebadges)
         )
     );
-    $backpacksettings = new admin_settingpage('backpacksettings', new lang_string('backpacksettings', 'badges'),
-            array('moodle/badges:manageglobalsettings'), empty($CFG->enablebadges));
-
-    $backpacksettings->add(new admin_setting_configcheckbox('badges_allowexternalbackpack',
-            new lang_string('allowexternalbackpack', 'badges'),
-            new lang_string('allowexternalbackpack_desc', 'badges'), 1));
-
-    $backpacksettings->add(new admin_setting_configselect('badges_site_backpack',
-            new lang_string('sitebackpack', 'badges'),
-            new lang_string('sitebackpack_help', 'badges'),
-            $defaultchoice, $choices));
-
-    $ADMIN->add('badges', $backpacksettings);
 
     $ADMIN->add('badges',
         new admin_externalpage('managebackpacks',

diff --git a/badges/backpacks.php b/badges/backpacks.php
@@ -56,6 +56,17 @@
     } else {
         $msg = get_string('sitebackpacknotdeleted', 'badges');
     }
+} else if ($action == 'moveup' || $action == 'movedown') {
+    // If no backpack has been selected, there isn't anything to move.
+    if (empty($id)) {
+        redirect($url);
+    }
+
+    $direction = BACKPACK_MOVE_DOWN;
+    if ($action == 'moveup') {
+        $direction = BACKPACK_MOVE_UP;
+    }
+    badges_change_sortorder_backpacks($id, $direction);
 }
 
 if ($action == 'edit') {

diff --git a/badges/classes/form/backpack.php b/badges/classes/form/backpack.php
@@ -78,7 +78,8 @@ public function definition() {
             }
             $mform->addElement('select', 'externalbackpackid', get_string('backpackprovider', 'badges'), $choices);
             $mform->setType('externalbackpackid', PARAM_INT);
-            $mform->setDefault('externalbackpackid', $CFG->badges_site_backpack);
+            $defaultbackpack = badges_get_site_primary_backpack();
+            $mform->setDefault('externalbackpackid', $defaultbackpack->id);
             $mform->hideIf('password', 'externalbackpackid', 'in', $restrictedoptions);
             $mform->hideIf('backpackemail', 'externalbackpackid', 'in', $restrictedoptions);
 

diff --git a/badges/classes/output/external_backpacks_page.php b/badges/classes/output/external_backpacks_page.php
@@ -56,10 +56,13 @@ public function __construct(\moodle_url $url) {
      * @return stdClass
      */
     public function export_for_template(\renderer_base $output) {
-        global $CFG, $PAGE;
+        global $PAGE;
 
         $PAGE->requires->js_call_amd('core_badges/backpackactions', 'init');
 
+        $rownumber = 0;
+        $rowcount = count($this->backpacks);
+
         $data = new \stdClass();
         $data->baseurl = $this->url;
         $data->backpacks = array();
@@ -68,9 +71,11 @@ public function export_for_template(\renderer_base $output) {
             $exporter = new backpack_exporter($backpack);
             $backpack = $exporter->export($output);
             $backpack->cantest = ($backpack->apiversion == OPEN_BADGES_V2);
-            $backpack->iscurrent = ($backpack->id == $CFG->badges_site_backpack);
+            $backpack->canmoveup = $rownumber > 0;
+            $backpack->canmovedown = $rownumber < $rowcount - 1;
 
             $data->backpacks[] = $backpack;
+            $rownumber++;
         }
 
         return $data;

diff --git a/badges/templates/external_backpacks_page.mustache b/badges/templates/external_backpacks_page.mustache
@@ -25,7 +25,13 @@
     Example context (json):
     {
         "backpacks": [
-            {"backpackweburl": "http://localhost/", "sitebackpack": true, "cantest": true}
+            {
+                "backpackweburl": "http://localhost/",
+                "sitebackpack": true,
+                "cantest": true,
+                "canmoveup": true,
+                "canmovedown": false
+            }
         ]
     }
 }}
@@ -40,27 +46,40 @@
     <thead>
         <tr>
             <th scope="col">{{#str}}backpackweburl, core_badges{{/str}}</th>
-            <th scope="col">{{#str}}sitebackpack, core_badges{{/str}}</th>
+            <th scope="col">{{#str}}order{{/str}}</th>
             <th scope="col">{{#str}}actions, core_badges{{/str}}</th>
         </tr>
     </thead>
     <tbody>
         {{#backpacks}}
         <tr data-backpackurl="{{{backpackweburl}}}">
             <td> {{{backpackweburl}}} </td>
-            <td> {{#sitebackpack}}Yes{{/sitebackpack}} </td>
+            <td>
+                {{#canmoveup}}
+                    <a href="{{baseurl}}?id={{id}}&action=moveup">{{#pix}}t/up, core,{{#str}}moveup{{/str}}{{/pix}}</a>
+                {{/canmoveup}}
+                {{^canmoveup}}
+                    {{#pix}}spacer, moodle{{/pix}}
+                {{/canmoveup}}
+                {{#canmovedown}}
+                    <a href="{{baseurl}}?id={{id}}&action=movedown">{{#pix}}t/down, core,{{#str}}movedown{{/str}}{{/pix}}</a>
+                {{/canmovedown}}
+                {{^canmovedown}}
+                    {{#pix}}spacer, moodle{{/pix}}
+                {{/canmovedown}}
+            </td>
             <td>
                 <a href="{{baseurl}}?id={{id}}&action=edit">{{#pix}}t/edit, core,{{#str}}editsettings{{/str}}{{/pix}}</a>
-            {{^iscurrent}}
+            {{^sitebackpack}}
                 <a href="{{baseurl}}?id={{id}}&action=delete" role="button" data-action="deletebackpack">
                     {{#pix}}t/delete, core,{{#str}}delete{{/str}}{{/pix}}
                 </a>
-            {{/iscurrent}}
+            {{/sitebackpack}}
             {{#cantest}}
                 <a href="{{baseurl}}?id={{id}}&action=test">{{#pix}}t/check, core,{{#str}}testsettings, core_badges{{/str}}{{/pix}}</a>
             {{/cantest}}
             </td>
         </tr>
         {{/backpacks}}
     </tbody>
-</table>
+</table>
diff --git a/badges/tests/badgeslib_test.php b/badges/tests/badgeslib_test.php
@@ -1196,6 +1196,7 @@ public function test_badges_open_badges_backpack_api() {
             'apiversion' => 2,
             'backpackapiurl' => 'https://api.ca.badgr.io/v2',
             'backpackweburl' => 'https://ca.badgr.io',
+            'sortorder' => 2,
         ];
 
         // Given a complete set of unique data, a new backpack and auth records should exist in the tables.
@@ -1204,10 +1205,13 @@ public function test_badges_open_badges_backpack_api() {
         $backpack1 = badges_save_external_backpack((object) $data);
         $data['backpackweburl'] = 'https://eu.badgr.io';
         $data['backpackapiurl'] = 'https://api.eu.badgr.io/v2';
-        $data['apiversion'] = 2.1;
+        $data['apiversion'] = '2.1';
+        $data['sortorder'] = 3;
         $backpack2 = badges_save_external_backpack((object) $data);
 
-        set_config('badges_site_backpack', $backpack2);
+        // Move backpack2 to the first position to set it as primary site backpack.
+        $this->move_backpack_to_first_position($backpack2);
+
         // The default response should check the default site backpack api version.
         $this->assertEquals(2.1, badges_open_badges_backpack_api());
         // Check the api version for the other backpack created.
@@ -1308,6 +1312,7 @@ public function test_badges_get_site_primary_backpack($withauth) {
             'apiversion' => '2',
             'backpackapiurl' => 'https://api.ca.badgr.io/v2',
             'backpackweburl' => 'https://ca.badgr.io',
+            'sortorder' => '2',
         ];
         if ($withauth) {
             $data = array_merge($data, [
@@ -1317,7 +1322,13 @@ public function test_badges_get_site_primary_backpack($withauth) {
         }
         $backpack = badges_save_external_backpack((object) $data);
 
-        set_config('badges_site_backpack', $backpack);
+        // Check the backpack created is not the primary one.
+        $sitebackpack = badges_get_site_primary_backpack();
+        $this->assertNotEquals($backpack, $sitebackpack->id);
+
+        // Move backpack to the first position to set it as primary site backpack.
+        $this->move_backpack_to_first_position($backpack);
+
         $sitebackpack = badges_get_site_primary_backpack();
         $this->assertEquals($backpack, $sitebackpack->id);
 
@@ -1344,6 +1355,84 @@ public function badges_get_site_primary_backpack_provider() {
         ];
     }
 
+    /**
+     * Test badges_change_sortorder_backpacks().
+     *
+     * @dataProvider badges_change_sortorder_backpacks_provider
+     * @covers ::badges_change_sortorder_backpacks
+     *
+     * @param int $backpacktomove Backpack index to move (from 0 to 5).
+     * @param int $direction Direction to move the backpack.
+     * @param int|null $expectedsortorder Expected sortorder or null if an exception is expected.
+     */
+    public function test_badges_change_sortorder_backpacks(int $backpacktomove, int $direction, ?int $expectedsortorder): void {
+        global $DB;
+
+        $this->resetAfterTest();
+        $this->setAdminUser();
+
+        // Create 5 more backpacks.
+        for ($i = 0; $i < 5; $i++) {
+            $data = new \stdClass();
+            $data->apiversion = OPEN_BADGES_V2P1;
+            $data->backpackapiurl = "https://myurl$i.cat/ob/v2p1";
+            $data->backpackweburl = "https://myurl$i.cat";
+            badges_create_site_backpack($data);
+        }
+
+        // Check there are 6 backpacks (1 pre-existing + 5 news).
+        $total = $DB->count_records('badge_external_backpack');
+        $this->assertEquals(6, $total);
+        $backpacks = array_values(badges_get_site_backpacks());
+
+        if (is_null($expectedsortorder)) {
+            $this->expectException('moodle_exception');
+        }
+
+        // Move the backpack.
+        badges_change_sortorder_backpacks($backpacks[$backpacktomove]->id, $direction);
+
+        if (!is_null($expectedsortorder)) {
+            $backpack = badges_get_site_backpack($backpacks[$backpacktomove]->id);
+            $this->assertEquals($expectedsortorder, $backpack->sortorder);
+        }
+    }
+
+    /**
+     * Provider for test_badges_change_sortorder_backpacks.
+     *
+     * @return array
+     */
+    public function badges_change_sortorder_backpacks_provider(): array {
+        return [
+            "Test up" => [
+                'backpacktomove' => 1,
+                'direction' => BACKPACK_MOVE_UP,
+                'expectedsortorder' => 1,
+            ],
+            "Test down" => [
+                'backpacktomove' => 1,
+                'direction' => BACKPACK_MOVE_DOWN,
+                'expectedsortorder' => 3,
+            ],
+            "Test up the very first element" => [
+                'backpacktomove' => 0,
+                'direction' => BACKPACK_MOVE_UP,
+                'expectedsortorder' => 1,
+            ],
+            "Test down the very last element" => [
+                'backpacktomove' => 5,
+                'direction' => BACKPACK_MOVE_DOWN,
+                'expectedsortorder' => 6,
+            ],
+            "Test with an invalid direction value" => [
+                'backpacktomove' => 1,
+                'direction' => 10,
+                'expectedsortorder' => null,
+            ],
+        ];
+    }
+
     /**
      * Test the Badgr URL generator function
      *
@@ -1434,4 +1523,17 @@ public function badges_external_get_mapping_provider() {
             ]
         ];
     }
+
+    /**
+     * Move the backpack to the first position, to set it as primary site backpack.
+     *
+     * @param int $backpackid The backpack identifier.
+     */
+    private function move_backpack_to_first_position(int $backpackid): void {
+        $backpack = badges_get_site_backpack($backpackid);
+        while ($backpack->sortorder > 1) {
+            badges_change_sortorder_backpacks($backpackid, BACKPACK_MOVE_UP);
+            $backpack = badges_get_site_backpack($backpackid);
+        }
+    }
 }
diff --git a/badges/tests/behat/backpack.feature b/badges/tests/behat/backpack.feature
@@ -5,9 +5,9 @@ Feature: Backpack badges
 
   Background:
     Given the following "badge external backpacks" exist:
-      | backpackapiurl                               | backpackweburl           | apiversion |
-      | https://dc.imsglobal.org/obchost/ims/ob/v2p1 | https://dc.imsglobal.org | 2.1        |
-      | https://test.com/                            | https://test.com/        | 2          |
+      | backpackapiurl                               | backpackweburl           | apiversion | sortorder |
+      | https://dc.imsglobal.org/obchost/ims/ob/v2p1 | https://dc.imsglobal.org | 2.1        | 2         |
+      | https://test.com/                            | https://test.com/        | 2          | 3         |
     And the following "users" exist:
       | username | firstname | lastname | email                |
       | student1 | Student   | 1        | student1@example.com |
@@ -17,15 +17,34 @@ Feature: Backpack badges
     And I set the field "Badge issuer email address" to "[email protected]"
     And I log out
 
+  @javascript
+  Scenario: If external backpack connection is disabled, backpack settings should not be displayed
+    Given I am on homepage
+    And I log in as "admin"
+    And I navigate to "Badges > Badges settings" in site administration
+    And I set the following fields to these values:
+      | External backpack connection | 0                        |
+    And I press "Save changes"
+    When I navigate to "Badges" in site administration
+    Then I should not see "Manage backpacks"
+    And I navigate to "Badges > Badges settings" in site administration
+    And I set the following fields to these values:
+      | External backpack connection | 1                        |
+    And I press "Save changes"
+    And I am on homepage
+    And I navigate to "Badges" in site administration
+    And I should see "Manage backpacks"
+
   @javascript
   Scenario: Verify backback settings
     Given I am on homepage
     And I log in as "admin"
-    And I navigate to "Badges > Backpack settings" in site administration
+    And I navigate to "Badges > Badges settings" in site administration
     And I set the following fields to these values:
       | External backpack connection | 1                        |
-      | Active external backpack     | https://dc.imsglobal.org |
     And I press "Save changes"
+    And I navigate to "Badges > Manage backpacks" in site administration
+    And I click on "Move up" "link" in the "https://dc.imsglobal.org" "table_row"
     And I navigate to "Badges > Add a new badge" in site administration
     And I set the following fields to these values:
       | Name          | Test badge verify backpack |
@@ -57,11 +76,12 @@ Feature: Backpack badges
   Scenario: User has been connected backpack
     Given I am on homepage
     And I log in as "admin"
-    And I navigate to "Badges > Backpack settings" in site administration
+    And I navigate to "Badges > Badges settings" in site administration
     And I set the following fields to these values:
       | External backpack connection | 1                        |
-      | Active external backpack     | https://dc.imsglobal.org |
     And I press "Save changes"
+    And I navigate to "Badges > Manage backpacks" in site administration
+    And I click on "Move up" "link" in the "https://dc.imsglobal.org" "table_row"
     And I navigate to "Badges > Add a new badge" in site administration
     And I set the following fields to these values:
       | Name           | Test badge verify backpack |
@@ -122,6 +142,21 @@ Feature: Backpack badges
     And I should not see "https://dc.imsglobal.org"
     And "Delete" "button" should not exist
 
+  @javascript
+  Scenario: Move up and down site backpack
+    Given I am on homepage
+    And I log in as "admin"
+    And I navigate to "Badges > Manage backpacks" in site administration
+    And "Move up" "icon" should exist in the "https://dc.imsglobal.org" "table_row"
+    And "Move down" "icon" should exist in the "https://dc.imsglobal.org" "table_row"
+    When I click on "Move up" "link" in the "https://dc.imsglobal.org" "table_row"
+    Then "Move up" "icon" should not exist in the "https://dc.imsglobal.org" "table_row"
+    And "Move down" "icon" should exist in the "https://dc.imsglobal.org" "table_row"
+    And I click on "Move down" "link" in the "https://dc.imsglobal.org" "table_row"
+    And I click on "Move down" "link" in the "https://dc.imsglobal.org" "table_row"
+    And "Move up" "icon" should exist in the "https://dc.imsglobal.org" "table_row"
+    And "Move down" "icon" should not exist in the "https://dc.imsglobal.org" "table_row"
+
   @javascript
   Scenario: Add a new site backpack with authentication details checkbox
     Given I am on homepage