Skip to content

Commit

Permalink
Merge branch 'MDL-69089-master' of git://github.com/aanabit/moodle
Browse files Browse the repository at this point in the history
  • Loading branch information
@andrewnicols
andrewnicols committed Aug 19, 2020
2 parents dd844d9 + dfd8f6b commit 7cd63bd
Show file tree
Hide file tree
Showing 9 changed files with 67 additions and 34 deletions.
2 changes: 1 addition & 1 deletion contentbank/amd/build/actions.min.js
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion contentbank/amd/build/actions.min.js.map
View file

Large diffs are not rendered by default.

26 changes: 21 additions & 5 deletions contentbank/amd/src/actions.js
View file
Expand Up @@ -139,10 +139,26 @@ function($, Ajax, Notification, Str, Templates, Url, ModalFactory, ModalEvents)
});
}).then(function(modal) {
modal.setSaveButtonText(saveButtonText);
modal.getRoot().on(ModalEvents.save, function() {
modal.getRoot().on(ModalEvents.save, function(e) {
// The action is now confirmed, sending an action for it.
var newname = $("#newname").val();
return renameContent(contentid, newname);
var newname = $("#newname").val().trim();
if (newname) {
renameContent(contentid, newname);
} else {
var errorStrings = [
{
key: 'error',
},
{
key: 'emptynamenotallowed',
component: 'core_contentbank',
},
];
Str.get_strings(errorStrings).then(function(langStrings) {
Notification.alert(langStrings[0], langStrings[1]);
}).catch(Notification.exception);
e.preventDefault();
}
});

// Handle hidden event.
Expand Down Expand Up @@ -211,11 +227,11 @@ function($, Ajax, Notification, Str, Templates, Url, ModalFactory, ModalEvents)
};
var requestType = 'success';
Ajax.call([request])[0].then(function(data) {
if (data) {
if (data.result) {
return 'contentrenamed';
}
requestType = 'error';
return 'contentnotrenamed';
return data.warnings[0].message;

}).then(function(message) {
var params = null;
Expand Down
1 change: 1 addition & 0 deletions contentbank/classes/content.php
View file
Expand Up @@ -127,6 +127,7 @@ public function update_content(): bool {
* @throws \coding_exception if not loaded.
*/
public function set_name(string $name): bool {
$name = trim($name);
if (empty($name)) {
return false;
}
Expand Down
21 changes: 15 additions & 6 deletions contentbank/classes/external/rename_content.php
View file
Expand Up @@ -87,15 +87,24 @@ public static function execute(int $contentid, string $name): array {
$content = new $contentclass($record);
// Check capability.
if ($contenttype->can_manage($content)) {
// This content can be renamed.
if ($contenttype->rename_content($content, $params['name'])) {
$result = true;
} else {
if (empty(trim($name))) {
// If name is empty don't try to rename and return a more detailed message.
$warnings[] = [
'item' => $contentid,
'warningcode' => 'contentnotrenamed',
'message' => get_string('contentnotrenamed', 'core_contentbank')
'warningcode' => 'emptynamenotallowed',
'message' => get_string('emptynamenotallowed', 'core_contentbank')
];
} else {
// This content can be renamed.
if ($contenttype->rename_content($content, $params['name'])) {
$result = true;
} else {
$warnings[] = [
'item' => $contentid,
'warningcode' => 'contentnotrenamed',
'message' => get_string('contentnotrenamed', 'core_contentbank')
];
}
}
} else {
// The user has no permission to manage this content.
Expand Down
4 changes: 3 additions & 1 deletion contentbank/tests/content_test.php
View file
Expand Up @@ -81,7 +81,9 @@ public function set_name_provider() {
'Name with symbols' => ['Follow us: @moodle', 'Follow us: @moodle'],
'Name with tags' => ['This is <b>bold</b>', 'This is bold'],
'Long name' => [str_repeat('a', 100), str_repeat('a', 100)],
'Too long name' => [str_repeat('a', 300), str_repeat('a', 255)]
'Too long name' => [str_repeat('a', 300), str_repeat('a', 255)],
'Empty name' => ['', 'Old name'],
'Blanks only' => [' ', 'Old name'],
];
}

Expand Down
20 changes: 11 additions & 9 deletions contentbank/tests/contenttype_test.php
View file
Expand Up @@ -375,12 +375,14 @@ protected function contenttype_setup_scenario_data(): void {
*/
public function rename_content_provider() {
return [
'Standard name' => ['New name', 'New name'],
'Name with digits' => ['Today is 17/04/2017', 'Today is 17/04/2017'],
'Name with symbols' => ['Follow us: @moodle', 'Follow us: @moodle'],
'Name with tags' => ['This is <b>bold</b>', 'This is bold'],
'Long name' => [str_repeat('a', 100), str_repeat('a', 100)],
'Too long name' => [str_repeat('a', 300), str_repeat('a', 255)]
'Standard name' => ['New name', 'New name', true],
'Name with digits' => ['Today is 17/04/2017', 'Today is 17/04/2017', true],
'Name with symbols' => ['Follow us: @moodle', 'Follow us: @moodle', true],
'Name with tags' => ['This is <b>bold</b>', 'This is bold', true],
'Long name' => [str_repeat('a', 100), str_repeat('a', 100), true],
'Too long name' => [str_repeat('a', 300), str_repeat('a', 255), true],
'Empty name' => ['', 'Test content ', false],
'Blanks only' => [' ', 'Test content ', false],
];
}

Expand All @@ -390,10 +392,11 @@ public function rename_content_provider() {
* @dataProvider rename_content_provider
* @param string $newname The name to set
* @param string $expected The name result
* @param bool $result The bolean result expected when renaming
*
* @covers ::rename_content
*/
public function test_rename_content(string $newname, string $expected) {
public function test_rename_content(string $newname, string $expected, bool $result) {
global $DB;

$this->resetAfterTest();
Expand All @@ -414,9 +417,8 @@ public function test_rename_content(string $newname, string $expected) {

// Check the content is renamed as expected by a user with permission.
$renamed = $contenttype->rename_content($content, $newname);
$this->assertTrue($renamed);
$this->assertEquals($result, $renamed);
$record = $DB->get_record('contentbank_content', ['id' => $content->get_id()]);
$this->assertNotEquals($oldname, $record->name);
$this->assertEquals($expected, $record->name);
}

Expand Down
24 changes: 13 additions & 11 deletions contentbank/tests/external/rename_content_test.php
View file
Expand Up @@ -52,12 +52,14 @@ class rename_content_testcase extends \externallib_advanced_testcase {
*/
public function rename_content_provider() {
return [
'Standard name' => ['New name', 'New name'],
'Name with digits' => ['Today is 17/04/2017', 'Today is 17/04/2017'],
'Name with symbols' => ['Follow us: @moodle', 'Follow us: @moodle'],
'Name with tags' => ['This is <b>bold</b>', 'This is bold'],
'Long name' => [str_repeat('a', 100), str_repeat('a', 100)],
'Too long name' => [str_repeat('a', 300), str_repeat('a', 255)]
'Standard name' => ['New name', 'New name', true],
'Name with digits' => ['Today is 17/04/2017', 'Today is 17/04/2017', true],
'Name with symbols' => ['Follow us: @moodle', 'Follow us: @moodle', true],
'Name with tags' => ['This is <b>bold</b>', 'This is bold', true],
'Long name' => [str_repeat('a', 100), str_repeat('a', 100), true],
'Too long name' => [str_repeat('a', 300), str_repeat('a', 255), true],
'Empty name' => ['', 'Test content ', false],
'Blanks only' => [' ', 'Test content ', false],
];
}

Expand All @@ -66,11 +68,12 @@ public function rename_content_provider() {
*
* @dataProvider rename_content_provider
* @param string $newname The name to set
* @param string $expected The name result
* @param string $expectedname The name result
* @param bool $expectedresult The bolean result expected when renaming
*
* @covers ::execute
*/
public function test_rename_content_with_permission(string $newname, string $expected) {
public function test_rename_content_with_permission(string $newname, string $expectedname, bool $expectedresult) {
global $DB;
$this->resetAfterTest();

Expand All @@ -91,10 +94,9 @@ public function test_rename_content_with_permission(string $newname, string $exp
// Call the WS and check the content is renamed as expected.
$result = rename_content::execute($content->get_id(), $newname);
$result = external_api::clean_returnvalue(rename_content::execute_returns(), $result);
$this->assertTrue($result['result']);
$this->assertEquals($expectedresult, $result['result']);
$record = $DB->get_record('contentbank_content', ['id' => $content->get_id()]);
$this->assertNotEquals($oldname, $record->name);
$this->assertEquals($expected, $record->name);
$this->assertEquals($expectedname, $record->name);

// Call the WS using an unexisting contentid and check an error is thrown.
$this->expectException(\invalid_response_exception::class);
Expand Down
1 change: 1 addition & 0 deletions lang/en/contentbank.php
View file
Expand Up @@ -33,6 +33,7 @@
$string['contentsmoved'] = 'Content bank contents moved to {$a}.';
$string['contenttypenoaccess'] = 'You cannot view this {$a} instance.';
$string['contenttypenoedit'] = 'You can not edit this content';
$string['emptynamenotallowed'] = 'Empty name is not allowed';
$string['eventcontentcreated'] = 'Content created';
$string['eventcontentdeleted'] = 'Content deleted';
$string['eventcontentupdated'] = 'Content updated';
Expand Down

0 comments on commit 7cd63bd

Please sign in to comment.