From 11b276fc1b53082f3038f18428a6a60f1d9884fc Mon Sep 17 00:00:00 2001 From: Sara Arjona Date: Fri, 7 Jun 2024 16:42:26 +0200 Subject: [PATCH] MDL-43938 badges: Bagde names are not unique anymore The restriction for the badge name to be unique has been removed so, from now on, the badge names can be duplicated, even inside a course. --- badges/classes/badge.php | 4 +-- badges/classes/form/badge.php | 39 ++++++++++++------------ badges/tests/behat/manage_badges.feature | 30 ++++++++++++++++++ lang/en/badges.php | 6 +++- lang/en/deprecated.txt | 1 + 5 files changed, 57 insertions(+), 23 deletions(-) diff --git a/badges/classes/badge.php b/badges/classes/badge.php index 77765558233..f3d5da7e8ff 100644 --- a/badges/classes/badge.php +++ b/badges/classes/badge.php @@ -1008,7 +1008,7 @@ class badge { $fordb->id = null; $fordb->courseid = $courseid; $fordb->type = $courseid ? BADGE_TYPE_COURSE : BADGE_TYPE_SITE; - $fordb->name = $data->name; + $fordb->name = trim($data->name); $fordb->version = $data->version; $fordb->language = $data->language; $fordb->description = $data->description; @@ -1078,7 +1078,7 @@ class badge { public function update(stdClass $data): bool { global $USER; - $this->name = $data->name; + $this->name = trim($data->name); $this->version = trim($data->version); $this->language = $data->language; $this->description = $data->description; diff --git a/badges/classes/form/badge.php b/badges/classes/form/badge.php index a3d6ed6fba7..162d84ab157 100644 --- a/badges/classes/form/badge.php +++ b/badges/classes/form/badge.php @@ -43,6 +43,15 @@ class badge extends moodleform { $mform = $this->_form; $badge = (isset($this->_customdata['badge'])) ? $this->_customdata['badge'] : false; $action = $this->_customdata['action']; + if (array_key_exists('courseid', $this->_customdata)) { + $courseid = $this->_customdata['courseid']; + } else if (array_key_exists('badge', $this->_customdata)) { + $courseid = $this->_customdata['badge']->courseid; + } + if (!empty($courseid)) { + $mform->addElement('hidden', 'courseid', $courseid); + $mform->setType('courseid', PARAM_INT); + } $mform->addElement('header', 'badgedetails', get_string('badgedetails', 'badges')); $mform->addElement('text', 'name', get_string('name'), ['size' => '70']); @@ -180,6 +189,11 @@ class badge extends moodleform { $defaultvalues['expiry'] = 2; $defaultvalues['expireperiod'] = $badge->expireperiod; } + + if (!empty($badge->name)) { + $defaultvalues['name'] = trim($badge->name); + } + $defaultvalues['tags'] = \core_tag_tag::get_item_tags_array('core_badges', 'badge', $badge->id); $defaultvalues['currentimage'] = print_badge_image($badge, $badge->get_context(), 'large'); @@ -190,7 +204,11 @@ class badge extends moodleform { * Validates form data */ public function validation($data, $files) { - global $DB; + global $DB, $SITE; + + // Trim badge name (to guarantee no badges are created with the same name but some extra spaces). + $data['name'] = trim($data['name']); + $errors = parent::validation($data, $files); if (badges_open_badges_backpack_api() == OPEN_BADGES_V1) { @@ -211,25 +229,6 @@ class badge extends moodleform { $errors['imageauthoremail'] = get_string('invalidemail'); } - // Check for duplicate badge names. - if ($data['action'] == 'new') { - $duplicate = $DB->record_exists_select( - 'badge', - 'name = :name AND status != :deleted', - ['name' => $data['name'], 'deleted' => BADGE_STATUS_ARCHIVED], - ); - } else { - $duplicate = $DB->record_exists_select( - 'badge', - 'name = :name AND id != :badgeid AND status != :deleted', - ['name' => $data['name'], 'badgeid' => $data['id'], 'deleted' => BADGE_STATUS_ARCHIVED], - ); - } - - if ($duplicate) { - $errors['name'] = get_string('error:duplicatename', 'badges'); - } - if ($data['imageauthorurl'] && !preg_match('@^https?://.+@', $data['imageauthorurl'])) { $errors['imageauthorurl'] = get_string('invalidurl', 'badges'); } diff --git a/badges/tests/behat/manage_badges.feature b/badges/tests/behat/manage_badges.feature index 61648dc4525..072fb9b55bc 100644 --- a/badges/tests/behat/manage_badges.feature +++ b/badges/tests/behat/manage_badges.feature @@ -129,3 +129,33 @@ Feature: Manage badges | Badge #1 | Not available | 2 | | Badge #2 | Available | 1 | | Badge #3 | Available | 0 | + + @_file_upload + Scenario: Badge names are not unique anymore + Given the following "courses" exist: + | fullname | shortname | category | + | Course 1 | C1 | 0 | + And the following "core_badges > Badge" exists: + | name | Badge #2 | + | status | 0 | + | course | C1 | + | type | 1 | + | version | 1.0 | + | language | en | + | description | Test badge description | + | image | badges/tests/behat/badge.png | + | imageauthorurl | http://author.example.com | + | imagecaption | Test caption image | + And I log in as "admin" + And I navigate to "Badges > Add a new badge" in site administration + And I set the following fields to these values: + | name | Badge #1 | + | description | Test badge description | + And I upload "badges/tests/behat/badge.png" file to "Image" filemanager + When I press "Create badge" + Then I should see "Criteria for this badge have not been set up yet." + And I select "Edit details" from the "jump" singleselect + # Set name for a site badge with existing badge name in a course is also allowed. + And I set the field "name" to "Badge #2" + And I press "Save changes" + And I should see "Changes saved" diff --git a/lang/en/badges.php b/lang/en/badges.php index 04f87eecc42..eb7686e4622 100644 --- a/lang/en/badges.php +++ b/lang/en/badges.php @@ -302,7 +302,6 @@ $string['error:cannotrevokebadge'] = 'Cannot revoke badge from a user.'; $string['error:cannotdeletecriterion'] = 'This criterion cannot be deleted. '; $string['error:connectionunknownreason'] = 'The connection was unsuccessful but no reason was given.'; $string['error:clone'] = 'Cannot clone the badge.'; -$string['error:duplicatename'] = 'Badge with such name already exists in the system.'; $string['error:externalbadgedoesntexist'] = 'Badge not found'; $string['error:guestuseraccess'] = 'You are currently using guest access. To see badges you need to log in with your user account.'; $string['error:invalidcriteriatype'] = 'Invalid criteria type.'; @@ -421,6 +420,8 @@ $string['namewithlink'] = 'Name with link'; $string['never'] = 'Never'; $string['newbackpack'] = 'Add a new backpack'; $string['newbadge'] = 'Add a new badge'; +$string['newbadgedeprecated'] = 'You have been redirected from badges/newbadge.php. Please note that badges/newbadge.php will be removed in the near future. +
Update links and bookmarks to use the current page badges/edit.php.'; $string['newimage'] = 'New image'; $string['noalignment'] = 'This badge does not have any external skills or standards specified.'; $string['noawards'] = 'This badge has not been earned yet.'; @@ -595,3 +596,6 @@ $string['includeauthdetails'] = "Include authentication details with the backpac // Deprecated since Moodle 4.3. $string['backpackemail'] = 'Email address'; $string['backpackemail_help'] = 'The email address associated with your backpack. While you are connected, any badges earned on this site will be associated with this email address.'; + +// Deprecated since Moodle 4.5. +$string['error:duplicatename'] = 'Badge with such name already exists in the system.'; diff --git a/lang/en/deprecated.txt b/lang/en/deprecated.txt index b934616128d..37bb5465b52 100644 --- a/lang/en/deprecated.txt +++ b/lang/en/deprecated.txt @@ -116,3 +116,4 @@ coursecalendar,core_calendar importcalendarexternal,core_calendar nocalendarsubscriptions,core_calendar datechanged,core +error:duplicatename,core_badges