MDL-65032 mod_forum: Updates based on Jun's feedback

This commit is contained in:
Peter
2019-04-24 12:49:41 +08:00
parent 0547bed1a5
commit bdb4a87daa
20 changed files with 101 additions and 152 deletions
+1 -1
View File
@@ -1 +1 @@
define(["jquery","core/templates","core/notification","mod_forum/repository","mod_forum/selectors"],function(a,b,c,d,e){var f=function(f){f.on("click",e.lock.toggle,function(e){var f=a(this),g=f.data("forumid"),h=f.data("discussionid"),i=f.data("state");d.setDiscussionLockState(g,h,i).then(function(a){return b.render("mod_forum/discussion_lock_toggle",a)}).then(function(a,c){return b.replaceNode(f,a,c)})["catch"](c.exception),e.preventDefault()})};return{init:function(a){f(a)}}});
define(["jquery","core/templates","core/notification","mod_forum/repository","mod_forum/selectors"],function(a,b,c,d,e){var f=function(b){b.on("click",e.lock.toggle,function(b){var e=a(this),f=e.data("forumid"),g=e.data("discussionid"),h=e.data("state");d.setDiscussionLockState(f,g,h).then(function(){return location.reload()})["catch"](c.exception),b.preventDefault()})};return{init:function(a){f(a)}}});
+2 -5
View File
@@ -48,11 +48,8 @@ define([
var state = toggleElement.data('state');
Repository.setDiscussionLockState(forumId, discussionId, state)
.then(function(context) {
return Templates.render('mod_forum/discussion_lock_toggle', context);
})
.then(function(html, js) {
return Templates.replaceNode(toggleElement, html, js);
.then(function() {
return location.reload();
})
.catch(Notification.exception);
@@ -58,7 +58,7 @@ class discussion {
'timestart' => $discussion->get_time_start(),
'timeend' => $discussion->get_time_end(),
'pinned' => $discussion->is_pinned(),
'locked' => $discussion->get_locked()
'timelocked' => $discussion->get_locked()
];
}, $discussions);
}
@@ -234,7 +234,7 @@ class discussion {
}
/**
* Check if this discussion is pinned.
* Get the locked time of this discussion.
*
* @return bool
*/
@@ -254,11 +254,11 @@ class discussion {
/**
* Set the locked timestamp
*
* @param int $timestamp
* @param int $timestamp The value we want to store into 'locked'
*/
public function toggle_locked_state(int $timestamp) {
// If it is locked already then unlock else set it to the timestamp.
$this->timelocked = ($this->timelocked ? 0 : $timestamp);
// Check the current value against what we want the value to be i.e. '$timestamp'.
$this->timelocked = ($this->timelocked && $timestamp ? $this->timelocked : $timestamp);
}
/**
+17 -7
View File
@@ -539,16 +539,12 @@ class forum {
}
/**
* Is the discussion locked?
* Check whether the discussion is locked based on forum's time based locking criteria
*
* @param discussion_entity $discussion The discussion to check
* @param discussion_entity $discussion
* @return bool
*/
public function is_discussion_locked(discussion_entity $discussion) : bool {
if ($discussion->is_locked()) {
return true;
}
public function is_discussion_time_locked(discussion_entity $discussion) : bool {
if (!$this->has_lock_discussions_after()) {
return false;
}
@@ -622,4 +618,18 @@ class forum {
return false;
}
/**
* Is the discussion locked? - Takes into account both discussion settings AND forum's criteria
*
* @param discussion_entity $discussion The discussion to check
* @return bool
*/
public function is_discussion_locked(discussion_entity $discussion) : bool {
if ($discussion->is_locked()) {
return true;
}
return $this->is_discussion_time_locked($discussion);
}
}
@@ -64,6 +64,8 @@ class discussion extends exporter {
'id' => ['type' => PARAM_INT],
'forumid' => ['type' => PARAM_INT],
'pinned' => ['type' => PARAM_BOOL],
'locked' => ['type' => PARAM_BOOL],
'istimelocked' => ['type' => PARAM_BOOL],
'name' => ['type' => PARAM_TEXT],
'group' => [
'optional' => true,
@@ -94,7 +96,6 @@ class discussion extends exporter {
'userstate' => [
'type' => [
'subscribed' => ['type' => PARAM_BOOL],
'locked' => ['type' => PARAM_BOOL],
],
],
'capabilities' => [
@@ -182,6 +183,8 @@ class discussion extends exporter {
'id' => $discussion->get_id(),
'forumid' => $forum->get_id(),
'pinned' => $discussion->is_pinned(),
'locked' => $forum->is_discussion_locked($discussion),
'istimelocked' => $forum->is_discussion_time_locked($discussion),
'name' => format_string($discussion->get_name(), true, [
'context' => $this->related['context']
]),
@@ -192,8 +195,7 @@ class discussion extends exporter {
'locked' => $discussion->get_locked()
],
'userstate' => [
'subscribed' => \mod_forum\subscriptions::is_subscribed($user->id, $forumrecord, $discussion->get_id()),
'locked' => $discussion->is_locked()
'subscribed' => \mod_forum\subscriptions::is_subscribed($user->id, $forumrecord, $discussion->get_id())
],
'capabilities' => [
'subscribe' => $capabilitymanager->can_subscribe_to_discussion($user, $discussion),
@@ -201,10 +201,6 @@ class discussion {
$exporteddiscussion['html']['pindiscussion'] = $this->get_pin_discussion_html();
}
if ($capabilities['manage']) {
$exporteddiscussion['html']['lockdiscussion'] = $this->get_lock_discussion_button_html();
}
return $this->renderer->render_from_template('mod_forum/forum_discussion', $exporteddiscussion);
}
@@ -278,24 +274,6 @@ class discussion {
return $html;
}
/**
* Get the HTML to render the subscription button.
*
* @return string
*/
private function get_lock_discussion_button_html() : string {
global $PAGE;
$forumrecord = $this->forumrecord;
$discussionrecord = $this->discussionrecord;
$html = html_writer::div(
forum_get_lock_discussion_icon($forumrecord, $discussionrecord, null, true),
'discussionlock'
);
return $html;
}
/**
* Get the HTML to render the move discussion selector and button.
*
@@ -53,7 +53,7 @@ abstract class db_table_vault {
public function __construct(
moodle_database $db,
entity_factory $entityfactory,
$legacyfactory
object $legacyfactory
) {
$this->db = $db;
$this->entityfactory = $entityfactory;
+1
View File
@@ -142,6 +142,7 @@ $functions = array(
'description' => 'Set the lock state for the discussion',
'type' => 'write',
'ajax' => true,
'capabilities' => 'moodle/course:manageactivities',
'services' => array(MOODLE_OFFICIAL_MOBILE_SERVICE),
),
);
+2 -2
View File
@@ -140,7 +140,7 @@ function xmldb_forum_upgrade($oldversion) {
upgrade_mod_savepoint(true, 2019040400, 'forum');
}
if ($oldversion < 2019040401) {
if ($oldversion < 2019040402) {
// Define field deleted to be added to forum_posts.
$table = new xmldb_table('forum_discussions');
$field = new xmldb_field('timelocked', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, '0', 'pinned');
@@ -151,7 +151,7 @@ function xmldb_forum_upgrade($oldversion) {
}
// Forum savepoint reached.
upgrade_mod_savepoint(true, 2019040401, 'forum');
upgrade_mod_savepoint(true, 2019040402, 'forum');
}
return true;
+18 -9
View File
@@ -1532,22 +1532,25 @@ class mod_forum_external extends external_api {
$vaultfactory = mod_forum\local\container::get_vault_factory();
$forumvault = $vaultfactory->get_forum_vault();
$forum = $forumvault->get_from_id($params['forumid']);
// If the targetstate(currentstate) is not 0 then it should be set to the current time.
$targetstate = $targetstate ? 0 : time();
self::validate_context($forum->get_context());
$managerfactory = mod_forum\local\container::get_manager_factory();
$capabilitymanager = $managerfactory->get_capability_manager($forum);
if (!$capabilitymanager->can_manage_forum($USER)) {
throw new moodle_exception('errorcannotlock', 'forum');
}
// If the targetstate(currentstate) is not 0 then it should be set to the current time.
$lockedvalue = $targetstate ? 0 : time();
self::validate_context($forum->get_context());
$discussionvault = $vaultfactory->get_discussion_vault();
$discussion = $discussionvault->get_from_id($params['discussionid']);
// If the current state doesn't equal the desired state then update the current.
// state to the desired state.
if ($capabilitymanager->can_manage_forum($USER)) {
$discussion->toggle_locked_state($targetstate);
$response = $discussionvault->update_discussion($discussion);
$discussion = !$response ? $response : $discussion;
}
$discussion->toggle_locked_state($lockedvalue);
$response = $discussionvault->update_discussion($discussion);
$discussion = !$response ? $response : $discussion;
$exporterfactory = mod_forum\local\container::get_exporter_factory();
$exporter = $exporterfactory->get_discussion_exporter($USER, $forum, $discussion);
@@ -1575,6 +1578,12 @@ class mod_forum_external extends external_api {
* @return external_description
*/
public static function set_lock_state_returns() {
return \mod_forum\local\exporters\discussion::get_read_structure();
return new external_single_structure([
'id' => new external_value(PARAM_INT, 'The discussion we are locking.'),
'locked' => new external_value(PARAM_BOOL, 'The locked state of the discussion.'),
'times' => new external_single_structure([
'locked' => new external_value(PARAM_INT, 'The locked time of the discussion.'),
])
]);
}
}
+1
View File
@@ -223,6 +223,7 @@ $string['erroremptymessage'] = 'Post message cannot be empty';
$string['erroremptysubject'] = 'Post subject cannot be empty.';
$string['errorenrolmentrequired'] = 'You must be enrolled in this course to access this content';
$string['errorwhiledelete'] = 'An error occurred while deleting record.';
$string['errorcannotlock'] = 'You do not have the permission to lock discussions.';
$string['eventassessableuploaded'] = 'Some content has been posted.';
$string['everyonecanchoose'] = 'Everyone can choose to be subscribed';
$string['everyonecannowchoose'] = 'Everyone can now choose to be subscribed';
+1 -61
View File
@@ -1824,7 +1824,7 @@ function forum_get_discussions($cm, $forumsort="", $fullpost=true, $unused=-1, $
}
$discussionfields = "d.id as discussionid, d.course, d.forum, d.name, d.firstpost, d.userid, d.groupid, d.assessed," .
" d.timemodified, d.usermodified, d.timestart, d.timeend, d.pinned, d.locked";
" d.timemodified, d.usermodified, d.timestart, d.timeend, d.pinned, d.timelocked";
$allnames = get_all_user_name_fields(true, 'u');
$sql = "SELECT $postdata, $discussionfields,
@@ -2583,66 +2583,6 @@ function forum_print_discussion_header(&$post, $forum, $group = -1, $datestring
}
/**
* Return the markup for the discussion lock toggling icon.
* @param stdClass $forum forum record
* @param stdClass $discussion discussion record
* @param null $returnurl the return url to use
* @param bool $includetext Whether or not to include the text with the icon
* @return string
* @throws coding_exception
* @throws moodle_exception
*/
function forum_get_lock_discussion_icon($forum, $discussion, $returnurl = null, $includetext = false) {
global $USER, $OUTPUT, $PAGE;
if ($returnurl === null && $PAGE->url) {
$returnurl = $PAGE->url->out();
}
$discussionid = $discussion->id;
$lockstatus = forum_discussion_is_locked($forum, $discussion);
$subscriptionlink = new moodle_url('/mod/forum/lockdiscussion.php', array(
'sesskey' => sesskey(),
'id' => $forum->id,
'd' => $discussion->id,
'returnurl' => $returnurl,
));
if ($lockstatus) {
$output = $OUTPUT->pix_icon('t/unlock', get_string('clicktounlockdiscussion', 'forum'), 'core');
if ($includetext) {
$output .= get_string('locked', 'mod_forum');
}
return html_writer::link($subscriptionlink, $output, array(
'title' => get_string('clicktounlockdiscussion', 'forum'),
'class' => 'iconsmall',
'data-forumid' => $forum->id,
'data-discussionid' => $discussionid,
'data-action' => 'toggle',
'data-type' => 'lock-toggle',
'data-state' => $discussion->locked
));
} else {
$output = $OUTPUT->pix_icon('t/lock', get_string('clicktolockdiscussion', 'forum'), 'core');
if ($includetext) {
$output .= get_string('notlocked', 'mod_forum');
}
return html_writer::link("#", $output, array(
'title' => get_string('clicktolockdiscussion', 'forum'),
'class' => 'iconsmall',
'data-forumid' => $forum->id,
'data-discussionid' => $discussionid,
'data-action' => 'toggle',
'data-type' => 'lock-toggle',
'data-state' => $discussion->locked
));
}
}
/**
* Return the markup for the discussion subscription toggling icon.
*
+5
View File
@@ -217,6 +217,11 @@ if (!empty($forum)) {
'returnurl' => '/mod/forum/view.php?f=' . $forum->id)),
get_string('youneedtoenrol'));
}
// The forum has been locked. Just redirect back to the discussion page.
if (forum_discussion_is_locked($forum, $discussion)) {
redirect(new moodle_url('/mod/forum/discuss.php', array('d' => $discussion->id)));
}
}
print_error('nopostforum', 'forum');
}
@@ -30,31 +30,29 @@
Example context (json):
{
"id": 0,
"locked": 1
}
}}
{{#capabilities.manage}}
<a
class="iconsmall"
data-type="lock-toggle"
data-action="toggle"
data-discussionid="{{id}}"
data-forumid="{{forumid}}"
data-state="{{times.locked}}"
tabindex="-1"
{{#userstate.locked}}
title="{{#str}}locked, forum{{/str}}"
{{/userstate.locked}}
{{^userstate.locked}}
title="{{#str}}notlocked, forum{{/str}}"
{{/userstate.locked}}
>
{{#userstate.locked}}
{{#pix}}t/unlock, core, {{#str}}clicktounlockdiscussion, forum{{/str}}{{/pix}}{{#str}}locked, forum{{/str}}
{{/userstate.locked}}
{{^userstate.locked}}
{{#pix}}t/lock, core, {{#str}}clicktolockdiscussion, forum{{/str}}{{/pix}}{{#str}}notlocked, forum{{/str}}
{{/userstate.locked}}
</a>
{{/capabilities.manage}}
<a
class="iconsmall"
data-type="lock-toggle"
data-action="toggle"
data-discussionid="{{id}}"
data-forumid="{{forumid}}"
data-state="{{locked}}"
href="#"
{{#locked}}
title="{{#str}}clicktounlockdiscussion, forum{{/str}}"
{{/locked}}
{{^locked}}
title="{{#str}}clicktolockdiscussion, forum{{/str}}"
{{/locked}}
>
{{#locked}}
{{#pix}}t/unlock, core, {{#str}}clicktounlockdiscussion, forum{{/str}}{{/pix}}{{#str}}locked, forum{{/str}}
{{/locked}}
{{^locked}}
{{#pix}}t/lock, core, {{#str}}clicktolockdiscussion, forum{{/str}}{{/pix}}{{#str}}notlocked, forum{{/str}}
{{/locked}}
</a>
@@ -33,7 +33,13 @@
<div id="discussion-container-{{uniqid}}" data-content="forum-discussion">
{{#html}}
<div class="d-flex flex-wrap flex-row-reverse m-b-1" data-container="discussion-tools" style="text-align: right;">
<div class="pl-1">{{{lockdiscussion}}}</div>
{{#capabilities.manage}}
{{^timelocked}}
<div class="pl-1 discussionlock">
{{> forum/discussion_lock_toggle }}
</div>
{{/timelocked}}
{{/capabilities.manage}}
<div class="pl-1">{{{subscribe}}}</div>
</div>
{{{neighbourlinks}}}
@@ -38,7 +38,6 @@ Feature: As a teacher, you can manually lock individual discussions when viewing
And I follow "Lock"
Then "a[@title='Lock']" "css_element" should not be visible
Then "Locked" "link" should be visible
And I reload the page
Then I should see "This discussion has been locked so you can no longer reply to it."
And I follow "Discussion 2"
Then I should not see "This discussion has been locked so you can no longer reply to it."
+2 -1
View File
@@ -148,7 +148,8 @@ class mod_forum_entities_discussion_testcase extends advanced_testcase {
$basetime,
$starttime,
$endtime,
false
false,
0
);
$CFG->forum_enabletimedposts = true;
+8 -6
View File
@@ -1451,22 +1451,24 @@ class mod_forum_external_testcase extends externallib_advanced_testcase {
$this->getDataGenerator()->enrol_user($user->id, $course->id, $studentrole->id, 'manual');
// Only a teacher should be able to lock a discussion.
$result = mod_forum_external::set_lock_state($forum->id, $discussion->id, 0);
$result = external_api::clean_returnvalue(mod_forum_external::set_lock_state_returns(), $result);
$this->assertFalse($result['userstate']['locked']);
$this->assertEquals('0', $result['times']['locked']);
try {
$result = mod_forum_external::set_lock_state($forum->id, $discussion->id, 0);
$this->fail('Exception expected due to missing capability.');
} catch (moodle_exception $e) {
$this->assertEquals('errorcannotlock', $e->errorcode);
}
// Set the lock.
self::setAdminUser();
$result = mod_forum_external::set_lock_state($forum->id, $discussion->id, 0);
$result = external_api::clean_returnvalue(mod_forum_external::set_lock_state_returns(), $result);
$this->assertTrue($result['userstate']['locked']);
$this->assertTrue($result['locked']);
$this->assertNotEquals(0, $result['times']['locked']);
// Unset the lock.
$result = mod_forum_external::set_lock_state($forum->id, $discussion->id, time());
$result = external_api::clean_returnvalue(mod_forum_external::set_lock_state_returns(), $result);
$this->assertFalse($result['userstate']['locked']);
$this->assertFalse($result['locked']);
$this->assertEquals('0', $result['times']['locked']);
}
+1 -1
View File
@@ -24,6 +24,6 @@
defined('MOODLE_INTERNAL') || die();
$plugin->version = 2019040401; // The current module version (Date: YYYYMMDDXX)
$plugin->version = 2019040402; // The current module version (Date: YYYYMMDDXX)
$plugin->requires = 2018112800; // Requires this Moodle version
$plugin->component = 'mod_forum'; // Full name of the plugin (used for diagnostics)