[ticket/11103] Make sure notifications are marked read when clicking them

How do we do this? If an item is unread, the URL to view that item will
be the URL to mark it as read (index.php?mark_notification=$id). When the
URL is visited it marks the item as read and redirects them to the correct
URL for the item.

If the item is read, the URL is directly to the item.

Prettify the html output

PHPBB-11103
This commit is contained in:
Nathan Guse 2012-10-14 12:35:35 -05:00
parent 716635c834
commit a48f090338
7 changed files with 63 additions and 35 deletions

View file

@ -165,7 +165,7 @@ class phpbb_notification_manager
} }
$load_special[$row['item_type']] = array_merge($load_special[$row['item_type']], $notification->get_load_special()); $load_special[$row['item_type']] = array_merge($load_special[$row['item_type']], $notification->get_load_special());
$notifications[] = $notification; $notifications[$row['notification_id']] = $notification;
} }
$this->load_users($user_ids); $this->load_users($user_ids);

View file

@ -138,7 +138,7 @@ abstract class phpbb_notification_type_base implements phpbb_notification_type_i
'UNREAD' => $this->unread, 'UNREAD' => $this->unread,
'U_MARK_READ' => append_sid($this->phpbb_root_path . 'index.' . $this->php_ext, 'mark_notification[]=' . $this->notification_id), 'U_MARK_READ' => append_sid($this->phpbb_root_path . 'index.' . $this->php_ext, 'mark_notification=' . $this->notification_id),
); );
} }
@ -148,7 +148,7 @@ abstract class phpbb_notification_type_base implements phpbb_notification_type_i
* @param bool $return True to return a string containing the SQL code to update this item, False to execute it (Default: False) * @param bool $return True to return a string containing the SQL code to update this item, False to execute it (Default: False)
* @return string * @return string
*/ */
public function mark_read($return = true) public function mark_read($return = false)
{ {
return $this->mark(false, $return); return $this->mark(false, $return);
} }
@ -159,7 +159,7 @@ abstract class phpbb_notification_type_base implements phpbb_notification_type_i
* @param bool $return True to return a string containing the SQL code to update this item, False to execute it (Default: False) * @param bool $return True to return a string containing the SQL code to update this item, False to execute it (Default: False)
* @return string * @return string
*/ */
public function mark_unread($return = true) public function mark_unread($return = false)
{ {
return $this->mark(true, $return); return $this->mark(true, $return);
} }
@ -352,11 +352,11 @@ abstract class phpbb_notification_type_base implements phpbb_notification_type_i
$this->unread = (bool) $unread; $this->unread = (bool) $unread;
$where = array( $where = array(
'item_type = ' . $this->db->sql_escape($this->item_type), "item_type = '" . $this->db->sql_escape($this->item_type) . "'",
'item_id = ' . (int) $this->item_id, 'item_id = ' . (int) $this->item_id,
'user_id = ' . (int) $this->user_id, 'user_id = ' . (int) $this->user_id,
); );
$where = implode(' AND ' . $where); $where = implode(' AND ', $where);
if ($return) if ($return)
{ {
@ -364,7 +364,7 @@ abstract class phpbb_notification_type_base implements phpbb_notification_type_i
} }
$sql = 'UPDATE ' . NOTIFICATIONS_TABLE . ' $sql = 'UPDATE ' . NOTIFICATIONS_TABLE . '
SET unread = ' . $this->unread . ' SET unread = ' . (int) $this->unread . '
WHERE ' . $where; WHERE ' . $where;
$this->db->sql_query($sql); $this->db->sql_query($sql);
} }

View file

@ -23,6 +23,23 @@ $user->session_begin();
$auth->acl($user->data); $auth->acl($user->data);
$user->setup(); $user->setup();
// Mark notifications read
if (($mark_notification = request_var('mark_notification', 0)))
{
$notification = $phpbb_notifications->load_notifications(array(
'notification_id' => $mark_notification
));
if (isset($notification['notifications'][$mark_notification]))
{
$notification = $notification['notifications'][$mark_notification];
$notification->mark_read();
redirect($notification->get_url());
}
}
// Handle the display of extension front pages // Handle the display of extension front pages
if ($ext = $request->variable('ext', '')) if ($ext = $request->variable('ext', ''))
{ {
@ -56,13 +73,6 @@ if ($ext = $request->variable('ext', ''))
exit_handler(); exit_handler();
} }
// Mark notifications read
$mark_notifications = request_var('mark_notification', array(0));
if (!empty($mark_notifications))
{
$phpbb_notifications->mark_notifications_read_by_id($mark_notifications);
}
include($phpbb_root_path . 'includes/functions_display.' . $phpEx); include($phpbb_root_path . 'includes/functions_display.' . $phpEx);
$user->add_lang('viewforum'); $user->add_lang('viewforum');

View file

@ -395,7 +395,7 @@ $lang = array_merge($lang, array(
'NOTIFICATION_POST' => '%1$s replied to the topic "%2$s".', 'NOTIFICATION_POST' => '%1$s replied to the topic "%2$s".',
'NOTIFICATION_POST_APPROVED' => 'Your post was approved "%2$s".', 'NOTIFICATION_POST_APPROVED' => 'Your post was approved "%2$s".',
'NOTIFICATION_POST_DISAPPROVED' => 'Your post "%1$s" was disapproved for reason: "%2$s".', 'NOTIFICATION_POST_DISAPPROVED' => 'Your post "%1$s" was disapproved for reason: "%2$s".',
'NOTIFICATION_POST_IN_QUEUE' => 'A new post titled "%2$s" was posted by "%1$s" and needs approval.', 'NOTIFICATION_POST_IN_QUEUE' => 'A new post titled "%2$s" was posted by %1$s and needs approval.',
'NOTIFICATION_QUOTE' => '%1$s quoted you in the post "%2$s".', 'NOTIFICATION_QUOTE' => '%1$s quoted you in the post "%2$s".',
'NOTIFICATION_REPORT_PM' => '%1$s reported a Private Message "%2$s" for reason: "%3$s".', 'NOTIFICATION_REPORT_PM' => '%1$s reported a Private Message "%2$s" for reason: "%3$s".',
'NOTIFICATION_REPORT_POST' => '%1$s reported a post "%2$s" for reason: "%3$s".', 'NOTIFICATION_REPORT_POST' => '%1$s reported a post "%2$s" for reason: "%3$s".',
@ -403,7 +403,7 @@ $lang = array_merge($lang, array(
'NOTIFICATION_TOPIC' => '%1$s posted a new topic "%2$s" in the forum "%3$s".', 'NOTIFICATION_TOPIC' => '%1$s posted a new topic "%2$s" in the forum "%3$s".',
'NOTIFICATION_TOPIC_APPROVED' => 'Your topic "%2$s" in the forum "%3$s" was approved.', 'NOTIFICATION_TOPIC_APPROVED' => 'Your topic "%2$s" in the forum "%3$s" was approved.',
'NOTIFICATION_TOPIC_DISAPPROVED' => 'Your topic "%1$s" was disapproved for reason: "%2$s".', 'NOTIFICATION_TOPIC_DISAPPROVED' => 'Your topic "%1$s" was disapproved for reason: "%2$s".',
'NOTIFICATION_TOPIC_IN_QUEUE' => 'A new topic titled "%2$s" was posted by "%1$s" and needs approval.', 'NOTIFICATION_TOPIC_IN_QUEUE' => 'A new topic titled "%2$s" was posted by %1$s and needs approval.',
'NOTIFY_ADMIN' => 'Please notify the board administrator or webmaster.', 'NOTIFY_ADMIN' => 'Please notify the board administrator or webmaster.',
'NOTIFY_ADMIN_EMAIL' => 'Please notify the board administrator or webmaster: <a href="mailto:%1$s">%1$s</a>', 'NOTIFY_ADMIN_EMAIL' => 'Please notify the board administrator or webmaster: <a href="mailto:%1$s">%1$s</a>',
'NO_ACCESS_ATTACHMENT' => 'You are not allowed to access this file.', 'NO_ACCESS_ATTACHMENT' => 'You are not allowed to access this file.',

View file

@ -137,14 +137,15 @@
<ul class="topiclist forums"> <ul class="topiclist forums">
<!-- BEGIN notifications --> <!-- BEGIN notifications -->
<li class="row<!-- IF notifications.UNREAD --> bg2<!-- ENDIF -->"> <li class="row<!-- IF notifications.UNREAD --> bg2<!-- ENDIF -->">
<!-- IF notifications.URL --><a href="{notifications.URL}"><!-- ENDIF --> <!-- IF notifications.URL or notifications.U_MARK_READ --><a href="<!-- IF notifications.UNREAD -->{notifications.U_MARK_READ}<!-- ELSE -->{notifications.URL}<!-- ENDIF -->"><!-- ENDIF -->
{notifications.AVATAR} <span>
<div> {notifications.AVATAR}
{notifications.FORMATTED_TITLE}<br /> <span class="notification_title">
{notifications.TIME} {notifications.FORMATTED_TITLE}
</div> <div class="notification_time">{notifications.TIME}</div>
</span>
</span>
<!-- IF notifications.URL --></a><!-- ENDIF --> <!-- IF notifications.URL --></a><!-- ENDIF -->
<!-- IF notifications.UNREAD --><a href="{notifications.U_MARK_READ}" title="{L_MARK_READ}">{L_MARK_READ}</a><!-- ENDIF -->
</li> </li>
<!-- END notifications --> <!-- END notifications -->
</ul> </ul>

View file

@ -77,14 +77,17 @@
<li class="row<!-- IF notification_list.UNREAD --> bg3<!-- ELSE --><!-- IF notification_list.S_ROW_COUNT is odd --> bg1<!-- ELSE --> bg2<!-- ENDIF --><!-- ENDIF -->"> <li class="row<!-- IF notification_list.UNREAD --> bg3<!-- ELSE --><!-- IF notification_list.S_ROW_COUNT is odd --> bg1<!-- ELSE --> bg2<!-- ENDIF --><!-- ENDIF -->">
<dl> <dl>
<dt> <dt>
<!-- IF notification_list.URL --><a href="{notification_list.URL}"><!-- ENDIF --> <!-- IF notification_list.URL or notification_list.U_MARK_READ --><a href="<!-- IF notification_list.UNREAD -->{notification_list.U_MARK_READ}<!-- ELSE -->{notification_list.URL}<!-- ENDIF -->"><!-- ENDIF -->
{notification_list.AVATAR} <span>
<div> {notification_list.AVATAR}
{notification_list.FORMATTED_TITLE}<br /> <span class="notification_title">
{notification_list.TIME} {notification_list.FORMATTED_TITLE}
</div> <div class="notification_time">{notification_list.TIME}</div>
</span>
</span>
<!-- IF notification_list.URL --></a><!-- ENDIF --> <!-- IF notification_list.URL --></a><!-- ENDIF -->
</dt> </dt>
<dd class="mark"><!-- IF notification_list.UNREAD --><input type="checkbox" name="mark[]" value="{notification_list.NOTIFICATION_ID}" /> <dfn>{L_MARK_READ}</dfn><!-- ENDIF --></dd> <dd class="mark"><!-- IF notification_list.UNREAD --><input type="checkbox" name="mark[]" value="{notification_list.NOTIFICATION_ID}" /> <dfn>{L_MARK_READ}</dfn><!-- ENDIF --></dd>
</dl> </dl>
</li> </li>

View file

@ -680,15 +680,29 @@ p.rules a {
#notification_list ul li { #notification_list ul li {
padding: 10px; padding: 10px;
width: 310px; width: 310px;
line-height: 1.5em;
} }
.notification_list ul li a { .notification_list ul li a span img {
text-decoration: none;
}
.notification_list ul li img {
float: left; float: left;
padding: 0 10px 10px 0;
max-width: 50px; max-width: 50px;
max-height: 50px; max-height: 50px;
} }
.notification_list ul li span .notification_title {
float: left;
width: 240px;
margin: 0 0 0 5px;
word-wrap: break-word;
position: relative;
top: -0.2em;
}
.notification_list ul li dl dt span .notification_title {
width: auto;
padding: 10px 0 0 0;
}
.notification_time {
margin-top: 0.5em;
}