From a48f09033810148fd9b2d5a0b6a683f14ac73a6a Mon Sep 17 00:00:00 2001 From: Nathan Guse Date: Sun, 14 Oct 2012 12:35:35 -0500 Subject: [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 --- phpBB/includes/notification/manager.php | 2 +- phpBB/includes/notification/type/base.php | 12 +++++----- phpBB/index.php | 24 ++++++++++++++------ phpBB/language/en/common.php | 4 ++-- .../styles/prosilver/template/overall_header.html | 15 +++++++------ .../prosilver/template/ucp_notifications.html | 15 ++++++++----- phpBB/styles/prosilver/theme/common.css | 26 +++++++++++++++++----- 7 files changed, 63 insertions(+), 35 deletions(-) diff --git a/phpBB/includes/notification/manager.php b/phpBB/includes/notification/manager.php index fc9b48c624..03776de2b4 100644 --- a/phpBB/includes/notification/manager.php +++ b/phpBB/includes/notification/manager.php @@ -165,7 +165,7 @@ class phpbb_notification_manager } $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); diff --git a/phpBB/includes/notification/type/base.php b/phpBB/includes/notification/type/base.php index e8959d1352..9ef0e71009 100644 --- a/phpBB/includes/notification/type/base.php +++ b/phpBB/includes/notification/type/base.php @@ -138,7 +138,7 @@ abstract class phpbb_notification_type_base implements phpbb_notification_type_i '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) * @return string */ - public function mark_read($return = true) + public function mark_read($return = false) { 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) * @return string */ - public function mark_unread($return = true) + public function mark_unread($return = false) { return $this->mark(true, $return); } @@ -352,11 +352,11 @@ abstract class phpbb_notification_type_base implements phpbb_notification_type_i $this->unread = (bool) $unread; $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, 'user_id = ' . (int) $this->user_id, ); - $where = implode(' AND ' . $where); + $where = implode(' AND ', $where); if ($return) { @@ -364,7 +364,7 @@ abstract class phpbb_notification_type_base implements phpbb_notification_type_i } $sql = 'UPDATE ' . NOTIFICATIONS_TABLE . ' - SET unread = ' . $this->unread . ' + SET unread = ' . (int) $this->unread . ' WHERE ' . $where; $this->db->sql_query($sql); } diff --git a/phpBB/index.php b/phpBB/index.php index af0441f5e3..7f85879d23 100644 --- a/phpBB/index.php +++ b/phpBB/index.php @@ -23,6 +23,23 @@ $user->session_begin(); $auth->acl($user->data); $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 if ($ext = $request->variable('ext', '')) { @@ -56,13 +73,6 @@ if ($ext = $request->variable('ext', '')) 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); $user->add_lang('viewforum'); diff --git a/phpBB/language/en/common.php b/phpBB/language/en/common.php index 2f9f646cd3..8980c274ef 100644 --- a/phpBB/language/en/common.php +++ b/phpBB/language/en/common.php @@ -395,7 +395,7 @@ $lang = array_merge($lang, array( 'NOTIFICATION_POST' => '%1$s replied to the topic "%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_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_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".', @@ -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_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_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_EMAIL' => 'Please notify the board administrator or webmaster: %1$s', 'NO_ACCESS_ATTACHMENT' => 'You are not allowed to access this file.', diff --git a/phpBB/styles/prosilver/template/overall_header.html b/phpBB/styles/prosilver/template/overall_header.html index e00ad555ff..20b98c6f8c 100644 --- a/phpBB/styles/prosilver/template/overall_header.html +++ b/phpBB/styles/prosilver/template/overall_header.html @@ -137,14 +137,15 @@ diff --git a/phpBB/styles/prosilver/template/ucp_notifications.html b/phpBB/styles/prosilver/template/ucp_notifications.html index 94e8432508..f71d8698b9 100644 --- a/phpBB/styles/prosilver/template/ucp_notifications.html +++ b/phpBB/styles/prosilver/template/ucp_notifications.html @@ -77,14 +77,17 @@
  • - - {notification_list.AVATAR} -
    - {notification_list.FORMATTED_TITLE}
    - {notification_list.TIME} -
    +     + + {notification_list.AVATAR} + + {notification_list.FORMATTED_TITLE} +
    {notification_list.TIME}
    +     +
    +
    {L_MARK_READ}
    • diff --git a/phpBB/styles/prosilver/theme/common.css b/phpBB/styles/prosilver/theme/common.css index 3d8d640598..c82fa5b7c8 100644 --- a/phpBB/styles/prosilver/theme/common.css +++ b/phpBB/styles/prosilver/theme/common.css @@ -680,15 +680,29 @@ p.rules a { #notification_list ul li { padding: 10px; width: 310px; + line-height: 1.5em; } -.notification_list ul li a { - text-decoration: none; -} - -.notification_list ul li img { +.notification_list ul li a span img { float: left; - padding: 0 10px 10px 0; max-width: 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; +} -- cgit v1.2.1