Commit 4083787b authored by Naomi Guyer's avatar Naomi Guyer Committed by Robert Lyon

FIX: Notifications bugs (BUG: 1465107)

* delete, mark as read, pagination filtering, and reply/reply all

Change-Id: I55f044c478f570be726c402c222f78b90bd810f7
parent 29c64425
......@@ -78,6 +78,34 @@ $activitylist = activitylistin_html($type);
$strread = json_encode(get_string('read', 'activity'));
$strnodelete = json_encode(get_string('nodelete', 'activity'));
$paginationjavascript = <<<JAVASCRIPT
// NOTE: most js is in the notification.js file, but we found
// this part much more difficult to relocate
jQuery(function($) {
// We want the paginator to tell us when a page gets changed.
function PaginatorData() {
var self = this;
var params = {};
this.pageChanged = function(data) {
self.params = {
'offset': data.offset,
'limit': data.limit,
'type': data.type
}
}
paginatorProxy.addObserver(self);
connect(self, 'pagechanged', self.pageChanged);
}
window.paginatorData = new PaginatorData();
addLoadEvent(function () {
window.paginator = {$activitylist['pagination_js']}
});
});
JAVASCRIPT;
$deleteall = pieform(array(
'name' => 'delete_all_notifications',
'class' => 'form-deleteall sr-only',
......@@ -183,7 +211,7 @@ function delete_all_notifications_submit() {
$smarty = smarty(array('paginator'));
$smarty->assign('options', $options);
$smarty->assign('type', $type);
$smarty->assign('paginatorData', $activitylist['pagination_js']);
$smarty->assign('INLINEJAVASCRIPT', $paginationjavascript);
// Adding the links to out- and inbox
$smarty->assign('PAGEHEADING', TITLE);
......
......@@ -19,7 +19,7 @@ global $USER;
global $THEME;
$readone = param_integer('readone', 0);
$list = param_alphanumext('table', 'notification_internal_activity');
$list = param_alphanumext('list', 'notification_internal_activity');
$markasread = param_integer('markasread', 0);
$delete = param_integer('delete', 0);
......@@ -87,6 +87,7 @@ else if ($delete) {
}
}
}
db_begin();
$countdeleted = 0;
foreach ($ids as $list => $idsperlist) {
......@@ -99,7 +100,8 @@ else if ($delete) {
$userid = $USER->get('id');
// Ignore message ids that do not belong to the current user to stop
// hacking of the form allowing the deletion of messages owned by other users.
$rawstrids = join(',', array_map('db_quote', $idspertable));
$rawstrids = join(',', array_map('db_quote', $idsperlist));
$ids = get_column_sql(
"SELECT id FROM {notification_internal_activity}
WHERE id IN ($rawstrids) AND usr = ?",
......@@ -123,7 +125,7 @@ else if ($delete) {
}
}
}
$countdeleted += ($ids) ? count($ids) : count($idspertable);
$countdeleted += ($ids) ? count($ids) : count($idsperlist);
}
db_commit();
$message = get_string('deletednotifications1', 'activity', $countdeleted);
......
......@@ -19,14 +19,20 @@ global $USER;
global $THEME;
$readone = param_integer('readone', 0);
$list = param_alphanumext('list', 'notification_internal_activity');
$markasread = param_integer('markasread', 0);
$delete = param_integer('delete', 0);
if ($readone) {
set_field('notification_internal_activity', 'read', 1, 'id', $readone, 'usr', $USER->get('id'));
if ('notification_internal_activity' === $list) {
set_field($list, 'read', 1, 'id', $readone, 'usr', $USER->get('id'));
}
else if ('artefact_multirecipient_notification' === $list) {
mark_as_read_mr(array($readone), $USER->get('id'));
}
$unread = $USER->add_unread(-1);
$data = array(
'newunreadcount' => $unread,
'newunreadcount' => $unread
);
json_reply(false, array('data' => $data));
}
......@@ -39,42 +45,24 @@ $offset = param_integer('offset', 0);
$message = false;
if ($markasread) {
$ids = array();
$m = array();
foreach ($_GET as $k => $v) {
if (preg_match('/^unread\-(\d+)$/',$k,$m)) {
$ids[] = $m[1];
}
}
if ($ids) {
set_field_select(
'notification_internal_activity', 'read', 1,
'id IN (' . join(',', array_map('db_quote', $ids)) . ') AND usr = ?',
array($USER->get('id'))
);
$newunread = $USER->add_unread(-count($ids));
}
$message = get_string('markedasread', 'activity');
}
else if ($delete) {
if ($delete) {
$ids = array();
$deleteunread = 0; // Remember the number of unread messages being deleted
foreach ($_GET as $k => $v) {
if (preg_match('/^delete\-([a-zA-Z_]+)\-(\d+)$/',$k,$m)) {
$table = $m[1];
$ids[$table][] = $m[2];
if (isset($_GET['unread-' . $table . '-' . $m[2]])) {
if (preg_match('/^select\-([a-zA-Z_]+)\-(\d+)$/',$k,$m)) {
$list = $m[1];
$ids[$list][] = $m[2];
if (isset($_GET['unread-' . $list . '-' . $m[2]])) {
$deleteunread++;
}
}
}
$countdeleted = 0;
db_begin();
foreach ($ids as $table => $idspertable) {
if ('artefact_multirecipient_notification' === $table) {
delete_messages_mr($idspertable, $USER->get('id'));
$countdeleted += count($idspertable);
$countdeleted = 0;
foreach ($ids as $list => $idsperlist) {
if ('artefact_multirecipient_notification' === $list) {
delete_messages_mr($idsperlist, $USER->get('id'));
$countdeleted += count($idsperlist);
}
}
db_commit();
......
......@@ -500,6 +500,7 @@ function activitylistout_html($type='all', $limit=10, $offset=0) {
continue;
}
$record = $recordsarray[0];
$record->self = false;
$record->canreplyall = false;
$record->canreply = false;
$record->startnewthread = true;
......@@ -516,6 +517,7 @@ function activitylistout_html($type='all', $limit=10, $offset=0) {
$record->tousr = array (
$tousrarray,
);
$record->self = ($record->from == $USER->get('id'));
}
else {
$record->tousr = array(
......@@ -524,11 +526,11 @@ function activitylistout_html($type='all', $limit=10, $offset=0) {
'link' => null,
),
);
$record->self = true;
}
// read out sender name
if (isset($record->from)) {
$record->fromusr = $record->from;
// $record->fromusr = $record->from;
}
else {
// we're in the outbox, so basically, this should hold for all messages
......@@ -556,6 +558,7 @@ function activitylistout_html($type='all', $limit=10, $offset=0) {
// applicable - we don't link to deleted users. Those will be summed
// up in a single entry at the end of the list
$deletedcount = 0;
$record->self = false;
$record->canreply = false;
$record->canreplyall = false;
$record->startnewthread = false;
......@@ -582,6 +585,7 @@ function activitylistout_html($type='all', $limit=10, $offset=0) {
if ($deletedcount < count($record->userids)) {
if ((count($record->userids) - $deletedcount) == 1) {
$record->canreply = true;
$record->self = ($record->fromid == $USER->get('id'));
}
else {
$record->canreplyall = true;
......
......@@ -71,6 +71,35 @@ $activitylist = activitylistout_html($type);
$strread = json_encode(get_string('read', 'activity'));
$strnodelete = json_encode(get_string('nodelete', 'activity'));
$paginationjavascript = <<<JAVASCRIPT
// NOTE: most js is in the notification.js file, but we found
// this part much more difficult to relocate
jQuery(function($) {
// We want the paginator to tell us when a page gets changed.
function PaginatorData() {
var self = this;
var params = {};
this.pageChanged = function(data) {
self.params = {
'offset': data.offset,
'limit': data.limit,
'type': data.type
}
}
paginatorProxy.addObserver(self);
connect(self, 'pagechanged', self.pageChanged);
}
window.paginatorData = new PaginatorData();
addLoadEvent(function () {
window.paginator = {$activitylist['pagination_js']}
});
});
JAVASCRIPT;
$deleteall = pieform(array(
'name' => 'delete_all_notifications',
'class' => 'form-deleteall sr-only',
......@@ -133,6 +162,8 @@ $smarty = smarty(array('paginator'));
$smarty->assign('options', $options);
$smarty->assign('type', $type);
$smarty->assign('INLINEJAVASCRIPT', $paginationjavascript);
// Adding the links to out- and inbox
$smarty->assign('PAGEHEADING', TITLE);
$smarty->assign('subsectionheading', get_string('labeloutbox1', 'artefact.multirecipientnotification'));
......
......@@ -38,13 +38,16 @@ function get_string(s) {
* Getting the string via ajax as deferred object
*/
function get_string_ajax(str, section) {
// If string already exists in strings object
if (typeof(strings[str]) !== 'undefined') {
return get_string.apply(arguments);
// need to pass all the arguments except 'str'
// in case there are other %s variables
return get_string.apply(str, [].slice.call(arguments));
}
var rnd = randString(10);
var placeholder = '<span id="str_' + rnd + '">' + get_string(str, section) + '</span>';
var placeholder = '<span id="str_' + rnd + '">' + str + '</span>';
get_string_ajax_call.apply(this, arguments).done(function(r) {
// need to find the random string in the text and replace it with our lang string
jQuery('#str_' + rnd).replaceWith(r.message.data.string);
......
......@@ -14,22 +14,27 @@
* and rewrites it to be javascript aware
*
* @param id The ID of the div that contains the pagination
* @param datatable The ID of the table containing the paginated data
* @param list The ID of the table containing the paginated data
* @param script The URL (from wwwroot) of the script to hit to get new
* pagination data
* @param limit Extra data to pass back in the ajax requests to the script
*/
var Paginator = function(id, datatable, heading, script, extradata) {
var Paginator = function(id, list, heading, script, extradata) {
var self = this;
this.init = function(id, datatable, heading, script, extradata) {
this.init = function(id, list, heading, script, extradata) {
self.id = id;
if (script && script.length !== 0) {
self.datatable = $(datatable);
self.list = $(list);
self.heading = $(heading);
self.jsonScript = config['wwwroot'] + script;
self.extraData = extradata;
if (self.list !== null && self.list.tagName == 'TABLE') {
self.isTable = true;
}
var index = location.href.indexOf('?');
if (index >= 0) {
var querystring = parseQueryString(location.href.substr(index));
......@@ -127,18 +132,22 @@ var Paginator = function(id, datatable, heading, script, extradata) {
};
this.updateResults = function (data, params, changedPage) {
var container = self.datatable;
if (self.datatable.tagName == 'TABLE') {
container = getFirstElementByTagAndClassName('tbody', null, self.datatable);
var container = self.isTable ? getFirstElementByTagAndClassName('tbody', null, self.list) : self.list
listdata = data.data.html ? data.data.html : data.data.tablerows,
paginationdata = data.data.pagination;
if (listdata.length === 0) {
listdata = '<p class="no-results">' + get_string_ajax('noresultsfound', 'mahara') + '</p>';
}
if (container) {
// You can't write to table nodes innerHTML in IE and
// konqueror, so this workaround detects them and does
// things differently
if ((document.all && !window.opera) || (/Konqueror|AppleWebKit|KHTML/.test(navigator.userAgent))) {
if (self.isTable && ((document.all && !window.opera) || (/Konqueror|AppleWebKit|KHTML/.test(navigator.userAgent)))) {
var temp = DIV({'id':'ie-workaround'});
if (container.tagName == 'TBODY') {
temp.innerHTML = '<table><tbody>' + data.data.tablerows + '</tbody></table>';
temp.innerHTML = '<table><tbody>' + listdata + '</tbody></table>';
swapDOM(container, temp.childNodes[0].childNodes[0]);
}
else {
......@@ -147,18 +156,18 @@ var Paginator = function(id, datatable, heading, script, extradata) {
}
}
else {
container.innerHTML = data['data']['tablerows'];
container.innerHTML = listdata;
}
// In Chrome, tbody remains set to the value before tbody.innerHTML was modified
// to fix that, we re-initialize tbody using getFirstElementByTagAndClassName
if (/chrome/.test(navigator.userAgent.toLowerCase()) && container.tagName == 'TBODY') {
container = getFirstElementByTagAndClassName('tbody', null, self.datatable);
container = getFirstElementByTagAndClassName('tbody', null, self.list);
}
// Pieforms should probably separate its js from its html. For
// Pieforms should separate its js from its html. For
// now, be evil: scrape it out of the script elements and eval
// it every time the page changes.
// it every time the page changes. :(
forEach(getElementsByTagAndClassName('script', null, container), function(s) {
var m = scrapeText(s).match(new RegExp('^(new Pieform\\\(.*?\\\);)$'));
if (m && m[1]) {
......@@ -172,7 +181,7 @@ var Paginator = function(id, datatable, heading, script, extradata) {
// Update the pagination
if ($(self.id)) {
var tmp = DIV();
tmp.innerHTML = data['data']['pagination'];
tmp.innerHTML = paginationdata;
swapDOM(self.id, tmp.firstChild);
// Run the pagination js to make it live
......@@ -245,7 +254,7 @@ var Paginator = function(id, datatable, heading, script, extradata) {
}
};
this.init(id, datatable, heading, script, extradata);
this.init(id, list, heading, script, extradata);
};
/**
......
This source diff could not be displayed because it is too large. You can view the blob instead.
......@@ -3,32 +3,35 @@
jQuery(function($) {
"use strict";
var paginatorData = window.paginatorData,
requesturl = 'indexin.json.php';
function markread(e, self, paginatorData) {
var checked = $(self).closest('.notification-parent').find('.js-notifications .control.unread input:checked'),
i,
paginator = {};
requestdata = {};
if(checked.length < 1){
//@todo maybe tell the user they need soemthign valid cvhecked
//@todo maybe tell the user they need something valid checked
return; //no valid items selected
}
for (i = 0; i < checked.length; i++) {
paginator[checked[i].name] = 1;
requestdata[checked[i].name] = 1;
}
paginator['markasread'] = 1;
requestdata['markasread'] = 1;
if (paginatorData) {
for (page in paginatorData.params) {
if(paginatorData.params.hasOwnProperty(page)){
paginator[page] = paginatorData.params[page];
requestdata[page] = paginatorData.params[page];
}
}
}
sendjsonrequest('indexin.json.php', paginator, 'GET', function (data) {
sendjsonrequest(requesturl, requestdata, 'GET', function (data) {
updateUnread(data, false);
});
}
......@@ -37,31 +40,30 @@ jQuery(function($) {
var checked = $(self).closest('.notification-parent').find('.js-notifications .control input:checked'),
i,
paginator = {};
requestdata = {};
if(checked.length < 1){
//@todo maybe tell the user they need something valid checked
//@todo tell the user they need something valid checked
return; //no valid items selected
}
for (i = 0; i < checked.length; i++) {
paginator[checked[i].name] = 0;
requestdata[checked[i].name] = 0;
}
paginator['delete'] = 1;
requestdata['delete'] = 1;
if (paginatorData) {
for (page in paginatorData.params) {
if(paginatorData.params.hasOwnProperty(page)){
paginator[page] = paginatorData.params[page];
requestdata[page] = paginatorData.params[page];
}
}
}
sendjsonrequest('indexin.json.php', paginator, 'GET', function (data) {
// paginator.updateResults(data);
sendjsonrequest(requesturl, requestdata, 'GET', function (data) {
updateUnread(data, false);
window.paginator.updateResults(data);
});
}
......@@ -70,29 +72,28 @@ jQuery(function($) {
var checked = $(self).find('.control.unread input.tocheck'),
item = self,
i,
paginator = {};
requestdata = {};
if(checked.length < 1){
return; //no valid items selected
}
for (i = 0; i < checked.length; i++) {
paginator[checked[i].name] = 1;
requestdata[checked[i].name] = 1;
}
paginator['table'] = $(self).find('a[data-table]').attr('data-table');
paginator['readone'] = $(self).find('a[data-id]').attr('data-id');
requestdata['list'] = $(self).find('a[data-table]').attr('data-table');
requestdata['readone'] = $(self).find('a[data-id]').attr('data-id');
if (paginatorData) {
for (page in paginatorData.params) {
if(paginatorData.params.hasOwnProperty(page)){
paginator[page] = paginatorData.params[page];
requestdata[page] = paginatorData.params[page];
}
}
}
sendjsonrequest('indexin.json.php', paginator, 'GET', function (data) {
sendjsonrequest(requesturl, requestdata, 'GET', function (data) {
updateUnread(data, item);
});
}
......@@ -119,44 +120,22 @@ jQuery(function($) {
}
}
function changeactivitytype() {
var delallform = document.forms['delete_all_notifications'];
delallform.elements['type'].value = this.options[this.selectedIndex].value;
var params = {'type': this.options[this.selectedIndex].value};
sendjsonrequest('indexin.json.php', params, 'GET', function(data) {
paginator.updateResults(data);
});
}
// We want the paginator to tell us when a page gets changed.
// @todo: remember checked/unchecked state when changing pages
function PaginatorData() {
var self = this;
var params = {};
this.pageChanged = function(data) {
self.params = {
'offset': data.offset,
'limit': data.limit,
'type': data.type
}
}
paginatorProxy.addObserver(self);
connect(self, 'pagechanged', self.pageChanged);
}
if($('.notification-list').attr('[data-paginator]') !== undefined){
var paginator;
var paginatorData = new PaginatorData();
function changeactivitytype(e) {
var delallform = document.forms['delete_all_notifications'],
params,
query = $(e.currentTarget).val();
addLoadEvent(function () {
paginator = $('.notification-list').attr('[data-paginator]');
delallform.elements['type'].value = query;
params = {'type': query};
connect('notifications_type', 'onchange', changeactivitytype);
sendjsonrequest(requesturl, params, 'GET', function(data) {
window.paginator.updateResults(data);
});
}
if ($('[data-requesturl]').length > 0) {
requesturl = $('[data-requesturl]').attr('data-requesturl');
}
$('.notification .control-wrapper').on('click', function(e) {
e.stopPropagation();
......@@ -203,4 +182,8 @@ jQuery(function($) {
markthisread(e, this, paginatorData);
});
$('.js-notifications-type').on('change', function(e){
changeactivitytype(e);
});
});
......@@ -10,7 +10,7 @@
</span>
</label>
<a class="collapsed" href="#notification-{$item->table}-{$item->id}" data-id="{$item->id}" data-table="{$item->table}" data-toggle="collapse" aria-expanded="1" aria-controls="notification-{$item->table}-{$item->id}">
<a class="collapsed" href="#notification-{$item->table}-{$item->id}" data-id="{$item->id}" data-list="{$item->table}" data-toggle="collapse" aria-expanded="1" aria-controls="notification-{$item->table}-{$item->id}">
<span class="details-group">
{if $item->read && $item->type == 'usermessage'}
<span class="icon icon-envelope type-icon prxl plxl"></span>
......
......@@ -8,7 +8,7 @@
<span class="sr-only">{str tag='select' section='mahara'}</span>
</span>
</label>
<a class="collapsed" href="#notification-{$item->id}" data-toggle="collapse" aria-expanded="1" aria-controls="notification-{$item->id}">
<a class="collapsed" href="#notification-{$item->id}" data-toggle="collapse" aria-expanded="1" aria-controls="notification-{$item->id}" data-list="{$item->table}">
<span class="details-group">
{if $item->read && $item->type == 'usermessage'}
<span class="icon icon-envelope type-icon prxl plxl"></span><span class="sr-only">{$item->strtype} - {str tag='read' section='activity'}</span>
......@@ -90,26 +90,23 @@
<p>{$item->message|safe}</p>
</div>
{/if}
{if ($item->canreply || $item->canreplyall)}
<div class="actions panel-footer mbl">
<div class="url">
{if $item->url}
<a class="action" href="{$WWWROOT}{$item->url}">
{if $item->canreply && !$item->self}
<a class="action" href="{$WWWROOT}artefact/multirecipientnotification/sendmessage.php?id={$item->fromusr}{if !$item->startnewthread}&replyto={$item->id}{/if}&returnto=outbox">
<span class="icon icon-reply"></span>
<span class="icon icon-arrow-right"></span>
{if $item->urltext}
{$item->urltext}
{else}
<span class="sr-only">{str tag="more..."}</span>
{/if}
{str tag=reply section=artefact.multirecipientnotification}
</a>
{/if}
{if $item->return}
<a class="action" href="{$WWWROOT}{$item->return}">
<span class="icon icon-reply-all"></span> {$item->returnoutput}
{if $item->canreplyall}
<a class="action" href="{$WWWROOT}artefact/multirecipientnotification/sendmessage.php?replyto={$item->id}&returnto=outbox">
<span class="icon icon-reply-all"></span> {str tag=replyall section=artefact.multirecipientnotification}
</a>
{/if}
</div>
</div>
{/if}
</div>
</div>
{/foreach}
......
{include file="header.tpl"}
<a title="{str section='artefact.multirecipientnotification' tag='composemessagedesc'}" class="btn-with-heading btn-lg btn btn-default" href="{$WWWROOT}artefact/multirecipientnotification/sendmessage.php">
<span class="icon icon-edit"></span>
{str section='artefact.multirecipientnotification' tag='composemessage'}
......@@ -8,7 +7,7 @@