View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 961 | Composr | core_notifications | public | 2013-01-03 11:17 | 2013-06-23 14:13 |
| Reporter | Chris Graham | Assigned To | Chris Graham | ||
| Priority | normal | Severity | feature | ||
| Status | resolved | Resolution | fixed | ||
| Summary | 961: Smarter topic notifications | ||||
| Description | Ability for members to say that they won't receive a topic notification for any topic that they currently have in an 'unread' state. | ||||
| Additional Information | The effect is they get a notification for the first reply, then no more until they've read that. It stops them getting bombarded. | ||||
| Tags | No tags attached. | ||||
| Attach Tags | |||||
| Attached Files | notifications.php (760 bytes)
<?php /*
ocPortal
Copyright (c) ocProducts, 2004-2013
See text/EN/licence.txt for full licencing information.
NOTE TO PROGRAMMERS:
Do not edit this file. If you need to make changes, save your changed file to the appropriate *_custom folder
**** If you ignore this advice, then your website upgrades (e.g. for bug fixes) will likely kill your changes ****
*/
/**
* @license http://opensource.org/licenses/cpal_1.0 Common Public Attribution License
* @copyright ocProducts Ltd
* @package ocf_forum
*/
class Hook_ocf_cpf_filter_notifications
{
/**
* Find which special CPF's to enable.
*
* @return array A list of CPF's to enable
*/
function to_enable()
{
$cpf=array('smart_topic_notification'=>1);
return $cpf;
}
}
| ||||
| Time estimation (hours) | 3 | ||||
| Sponsorship open | |||||
|
|
Thanks for the sponsorship guys. This has been completed and is currently pending a code review. That should happen within the next few days :). |
|
|
Excellent - look forward to adding it Chris |
|
|
This is now implemented in a v9 branch (smarter_topic_notifications). A zip of the changed files is attached. Credits have been charged. The issue is left open as we haven't merged into v10 yet. I've monitored it as a reminder to myself that we need to do that. The setting to enable the new functionality can be found by a member on their notification settings under their account. You may not want to upload lang/EN/cns.ini because it may compete with other changes made to that file. Other feature development of hot-fixes are very likely to touch it. Therefore you can manually paste this onto the bottom of your existing file: SPECIAL_CPF__cms_smart_topic_notification=Smart topic notification SMART_TOPIC_NOTIFICATION=Smart topic notification DESCRIPTION_SMART_TOPIC_NOTIFICATION=Only send one new-post notification for a topic up until you've viewed it again. There is a theme change to global.css. In your theme you need to change: .simple_neat_checkbox span { float: {!en_right}; margin-{!en_left}: 2em; margin-top: -1.5em; } to: .simple_neat_checkbox span { float: {!en_right}; padding-{!en_left}: 2em; margin-top: -1.5em; width: 100%; {$BETA_CSS_PROPERTY,box-sizing: border-box;} } Finally, once the files are uploaded, you need to run http://baseurl/data_custom/execute_temp.php. This will install the new member notifications setting. Thanks :). |
|
|
Hi Chris, There is something not quite right with this. Since turning on the smart topic notifications I have not had any notifications come through. I checked and there are some posts that should of generated a notification but didn't. I then used the su to test the whole thing out and it worked as expected, ie I replied to a topic that I started with the su user, email came through, replied again, no email, read the topic as my user, replied again and email came through. So I am not too sure as to what is going on. Take this topic for example http://vwgolfmk1.org.uk/forum/index.php?page=topicview&id=website-issues_2%2Fsite-closure-for-some&redirected=1#post_1264215 I did not receive any notifications for this one at all and I enabled the smart topic notifications before reopening the site. Any ideas Chris ? Cheers Ade |
|
|
There has just been a reply to the same topic and no notification went out. I am going to turn the smart topic notification off and see if they start again . |
|
|
Hi, The system works via the has-read tracking (i.e. there is no new table that is going to be used to start tracking what notifications went out already). So is it possible there was already an unread reply on that topic before you turned on the feature? If that was the case, the last notification you had on it would still count, because it knows you haven't read that topic since. |
|
|
Another way of thinking about it is this... If there are 2 or more posts since the last read time of a topic for member X, then member X does not receive a notification. (Notifications are sent after the post is added, so that's why it is '2 or more' rather than '1 or more' - it counts the post being notified as one of those unread posts) (This is how the code actually works) |
|
|
Ok, I understand that logic. However the example I have I read the previous notifications and the next post was the only outstanding one. But I did not get a notification. I have switched it off now because I didn't get a single notification with it on - which is very unusual. Can the read counts be reset ? Cheers Ade |
|
|
You're right, there was a nasty bug. It was counting new posts across the forum, not constraining to the topic. Sorry about that. The logic of this confuses me a little too, the way it's interpreting data for a different purpose (not that the implementation was lazy - it's just a bit hard to get your head around I think). I have reuploaded, sources/hooks/systems/notifications/cns_topic.php is the changed file. |
|
|
Excellent - thanks for fixing it Chris. I have re enabled it and will let you know how it goes. Cheer Ade |
|
|
Merged into v10 |
|
|
Hi Chris, We have been using this successfully. Is there an easy tweak to allow this to be set for new users that sign up to the forum, I searched but I could not find an admin setting to make it the default. Cheers Ade |
|
|
Okay, this isn't really supported, but it can be done via getting into the guts of it, which is driven by a CPF. I will assume this is for new members, not switching it over for all existing members. (For existing members you'd need to run an SQL query on the f_member_custom_fields table, for the field_<cpfid> column, whatever your <cpfid> is) I have attached a sources/hooks/systems/cns_cpf_filter/notifications.php file. This will make the "Smart topic notification" CPF editable, from Admin Zone > Tools > Members > Custom Profile Fields > Edit Custom Profile Field. Edit it, change the default value to "1". |
|
|
Oh and you probably want to delete sources/hooks/systems/cns_cpf_filter/notifications.php when you're done, else the CPF will be editable in member profiles as well as the member's notifications tab. For cms v10 I am coding up a special developer switch that makes all the CPFs editable. It still won't really be supported as an option but will be achievable with a little digging. |
|
|
Thanks for that Chris - works a treat, been away this week, so only just getting around to trying it out. (I only wanted it to work for new users). I am not sure whether this is related to smarter topic notifications or not, but I am starting to get quite a lot of complaints that people are receiving lots of notifications after joining our site. In most cases people seem unable to grasp the fact that they need to go and click the advanced button to set the finer grained settings - I think they find it non intuitive if the front page does not show a ticked boxed. The last complaint was from a rossco-pcoltrane who joined last Wednesday. When I checked his advanced notifications for Topic positing I found some of the classified areas ticked. The strange thing is that I created a brand new user myself and none of the advanced boxes were ticked, so something weird is happening, but I just cannot see it. I dont know if the fact that newbies cannot post in classifieds until they reach a certain points level is having anything to do with it. It appears that these notifications have been going on for a while but people have just put up with them. If you want me to raise this as a potential bug separate to this then I am happy to do that. Cheers Ade |
|
|
Right, sorry for the delay on the reply... I can't really speak of the implied "random notification settings" above. All I can think is he might have set it in some kind of confusion. I checked for a relatively new member on your site, and they had no forum/topic notifications set. If you can find a reproduction case, please do report. As for the point of confusion, yes I can see this. I just did some research and I found HTML5 has made standard something Microsoft invented for IE, indeterminate checkboxes. I was pleased and surprised to find that all the main browsers now have it. I have implemented this in the notification system, showing an indeterminate state for a notification code/contact method when there are category-overridden notification settings for that code/contact method and the checkbox is not ticked. It sounds more complex than it is. In simpler terms: the checkboxes will show as half-ticked if they would have previously shown as unticked yet have overrides buried in there. I have deployed for you. |
|
|
No worries Chris - I hear you have been ill, hope that you are feeling better !! I think you have the issue nailed and it is only coming from those using mobile devices. The problem is that on mobile devices you cannot scroll the advanced window as the background window scrolls instead - is there an easy way to correct this fro mobile? perhaps open a full page instead of a pop up window? We have more mobile device users than anything else. The indeterminate checkbox works well on desktop/laptop browsers but not on mobile, they just put up a unchecked box as before, hopefully the browsers will catch up. Thanks for doing the update. |
|
|
On v10 we are changing overlays to not have an internal scroll, due to this silly deficiency in the mobile browsers ;). We'll include that in the next patch also. I just put up a workaround for you that changes it for ios/android to scroll with the main window. Apparently also you can do a "two finger scroll" to make internal frames scroll independently,. Regarding indeterminate state checkboxes, I see you are correct. It seems the widget libraries on ios and android do not support it. Webkit itself does. Add this to your global.css of your theme as a workaround... input[type="checkbox"]:indeterminate { background: {$GET,native_ui_selected_background}; } |
|
|
Hi Chris, Added that to global.css - gives a shaded box now, which is better than it was before - did you expect it to be a minus sign ? Two finger scroll still scrolls the back screen :-( I did try several finger combinations ! I even tried Chrome on my iPhone/ipad and it was the same :-) Did you up load the workaround? I have just tried and I cannot scroll the pop up window Cheers Ade |
|
|
Just a shaded box, it was the best I could think to do without getting really device-specific, into the world of "shadow DOM". I think it is possible two-finger scroll was dropped. I'm not 100% sure, as it was an obscure feature originally so discussion on it is confused anyway. |
