Wedge

Public area => The Pub => Plugins => Topic started by: Dragooon on June 10th, 2012, 08:14 PM

Title: [Plugin] Notifications system (1.0)
Post by: Dragooon on June 10th, 2012, 08:14 PM
I thought I'd announce this before I go, anyway, I've been working on a full fledged notifications of Wedge which is meant to serve as a replacement for all the existing notification with a fully pluggable notifications system.

The system plugin by default provides no notifications, but various plugin that will come with it that provides notifications for various actions. Currently implemented features:

- Allow plugins to easily issue notifications without much hassle
- Provide the user an easy interface to view those and mark as read, currently it's displaying 5 latest unread notifications on the sidebar with an option to view all notifications. The notifications are marked as read automatically as soon as they're clicked.
- Allow individual notifiers to be disabled per user, i.e. an user can choose to not receive notification for topic replies etc.
- Allow notifiers to send e-mails for notifications, which can be disabled on a per notifier basis by the user
- Allow e-mails to be received instantly, daily or weekly (not implemented yet)

The aim of the system is to provide an easy API for plugins to notify users without having to re-implement all the features again. Plus it provides a centralized system for receiving/viewing notifications.

Notification extensions
These plugins use the notifications core to issue notifications in various events, the notifications core itself is useless. These can be used as a sample for notification extensions

ID: Dragooon:WeNotif-TopicReply

Notifies the author of a reply to their topic

ID: Dragooon:WeNotif-Quote

Notifies the author if someone has quoted their post

I'll be working on replacing the existing notifications next, by providing plugins that mimic the funcionality but work full and well with the notification system.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dismal Shadow on June 11th, 2012, 05:33 AM
Nice & elegant. :)

Does this have any impact the server?
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on June 11th, 2012, 05:14 PM
Quote from Nathan Sparrow on June 11th, 2012, 05:33 AM
Nice & elegant. :)

Does this have any impact the server?
It has some impact due to additional queries, but I'm trying to be very careful about minimizing any extra load and optimizing it as much as I can, plus I'm also utilizing caching wherever I can.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on January 13th, 2013, 01:55 PM
Just wanted to give this topic a bump, I'm still working on it (whenever I can, hardly getting time these days due to exams, then SMF4Mobile and all).  I wanted some opinions on what you guys think about the core plugin and all.

Some terminology, so that people don't get confused later.
Notifier: This is the object that actually issues notifications. For example, TopicReply is a notifier that issues notifications whenever a replied is posted to a person's topic.
Notification: Pretty obvious, individual messages sent to people to tell what's going on
NotifSubscriber: I probably need a better name for this, short for Notification Subscriber, this is another object which handles individual types of subscription. For example, there can be a subscriber for Board which handles sending out subscription notifications for any new topic in a specific board.

Basically, currently the core of notifications is divided into two. First is handling the notifications themselves, it allows plugins to have notifier and issue notifications to user. It also handles disabling of specific notifiers, individual preferences (if any) etc. in a single profile page. The second part handles subscriptions, basically anything that the user isn't directly linked to but wants to get an update of (same as current Wedge subscriptions). Subscription part is build on top of the notification core and is optional, it obviously aims at replacing the current mess of a subscription Wedge (and, by extension, SMF) has with a simpler UI and more modularity. My aim is to make the subscription option themselves simpler, an user can subscribe to a specific topic and see his or her subscription info from their profile, and remove them if they want. The advance notifying stuff (stuff like e-mail once every week etc.) gets moved to the notifications core. This way, other notifier can also, by default, take advantage of having some nice e-mailing options.

Currently subscriptions are divided into two parts, so one needs to code these two in order to add a new subscriptions . First is their subscriber, and second is their notifier. Subscriber handles all the subscription part of the equation, i.e. validating of topic (for example) they're trying to subscribe to  and issuing subscription notifications themselves whenever required (like when a new topic is posted). Notifier handles the notifications of the subscription, i.e. what to actually say when a notification is issued, what to do when there are multiple unread notifications of the same type, and notifier specific preferences (this is valid to other types of notifier as well and not only subscriptions).

To give an elaborate example for a subscription, if someone needs to create a plugin where anyone is notified when a specific user is quoted in a post (probably a stupid idea, but it gets the point across). A member will subscribe to another member to receive the notifications. One will firstly code the notifier itself. The notifier will have a few functions, firstly to return what the text should say for a specific notification, the e-mail text etc. and what to do if there are multiple unread notifications of the same type (so that it can say "admin has been quoted in "Re: Test" and 4 other messages"). Then comes the subscriber, which will handle identifying the subscriber itself (for internal purposes) and validating the member when someone is trying to subscribe to them, i.e. whether the member is valid and can be subscribed too or not. The plugin then will hook to post_form (or whichever hook is called after a post has been created),  and check if a member has been quoted, and if there has been a member who's been quoted, it will issue all the subscription notifications. The issuing part is mostly simple, you just need to call a function, say which subscriber you are (memberquote in this case) and for which object are notifications being issued (the member who has been quoted in this case) and pass any arbitrary data (post subject, etc.) that can be useful when displaying notification. The internal function (part of the core plugin, already coded, no need to code anything more here) will handle actually sending out the notifications to whoever has subscribed to that specific member. The plugin will also hook to appropriate places to add the button/UI for providing a link to actually subscribe to the place (the subscription action is handled by the core, but the UI is provided by the plugin which is adding a subscriber since it can be at any number of places).

I already have a couple of examples for simple notifiers, see them in my GH repository.

This can be a fairly bit complex to explain without code examples, but any feedback is appreciated. Is it too complex to extend? Any ideas which can probably make it simpler? Any examples of other systems which already do this and work well? Thanks :)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on January 13th, 2013, 03:52 PM
I think it's kind of hard to judge this without actually trying to make something myself with it.

Mind you, I've been of the opinion that we might need to add something along these lines into the core sometime.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Pandos on January 14th, 2013, 10:43 AM
Love this idea!
In my case we're using an external CMS. Layout works over SSI. So the comments and replys to an article made in the cms will be recognized . Very cool!
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on January 14th, 2013, 11:51 AM
Quote from Arantor on January 13th, 2013, 03:52 PM
I think it's kind of hard to judge this without actually trying to make something myself with it.

Mind you, I've been of the opinion that we might need to add something along these lines into the core sometime.
Fair enough, hopefully you'll be able to make something yourself soon.
Quote from Pandos on January 14th, 2013, 10:43 AM
Love this idea!
In my case we're using an external CMS. Layout works over SSI. So the comments and replys to an article made in the cms will be recognized . Very cool!
Ofcourse, you'll need to code that part :)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on January 20th, 2013, 12:39 AM
Okay, this is the first working example of a subscription plugin: https://github.com/Dragooon/WeNotif-Subs-Topics

It allows one to subscribe to topics, coded in about 30 minutes at 4:30 AM so it's still very rough and a bunch of things are still left to do, but it seems to be working fairly fine. You can see how a subscriber is implemented, this should give a nice example of one.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on January 20th, 2013, 01:32 AM
Shiny :)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on January 20th, 2013, 08:57 AM
Quote from Arantor on January 20th, 2013, 01:32 AM
Shiny :)
Yup :), it's working out okay. The core plugins still need a lot of work (I need to work on it during daytime more, the code needs tidying up), but the basic structure should remain same. The topics extension is more or less complete, I don't think it should need anything more except a couple of things that are todo, what do you think of the code (simplicity wise) itself? It's a complete implementation for topic subscription, any other plugin will have to do similar things to add their own notification subscriptions.
Title: Re: [Plugin] Notifications system (1.0)
Post by: agent47 on January 20th, 2013, 10:02 AM
Tried to give this plugin a test but got this error:
Quote
This plugin cannot be used because:
One or more features required by this plugin are not available. (notification_callback, notification_subscription)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on January 20th, 2013, 03:24 PM
Quote from agent47 on January 20th, 2013, 10:02 AM
Tried to give this plugin a test but got this error:
Quote
This plugin cannot be used because:
One or more features required by this plugin are not available. (notification_callback, notification_subscription)
You also need to enable WeNotif and WeNotif-Subs before being able to enable this plugin. And I seriously don't recommend trying it out just now, it's still very early in development.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on January 20th, 2013, 03:26 PM
Yay for a plugin system that won't let you install plugins without dependencies being met. :)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 10th, 2013, 08:34 AM
Ahah, this is the public topic....reminder: implement the option to automatically subscribe to your threads.

Also, for the others:
Quote
I know the plugin uses the sidebar but I'm not entirely convinced that's workable at present - I would never, ever see my own notifications on the desktop because I have my window sized at around 1000px, gives me enough room for dual pane work, but has the side effect that the sidebar is pushed to the bottom. Makes me wonder about having something up in the header area for the user.
Okay, this is one of the last issues I'd like to work out before giving it a go for installation on a live site for testing, any ideas where this can go? I'm thinking either:
1) Make a big number counter next to the site's title
2) Make a menu entry and display the counter there, which expands to show quick unread notification with the option to show all.
3) Make some sort of floating box?

EDIT: I'd like any suggestion anyone has
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on March 10th, 2013, 04:25 PM
What I'd be inclined to do is move the user info bit out of the sidebar and up into the top right under the search area, and put notifications up there.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 10th, 2013, 04:45 PM
Yeah that'll probably be the best option.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 11th, 2013, 09:01 PM
How's this? The entire notification thing is a popup (similar to the search popup)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on March 11th, 2013, 09:07 PM
I like the overall idea of the popup but the (1) bit needs to be bigger and more prominent and preferably with a label to indicate what it is.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 11th, 2013, 09:20 PM
Yeah, will work on it more tomorrow. I've finished most of other things with the plugin and further extensions, perhaps Nao can install them here in a couple of days (atleast the core and mentions plugin)?
Title: Re: [Plugin] Notifications system (1.0)
Post by: live627 on March 12th, 2013, 12:30 AM
Meh. Move the number, so the title is "Notifications ❶ (show all)".
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on March 12th, 2013, 12:31 AM
But if the number is part of the popup, what do users interact with to get the popup?
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 12th, 2013, 12:48 PM
How about this?
Title: Re: [Plugin] Notifications system (1.0)
Post by: MultiformeIngegno on March 12th, 2013, 12:53 PM
This can be useful also to notify the likes on posts (actually there's no notification of likes). :)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 12th, 2013, 12:55 PM
Quote from MultiformeIngegno on March 12th, 2013, 12:53 PM
This can be useful also to notify the likes on posts (actually there's no notification of likes). :)
Of course, that's one of the notification extensions I've planned.
Posted: March 12th, 2013, 12:54 PM

The idea is to replace the entire notification and subscription system Wedge currently has with this.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 13th, 2013, 08:07 PM
Quote from MultiformeIngegno on March 12th, 2013, 12:53 PM
This can be useful also to notify the likes on posts (actually there's no notification of likes). :)
And done(https://github.com/Dragooon/WeNotif-Likes)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on March 17th, 2013, 04:52 PM
I'd suggest using <h6> for the Notifications title. I'm trying to move all 'small titles' to use this ;) (see search popup.)

Also, a few remarks on the database structure... More specifically the members table.
- I'd rather we leave 'data' at the very end. It needn't be lost in that extremely crowded list of columns...
- Does the mod really, really need so many new columns..? Technically, all new member data should go to the data column. If you need an explanation of how it works (really, it's just an array that's serialized and unserialized as needed), you can ask me.
- What should be in data: anything that's not updated for all members at once, or at least not on a regular basis, and that isn't used as a search parameter. Considering no new keys are added, I'm inclined to think that it's all right.

What do you think...?
I'd like to make sure we all agree on the same thing before I actually go and manually add all of these entries to the Wedge.org database... ;) (Already done on my local install. I had to, if I wanted to ensure my 'close mini-menu on click' trick worked... :lol:)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 17th, 2013, 05:01 PM
Quote from Nao on March 17th, 2013, 04:52 PM
- Does the mod really, really need so many new columns..? Technically, all new member data should go to the data column. If you need an explanation of how it works (really, it's just an array that's serialized and unserialized as needed), you can ask me.
notfy_email_last_sent and notify_email_period definitely need to be columns since they are searched for scheduled tasks, unread_notifications are also used (I forgot to define Keys, I'm not sure how that'll work for these). I guess disabled_notifiers, notifiers_prefs and email_notifiers can be consolidated into data since the latter two are json_encoded arrays anyway.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on March 17th, 2013, 05:33 PM
The thing for keys is that the value as they stand didn't seem to me to be very key-like in structure. Text keys are next to useless in this case, as are keys on things like flags for the simple reason that unless they're decently selective, they're just bloat.

The email_last_sent and email_period might be indexable, but I don't know. Some stuff I can usually get a feel for indexes while writing it, other stuff I want to see in action to judge how useful an index would be - and this certainly is the case.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on March 18th, 2013, 05:59 PM
I'd really appreciate if Dragooon could rework these columns into data variables. If it's not too much to ask for.

Also, I'm looking at the CSS, and: (1) anything that's member-only should be within '@if member' blocks. This saves the extra load for guests... (2) What exactly is that ":parent" pseudo-class..? If you actually mean "select this selector's parent", this isn't defined anywhere in CSS, even in CSS4. (I should know, I read through their entire pseudo-class list for CSS4 no later than yesterday for some other reason.) At worst, you can use JavaScript for that, but...
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 18th, 2013, 06:05 PM
Quote from Nao on March 18th, 2013, 05:59 PM
I'd really appreciate if Dragooon could rework these columns into data variables. If it's not too much to ask for.
Yeah sure, but you'll have to wait till weekend until my exams are through.
Quote from Nao on March 18th, 2013, 05:59 PM
Also, I'm looking at the CSS, and: (1) anything that's member-only should be within '@if member' blocks. This saves the extra load for guests... (2) What exactly is that ":parent" pseudo-class..? If you actually mean "select this selector's parent", this isn't defined anywhere in CSS, even in CSS4. (I should know, I read through their entire pseudo-class list for CSS4 no later than yesterday for some other reason.) At worst, you can use JavaScript for that, but...
1) Ah okay
2) Ugh...I always thought CSS has a :parent selector, I'll fix that as well.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on March 19th, 2013, 09:58 AM
- Hmm, are you sure you can do it? Doesn't that bother you?

- @if member is a trick I only recently added, you'd have surprised me if you'd implemented it that way, actually ;)

- I can do that, if you want... CSS/JS is my area of expertise these days, after all :P

Also... Where the heck is the admin page for setting up notifications...? I couldn't find it. The language files mention "Admin > Plugins > Notifications", so I'm trying to update them to the correct position...

Ah, and the notification popup is incorrectly positioned in my local install. I was wondering, could you test on your side with the latest revision? I'm wondering if it's a browser issue or a bug I introduced when optimizing your JS code... (I did some pretty big changes and didn't know *where* to test, so I wouldn't be surprised.)
Posted: March 19th, 2013, 09:39 AM
Quote from Nao on March 19th, 2013, 09:39 AM
- @if member is a trick I only recently added, you'd have surprised me if you'd implemented it that way, actually ;)
I only recently added it in JS, sorry, it's been in Wess for a longer time ;)

I mixed things up in my head. Which reminds me: notifications.js was a stand-alone file because it was a plugin to begin with... Now that it's a core feature, we should be integrating it into script.js, all right? And add the infamous @if member to it, of course ;)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 19th, 2013, 10:16 AM
Quote from Nao on March 19th, 2013, 09:58 AM
- Hmm, are you sure you can do it? Doesn't that bother you?
Not sure why it would bother me, I can definitely do it :P.
Quote from Nao on March 19th, 2013, 09:58 AM
- @if member is a trick I only recently added, you'd have surprised me if you'd implemented it that way, actually ;)

- I can do that, if you want... CSS/JS is my area of expertise these days, after all :P
Go ahead, it's a part of Wedge anyway
Quote from Nao on March 19th, 2013, 09:58 AM
Also... Where the heck is the admin page for setting up notifications...? I couldn't find it. The language files mention "Admin > Plugins > Notifications", so I'm trying to update them to the correct position...
There isn't one :P, there is only one setting for setting the number of days before which read notifications are pruned and that doesn't have an UI yet.
Quote from Nao on March 19th, 2013, 09:58 AM
Ah, and the notification popup is incorrectly positioned in my local install. I was wondering, could you test on your side with the latest revision? I'm wondering if it's a browser issue or a bug I introduced when optimizing your JS code... (I did some pretty big changes and didn't know *where* to test, so I wouldn't be surprised.)
I can have a look in the evening.
Quote from Nao on March 19th, 2013, 09:58 AM
I mixed things up in my head. Which reminds me: notifications.js was a stand-alone file because it was a plugin to begin with... Now that it's a core feature, we should be integrating it into script.js, all right? And add the infamous @if member to it, of course ;)
Yeah sure. The patch file was a quick job I did in the evening, it definitely isn't finished yet.
Posted: March 19th, 2013, 10:11 AM

Okay, quickly looking over the popup it seems to be correctly positioned in the latest revision.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on March 21st, 2013, 10:09 AM
I've made a few changes to the notification stuff, and uploaded them here, but not committed, so you can easily compare with your local install...
More importantly -- I've (temporarily..?) disabled auto-markread when clicking the notification, so that I can actually have time to style the popup at all... :whistle:

I think it would be best if we went for an approach more similar to Facebook's. Not that all they do is great, but in particular, their notification system is pretty good, and pretty much the only thing I use on their site...
Quote from Dragooon on March 19th, 2013, 10:16 AM
Go ahead, it's a part of Wedge anyway
Hopefully today I'll have included that in script.js, but first I'll commit a changed version of notifications.js so that it's easier for you guys to track all the changes I made, then I'll make another commit where I merge both files together.

File size of original notifications.js when gzipped: 744 bytes
Current file size: 578 bytes

Ideally, getting it under 500 bytes before the merging would be satisfying for me. This will require rethinking some of the code, though, and I'll put the priority on getting it to look nice.
Quote
There isn't one :P, there is only one setting for setting the number of days before which read notifications are pruned and that doesn't have an UI yet.
Alrighty.
We should also make sure that all unread notifs are sent by e-mail before they're pruned... Is it accounted for?

Also, a strange error in the log...
A *guest* accessed a ;area=getunread page. I don't know how the hell they found the link to that..?! I tried logging off, but nothing showed up. Hmm, maybe it's in the footer JS...? I'll have a look again.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 21st, 2013, 10:19 AM
Quote
We should also make sure that all unread notifs are sent by e-mail before they're pruned... Is it accounted for?
Unread notifications aren't pruned, only read ones. By default no emails are sent, you can change that in the profile area. The email area is perhaps one of the most unfinished part, it needs refinement and perhaps use of email templates.
Quote
More importantly -- I've (temporarily..?) disabled auto-markread when clicking the notification, so that I can actually have time to style the popup at all... :whistle:
Did you disable mark read on topic display (See Notification::markReadForNotifier, it handles all auto mark reads for areas other than the notifications action)? Because otherwise one way or the another it'll end up mark it read on click.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on March 21st, 2013, 11:04 AM
Quote from Dragooon on March 21st, 2013, 10:19 AM
Quote
We should also make sure that all unread notifs are sent by e-mail before they're pruned... Is it accounted for?
Unread notifications aren't pruned, only read ones.
I'm not sure it works like this... It seems to me that as soon as you click the popup, it gets marked as read, even if not visiting the topic itself. I may be wrong, but this is what I gathered yesterday... And I haven't looked at the notification code too deeply for now.
Quote
Did you disable mark read on topic display (See Notification::markReadForNotifier, it handles all auto mark reads for areas other than the notifications action)? Because otherwise one way or the another it'll end up mark it read on click.
That's not a problem... As long as I don't read the Birthday topic (which by tradition I'm not supposed to post on for a while :P), it won't get marked in my notifications. (Oh, and I'd appreciate if someone could post another @Nao in that topic, so that I get two notifications to style... :P)

Oh, I love the popup that comes up when I mention someone... It will need some refining, though! For instance, sometimes it doesn't close... (Like now. It's still opened.) Also, I tried locally to position the popup next to the name I entered, and while it works, which is great, it adds another gzipped kilobyte of JS, so I'll try to make it shorter or something, or maybe it's worth it I don't know...
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 21st, 2013, 11:07 AM
Quote
I'm not sure it works like this... It seems to me that as soon as you click the popup, it gets marked as read, even if not visiting the topic itself. I may be wrong, but this is what I gathered yesterday... And I haven't looked at the notification code too deeply for now.
That should not happen, some systems do this but I don't intend that.
Quote
Oh, I love the popup that comes up when I mention someone... It will need some refining, though! For instance, sometimes it doesn't close... (Like now. It's still opened.) Also, I tried locally to position the popup next to the name I entered, and while it works, which is great, it adds another gzipped kilobyte of JS, so I'll try to make it shorter or something, or maybe it's worth it I don't know...
I wanted it positioned next to the cursor but couldn't figure it out, so thanks! I'd like to see how you did it even if you don't commit it.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on March 21st, 2013, 05:21 PM
There's an error been flagged in the log - and it still happened this morning but I'm not sure it's been fixed since.

(edited out)
 8: Undefined index: page_title_html_safe

(Equivalent of action=notification)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on March 21st, 2013, 05:40 PM
@Dragooon> I actually used an external jQuery plugin for that, but it needed to be rewritten a bit because it's old and incompatible. Basically, IE provides a x/y position in its select range object, so that's perfect, but other browsers don't, so the plugin creates a hidden textarea with no scrollbars, fills it with the text up to the caret position, and gets the caret position from that. Well, it's easy to get the vertical position, but as for the horizontal position, I'm not too sure, haven't delved too deeply into its code... All I know, is that it works ;)

@Arantor> Yup I did notice that, and I'm planning to fix it, but right now I'm lost in a larger rewrite of the notification code and that error gets in my way :lol: Also, I edited your link out -- it's a public page, and I don't want Google following a link that generates errors for guests..!
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on March 21st, 2013, 05:43 PM
Oops >_<

In other news, we really gotta move it up to the top of the page.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on March 22nd, 2013, 03:11 AM
Also, the x/y position is off; when using main reply, non-WYSIWYG, the suggester position is the bottom left of the textarea.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 22nd, 2013, 03:52 AM
Currently that's intended, I couldn't position it next to the cursor.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on March 22nd, 2013, 10:38 AM
Quote from Nao on March 21st, 2013, 05:40 PM
I actually used an external jQuery plugin for that, but it needed to be rewritten a bit because it's old and incompatible.
I'm the first surprised, but I managed to take it down to 340 gzipped bytes.... 8-)

Basically, I gave up on the plugin I was using, and took some ideas here and there. Which didn't work...

Here's how I did it. In times like that, I feel really smart... :lol:

- Getting the Y position: reproducing the same text (up to the caret position) in a div that inherits the textarea's font values. That's the classic way of doing it. It's quick, and it works: you just take the div's height (default overflow), and you're done :)

- Getting the X position: here's the tricky part -- the one I figured out by myself. I'm initializing a count variable, and getting the height from above. Then I add a single-pixel width span, and calculate the div's height again. If it's the same, increment the count and continue. If it's not, then it means the div overflowed, and I stop it. Well... By this point, guess what count stands for? It's the number of horizontal pixels between the caret and the right end of the div! Then, all I need to do is $test.width() - count... And voilà, we've got our X position!

Worst part is, it really works :lol:
It could be faster, though... I'll have to consider doing that in pure JavaScript (not jQuery), I don't know. I'll just see.

This isn't tested in Wysiwyg, though. I suspect it won't work there... And I'll have to do a different codepath for it. Which will increase the size of the code. Meh. Well, right now it's at 1130 bytes, gzipped, and it's short enough to warrant being enabled by default here. ;)
Posted: March 22nd, 2013, 10:34 AM

Bugger... Just did a quick benchmark.
With innerHTML: 8 seconds to execute!!
With .append(): 1.4 second.
It's still very slow... Dunno how I can optimize.
Title: Re: [Plugin] Notifications system (1.0)
Post by: live627 on March 22nd, 2013, 10:48 AM
Someone's good at math :lol:
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 22nd, 2013, 11:42 AM
Use pure javascript? IMO textarea keyup callbacks should really use that, it is a world of a difference when it comes to speed.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on March 22nd, 2013, 03:55 PM
'kay, back from the outer world, immediately jumped onto this...
I was hoping changing .height() to .innerHeight would help, but it didn't.
After multiple tests, I managed to reduce the delay to under 200ms. Another stroke of genius, or maybe not: I simply increased the width of the span... If it's 5px instead of 1px, you need 5 times less additions to get the width, obviously... And a 5px difference doesn't matter much in the final result. It's not rocket science...

I'll still try to find other solutions. Apparently, just changing the display:inline of that span to display:inline-block seems to increase the speed, but I'm not sure if it's related to that... :^^;:
Posted: March 22nd, 2013, 01:01 PM

Doing Wysiwyg is a nightmare... :-/
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 22nd, 2013, 04:05 PM
Have fun! :P
Posted: March 22nd, 2013, 04:03 PM

@Nao: This is slightly unrelated but IMO the suggestion box can use some spicing up with avatar and membergroup info perhaps?
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on March 22nd, 2013, 04:07 PM
Actually, that might nice to do now that we're not living in 2007 any more :P
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on March 22nd, 2013, 06:09 PM
I'm only getting started... ;)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dismal Shadow on March 22nd, 2013, 07:53 PM
Quote from Arantor on March 22nd, 2013, 04:07 PM
Actually, that might nice to do now that we're not living in 2007 any more :P
@Arantor, next we see people saying we're not living in 2013 anymore. :P
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on March 26th, 2013, 04:55 PM
@Arantor, @Dragooon, @Dismal Shadow

This is a work in progress, but have a look at your notifications... ;)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 26th, 2013, 04:56 PM
I have none :P. Something is wrong, my counter is not going down. I have no unread notifications but it's telling me I have two.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on March 26th, 2013, 04:59 PM
I have (1) but none turn up (seen this before, not sure why)

Clicking view all brings me to: Fatal error: Cannot use object of type Notification as array in Sources/Notifications.php on line 282

Also, having the avatar is fine, provided that the subject is made smaller because right now it's a bit big and means it takes up a lot of space - even for a modestly long title like this topic. I'd argue the name of the person making the notification should be bigger than it is, too.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on March 26th, 2013, 05:11 PM
@Arantor> I'm not getting the error but I suspect it's in /do/notification..? I'm not visiting the page, I'm focusing on the popup first :P
I tried fixing it, please tell me if it works.

The popup width will be increased soon enough, once I move the item to the header (instead of the sidebar). It will no longer be limited by an arbitrary width.
Isn't the font size okay..?

@Dragooon> There's a bug with @Dismal Shadow :whistle: Also, you can only blame yourself for the bug you mention... Honestly, I spent my entire morning on the notification code, and all I got out of it was a headache. It's interesting to split tasks into several tasks (Notifier, Notification and weNotifs), but really -- there's no obvious role to any of these. It looks like one is just there to hold pointers to notification objects, another is there for actual notification handling, and another has some helper functions. To me, it sounds like these three objects could be reduced to one. There's a lot of time wasted in calling getters and setters here, too.

Currently, I have "6 notifications", and clicking them shows me 4. But from time to time (and persistently), it'll show me all of them.
My goal, as I said, is to use the Notification.template.php template for the popup. It's what I'll be working on next. I'll be showing all items (read and unread), with a class to differentiate them.

Well, I'm pretty much emulating Facebook here, but in my own style, hopefully...
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 26th, 2013, 05:15 PM
I'm not throwing any blame but I've never seen it happen here or on my local code before, it's only doing that after recent updates. I'll take a look tomorrow and see if I can find the problem.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 26th, 2013, 05:20 PM
@Nao: Ugh...can it be related to caching? It's randomly juggling notifications for me and that's the only reasoning I can think of.

EDIT: Oh and how did you format the text the way you did?
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on March 26th, 2013, 06:50 PM
There's another issue, actually. If I open 5 tabs, e.g. 5 new topics, only the first post has any notifications in the popup, the rest just have the 'view all' bit, though all 5 tabs have the right number of notifications.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 26th, 2013, 07:00 PM
Quote from Arantor on March 26th, 2013, 06:50 PM
There's another issue, actually. If I open 5 tabs, e.g. 5 new topics, only the first post has any notifications in the popup, the rest just have the 'view all' bit, though all 5 tabs have the right number of notifications.
It has to be cache juggling things up, same query can't return two different results...
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dismal Shadow on March 26th, 2013, 07:52 PM
Could it be because I have space in my username? Just a thought...
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 26th, 2013, 07:54 PM
@Dismal Shadow

Ah..it only works if I put a space after your name, the breaking might be a little off in the mentions plugin. Will have a look in a while.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on March 26th, 2013, 09:18 PM
Do we want notifications if a post of ours is liked?

/meis thinking could be good for the core, good for me to practice on - and good for ensuring that we get the 'don't notify me about this' working.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Maxx on March 26th, 2013, 09:38 PM
I think it's best to just have counts ,like you have and who, but not every time, maybe if some one dis likes the post, and can it be shown in the author/poster's profile, ( amount of likes only??) or is that too much too much?

being notified on a busy board could be annoying at times ( I'd think).

regards,
Maxx
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on March 26th, 2013, 09:43 PM
Quote
maybe if some one dis likes the post
Not an option.
Quote
and can it be shown in the author/poster's profile,
If you go to the profile > show posts area for a user, you will see their posts, if the posts received likes that will be shown there.

But I'm not talking about any of that. I'm talking about the notifications system that appears (currently) in the top of the sidebar, that is like XenForo's system.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Maxx on March 26th, 2013, 09:51 PM
The way it is now looks great! if they like would it show on drop down. or hit view all?
anyway it's cool the way it, and if you add on the likes that sounds cool!

was think of something else sorry!

regards,
Maxx
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 26th, 2013, 10:01 PM
Well sure, the plug in should work although I haven't extensively tested it. It should also show how multiple notifications are grouped together.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 27th, 2013, 07:08 AM
@Arantor: Yay your first notifier, a couple of points though
1) It can use multiple handling, something like Arantor and 4 others liked your post "Re:Like my post" which is pretty simple to implement (mostly Notifier::getText and Notifier::handleMultiple stuff). Currently it'll just stop the notification and bump it, you can add a count in data and increment it when handleMultiple is called and add some conditional checks on getText
2) It should be LikesPost since it'll currently only handle that

EDIT:
@Nao: Can you temporarily remove caching stuff from weNotif::get_quick_notifications on this site? I want to see if that fixes things.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 27th, 2013, 11:06 AM
@Nao: Okay, improved name detection. Apparently preparsecode's changing of \n to <br> was breaking @Dismal Shadow's name detection (or anyone else followed by a line break). https://github.com/Dragooon/WeMentions/commit/efcd547c925f572e94c5a51fb37b6aa5b62082ee
Title: Re: [Plugin] Notifications system (1.0)
Post by: MultiformeIngegno on March 27th, 2013, 12:40 PM
Uhm, this is strange. I tried to notify Arantor, so I typed @ Aran (of course without the space) but Chrome throwed the popups attached, every time with a different number.

To reproduce: type @ Aran (without space) then close the first popup, press ESC and a new popup with a different number should appear, close it and press ESC again ...
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on March 27th, 2013, 01:57 PM
Quote from Dragooon on March 27th, 2013, 07:08 AM
@Arantor: Yay your first notifier, a couple of points though
1) It can use multiple handling, something like Arantor and 4 others liked your post "Re:Like my post" which is pretty simple to implement (mostly Notifier::getText and Notifier::handleMultiple stuff). Currently it'll just stop the notification and bump it, you can add a count in data and increment it when handleMultiple is called and add some conditional checks on getText
2) It should be LikesPost since it'll currently only handle that
It was a quick thing I did late last night ;) I did skim over the handleMultiple but I wasn't quite sure what I was supposed to do with it.

As far as the name goes, I did envision it supporting likes on thoughts too but I ran out of time last night to work on it.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 27th, 2013, 06:04 PM
Quote from MultiformeIngegno on March 27th, 2013, 12:40 PM
Uhm, this is strange. I tried to notify Arantor, so I typed @ Aran (of course without the space) but Chrome throwed the popups attached, every time with a different number.

   To reproduce: type @ Aran (without space) then close the first popup, press ESC and a new popup with a different number should appear, close it and press ESC again ...
You can freely use @Aran, see it won't do anything. BTW @Nao, you forgot to remove debugging code :P
Title: Re: [Plugin] Notifications system (1.0)
Post by: MultiformeIngegno on March 27th, 2013, 06:06 PM
No, I explained myself badly. When I try to type @Arantor, as long as I arrive at typing @Aran, those popups with the numbers appear.

EDIT: oh so they was for debug?
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 27th, 2013, 06:12 PM
Quote from MultiformeIngegno on March 27th, 2013, 06:06 PM
No, I explained myself badly. When I try to type @Arantor, as long as I arrive at typing @Aran, those popups with the numbers appear.

   EDIT: oh so they was for debug?
Yeah I got what you meant, what I meant was you didn't need to add a space after @ :P.
Title: Re: [Plugin] Notifications system (1.0)
Post by: live627 on March 27th, 2013, 11:32 PM
Quote from MultiformeIngegno on March 27th, 2013, 12:40 PM
Uhm, this is strange. I tried to notify Arantor, so I typed @ Aran (of course without the space) but Chrome throwed the popups attached, every time with a different number.

To reproduce: type @ Aran (without space) then close the first popup, press ESC and a new popup with a different number should appear, close it and press ESC again ...
Quote from Dragooon on March 27th, 2013, 06:12 PM
Yeah I got what you meant, what I meant was you didn't need to add a space after @ :P.
Is someone confused? ::P
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on March 27th, 2013, 11:34 PM
I am, I don't know about anyone else.
Title: Re: [Plugin] Notifications system (1.0)
Post by: MultiformeIngegno on March 28th, 2013, 12:13 AM
Should I write again what I'm trying to say or is it clear enough? :P
Was someone able to reproduce that?
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on March 28th, 2013, 12:14 AM
Sure it was... it's debugging information in trying to figure out where the popup should be displayed.
Title: Re: [Plugin] Notifications system (1.0)
Post by: MultiformeIngegno on March 28th, 2013, 12:18 AM
Ok phew. I feared I couldn't even explain a basic thing like that in English :P
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 28th, 2013, 04:17 PM
Meh forget about it, it was actually a stupid joke :P.

I've been thinking, is it a good idea to move the notification list (all of them) to the profile area instead of an independent action? The notifications action will still remain still it's used for more than that but the list goes into the profile. XF does something similar and it's not a bad idea.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on March 28th, 2013, 04:19 PM
That works for me.

Also, when looking through the code the other day, I realised the subtle change that occurs... it actually replaces the old notifications area for board/topic notifications by replacing area=notification in the profile. I take it that was intentional?
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 28th, 2013, 04:31 PM
Quote from Arantor on March 28th, 2013, 04:19 PM
That works for me.

   Also, when looking through the code the other day, I realised the subtle change that occurs... it actually replaces the old notifications area for board/topic notifications by replacing area=notification in the profile. I take it that was intentional?
Yes, the entire old notification and subscription system is meant to be replaced by this (hence the entire reason I added e-mail grouping and notifications)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on March 28th, 2013, 04:32 PM
Just checking ;)

That would also mean we need to figure out some way of getting people to notify for a given board or topic that they're watching... and that does need to be in the core.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 28th, 2013, 04:33 PM
Quote from Arantor on March 28th, 2013, 04:32 PM
Just checking ;)

   That would also mean we need to figure out some way of getting people to notify for a given board or topic that they're watching... and that does need to be in the core.
Already taken care of, just need to be incorporated into core. That part is handled by my notification subscriptions core + subscription handlers. I have coded them for topics, need to work on board but it's pretty simple.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on March 28th, 2013, 04:37 PM
Is that notification on 'new topics', 'new posts in topics you're watching' and 'new posts in topics you're watching but only for the first unread post'?

As for boards, is that 'new topics', 'new topics and new posts in topics you're watching', 'new topics and all new posts'?

There are people who want this stuff >_<

(I'm not sure whether it *should* be, I'm just trying to get a feel for what it currently is and what the plans are for it rather than whether it should do these things.)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 28th, 2013, 04:39 PM
Quote from Arantor on March 28th, 2013, 04:37 PM
Is that notification on 'new topics', 'new posts in topics you're watching' and 'new posts in topics you're watching but only for the first unread post'?

   As for boards, is that 'new topics', 'new topics and new posts in topics you're watching', 'new topics and all new posts'?

   There are people who want this stuff >_<

(I'm not sure whether it *should* be, I'm just trying to get a feel for what it currently is and what the plans are for it rather than whether it should do these things.)
By default it's for new posts but can be extended in multiple ways to incorporate those option. By automatically subscribing to topics you're watching (it also has the option to automatically subscribe to topics you post or reply, I'm not sure if it works ATM :P), and by giving options. I haven't done it but porting those options should be a simple affair. What is the difference between "notify" and "watching" a topic? Seems redundant to me?
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on March 28th, 2013, 04:43 PM
Quote
What is the difference between "notify" and "watching" a topic? Seems redundant to me?
Oh, it's the same thing, just different terminology (and more sane terminology than 'Notify' seems to be). But there is a big difference between 'all new' and 'new in things you're specifically interested in'. I can well imagine some people wanting notifications on all posts, and some people only wanting notifications on posts they were watching.

(And I can imagine admins wanting to beat their users with a stick YOU WILL HAVE NOTIFICATIONS ON ALL POSTS AND YOU WILL LIKE IT >_<)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 28th, 2013, 04:47 PM
Totally unrelated bug, I can't do cmd + b for bold on Mac.
Quote from Arantor on March 28th, 2013, 04:43 PM
Oh, it's the same thing, just different terminology (and more sane terminology than 'Notify' seems to be). But there is a big difference between 'all new' and 'new in things you're specifically interested in'. I can well imagine some people wanting notifications on all posts, and some people only wanting notifications on posts they were watching.
I'm going to assume you meant topics they are watching, because watching posts is not only creepy, it's useless :P. But they are options that aren't hard to implement (i.e. difference between notify board and notify topic), plus the notify on all vs notify on unread is something that's easily resolvable via handleMultiple as far as I can think.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on March 28th, 2013, 04:51 PM
Well, yes, that's what I meant :P But you see what I'm getting at - some people will want notifications on *everything* and some just on what interests them.
Quote
plus the notify on all vs notify on unread is something that's easily resolvable via handleMultiple as far as I can think.
Sounds legit :bravo:
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on March 28th, 2013, 04:52 PM
And giving them that option won't be hard. Hell I can easily make a plugin that allows a person to subscribe to another member. Not sure if it'll be well received :P.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on March 28th, 2013, 04:54 PM
It's no more creepy than Facebook is, after all ;)

(I think XF has that as an option, actually)
Title: Re: [Plugin] Notifications system (1.0)
Post by: MultiformeIngegno on April 5th, 2013, 06:01 PM
This would be so geek to have :P
http://josephscott.org/archives/2013/04/favicon-alerts-with-tinycon/
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on April 5th, 2013, 07:24 PM
You know I wrote half a plugin for this very thing, right?
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on April 5th, 2013, 07:25 PM
Quote from Arantor on April 5th, 2013, 07:24 PM
You know I wrote half a plugin for this very thing, right?
Uhm, what do you mean?
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on April 5th, 2013, 07:31 PM
The post above mine about using TinyCon - I did most of a plugin to put the number of unread PMs into the favicon but for some reason I never finished it.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on April 5th, 2013, 07:31 PM
Quote from Arantor on April 5th, 2013, 07:31 PM
The post above mine about using TinyCon - I did most of a plugin to put the number of unread PMs into the favicon but for some reason I never finished it.
Why not? That sounds interesting.
Title: Re: [Plugin] Notifications system (1.0)
Post by: MultiformeIngegno on April 5th, 2013, 07:33 PM
Quote from Dragooon on April 5th, 2013, 07:25 PM
Quote from Arantor on April 5th, 2013, 07:24 PM
You know I wrote half a plugin for this very thing, right?
Uhm, what do you mean?
Didn't know that
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on April 5th, 2013, 07:36 PM
Lemme dig it out, I'm sure I can convert it to use the regular notifications system without too much trouble.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on April 9th, 2013, 09:01 AM
Quote from Arantor on April 5th, 2013, 07:36 PM
Lemme dig it out, I'm sure I can convert it to use the regular notifications system without too much trouble.
Which reminds me, PM also have notifications. Hm, leave it alone or make the entire thing use notifications core? Also, do you have any ideas on how to handle the Email templates? You have some idea how I'm intending emails to be handled, but I'm not really sure how to go about it (Currently they suck, I had no idea where I was mentioned and the email was without any clickable link :P).

@Nao: Any summary of changes left for the notifications/not in SVN yet? Oh and can you make my unread_notifications count 0? it's driving me crazy :P

Also, did anyone test periodical notifications? Do they work? :P
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on April 9th, 2013, 12:03 PM
- Dynamic favicons: I don't like the demo I saw for Tinycon... I don't know. One of the things is that when you have plenty of tabs opened, once a notification is there, all of your favicons are going to be updated...

Ah... If only we could send a message across all tabs, stopping timers, things like that...!
Hmm... Thinking about it... That might be a good opportunity to start using local storage...?!
For instance, on each server request, I could store the latest request time & response. And maybe a random ID that stays the same for the duration of the page. Then every tab checks the same local storage... Before doing a server request, they can check for the content stored in that. If the last time isn't expired, then take that data and cancel the server request. This would allow tabs to do their requests more frequently, e.g. less than a minute, rather than 10 minutes for now (for inactive tabs).
If one tab is active, show the notification in its title, hide it from others. If no tab is active, show notification in all tabs, so as to REALLY draw your attention, maybe..?
There's so much that could be done with notifications, really...
Very strange that I'm spending so much time on a feature I didn't write, didn't think of, and didn't even really want in the first place... :^^;:

- Never tested periodical notifications. Never received any e-mails.

- If it's not zero, then you have unread notifications... Don't you..? Wedge.org should have the ability to mark them as read, AFAIK..?

- PM notifications should become normal notifications, I agree, *but* they should have high priority, and that's one of the things that bother me... If you receive tons of notifications for 'likes', you'll tend to ignore ALL notifications. Eventually, we'll need to add more user settings, such as 'ignore notifications by type', 'ignore notifications by user', etc...

- I'm also thinking of giving the user a strong visual hint when a new notification shows up when they're using the page. For instance, automatically open the popup...? And show it as position:fixed to ensure it shows up whatever the position in the page. Or just have a small, unrelated popup in the top left corner, saying "You've got a new message!", and then closing itself after a few seconds or on click...?
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on April 9th, 2013, 12:51 PM
Will reply to the rest of the things later, but you can disable individual types of notifications, see your profile area. And user configurable priority is something I thought of but didn't really do it, it's not a bad idea. And I have no actual unread notifications, just a counter saying 1, it got messed up sometime when the site was being modified or something.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on April 9th, 2013, 04:12 PM
It's interesting that I didn't remember that area, even though I translated it all to French... ;)
Maybe it should be made more prominent. Right now, it sits where IIRC there was already a 'Notifications' area, but it's different now... Anyway.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on April 9th, 2013, 04:14 PM
Yes, that was intentional - but we can't really remove it until we also fix up notifications for topics and replies and stuff (and have a way for users to unsubscribe from boards/topics/whatever on a selective basis)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on April 9th, 2013, 04:28 PM
You don't have to use permissions for that...
I'd say there are things that should be left to the user's discretion, rather than the admin's.

For instance, what if I don't want users to access my profile? Or I'm okay with that, but not with reading my latest posts? Or my galleries? Or my posting statistics..?
Members have a data field, it's not for nothing. :)

Things that can be stored in data fields:
- member data: whatever notifications I want to ignore; whatever profile areas I want to hide from whatever membergroup(s) or member ID(s). Plenty of other things, I'm sure...
- topic data: set a 'fallback' topic for a particular topic; if it's locked, Quote and Reply buttons will still show up, but will be redirect to said fallback topic. Set membergroups or member ID(s) that can keep quoting, or posting. e.g. in here, 'New Revs' would be a perfect candidate for that. Set all users to be redirected to 'New Revs Public Comments', set Arantor and Nao to keep the Reply (or Quick Reply) button (but not their Quote buttons). This allows users to actually quote a post without having to manually copy and paste it...! (I think there was a discussion regarding this at Elk, recently. I was surprised we had a similar idea, but apparently our implementations would be different.)

Of course, all that matters here, is making a suitable UI for these... And I'm not exactly the greatest at UIs, I'm afraid, so it's not even in my to-do list I think...!
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on April 9th, 2013, 05:09 PM
Quote
You don't have to use permissions for that...
I'd say there are things that should be left to the user's discretion, rather than the admin's.
Wait, what? How did we get to this?

Oh, I agree, but there are things admins will want to change the defaults for. For example, there is currently no way to reset the default PM mode from the admin panel. Nor is there a way to override notifications to set things for people by default (the number of people who want to opt everyone in to notifications on everything is slightly frightening but there are some legitimate cases for it)
Quote
For instance, what if I don't want users to access my profile? Or I'm okay with that, but not with reading my latest posts? Or my galleries? Or my posting statistics..?
Well, they can see your latest posts if they can see the boards you posted in... same for galleries. Why you'd want to hide profile in general or stats... not so sure. But I can see the logic of it.
Quote
Things that can be stored in data fields:
- what sanctions are currently in force on the account and when they expire ;)
Quote
- topic data: set a 'fallback' topic for a particular topic; if it's locked, Quote and Reply buttons will still show up, but will be redirect to said fallback topic.
Wait... neither the messages nor the topics tables have data fields... Though a data field on topics would be quite nice, could do all sorts of fun things like auto redirection topics then.

Though the concept of a fallback topic is an interesting one. I'm not sure what the UI would look like but it's interesting nonetheless. What Elk proposes is 'quote to new topic' which is a different variation on the theme.
Quote
Of course, all that matters here, is making a suitable UI for these... And I'm not exactly the greatest at UIs, I'm afraid, so it's not even in my to-do list I think...!
I'm not the greatest at UI either but I figure if I keep cracking on with them I'll get better :P
Posted: April 9th, 2013, 04:41 PM

Actually, I'd quite like a data field on messages too, I can have some fun warning stuff in there, but I can safely live without that for now ;)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on April 10th, 2013, 11:09 AM
@Nao: We can use Chrome's Desktop notifications as well, they're pretty nice, http://www.html5rocks.com/en/tutorials/notifications/quick/ and http://www.chromium.org/developers/design-documents/desktop-notifications/api-specification and fairly simple to implement.

Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on April 10th, 2013, 01:31 PM
Other than being Webkit only, and don't actually appear to work for me (the example about grabbing tweets from Twitter didn't show me a single notification even when I specifically gave it one to test)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on April 10th, 2013, 02:27 PM
Quote from Arantor on April 10th, 2013, 01:31 PM
Other than being Webkit only, and don't actually appear to work for me (the example about grabbing tweets from Twitter didn't show me a single notification even when I specifically gave it one to test)
I think it's chrome only, you sure you gave it permission? There was another demo which asked for a title and message which worked fine for me, plus I've been using these for Gmail since forever.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on April 10th, 2013, 07:28 PM
@Arantor: http://dangercove.com/html5-notifications/ works well
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on April 10th, 2013, 10:52 PM
Yes, I gave it permission. I may be many things but not that naive (I hope :P)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on April 13th, 2013, 01:55 PM
Honestly... To me, it's a given, I will (or someone else will) add support for Chrome notifications in the future... It doesn't seem to be that hard to add, and it solves a lot of problems (including providing a mutex for each notification, allowing for multiple tabs with the same code). I, for one, would be delighted to get these notifications every time someone posts on Wedge or whatever... ;)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on April 13th, 2013, 01:58 PM
Quote from Arantor on April 10th, 2013, 10:52 PM
Yes, I gave it permission. I may be many things but not that naive (I hope :P)
So, was that sarcasm..? (I'm having a Sheldon moment here.)
Because I don't see any reason to be paranoid about this? Notifications only show up when you have a tab open with that website in it...
Title: Re: [Plugin] Notifications system (1.0)
Post by: MultiformeIngegno on April 13th, 2013, 02:37 PM
It works quite well with Gmail
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on April 13th, 2013, 04:25 PM
Quote
So, was that sarcasm..? (I'm having a Sheldon moment here.)
No, it wasn't. The question was: did I give the page permission to use notifications. Yes, I did. While many users might be like 'wut, permission? derp derp', I was aware I would need to give it permission, which I did and I told it to use my own Twitter account for testing, and as such I added a tweet and it didn't show - and yes, I left the tab open for a while.

I'm not opposed to it, but I don't really like the idea of a Webkit only feature in the core right now.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on April 14th, 2013, 04:48 PM
It'll soon be WebKit AND Blink only... :P
That is, enough to warrant standardization. Which, BTW, is underway... :)

Anyway.

A few issues I'm having with notification emails...

- I noticed that Notifications.php calls sendmail() with $send_html set to true. So, it does intend to transmit links, but unfortunately all I'm receiving is raw HTML, i.e. tags show up in the mail... (That's with gmail, I should add.)

- Even if we managed to have e-mails sent through HTML, there's the tiny problem of membergroups... Ever since rev 2031, member colors are no longer stored, rather groups, which is fine by me, be in an e-mail, you don't have access to the website's CSS, so these class names just return nothing. Is it worth including a link to our CSS files in the e-mails...? Hmm... I dunno...
Title: Re: [Plugin] Notifications system (1.0)
Post by: spoogs on May 10th, 2013, 11:25 PM
So I just recently got my first few notifications and I actually really like how it works,

I pretty much never check my email so I did not use the previous method of being notified by email of replies, PM's, etc. Needless to say that many things have gone missed by me because eventually I forget that i wanted to keep up with a particular topic, had I been notified via email it would have yielded the same result anyway.

Once the notification system was in use here I looked at the settings and chose to not be notified via email so that was great for me, no need to get emails I'll not see for months after they've stopped being relevant right. I have surprisingly been caught up on all the topics so no particular topic struck me as need to be notified so I sat jealously and patiently by reading how nice the system was and would ya know it @Arantor liked a post of mine and boom now I can see this thing in action from a true use case perspective without having being notified just for the hell of it. Then @Nao mentions me and I got that notification as well.

I gotta say great work here @Dragooon and guys. This is how I would love to have always received notifications on forums I visit.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on May 16th, 2013, 09:52 AM
@spoogs: Well, thank you! It's far from finished though, I have a lot of WIP stuff that still needs to be merged into SVN. Hopefully I'll get more free time by end of May.

I just had an idea though, I got a Facebook plugin that works, I can make it so that it pushes any notification on Wedge to your Facebook's notification stream. Moar notifications!
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on May 16th, 2013, 10:45 AM
Quote from Dragooon on May 16th, 2013, 09:52 AM
@spoogs: Well, thank you! It's far from finished though, I have a lot of WIP stuff that still needs to be merged into SVN. Hopefully I'll get more free time by end of May.
And I rewrote so much of your code, it feels like Aeva Media all over again, don't you think..? I hope you're not upset by that. The fact that your code both times managed to draw my attention long enough to make it me work weeks on it (even years, in the case of SMG), should be enough, eh eh. I mean, I suck at 'starting new features', but you're good at that, and I rock when it comes to making them shine :P

Still, there's at least one of your WIPs that we need to determine whether it makes it into the main code: properly saving the avatar and member ID... Currently, Wedge uses a modified version of the original code, that dates the member data and loads it 'manually', but it would definitely benefit from taking the member ID from a dedicated field in the notifications table, which your WIP had, and Wedge doesn't (for now). I've always wondered whether I should take your patch and try to put it into the Wedge codebase, or if I should wait for you to take the Wedge codebase (which hopefully on your side has the very code we're looking for), and trim the elements that are no longer necessary once your code is taken into account...
What do you think..?

Since we're planning to release a public alpha this spring (yeah, one can always hope :P), it would be nice to have the notification code 100% complete before we do that, so that people can start writing notif plugins if they want ;)
Quote
I just had an idea though, I got a Facebook plugin that works, I can make it so that it pushes any notification on Wedge to your Facebook's notification stream. Moar notifications!
I don't use Facebook (much), so I wouldn't want it core, and generally anything that interfaces deeply with another third party is a no-no for core, but as a plugin it'd be cool, yeah!

Just like I still have that 100-year old to-do entry, "allow reposting thoughts to my Facebook/Twitter feed"... :lol:
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on May 16th, 2013, 05:36 PM
No I don't really mind, plus you've mostly worked on the UI, an area I didn't really spend time on and an area I'm not too good at so its all good and appreciated. You probably managed to make it better than I could've, so thanks for that :).

I haven't looked into the avatar stuff but I'll probably go with making it global so that its not notifier dependent with the ability for a notifier to override the default avatar as an icon. I got an idea for this just need some time to actually implement.

The Facebook part was definitely plugin stuff.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on May 28th, 2013, 02:07 PM
So, as I mentioned in my thoughts, I've been working on an alternative way of showing notif previews, which would work on mobile...
I'm pretty much finished with it, and it works great on mobile. It's also about 100 bytes smaller in the js.gz, and also does without the extra #notprev styling I added recently.

I'm planning to upload it here to have you guys review it and tell me what you prefer.
The main issue is, I like the current one better... :-/
The difference is, the new one doesn't work on hover but on click, meaning it's less taxing on the server, but also requires more attention to get the preview. Probably needs a visual hint that you have to click, etc...
I also modified the popup to ensure it can extend to the full window height, so I can easily add previews BELOW the notifications without making it too hard to navigate.

So... Please everyone try the current version a bit, then tell me you're ready for the new version, or something... I'll upload it, and you can start comparing ;)
Title: Re: [Plugin] Notifications system (1.0)
Post by: MultiformeIngegno on May 28th, 2013, 06:19 PM
Uhm.. I don't know.. I think I like this version too. Maybe the only note is that it takes almost 1 second the first time to load the animation...
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on May 28th, 2013, 06:29 PM
Actually, I added a 0.3s delay to avoid sending needless Ajax requests to the server... ;)
Title: Re: [Plugin] Notifications system (1.0)
Post by: MultiformeIngegno on May 28th, 2013, 06:34 PM
Quote from Nao on May 28th, 2013, 06:29 PM
Actually, I added a 0.3s delay to avoid sending needless Ajax requests to the server... ;)
But only the first time, right? After the first click it's instantaneous. The delay makes it seem sluggish.. :P
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on May 28th, 2013, 06:42 PM
Nope, there's always this delay... Although if you'd start switching between notification items, once they're all loaded, it'd be quicker to load because they're all cached in memory.

Anyway-- I've uploaded the alternative version, which as I said, has the advantage of working just the same on mobile devices.
Click anywhere inside a notification to expand it. Err, just don't click on the links, though, obviously... ;)

Opinions, please..!
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on May 28th, 2013, 06:45 PM
I like it :) Gut reaction is that 1) it's not clear that we need to click on it, 2) no ability to collapse them again, 3) it does feel a little sluggish clicking on it without some kind of visual feedback that it's loading.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on May 28th, 2013, 06:52 PM
1/ As I said. And, well, if I settle on this, I'll really add some kind of visual hint...
2/ Yeah, I know, but I felt it wasn't that useful... If you noticed that too, then I guess I should add it. It's just a few bytes of JS...
3/ Hmm, yeah. But calling the Ajax loader might be overkill, I don't know..?

So, do you prefer this one or the previous..? You can always check from the repo to compare, if you need, or I can re-upload the SVN version. (It's just three changed files, really...)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on May 28th, 2013, 06:54 PM
I prefer this one, this works on mobile and feels more natural. However, I might be inclined to suggest we do some kind of cut on the preview text simply because it's kinda big in some cases. For example I got a like on one of my New Revs posts which was pretty lengthy and it only just fits on the screen vertically when wrapped into that width.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on May 28th, 2013, 07:01 PM
Quote from Arantor on May 28th, 2013, 06:54 PM
I prefer this one, this works on mobile and feels more natural.
I could possibly have both -- and determine which to show depending on the device, but it would probably be wiser to keep it simple.
Quote from Arantor on May 28th, 2013, 06:54 PM
However, I might be inclined to suggest we do some kind of cut on the preview text simply because it's kinda big in some cases. For example I got a like on one of my New Revs posts which was pretty lengthy and it only just fits on the screen vertically when wrapped into that width.
It should be creating a scrollbar, then..?

And as you can see, I rewrote the code to allow for the popup to extend to the window boundaries, and create a scrollbar if it goes beyond that. Actually, I don't know why I didn't do it like this in the first place...

PS: sorry for the bug with select box padding. I'll fix that soon. It's a debug thing...
Title: Re: [Plugin] Notifications system (1.0)
Post by: godboko71 on May 28th, 2013, 07:04 PM
I like the current one works well on my old ipad 2
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on May 28th, 2013, 07:05 PM
Oh, it is creating a scrollbar but the fact is it's *still* the size of the screen. See attached.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on May 28th, 2013, 07:16 PM
Well, that's normal... Apparently, that's a very, very long post... :lol:

Anyway -- enabled the 'click to fold' effect. Adds 17 bytes, thought I could do it with less... Ah, well.

Also, has the show_ajax() stuff. I really need to rewrite this into a positioned (centered by default) animation... Like for Zoomedia, see..?
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on May 28th, 2013, 07:23 PM
See, that's why I suggested a cut on the string ;) Since if it's a shorter string, it won't be a huge thing like this...
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on May 28th, 2013, 11:13 PM
A cut? You mean, having to actually visit the topic to see the full post? Meh...

So, I've committed the update. Everyone happy with it..?
I'm a bit disappointed with the size savings (even after I saved an additional 10+ bytes with a small rewrite in notload(), *sigh*), but it's still better than nothing..! I'm just too much of a bytenazi, I guess...

Bed time!
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on May 28th, 2013, 11:49 PM
Supposing the post has, I dunno, a couple of YouTube videos in it? Suppose it's a dirty great long post... there are plenty of posts of mine on this forum that, when shrunken into that width, certainly wouldn't fit on the page... it's supposed to be a preview, not an alternative thread view.
Title: Re: [Plugin] Notifications system (1.0)
Post by: icari on May 28th, 2013, 11:54 PM
i have not had to really see how the notification system works that well, but does that preview also parse bbc? it might be worth not parsing bbc especially if it only shows a small preview of a message
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on May 29th, 2013, 12:09 AM
The preview pane does indeed parse bbc.

Sending @icari a notification...
Title: Re: [Plugin] Notifications system (1.0)
Post by: icari on May 29th, 2013, 01:49 AM
it did not include the text of the message, interesting. also with the delay on loading the popup i thought it was not going to come up at all or i had clicked in the "wrong" spot.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on May 29th, 2013, 02:02 AM
You have to click on the bit that isn't the wording for it to show up... it won't just include the full message of the text straight off...
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on May 29th, 2013, 02:42 PM
This is pretty nice, I really like it :D. Although get_single_post can cause a lot of queries, perhaps some caching or grouping can be useful here? This can get slightly tricky given the modular nature of notifications. Or store the body in data column of notifications for preview?

PS: Bug, marking a notification as read when the preview is open doesn't remove the preview container but just the notification
Posted: May 29th, 2013, 02:37 PM

I had another idea though, make the notification's preview appear on hover and on swipedown event for touch devices? That'll be pretty cool.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on May 29th, 2013, 03:51 PM
- Pete: if you have large videos, then it creates a scrollbar, but the other notifications will still be readable in the same pane -- they won't take the entire width, only the overflowing elements will actually overflow, so I thought it was fine, but would you rather I overflow:hidden the entire preview..?

- Shitiz:

- get_single_post is a single query, using a very obvious index key, so it's fast to execute... And, it's only executed once per Ajax query, so you shouldn't be worried about THAT query, more about the queries that are executed before get_single_post even gets to be executed, eh... ;)

- I'm still waiting for your updated code to store the notification issuer in a row, rather than in the data field... If you want to update it, of course... :^^;:

- I'm aware of this issue, and I actually wrote a fix for it, but I couldn't figure out whether it was "all right", or if I could do a better animation... I'll probably end up using this, as it's really short and sweet: just add .next('.n_prev').andSelf() right before the .hide() call in the mark read code path.
Posted: May 29th, 2013, 03:50 PM
Quote from icari on May 28th, 2013, 11:54 PM
i have not had to really see how the notification system works that well, but does that preview also parse bbc? it might be worth not parsing bbc especially if it only shows a small preview of a message
Showing unparsed messages should never happen... It's okay if you're the author, of course, but the viewer (guest or whatever), should never see the cogs in the machine, it just doesn't make sense to make them wonder what they're reading.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on May 29th, 2013, 03:58 PM
Yeah, I'm inclined to think overflow:hidden might help but I still think it makes it overly long vertically.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on May 29th, 2013, 04:04 PM
Quote
- I'm still waiting for your updated code to store the notification issuer in a row, rather than in the data field... If you want to update it, of course... :^^;:
I know, I have my last entrance exam on 2nd June (2 papers, 3 hours each and an hour break in between) so once I'm through with that I'm back on programming :D. I'll work on the patches after that.
Title: Re: [Plugin] Notifications system (1.0)
Post by: agent47 on May 30th, 2013, 06:10 PM
I don't know if you've moved passed this and have already come up with an idea for this but there needs to be some sort of indication that a preview is available. If you've come up with something, do share if you don't mind.

P.S: If I may, the top [orange and white] loading bar looks very old-school and doesn't go along with the feel of Wedge.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on May 30th, 2013, 06:19 PM
Nope, haven't moved past this... I'm just taking a break from notifications, after working on them for so long... :P

Anyway!

And yes, the show_ajax is old-school, I even said so in this topic I believe, but what alternative can you offer..? My current plan is to use Zoomedia's ajax stuff as replacement, but... Well, it's not my priority.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on May 30th, 2013, 06:38 PM
Has it even been modified style-wise since we inherited it? At least it's not the white-on-green on 1.1.x...
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on May 31st, 2013, 03:20 PM
Okay, so I take a lot of inspiration from Android these days (see SMF4Mobile 1.2 for example), even the notifications area is somewhat inspired (centralised notifications for all) from Android as well as XF's implementation. One new nifty feature in Android (since 4.1) is the ability for notifications to have actions, so you can do stuff right from the notifications without going into the app (archive e-mail, share a new screenshot etc), and I think it can be useful for this as well? Like for reported post one can provide actions to close, delete post etc.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on May 31st, 2013, 03:51 PM
I know the plan is to migrate all principle notifications to the shiny notifications system, but I actually found myself intrigued by what IPB offers in this direction.

They allow for notifications being controlled by admin as to whether email or notification is sent, and whether the user has a choice in the matter.

The other aspect about it that's quite important is that I don't really see how board/topic (set to notify etc.) will fit into what the notifications configuration currently has... right now there's facilities for selecting whether you want it or not and whether you want emails on it as well/or not. But boards/topics explicitly require more complex notifications.

Right now the choices are 'new topics in a board I have set to notify me about' and 'new replies to topics I have set to notify me about' but I can well imagine that people would also want 'notifications of topics I've previously replied to' (which amounts to turning notifications on for a topic when replying, at least by default)

And since there are likely to be many... we come back to the other notifications page that we haven't killed off yet.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on May 31st, 2013, 03:53 PM
I already got that covered, that's what the notification subscriptions core is all about (and the topic subscription automatically subscribes as well), I just need to make an up to date patch.
Quote
They allow for notifications being controlled by admin as to whether email or notification is sent, and whether the user has a choice in the matter.
I've always meant to improve email handling, so this is a pretty good idea.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on May 31st, 2013, 04:12 PM
I'm not so bothered by the core part of it, because I know it can handle it - it's the UI for the user, selecting what topics they want - and more importantly opting out of topics - that I'm a bit fuzzier on. Essentially we need to somehow splice the old thing into the shiny thing :P
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on May 31st, 2013, 04:12 PM
Quote from Arantor on May 31st, 2013, 04:12 PM
I'm not so bothered by the core part of it, because I know it can handle it - it's the UI for the user, selecting what topics they want - and more importantly opting out of topics - that I'm a bit fuzzier on. Essentially we need to somehow splice the old thing into the shiny thing :P
Well I'm currently keeping the UI more or less the same, if you got any better ideas I'm all ears.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on May 31st, 2013, 04:15 PM
All in one page? Right now having two pages is confusing ;) I got no problems with AJAXifying stuff as needed.

Also, something I noticed... the notifications core plus all notifications that hook into it are all loaded every page load... do we need to load them every page load or only if we're actually going to send notifications (or do something else notification related)?
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on May 31st, 2013, 04:29 PM
Quote from Arantor on May 31st, 2013, 04:15 PM
All in one page? Right now having two pages is confusing ;) I got no problems with AJAXifying stuff as needed.

Also, something I noticed... the notifications core plus all notifications that hook into it are all loaded every page load... do we need to load them every page load or only if we're actually going to send notifications (or do something else notification related)?
Ah, okay. I'll look into simplifying them. Yes, they're required to view the notifications as well.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on May 31st, 2013, 04:34 PM
Yes, they're required to view notifications, however the notifications are pulled AJAXively, yes? In which case that would presume you only need to load it for the cases when a user is actually viewing them (i.e. as part of the AJAX service) rather than every page load.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on May 31st, 2013, 04:42 PM
Quote from Arantor on May 31st, 2013, 04:34 PM
Yes, they're required to view notifications, however the notifications are pulled AJAXively, yes? In which case that would presume you only need to load it for the cases when a user is actually viewing them (i.e. as part of the AJAX service) rather than every page load.
Hm, that's a good point. It should be possible to load them on demand but I'll have to take a look at it. Since they don't really cause a lot of load it's not high on my priority atm.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on May 31st, 2013, 08:25 PM
See, I'm looking at http://xenforo.com/community/threads/watching-a-forum.51048/ and thinking whether that's the sort of thing we need to be able to handle.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on May 31st, 2013, 08:35 PM
So basically what SMF is already doing?
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on May 31st, 2013, 08:38 PM
Yeah, essentially. I just don't quite see how to get from what SMF has to something unified with your notifications system.

I actually don't use what SMF has, so it's hard for me to understand how to make it work, as it were.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on May 31st, 2013, 08:40 PM
But I already have it working :P, at least I think I do. I'll give a detailed post tomorrow.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on May 31st, 2013, 08:42 PM
I'd be interested to see it :)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on June 2nd, 2013, 07:24 PM
Got kind of late, ran into a terrible viral yesterday. Anyway, this(http://github.com/Dragooon/WeNotif-Subs) is what I meant. It's not a new notifier but an extension to notifications itself, which facilitates providing notifications without any direct link but rather through an opt-in mode, hence the whole subscriptions thing. It works on similar principals of Notifications, a notifier which wants to be able to be subscribed has to declare a new class and specify it's behaviour, and call the subscription's publish options. Then the core pushes notifications (via the same notifier) to all the subscribed members. It doesn't really add much to the UI except a profile page to view notifications, most of it is internal stuff so that one can modularly add subscription features. Here's(https://github.com/Dragooon/WeNotif-Subs-Topics) an example (topic subscriptions) which works just fine. I believe this is what you meant?
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on June 2nd, 2013, 07:31 PM
I guess I need to install that to understand how it works... screenshots would have been nice :P
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on June 2nd, 2013, 07:49 PM
Quote from Arantor on May 31st, 2013, 04:34 PM
Yes, they're required to view notifications, however the notifications are pulled AJAXively, yes? In which case that would presume you only need to load it for the cases when a user is actually viewing them (i.e. as part of the AJAX service) rather than every page load.
To his credit, IIRC I wrote the Ajaxification of the code, so before he released it to us, it had every reason to be loaded on every page.

Currently, only Ajax + the actual /do/notification page need initializing.
(Although I'm planning to redo the /do/ one, because it's... in need of some love, I guess.)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on June 2nd, 2013, 08:21 PM
Quote from Arantor on June 2nd, 2013, 07:31 PM
I guess I need to install that to understand how it works... screenshots would have been nice :P
The UI is same, its all about the back end.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on June 2nd, 2013, 08:38 PM
So we have two notifications pages then?
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on June 2nd, 2013, 10:05 PM
Quote from Arantor on June 2nd, 2013, 08:38 PM
So we have two notifications pages then?
No, I'm not being clear I guess. The subscription plugin just adds the ability for existing notifier to have the ability to be subscribed to. The current page/UI is still used for preferences as well as for the notifications. Basically, it just adds a layer on top of the current system so that a plugin or core feature can add the ability to subscribe to any object (board, topic etc) which's notification is pushed through the current system. It adds no new UI element (the topic subscription one just replaces the URL of the current Notify button).

Basically here's how it goes (topic subscription plugin is an arbitrary example)
A user clicks "Notify" button on a topic -> the request gets sent to the subscription core which registers the ID and type of the subscription.
Another random user posts a reply -> the topic subscription plugin has a new_post callback which calls the subscription core to issue notifications
The subscription core issues notifications passing the given data to every subscribed member -> The topic subscription notifier kicks in and delivers all the notifications

You can install and try, or I can finish the patch in a couple of days (already WIP) and post it to merge.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on June 2nd, 2013, 10:07 PM
Yeah, I figured about that much - but my concern is still there: how does the user see what they have subscribed to and optionally unsubscribe to those? That's what the old Profile > Notifications is for, giving you somewhere to review your subscriptions and to remove them - and right now we either have to figure out how to present the two separate notifications pages or fuse the list of things you're subscribed to into the notifications settings page...
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on June 2nd, 2013, 10:10 PM
Quote from Arantor on June 2nd, 2013, 10:07 PM
Yeah, I figured about that much - but my concern is still there: how does the user see what they have subscribed to and optionally unsubscribe to those? That's what the old Profile > Notifications is for, giving you somewhere to review your subscriptions and to remove them - and right now we either have to figure out how to present the two separate notifications pages or fuse the list of things you're subscribed to into the notifications settings page...
Currently I'm adding a new page which lists all the current subscriptions (which works) subscriber wise. Considering this'll go in the core, we can do anything I guess. I can't do a screenshot since my localhost is messed up.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on June 2nd, 2013, 10:11 PM
Why not just rewrite the current page that already covers existing subscriptions to boards/topics?
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on June 2nd, 2013, 10:11 PM
Quote from Arantor on June 2nd, 2013, 10:11 PM
Why not just rewrite the current page that already covers existing subscriptions to boards/topics?
Because the existing stuff goes out the window :P
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on June 2nd, 2013, 10:17 PM
I got no problem with that provided that the UI is usable :)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on June 2nd, 2013, 10:18 PM
Quote from Arantor on June 2nd, 2013, 10:17 PM
I got no problem with that provided that the UI is usable :)
Yeah, of course. My notification system's aim was always to completely replace what was there, because I didn't want to be restricted by what's there because I didn't like it re-implementing the same things in different flavours.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on June 2nd, 2013, 10:20 PM
By the time we're done, I suspect there won't be that much original SMF code left!
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on June 2nd, 2013, 10:22 PM
@Nao: Can you re-enable markReadForNotifier on this site? It's annoying to have pending notifications of stuff I've already seen :P
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on June 3rd, 2013, 10:38 PM
What do you mean, exactly..? :^^;:

I probably should mark items as read as soon as they're previewed, though.

Also, I did this query...

Code: [Select]
SELECT m.id_member, m.real_name, m.unread_notifications, (SELECT COUNT(id_notification) FROM notifications AS n WHERE n.id_member = m.id_member AND n.unread != 0) AS real_count FROM members AS m WHERE m.unread_notifications > 0

Pointed out that I was off by 2 notifications, and you by 1, so I reset both our notification counts, to the correct number. :)

I also prepared the website to receive the new commit, with this:

Code: [Select]
SELECT COUNT(id_notification) AS co, id_notification, id_member, notifier, id_object, SUBSTRING_INDEX(data, 's:2:"id";s:', -1) AS truc FROM notifications WHERE id_member_from=0 GROUP BY truc ORDER BY co DESC

This gave me an ordered list of all notification issuers, and then I simply needed to update the tables manually, with another query... Yay.
Only took 20 minutes, though! Thankfully, the feature isn't old enough to be impossible to manage... :lol:

Dunno what's the point in keeping the member ID in the data field, though...! ;)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on June 3rd, 2013, 10:39 PM
Quote
I probably should mark items as read as soon as they're previewed, though.
+1
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on June 5th, 2013, 06:42 PM
Quote
Dunno what's the point in keeping the member ID in the data field, though...! ;)
Not much point anymore actually, I guess we can just store the name now.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on June 6th, 2013, 01:31 AM
I've left a few notifications pending for a bit, partly to prove emailing them works and partly to see what happens when a user accrues a lot of notifications (11 unread so far), and the sense I'm getting is for a "mark all read" button.

Thoughts?
Title: Re: [Plugin] Notifications system (1.0)
Post by: spoogs on June 6th, 2013, 04:28 AM
Sounds about right
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on June 6th, 2013, 04:31 AM
Also, a way to clearly indicate 'click me for a preview' and 'click me to take you to the content you've been notified about'
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on June 6th, 2013, 08:11 AM
Quote
What do you mean, exactly..? :^^;:
Didn't you disable Notifications::markReadForNotifier on this site? Which is preventing mine notifications to be automatically mark as read when I open a related thread.
Posted: June 6th, 2013, 08:10 AM
Quote from Arantor on June 6th, 2013, 01:31 AM
I've left a few notifications pending for a bit, partly to prove emailing them works and partly to see what happens when a user accrues a lot of notifications (11 unread so far), and the sense I'm getting is for a "mark all read" button.

Thoughts?
I'm having a feeling periodical e-mailing will not work. Mostly because I never tested it :P (not because I never meant to but I never had a localhost mailserver setup and didn't know how to exactly test it)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on June 6th, 2013, 02:28 PM
I have a solution for the whole local mailserver thing... it's called 'not worrying about it but making sure I have mail queue enabled'... because then I can see what entered the queue.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on June 6th, 2013, 02:39 PM
Quote from Arantor on June 6th, 2013, 02:28 PM
I have a solution for the whole local mailserver thing... it's called 'not worrying about it but making sure I have mail queue enabled'... because then I can see what entered the queue.
That's...a good idea...
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on June 6th, 2013, 02:39 PM
It's also how I discovered some of the bugs in the mail queue ;)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on June 6th, 2013, 10:44 PM
Also, the previews don't get parsed properly through the buffer. I have a notification where someone mentioned me and the preview includes the link to my profile name (of course, it's the parsed tag)... but it still has the <URL> in the link and isn't coloured because it's not pulled through the rewrite buffer.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on June 6th, 2013, 11:28 PM
You're talking about e-mails..?

- Color: that's logical, since you rewrote it to use classes for groups, instead of styles. The only way to fix, would be to have an alternate codepath where we do the old-school rewrite, but I don't know if it's worth it...

- We're still sending e-mails in text format, not HTML, so we still have <a ...> links, instead of text links.

- My local install has an alternative version, which at least fixes URLs to prevent them from being cut off (and, of course, goes through ob_sessrewrite in the e-mail parser). I have yet to commit it, because... Well, it's only worth it, if we're sending HTML...
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on June 6th, 2013, 11:42 PM
No, I'm talking about the *preview* pulled via AJAX... it has the wrong link in it and no styling.

See attached, notice the link that my cursor is under still has a <URL> in it and the preview doesn't have the colour groups.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on June 18th, 2013, 03:02 PM
So, I'm back working a bit on notifications, here's what I'm trying to add...

1/ Object type: return the type of the notification's object, of course. Right now, we're always assuming this to be a topic, when it could very well be something else, like a thought. Here's where I'm having trouble:

* The database only has an id_object, but no type for it. Meaning, at this point, you really can't specify a type per-object, unless I forgot something else... Can you help on this, Dragooon? Or anyone else... I don't see any way, other than adding a database entry... It was built to support multiple types, right..? Otherwise, it'd have been called id_topic, not id_object...

* What's the difference between these..? (In Class-Notification)

Code: [Select]
public function getText()
{
return $this->notifier->getText($this);
}

...

public function getObject()
{
return $this->id_object;
}

To be specific, I don't know if my getType() method should relay to $this->notifier, or not. I also really don't know why there's a difference, in here... (?)
I guess I'm having trouble, mostly with what a notifier is, and why it's needed, as opposed to just having everything in the notification class... (?)

2/ More options in notifications. This is quite simple, here... When you preview a message, you'll get a new line below it, with several links: "Mark as unread", "In context", and "First unread". "First unread" is only valid for topics, which is why I needed to implement (1), so whatever. It leads you to the first unread post in the related topic, or at least it tells you that there are unread posts *before* the notification's, allowing you to catch up with them, instead of losing the unread count when going to the notif post in context. I don't know if I'm clear enough, uh...? Just wanna know what you guys think about these...

3/ E-mail notifications. I'm working hard on this...

* The first thing I need to fix so hard, is the HTML e-mails. To solve this, I've decided to simply force plugin authors to add an extra language entry, so there's one for the popup notification, and one for the e-mail. That way, you can have a purely text e-mail, and still use HTML in the popup. I'm not comfortable with the various notification objects, so I'll just commit something a bit horrible, and I'm counting on you guys to offer shorter, better rewrites, as I'm only doing the bulk of it, and don't want to mess my mind with abstract stuff... ^^

* Second thing, I need to ensure that pretty URLs are applied to the message, but I can't go through ob_sessrewrite but it'll do other things, like cutting URLs. Two solutions: either I *do* go through ob_sessrewrite but with a special setting saying "Okay, this is for an e-mail, so don't screw up with it!", which is what is in my local code for now; or, I simply do it à la redirectexit(), which is call pretty URL transformation independently, and be done with it. I'm thinking I'll go with the second one, but I need to know if you guys agree, i.e. do you think there's something else to transform in e-mails..?

* Thirdly, I changed all %s and %d to {VARIABLE_NAMES}, like in EmailTemplates, which makes sense, since it has the same purpose. Plus, it's more self-explained... Don't really need an opinion on this, I'll do it this way anyway, from now on... ;) I also chose going with underscores, because e-mail templates use {VARIABLENAMES}, which I find less readable, but I can't bother to fix THESE now... Eh.

* And lastly, right now the mentions notification will show something like, "(link to user) mentioned you in (link to post)", while likes will show, "(user name) liked your post (post name)". The first has links, the second has raw text. Main advantage of the first one, you obviously can directly click on these links (which I don't like because of the "first unread" issue I discussed above), and also see the member type, since it has the associated class, which the likes notifications don't have. Of course, the first won't show correctly in e-mails, so I'm trying to determine which is best... (a) just let the HTML be as it is, with links and all, and turn them to pure text at notification e-mail sending time. It's easy enough to do, with just a simple strip_tags, and would save authors the hassle of providing two strongly different language strings. However, I was thinking, maybe I could go one step further, and instead of requiring two language strings, I could turn a regular link to Nao into "Nao (http://wedge.org/profile/Nao/)", right in the text. Thus, instead of having an e-mail string like "...mentioned you in {POST_TITLE}, which you can read here: {POST_URL}", we could have "...mentioned you in {POST}.", where {POST} is turned into a link in HTML, and when used for e-mails, into a simple text, "{POST_TITLE} ({POST_URL})"...
What do you think?

I know it's a lot, but I'm kind of stuck in all of these areas right now, and I don't know which direction to go, so any opinions, would be greatly appreciated... :)
Posted: June 18th, 2013, 02:59 PM

Oh... Or, I could do a 'generic' string for e-mails, appended to any notifications:

Link to the user's profile:
{MEMBER_HREF}

Link to the post: (if this is a topic, of course...)
{POST_HREF}

I could add all of these links by analyzing the contents of the string. If it has {MEMBER} in it, strip the link, then add it at the bottom, etc...
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on June 18th, 2013, 04:23 PM
@Nao: Long post is long, I'll answer the first point. "notifier" column is your type and it works fine with multiple types of notifiers. Basically, Notifier is your "type handler". It handles different types of objects, it already does what you're trying to do. Right now I'm not assuming jack, it's all handled on a per-notifier basis. Mentions is a "notifier" which issues Notifications of type mention, the core routes that back to the notifier and asks what to do (all the getText, getURL are for this). Similarly, Likes is also a notifier for specifically handling the notifications of "likes" type. For a plugin to implement a new type of notification, they create a new class derived from Notifier and add their own type. getType is already there as notifier is their type, so basically getNotifier is what you're looking for. If you want the string interpretaiton of a specific notifier, $notification->getNotifier()->getName() is what you want to do.

The difference between notifier and notification classes is:
Notification is a sort of helper class for handling individual rows of notifications in the DB, it doesn't do much except make our lives easier for storing new objects/reading existing ones.
Notifier is what actually handles the notifications, they interpret the data stored in the Notifications object and tell what should it's text, URL, preview etc. should be. Basically handle each type of notification's behavior. See the classes defined in the notifs directory, they tell different ways of handling different notifications.

If you're still unclear about how it all works and why it is the way it is, I can make a detailed documentation. Currently I'm out of home till late night so I can't do it right now.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on June 18th, 2013, 04:59 PM
getName() would return 'likes'. This would assume that all likes are for posts (or thoughts), when they can be for either of them...
Thus, getType() (or getObjectType(), if you prefer), would return 'topic' or 'thought' (or 'media' or whatever, once I implement liking media items...), depending on the target.

See what I mean..?

Anyway, other than that, happy to receive any feedback from you on the other matters..?
Sorry about the long post. Someone has to take Pete's place when he's on vacation... :lol:
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on June 18th, 2013, 05:02 PM
No no no, technically it should be likes_post and likes_thought. Like when it returns "mentions" it is assumed it's for a post since nothing else can be mentioned. Why would you want getObjectType though? Defining specific behaviour for specific types goes against the whole notifications idea.

I'll reply to the rest of it later tonight.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on June 18th, 2013, 06:08 PM
Using getName() for object type, implies that for instance, users can disable notifications for topics but not for media items, things like that...
It sounds like UI nightmare, to me. It should just be 'likes', and then have a getObjectType method, which returns 'topic' by default.

It really makes more sense to store the object type. I'll assume it's stored in the data field, in the meantime, until we agree on something...
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on June 18th, 2013, 06:14 PM
Quote from Nao on June 18th, 2013, 06:08 PM
Using getName() for object type, implies that for instance, users can disable notifications for topics but not for media items, things like that...
It sounds like UI nightmare, to me. It should just be 'likes', and then have a getObjectType method, which returns 'topic' by default.

It really makes more sense to store the object type. I'll assume it's stored in the data field, in the meantime, until we agree on something...
What you're trying to do sounds like a coding nightmare. Users can disable individual notifiers, and they can already do that. Not sure how it can be further simplified without complicating the coding process.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on June 18th, 2013, 09:17 PM
Quote from Dragooon on June 18th, 2013, 06:14 PM
What you're trying to do sounds like a coding nightmare.
How so?
Quote from Dragooon on June 18th, 2013, 06:14 PM
Users can disable individual notifiers, and they can already do that.
No. They can't disable likes notifs for topics, while keeping likes notifs for media items... See what I mean?
Quote from Dragooon on June 18th, 2013, 06:14 PM
Not sure how it can be further simplified without complicating the coding process.
Well, in any case, non-messages have to be taken into account... Otherwise, notifications won't be very far..!
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on June 19th, 2013, 12:28 AM
No feedback from anyone = no one cares about notifications...?

Should I remove the entire notification system, or just the new things I was working on..? ::)
Title: Re: [Plugin] Notifications system (1.0)
Post by: godboko71 on June 19th, 2013, 06:04 AM
Sorry work so not read the latest messages,
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on June 19th, 2013, 08:25 AM
Quote
No. They can't disable likes notifs for topics, while keeping likes notifs for media items... See what I mean?
They will be able to. Since the notifier for topics will never be same as the notifier for media items, it'll be a different notifier. Like there are different notifier for quotes and mentions. getObjectType is more or less redundant with individual notifier, the whole idea is to implement different notifiers for different individual items.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on June 19th, 2013, 01:27 PM
Quote
2/ More options in notifications. This is quite simple, here... When you preview a message, you'll get a new line below it, with several links: "Mark as unread", "In context", and "First unread". "First unread" is only valid for topics, which is why I needed to implement (1), so whatever. It leads you to the first unread post in the related topic, or at least it tells you that there are unread posts *before* the notification's, allowing you to catch up with them, instead of losing the unread count when going to the notif post in context. I don't know if I'm clear enough, uh...? Just wanna know what you guys think about these...
This seems better suited for my generic button controls idea. Basically I wanted notifiers to be able to add their own control buttons which can do whatever they want, that way they can have more control on a per-notifier basis instead of we determining their behaviour.

Do whatever the hell you want with emails, I can use all the help I can get there. I have no idea how to simplify those :P
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on June 19th, 2013, 06:05 PM
Quote from Dragooon on June 19th, 2013, 08:25 AM
Quote
No. They can't disable likes notifs for topics, while keeping likes notifs for media items... See what I mean?
They will be able to. Since the notifier for topics will never be same as the notifier for media items, it'll be a different notifier. Like there are different notifier for quotes and mentions. getObjectType is more or less redundant with individual notifier, the whole idea is to implement different notifiers for different individual items.
Why don't notifier (plugins) inherit from the Notifications class rather than Notifier, anyway..?
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on June 19th, 2013, 06:08 PM
Quote from Dragooon on June 19th, 2013, 01:27 PM
This seems better suited for my generic button controls idea. Basically I wanted notifiers to be able to add their own control buttons which can do whatever they want, that way they can have more control on a per-notifier basis instead of we determining their behaviour.
Hmmm... That, I could possibly do, I guess... I'll have a look. That would also, I guess, solve some of our problems...

Basically, we'd need to move the preview action to another function in the Notifier object, is that it..?
Quote
Do whatever the hell you want with emails, I can use all the help I can get there. I have no idea how to simplify those :P
Yeah, I still don't know either, since generic strings at the end of the e-mail are cool, but they also imply one shouldn't use HTML in the notification language string, either... Hmm...
Title: Re: [Plugin] Notifications system (1.0)
Post by: spoogs on June 20th, 2013, 04:11 AM
Probably Already reported but couldn't find it anywhere

I'm showing 1 new notification after having previewed and viewed the post I'm being notified about. The notification relates to likes for this post(http://wedge.org/pub/plugins/8199/popular-topics/msg290234/#msg290234).

Hmmm I clicked the "x" to close and it eventually marked it as read it seems since the entry can still be seen in "View All"... is this how it's supposed to work now?

Also @Nao in reference to feedback, I'm not sure I follow whats being said/asked, I actually assumed you were speaking to the more technically inclined. Could be that I'm usually with a drink at hand  most times as I'm browsing or brain fried late nights. Dont mind spitting my opinions around at all but when it seems to get technical I tend to leave that for those more in the know.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on June 20th, 2013, 05:20 PM
There's a filter for unread notifs (default), and a filter for all notifs, including read ones... Just in case you want to quickly access a notification you previewed and closed too quickly, for instance...

As for feedback, I just left it a couple of days, to see if anyone had anything to say, but I guess I'll have to make my decisions myself, eh... ;)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on June 20th, 2013, 06:09 PM
Quote from Nao on June 19th, 2013, 06:05 PM
Quote from Dragooon on June 19th, 2013, 08:25 AM
Quote
No. They can't disable likes notifs for topics, while keeping likes notifs for media items... See what I mean?
They will be able to. Since the notifier for topics will never be same as the notifier for media items, it'll be a different notifier. Like there are different notifier for quotes and mentions. getObjectType is more or less redundant with individual notifier, the whole idea is to implement different notifiers for different individual items.
Why don't notifier (plugins) inherit from the Notifications class rather than Notifier, anyway..?
Because they are Notifiers not Notifications :P, it's sort of subjective I guess. That made most sense to me.
Quote from Nao on June 19th, 2013, 06:08 PM
Quote from Dragooon on June 19th, 2013, 01:27 PM
This seems better suited for my generic button controls idea. Basically I wanted notifiers to be able to add their own control buttons which can do whatever they want, that way they can have more control on a per-notifier basis instead of we determining their behaviour.
Hmmm... That, I could possibly do, I guess... I'll have a look. That would also, I guess, solve some of our problems...

Basically, we'd need to move the preview action to another function in the Notifier object, is that it..?
I'm not sure, I was thinking of a new function returning an array of buttons which are then embedded.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on June 20th, 2013, 06:11 PM
Notifications = overall system, Notifier = something in the system of which many instances will be created.

It's like the difference between boards and topics ;)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on June 20th, 2013, 06:20 PM
The problem is that I often find myself stuck in object hell...

Example: want to have getPreview() return whatever the notifier wants, but have it return topic stuff by default. Moved code from Notifications.php to Class-Notifier.php, with no special treatment... And, I don't even know if it's the correct target. I suppose it is, but sure..? Nope.

Also, I really think it's best to store the object type in its own field in the database, rather than in the data field. Makes it easier to avoid duplicate entries... And to me, it's better than creating multiple notifier names, one for each object type... Urgh!
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on June 20th, 2013, 07:08 PM
Quote from Nao on June 20th, 2013, 06:20 PM
The problem is that I often find myself stuck in object hell...

Example: want to have getPreview() return whatever the notifier wants, but have it return topic stuff by default. Moved code from Notifications.php to Class-Notifier.php, with no special treatment... And, I don't even know if it's the correct target. I suppose it is, but sure..? Nope.

Also, I really think it's best to store the object type in its own field in the database, rather than in the data field. Makes it easier to avoid duplicate entries... And to me, it's better than creating multiple notifier names, one for each object type... Urgh!
Why would you want to have it return topic stuff by default? The thing is, I don't want to pre-define a lot of behaviour, mostly because then authors don't touch and the experience becomes bad again. Plus I don't want to add pre-defined behaviour for "special" object types, that goes against my model. I want it to be absolutely "core", in the sense it should be as unassuming as possible. I'm assuming a few cases (mostly such as getIcon in which it returns the avatar) because they are more or less global and can apply to all. That is why in my view getObjectType is absolutely redundent and not a good idea. I don't think having multiple notifiers is a bad thing, mostly because it gives every notifier an equal chance to implement the same experience for themselves and we don't define anything for them (any notifier that is in the core can't do anything more or less than any other notifier that might be a plugin)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on June 22nd, 2013, 11:52 AM
Quote from Dragooon on June 20th, 2013, 07:08 PM
Why would you want to have it return topic stuff by default?
Because that's what most notifications will be about... This is a forum, right..?
Quote
The thing is, I don't want to pre-define a lot of behaviour, mostly because then authors don't touch and the experience becomes bad again. Plus I don't want to add pre-defined behaviour for "special" object types, that goes against my model. I want it to be absolutely "core", in the sense it should be as unassuming as possible. I'm assuming a few cases (mostly such as getIcon in which it returns the avatar)
Perhaps because I imposed this one upon you... ;)
Quote
because they are more or less global and can apply to all.
Then it should be called getAvatar and not even be extensible... ;)
Quote
That is why in my view getObjectType is absolutely redundent and not a good idea. I don't think having multiple notifiers is a bad thing, mostly because it gives every notifier an equal chance to implement the same experience for themselves and we don't define anything for them (any notifier that is in the core can't do anything more or less than any other notifier that might be a plugin)
There are many problems in your implementation. There are good ideas, too, but there are problems. I didn't look much into your system until recently, because it was a lot to take in, and I should have really.
First of all, the different files are still fuzzy even to me. Class-Notification.php and Notifications.php are too much alike in their naming. I've come to learn that basically, Notifications.php should have been called Subs-Notification(s).php, since it's a helper class, but having the action handler within it makes little sense. This is probably why support for class::method wasn't added until recently to Wedge's action router, and it was added specifically for your code... Not a biggie, but it makes one wonder whether this was well thought out..? Maybe it should not be a class at all in the first place...

Also, maybe Notification::issue shouldn't be a method. Maybe it should be issue_notification(...), and this redirects to your Notification object, or something... Notification::issue implies that the notif object is loaded, which shouldn't be needed, at least not at all times. I don't know. Maybe it should, but it's just strange to me.

Notifiers are stored in the database with a plural in the name. A single notifications should be sorted as a 'like' or 'mention', not as a 'likes' or 'mentions'...

I have a hard time understanding why id_object gets its own database field. I mean, is that only for a case where you'd want to mark all notifications from a single topic as read once you read that topic..? But (1) there's no way to be sure the user has read the entire topic anyway, (2) I don't even think the code is currently checking unread notifications for a topic being read. Or is it..?

I could possibly agree with having different notifiers for the same notification type, but would have to be reassured on a couple of points...
- Could be done as an extra plugin, e.g. a class named Thought_Like(s)_Notifier, that extends Likes_Notifier, and can override all of its functions. Is that even doable..? (This would allow someone else to release a plugin that adds like notifications to their own plugin-created data types, without having to rewrite the notifier at all.)
- Could be done within the same notifier class, e.g. when returning the notifier name, getName() should return 'like' or 'like_thought', depending on the origin. Of course, this implies that the notifier keeps track of what type it is, internally.
- Doesn't disrupt all existing SQL queries that rely on id_object to filter items, especially since right now these queries seem to imply that id_object is always a topic.
(BTW, I'm surprised that id_object doesn't even get its own key..?!)

I think that's all for now, uh...
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on June 22nd, 2013, 05:33 PM
Without looking at the code (been snowed under with other code)... here's my take on a few things:
Quote
I have a hard time understanding why id_object gets its own database field.
For handling multiple operations to the same thing. E.g. if three people like a post, you'd want to tie them all together using the id_object. I mean we *don't* do that but we *could*. And it's on that field that we do it.
Quote
- Could be done as an extra plugin, e.g. a class named Thought_Like(s)_Notifier, that extends Likes_Notifier, and can override all of its functions. Is that even doable..?
It is, under certain conditions. Firstly, the parent class must be loaded (or invokable through an autoloader) before the child class, and secondly whatever invokes the child class will need the full class name and that seems a bit long for the current system.

Autoloading is currently tricky because the class names don't really match the filenames but we could do something with it if we wanted.
Quote
- Could be done within the same notifier class, e.g. when returning the notifier name, getName() should return 'like' or 'like_thought', depending on the origin. Of course, this implies that the notifier keeps track of what type it is, internally.
We have a gigantic data field for stuffing things like that into so it's no big deal to track it and then generate an appropriate message in getText().

See, there's more potential for likes than we're currently using; there's no reason why we couldn't attach likes to media items, for example. I know media items have ranking, but I can see the use for likes on media items as an additional option.
Quote
(BTW, I'm surprised that id_object doesn't even get its own key..?!)
It'd be better served as a key on (notifier (6), id_object) really from what I remember.
Quote
Also, maybe Notification::issue shouldn't be a method. Maybe it should be issue_notification(...), and this redirects to your Notification object, or something... Notification::issue implies that the notif object is loaded, which shouldn't be needed, at least not at all times. I don't know. Maybe it should, but it's just strange to me.
This is something I've raised in the past, actually. issue_notification() needs to handle loading all the relevant files (including the right file for a given notifier) and then we don't need to load it all each page (which would be handy)

Right now, the notifications core doesn't just load each page but every notifier does too. I can kind of see that need, to load each notifier, so that you know what you have available... I dunno.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on June 22nd, 2013, 06:13 PM
Quote
Because that's what most notifications will be about... This is a forum, right..?
I want it to be impartial, plus you got a huge freaking gallery inside the forum so I'd doubt that it's just a forum :P.
Quote
Perhaps because I imposed this one upon you... ;)
And it was a good idea, and it should be extensible. Maybe a crazy plugin author gets a good idea and implements something else for their notifier, it doesn't hurt to keep that option open.
Quote
First of all, the different files are still fuzzy even to me. Class-Notification.php and Notifications.php are too much alike in their naming. I've come to learn that basically, Notifications.php should have been called Subs-Notification(s).php, since it's a helper class, but having the action handler within it makes little sense. This is probably why support for class::method wasn't added until recently to Wedge's action router, and it was added specifically for your code... Not a biggie, but it makes one wonder whether this was well thought out..? Maybe it should not be a class at all in the first place...
Notifications.php is the "controller" here, it handles the notifications logic. If you have a better name go ahead, but I've thought a lot about my code and to me it makes perfect sense.
Quote
Also, maybe Notification::issue shouldn't be a method. Maybe it should be issue_notification(...), and this redirects to your Notification object, or something... Notification::issue implies that the notif object is loaded, which shouldn't be needed, at least not at all times. I don't know. Maybe it should, but it's just strange to me.
Hm, maybe. Notification::issue kind of made sense to me since Notification class is sort of the "model" for doing the DB interactions with individual Notification rows. Although it can be made different, not sure.
Quote
Notifiers are stored in the database with a plural in the name. A single notifications should be sorted as a 'like' or 'mention', not as a 'likes' or 'mentions'...
I read it like this notifier handles "likes" so it gets the name "likes", although this is a completely arbitrary issue since no one really reads the DB for grammar.
Quote
I have a hard time understanding why id_object gets its own database field. I mean, is that only for a case where you'd want to mark all notifications from a single topic as read once you read that topic..? But (1) there's no way to be sure the user has read the entire topic anyway, (2) I don't even think the code is currently checking unread notifications for a topic being read. Or is it..?
Why shouldn't id_object be a field? It is the object for which the notification is issued, and has a bunch of uses including multiple handling as @Arantor said. Anything stored in data field is mostly (almost purely) for presentation purposes only. As far as key goes...I forgot :P.

And in my world, there is no "object type". There is the notifications core, and then there are notifiers who do whatever they want. I don't want to abstract it further.
Title: Re: [Plugin] Notifications system (1.0)
Post by: live627 on June 22nd, 2013, 11:33 PM
Quote
And it was a good idea, and it should be extensible. Maybe a crazy plugin author gets a good idea and implements something else for their notifier, it doesn't hurt to keep that option open.
/mechuckles
And that crazy author is me.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on June 23rd, 2013, 01:36 AM
What did you do? :P
Title: Re: [Plugin] Notifications system (1.0)
Post by: live627 on June 23rd, 2013, 04:22 AM
hehehe award image instead of user avatar
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on June 23rd, 2013, 04:45 AM
Yeah, see, that makes sense to me.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on June 24th, 2013, 11:29 PM
Quote from Arantor on June 22nd, 2013, 05:33 PM
It is, under certain conditions. Firstly, the parent class must be loaded (or invokable through an autoloader) before the child class, and secondly whatever invokes the child class will need the full class name and that seems a bit long for the current system.
It's a bit fucked up... First of all, the notifiers aren't auto-loading, they're explicitly loaded and invoked in the initializer, and a hook is used to allow for other notifiers to plug into it. Seriously..? :-/
Wouldn't it be simpler to, I don't know... Force Notification::issue to require getNotifiers('Likes likes') or 'Likes:likes', or whatever structure that calls for the file to be loaded if not already done..?
Same for any situations where the 'likes' notifier is requested, anyway.
Quote
Autoloading is currently tricky because the class names don't really match the filenames but we could do something with it if we wanted.
Hmm, I think they do..? (Once you ucfirst them, of course...)
Quote
See, there's more potential for likes than we're currently using; there's no reason why we couldn't attach likes to media items, for example. I know media items have ranking, but I can see the use for likes on media items as an additional option.
It's actually quite tempting to ditch media ratings in favor of likes, but in that situation, things like fox.noisen.com lose some of their interest, at least for me -- since I like giving very precise ratings to these audio files. But really, in 95% of the cases, a like would even be preferable, because it's much easier to 'grasp' for users, who these days are so incredibly lazy... :P
Quote
Quote
(BTW, I'm surprised that id_object doesn't even get its own key..?!)
It'd be better served as a key on (notifier (6), id_object) really from what I remember.
Dunno if it matters that much -- if id_object is included as "AND id_object ...", MySQL will prioritize the first test (if it's keyed) before this one, right..?
Posted: June 24th, 2013, 11:14 PM
Quote from Dragooon on June 22nd, 2013, 06:13 PM
I want it to be impartial, plus you got a huge freaking gallery inside the forum so I'd doubt that it's just a forum :P.
Oh, you have no idea how many times I regretted including AeMe by default in Wedge... :^^;:
Not because it's not maintainable -- it is -- but for two reasons:
- too many 'loose licenses' in the code, meaning it's currently not compatible with Wedge's, and it will eventually force me to change most of the libraries to make it work, while I wouldn't have had to do it (so soon), had it been a plugin...
- my original plan was to ditch the attachment system, and replace it with media items uploaded to a default, fallback 'attachments' folder. Hmm... Well, I never got around to do that. Too much attachment code, too many things to change entirely. I could have done it at the beginning... But three years later, I'm just wary of huge code overhauls, and I don't see myself ever doing it, which is a pity...
Quote
And it was a good idea, and it should be extensible. Maybe a crazy plugin author gets a good idea and implements something else for their notifier, it doesn't hurt to keep that option open.
And a good idea from John, I think..!
Quote
Notifications.php is the "controller" here, it handles the notifications logic. If you have a better name go ahead, but I've thought a lot about my code and to me it makes perfect sense.
Why is Notifications a plural, and Class-Notification a singular, for instance..? :P
Why is weNotif called that way (Wedge standard is "wefirstletters", all lowercase[1]), and why is it an object to begin with?
Quote
Hm, maybe. Notification::issue kind of made sense to me since Notification class is sort of the "model" for doing the DB interactions with individual Notification rows. Although it can be made different, not sure.
issue_notification, really, and everything that's in the issue method...

Code: [Select]
Notification::issue($id_author, WeNotif::getNotifiers('likes'), $_REQUEST['msg'], array(
'topic' => $topic,
'subject' => $subject,
'member' => array(
'id' => we::$id,
'name' => we::$user['name'],
),
));

Possible change...

Code: [Select]
issue_notification($id_author, 'Likes likes post', $_REQUEST['msg'], array(
'topic' => $topic,
'subject' => $subject,
'member' => array(
'id' => we::$id,
'name' => we::$user['name'],
),
));

With the second member meaning: Likes.php notifier file, 'likes' notifier name, 'post' type.
This way, I can easily do a notification for a thought, with 'Likes likes thought' for instance... Likes.php could then get the 'thought' request, and if it doesn't support it, make a request to something, so that it autoloads it... From a plugin, for instance. But I'm not a specialist of object management, so... It's a bit over my skill level, I guess.
After that, in the database, either store as 'likes_thought', or as 'likes' with an extra field called 'type', as I suggested... Thus, id_object would immediately reflect the type of item it relates to; really, if you want to get all notifications related to post #123, you can't simply search for anything with id_object=123, since this would also return all notifications for thought #123 or media item #123, wouldn't it..?

Ah well...
I'm currently holding my changes, been doing so for a week now, I don't know where to go...

(Mind you, this isn't the only thing I've been stuck on; I'm very, extremely stuck on getting infinite scrolling to work 100%, and despite several internal rewrites, I still can't get it to behave better than it currently does.)

(I made it work on mobile devices, though... Which is an achievement of sorts, but it needs more love :P)

Also, I modified your template to ensure that the periodical e-mail doesn't use any HTML, which it did in your version, hmmm... Who cares, this e-mail has yet to be sent to anyone, anyway... :P
 1. Suggesting we should go for 'wenot', although it carries a bit of negativity... :lol:
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on June 25th, 2013, 12:08 AM
Quote
Hmm, I think they do..? (Once you ucfirst them, of course...)
class Likes_Notifier for example... you'd declare new Likes_Notifier and Likes_Notifier is going to be the information made available to the autoloader. Normally, in autoloading cases you'd have the filename (notifiers/Likes.php) match the class name (Likes_Notifier), which it doesn't.

And an autoloader wouldn't help in wedit, wezip, westr, wesql or similar cases (but most of those aren't dynamically loading) or any similar 5-letter class naming scheme because there's no way to derive the filename from those class names.

And in any case, an autoloader is going to be largely screwed if you want plugins to use them because the plugin folders aren't part of the autoload list. (XenForo solves this aspect by essentially expecting plugins to add to the master Library/ folder so then you have Library/Author/System/Component deals, giving you classes named Author_System_Component, which IMHO is hideously long) And plugins will want to declare their own notifications at times.
Quote
It's actually quite tempting to ditch media ratings in favor of likes, but in that situation, things like fox.noisen.com lose some of their interest, at least for me -- since I like giving very precise ratings to these audio files. But really, in 95% of the cases, a like would even be preferable, because it's much easier to 'grasp' for users, who these days are so incredibly lazy...
I wouldn't ditch them. I'd make it an option. There are times ratings will be useful. There are times likes will be useful. There are times when both are unnecessary.
Quote
Dunno if it matters that much -- if id_object is included as "AND id_object ...", MySQL will prioritize the first test (if it's keyed) before this one, right..?
Sort of. When you're going through and adding clauses in MySQL, you generally want them to follow the ordering of the indexes.

In hindsight my index suggestion was probably naive but I'm kind of thinking on the hoof a bit here. If you have two clauses in a WHERE, MySQL is going to run them in that order. That means if you're doing two WHEREs on table contents, the WHERE clauses should follow the index.

This is kind of a chicken/egg scenario, the query should follow the index but the index is for the query's benefit. What is ideal is the characteristic of selectivity. The more selective something is - i.e. the criteria that lets you select a suitably useful subset of rows with the fewest characters - the better the index will be.

I suggested notification then what the notification is about, because that's logically the direction of selectivity, but I suspect on profiling it, putting the index on id_object then the type of notification (but making the index cover both fields in all cases) would be even better. It's still selecting the same rows, still selecting the same data, but the way it goes about it (which is what EXPLAIN will tell you) is faster. If you have to go through something row by row, better to have the fewest rows possible first.
Quote
Oh, you have no idea how many times I regretted including AeMe by default in Wedge... :^^;:
Not because it's not maintainable -- it is -- but for two reasons:
- too many 'loose licenses' in the code, meaning it's currently not compatible with Wedge's, and it will eventually force me to change most of the libraries to make it work, while I wouldn't have had to do it (so soon), had it been a plugin...
- my original plan was to ditch the attachment system, and replace it with media items uploaded to a default, fallback 'attachments' folder. Hmm... Well, I never got around to do that. Too much attachment code, too many things to change entirely. I could have done it at the beginning... But three years later, I'm just wary of huge code overhauls, and I don't see myself ever doing it, which is a pity...
Do you still want to do it? Most of the licensing issues aren't really issues - the outstanding issues are Exifer/Exifixer/Exifier and JWPlayer. Everything else for that just isn't a problem.

And I suspect it wouldn't be such a big deal to do the replacement - the real issue is doing it for *here* at this point in time. Though I'd suggest refactoring Aeva's code at this point so that action=media itself might still delegate tasks everywhere but it doesn't need to load hundreds of KB of code to do so every time (for busy galleries it can actually hammer a server, to the point where one 'firewall' mod for SMF typically considers it a DOS risk)

If you like, it's something I can tackle when I'm back?
Quote
Why is Notifications a plural, and Class-Notification a singular, for instance..? :P
Why is weNotif called that way (Wedge standard is "wefirstletters", all lowercase[1]), and why is it an object to begin with?
I asked Dragooon about that a while ago. Basically, he wanted to just objectify it because that's how he likes working. There's nothing wrong with that, and it does have some certain conveniences in terms of keeping everything together; it implicitly namespaces everything related to notifications and to a point I'd sort of like to do that with some of the other subsystems in Wedge, e.g. mail sending.

Of course it would also encourage standardisation of the naming system like you wanted to do.
Quote
I'm currently holding my changes, been doing so for a week now, I don't know where to go...

(Mind you, this isn't the only thing I've been stuck on; I'm very, extremely stuck on getting infinite scrolling to work 100%, and despite several internal rewrites, I still can't get it to behave better than it currently does.)
Yeah, I know that feeling. Related, please add my last commit (to Who's Online) to this site... the lack of debugging information is proving a little awkward, especially as I'm fairly sure I've seen two new malware bots hanging around... :/

I do have some of the changes fixed for the /e modifier deprecation but I'm not yet ready to commit them for a similar sort of reason
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on June 25th, 2013, 07:56 AM
Not replying to all, kinda stuck IRL but will reply to a couple of points
Quote
It's a bit fucked up... First of all, the notifiers aren't auto-loading, they're explicitly loaded and invoked in the initializer, and a hook is used to allow for other notifiers to plug into it. Seriously..? :-/
How else do I figure out what all notifiers are available?
Quote
Why is weNotif called that way (Wedge standard is "wefirstletters", all lowercase), and why is it an object to begin with?
You renamed it weNotif, it was WeNotif :P. It's an object because I wuv objects, seriously, it's just a personal preference.
Quote
This way, I can easily do a notification for a thought, with 'Likes likes thought' for instance... Likes.php could then get the 'thought' request, and if it doesn't support it, make a request to something, so that it autoloads it... From a plugin, for instance. But I'm not a specialist of object management, so... It's a bit over my skill level, I guess.
In the process you end up making Notifiers an if/else maze since they will be acting like two notifiers in one (really, likes_thought and likes_post will have almost nothing in common), they're better served as independent notifiers. And I was passing notifier objects because of type strictness and all (less headache and more clean stuff).
Quote
Also, I modified your template to ensure that the periodical e-mail doesn't use any HTML, which it did in your version, hmmm... Who cares, this e-mail has yet to be sent to anyone, anyway... :P
You know right that sendmail function is perfectly capable of handling HTML content, you just need to set the $send_html flag to true? Periodical e-mail already does that and when I received my periodical from localhost it was displaying it fine. Yeah though I wasn't too sure it did any good (the HTML formatting)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on June 25th, 2013, 10:04 AM
Okay, how can I disable infinite scrolling? It's annoying when trying to quick reply.

Oh what the hell, the idea of object type just hit me. One can have likes_post, likes_thought or simply likes with post, thought, media_comment etc. and mentions with different object type. You mind if I do this?
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on June 27th, 2013, 03:22 PM
Quote from Arantor on June 25th, 2013, 12:08 AM
class Likes_Notifier for example... you'd declare new Likes_Notifier and Likes_Notifier is going to be the information made available to the autoloader. Normally, in autoloading cases you'd have the filename (notifiers/Likes.php) match the class name (Likes_Notifier), which it doesn't.
Could easily rebuild the filename from the requested filename, it's just a matter of setting the convention..?
Quote
And an autoloader wouldn't help in wedit, wezip, westr, wesql or similar cases (but most of those aren't dynamically loading) or any similar 5-letter class naming scheme because there's no way to derive the filename from those class names.
I don't think we have the same definition of an autoloader, though... You're talking about spl_autoload_register, right..? I was thinking of something entirely Wedge-made, myself...
Quote
I wouldn't ditch them. I'd make it an option. There are times ratings will be useful. There are times likes will be useful. There are times when both are unnecessary.
Per-album option, then..?
Quote
This is kind of a chicken/egg scenario, the query should follow the index but the index is for the query's benefit. What is ideal is the characteristic of selectivity. The more selective something is - i.e. the criteria that lets you select a suitably useful subset of rows with the fewest characters - the better the index will be.
If I was MySQL (but I am not), I would do it this way...
- gather all of the AND elements in the WHERE.
- go through each of them, see if there's a matching index, if yes, use index, otherwise, skip the test for now, and look for the next test that has an index.
- when the loop is finished, if it's still TRUE, then go back to the beginning, and run a regular unindexed sort on all remaining entries, until we get FALSE.

Wouldn't it, err... Make more sense, to run queries this way...?
But as I said, I'm not MySQL... These guys probably know better than me, don't they..?
Quote
I suggested notification then what the notification is about, because that's logically the direction of selectivity, but I suspect on profiling it, putting the index on id_object then the type of notification (but making the index cover both fields in all cases) would be even better. It's still selecting the same rows, still selecting the same data, but the way it goes about it (which is what EXPLAIN will tell you) is faster. If you have to go through something row by row, better to have the fewest rows possible first.
I think I'll let you work on this one, hmm... ;)

Of course, ideally the notif table would be as empty as possible, so it wouldn't be much of a problem. (Thinking about it, do we have an OPTIMIZE query for this one at any point..? Because, I don't think we do, and it'd be useful...)
Quote
Do you still want to do it?
Do what? Ditch attachments..? Nope. I'm always thinking, "this is Wedge 2.0 material... I'll just write a frigging script to convert all current attachments to media items, and be done with it..."
Quote
Most of the licensing issues aren't really issues - the outstanding issues are Exifer/Exifixer/Exifier and JWPlayer. Everything else for that just isn't a problem.
Yes, true enough, I fixed the Highslide license issue years ago, but it took me weeks of work to rebuild my very own (Zoomedia), and I'm not looking forward to rewriting the rest of the code for our licensing purposes, I reckon...
Quote
And I suspect it wouldn't be such a big deal to do the replacement - the real issue is doing it for *here* at this point in time. Though I'd suggest refactoring Aeva's code at this point so that action=media itself might still delegate tasks everywhere but it doesn't need to load hundreds of KB of code to do so every time (for busy galleries it can actually hammer a server, to the point where one 'firewall' mod for SMF typically considers it a DOS risk)
Hmm...? There's a mod for SMF that advises against using AeMe..? Really?
Quote
If you like, it's something I can tackle when I'm back?
As I told you before (I think), I always feel like it's "my area", but since I haven't even started work on it yet, feel free to do it, after all, I can always alter your changes later, it's really not a big deal, and it'd get me started, I guess...! (Replacing JWPlayer with MediaElementJS is something that really scares the bejesus out of me, I'll admit... And it's not even that big a deal, I suppose...)
Quote
Yeah, I know that feeling. Related, please add my last commit (to Who's Online) to this site... the lack of debugging information is proving a little awkward, especially as I'm fairly sure I've seen two new malware bots hanging around... :/
I think I did it a few days ago, I'm not 100% sure though, I'll have to check...
Quote
I do have some of the changes fixed for the /e modifier deprecation but I'm not yet ready to commit them for a similar sort of reason
This one really annoys me... Especially since we can't do anonymous functions, because PHP 5.3, while the most commonly used version, is still not overwhelmingly spread, and PHP 5.2 is still installed on something like 43-45% of all servers, so it's a no-no, unfortunately...
Posted: June 27th, 2013, 03:06 PM
Quote from Dragooon on June 25th, 2013, 07:56 AM
Quote
Why is weNotif called that way (Wedge standard is "wefirstletters", all lowercase), and why is it an object to begin with?
You renamed it weNotif, it was WeNotif :P. It's an object because I wuv objects, seriously, it's just a personal preference.
Ah, yes, I remember now... Well, I guess I tried to make a concession, by only lowercasing the first letter, and not all, and I figured renaming it to 'wenot' immediately might confuse you, I guess...
Quote
In the process you end up making Notifiers an if/else maze since they will be acting like two notifiers in one (really, likes_thought and likes_post will have almost nothing in common),
Hmm, they do..?
Okay, let's take Likes.php as an example... I'll list the functions that are currently in my own version (I removed getPreview, as it's for topics only, will re-add it to non-topics, I guess...!)

   public function getURL(Notification $notification) --> this could be handled by the parent, for topics by default...
   public function getName() --> all right...
   public function getText(Notification $notification, $is_email = false) --> this one's pretty mandatory (see how I added $is_mail, too?)
   public function handleMultiple(Notification $notification, array &$data, array &$email_data) --> does it really need to be in here..?
   public function getProfile($id_member) --> this is going to be the same everywhere, maybe we could just standardize it...? $txt['we' . getName() . '_likes'], etc...? That way, we can put it in the main object... Yeah, that'd be nice...
   public function getEmail(Notification $notification, array $email_data) --> same here, could be standardized, I say...

All in all, there's not much to be done in a notifier, and if you're writing a notification for a thought, I'd even dare to say, it's the same: you'd want to have the 'default' code in a parent object, and the actual notifiers would just need to trust it, is all.
The issue, though, might be to create a new parent object for thought notifications, or media notifications, and I don't know how best to do that, especially, hmm, if we can't determine first-hand what type the notification is, can we...?
Quote
they're better served as independent notifiers. And I was passing notifier objects because of type strictness and all (less headache and more clean stuff).
Less headaches for you, maybe... ;)
Quote
You know right that sendmail function is perfectly capable of handling HTML content,
Yes...
Quote
you just need to set the $send_html flag to true? Periodical e-mail already does that and when I received my periodical from localhost it was displaying it fine. Yeah though I wasn't too sure it did any good (the HTML formatting)
http://academy.hubspot.com/blog/bid/109587/
Things like that...
Basically, HTML e-mail = higher chance of being flagged as spam, and thus not seen by the end user.

HTML e-mail is fine for a marketing newsletter campaign, where you're precisely trying to attract someone's attention, but it's no good for something that's utilitarian, and it also makes it look less 'personalized', because you can't help thinking that it's an advertisement, even if it isn't.

Also, HTML takes more bandwidth (for your server...!), tends to irritate people more, is less accessible, and is opened to tons of issues, like you have to do inline styles, which I hate, etc, etc...

All in all, it's really best to do plain text, and I'm going to go that way, but I'll still keep the HTML versions anyway, so if in the future you want to offers users to ability to choose between plain text and HTML (with plain text as default), you can always do it... Okay?
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on June 27th, 2013, 03:23 PM
Quote from Dragooon on June 25th, 2013, 10:04 AM
Okay, how can I disable infinite scrolling? It's annoying when trying to quick reply.
Just... Don't keep moving your mouse wheel more than a couple of times after you reach the end of the page..?
Quote
Oh what the hell, the idea of object type just hit me. One can have likes_post, likes_thought or simply likes with post, thought, media_comment etc. and mentions with different object type. You mind if I do this?
I have no idea what you're suggesting, but I'm open to anything you have to offer...? Can you elaborate, then..?
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on June 27th, 2013, 06:31 PM
Quote
I don't think we have the same definition of an autoloader, though... You're talking about spl_autoload_register, right..? I was thinking of something entirely Wedge-made, myself...
Um... kind of. Let me walk you through what an autoloader actually does (and it's irrelevant whether it's __autoload or spl_autoload_register)

An autoloader is something that does the loading of a class when it's called for without explicitly loading it. You just say $var = new Classname(); and if it is already loaded, fine, if not the autoloader function is called, and given the Classname and essentially expected to figure out how to load it.

__autoload is the simple version, one function, application-wide, that will receive every call for a class that doesn't currently exist. spl_autoload_register is the swish version, which lets you declare multiple functions that act as __autoload functions. They will be called in order and attempt to find the class name. Bear in mind plugins may well want to declare autoloaders. I'm strongly thinking of making SSI auto-loader driven rather than including every function every time it's used.

Short of having a list of names to files (e.g. wedit => Class-Editor.php), there's simply no way to do it. If we're going to do *that* we might as well not bother with an autoloader in the first place.
Quote
Per-album option, then..?
I was actually thinking per installation myself.
Quote
If I was MySQL (but I am not), I would do it this way...
- gather all of the AND elements in the WHERE.
- go through each of them, see if there's a matching index, if yes, use index, otherwise, skip the test for now, and look for the next test that has an index.
- when the loop is finished, if it's still TRUE, then go back to the beginning, and run a regular unindexed sort on all remaining entries, until we get FALSE.

Wouldn't it, err... Make more sense, to run queries this way...?
But as I said, I'm not MySQL... These guys probably know better than me, don't they..?
There are surprisingly few cases where every AND criteria will be matched on a single index, at least not when you get multi-table queries going. Part keys can be useful too.
Quote
Basically, HTML e-mail = higher chance of being flagged as spam, and thus not seen by the end user.

HTML e-mail is fine for a marketing newsletter campaign, where you're precisely trying to attract someone's attention, but it's no good for something that's utilitarian, and it also makes it look less 'personalized', because you can't help thinking that it's an advertisement, even if it isn't.
I'd be inclined to leave it out for this purpose but perhaps include it for newsletters. Technically, though, if you include both versions (which IIRC you're actually supposed to do for specification compliance) it seems less likely to be misread as spam. But then you're including two copies per email which is not cheap (and the poor mail queue gets hammered even more by it because the bulk insert can't do so many per cycle)[1]
 1. For those not familiar with how the mail queue works, it does something unlike every other insert in SMF/Wedge. Most inserts, even those with bulk lines to insert, are never going to get anywhere near the query packet limit, but the mail queue insertion is potentially very likely to do so. So the query is built bit by bit and then thrown as a single big-ass query, typically around 800KB in length, into the queue table. I have to admit I was impressed when I first saw it because it's showing an awareness of a problem most people don't even know exists.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on June 27th, 2013, 11:11 PM
Autoload: we could easily call the file Class-wedit.php, it doesn't look as good, but it's not much of a problem..?

Per-album: it's the best solution, because one might want to have 'likes' on a regular album, and 'ratings' on an audio album with multiple tracks that the author wants to know who prefers what, etc... (I feel like I made a grammar mistake in that sentence, hmm...)

AND: see, that's where it goes right over my head... I still have no clue about how indexes work, and not that I care much, either...

Email: let's start with just raw text e-mails, all right..?



Other: I'm still lost with these notification objects... An example: getProfile.
Notifications.php does $subscriber->getProfile($memID).
Class-Notifier.php does abstract public function getProfile($id_member)
Likes.php does public function getProfile($id_member), but it doesn't use $id_member...
Notifications.template.php does weNotif::getNotifier($notifier)->getProfile() (without an id_member)...

All in all, I'm a bit lost, but it gets worse with this: I need to have a 'default' getProfile, and I don't know how to declare it.
I changed Class-Notifier's to remove the abstract keyword, but I need to access the current notifier's name, so how do I do it..?

Code: [Select]
public function getProfile($id_member)
{
$name = $notification->getName();
...
}

Where do I get $notification from..? Do I have to declare it in the param list, and then get rid of $memID and stuff which is never used anyway, or... What?

How complicated...

But, my ultimate goal is to enable notifications to be as simple as possible, and this is one of the obstacles on my way. ;)
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on June 27th, 2013, 11:22 PM
Quote
Autoload: we could easily call the file Class-wedit.php, it doesn't look as good, but it's not much of a problem..?
Yeah, we'd just have to do that for all the classes.
Quote
Per-album: it's the best solution, because one might want to have 'likes' on a regular album, and 'ratings' on an audio album with multiple tracks that the author wants to know who prefers what, etc... (I feel like I made a grammar mistake in that sentence, hmm...)
(There's no grammar mistake I can discern) I can get behind that.
Quote
AND: see, that's where it goes right over my head... I still have no clue about how indexes work, and not that I care much, either...
If you wanted to know, have a read of "High Performance MySQL" (2nd edition at least) by Baron Schwartz and others, it's pretty awesome. But very specialised and generally not worth worrying about too much. Basically, as long as a query doesn't generally have a filesort in it when EXPLAINed.
Quote
Email: let's start with just raw text e-mails, all right..?
Works for me.
Quote
I changed Class-Notifier's to remove the abstract keyword, but I need to access the current notifier's name, so how do I do it..?
That would make sense, but how often are we actually going to see notifiers that actually have commonality?

As far as current notifier's name, get_class($this) seems the likely candidate?
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on June 27th, 2013, 11:43 PM
- Is it feasible?

- It just needs implementing... And, well... As usual, AeMe is not my priority..... :^^;:

- I hate the very fact that explains are so obscure to a non-specialist, to me it makes no sense, and thus has limited interest.

- Good!

- Hmm, I just realized how silly my question was... Doing $this->getName() was enough, uh. Now, if I could understand why getData and getObject are defined in Class-Notification, rather than Class-Notifier..? Shouldn't notifiers be allowed to tamper with these, in the first place..?
Title: Re: [Plugin] Notifications system (1.0)
Post by: Arantor on June 27th, 2013, 11:51 PM
Quote
- Is it feasible?
Sure it is. We just rename the classes, declare an autoloader, and do a loadSource on the name if it fits our normal naming convention (or the file exists with the right name). We just get to stop doing loadSource() explicitly everywhere, which is kind of nice.
Quote
- I hate the very fact that explains are so obscure to a non-specialist, to me it makes no sense, and thus has limited interest.
They're obscure, sure, but they contain a *lot* of information in a small space, which is why they're obscure.

As for notifications, @Dragooon is probably the person to ask.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on June 28th, 2013, 02:43 PM
I'm replying in random order, too much stuff. If I miss something let me know.
Quote
- Hmm, I just realized how silly my question was... Doing $this->getName() was enough, uh. Now, if I could understand why getData and getObject are defined in Class-Notification, rather than Class-Notifier..? Shouldn't notifiers be allowed to tamper with these, in the first place..?
The plugin handling notifier sends the data anyway, so notifier doesn't need to really tamper once the notification is issued. Plus you can't change ID of the object in any case, since that's fixed for a notification.
Quote
I have no idea what you're suggesting, but I'm open to anything you have to offer...? Can you elaborate, then..?
I'm suggesting what you've been saying, implement a new column called object_type and add that to the classes. Basically, that way likes can be for thought, post, media, media_comment etc. and similarly mentions can be for thought, post etc. without having multiple notifiers.
Quote
Just... Don't keep moving your mouse wheel more than a couple of times after you reach the end of the page..?
Very hard to do on a trackpad :P
Quote
I changed Class-Notifier's to remove the abstract keyword, but I need to access the current notifier's name, so how do I do it..?
Wait, what? Why? That's an abstract class, I'm fairly sure it'll cause a fatal error if you just remove the abstract keyword...even if it doesn't have any abstract function it'll always be an abstract class. It's not supposed to be created as an instance.
Quote
All in all, there's not much to be done in a notifier, and if you're writing a notification for a thought, I'd even dare to say, it's the same: you'd want to have the 'default' code in a parent object, and the actual notifiers would just need to trust it, is all.
If code can be standardised, go for it. However we can always leave them if a notifier needs to do something. getProfile also handles profile configuration fields (which I actually use for some of my notifiers), so it's useful for that but can have a default implementation I guess.
Quote
The issue, though, might be to create a new parent object for thought notifications, or media notifications, and I don't know how best to do that, especially, hmm, if we can't determine first-hand what type the notification is, can we...?
No point, really. There will be slight differences which need to be handled on a per notifier basis, I think it's best to abstract things into notifier and type and let notifier handle everything instead of standardising too much stuff.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on June 28th, 2013, 02:47 PM
Dragooon, can you tell me something? It may not be you, maybe someone else, but I noticed that one notification type (the default, I think) uses id_object to store the topic ID and $data['id_msg'] to store the message ID, while another uses id_object for id_msg, and $data['topic'] (in mentions, I think) for id_topic...

That doesn't make sense to me, because both point to a post, so which is the better one..? Which makes the most sense, in id_object? I would say id_msg, right..? But that's not the default...
Title: Re: [Plugin] Notifications system (1.0)
Post by: Dragooon on June 28th, 2013, 02:53 PM
It's on a case by case basis, it has nothing to do with what's the best behaviour, mostly related to how one would want to be notified if they're to be notified multiple times on that object (handleMultiple)

Basically, in a case of mentions, I always believed any individual post having a mention should be told to the user in a separate notification, hence I chose id_msg as the object. However in the case of Quote, I thought it's enough to be notified once in the topic that you've been quoted and then the guy can catch up with the rest. I'm not sure what Likes does since I didn't code that one, although I guess it should be on a per-msg basis and use id_msg as the object. The counterpart is stored in $data mostly for generating the complete URL.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on June 28th, 2013, 03:05 PM
Quote from Dragooon on June 28th, 2013, 02:43 PM
I'm replying in random order, too much stuff. If I miss something let me know.
I don't re-read my old posts, so it doesn't matter, when I need to make a decision, I'll just go with my instincts, if you didn't comment on it earlier.
Quote
The plugin handling notifier sends the data anyway, so notifier doesn't need to really tamper once the notification is issued. Plus you can't change ID of the object in any case, since that's fixed for a notification.
I'm using get_class, as per Pete's suggestion (more specifically, get_class($this), since get_class() returns the parent' class.)

I'm still not sure why we should provide a Notification object to so many notifier methods, since AFAIK, we could easily store that in the notifier object's properties themselves, couldn't we..??
Actually, I'd be inclined to do that, in the first place...

Also, I'll be doing things this way, I think...:

- Notifier (for posts)
  - Likes_Notifier
  - Notifier_Thoughts (for thoughts)
     - Likes_Notifier_Thoughts (or something in that line, for thought likes.)

Notifier_Thoughts can basically inherit from Notifier, and override all functions that directly use a post ID, and feed them a thought ID instead, making it simpler to inherit when you create a new notification, e.g. 'reply to your post', and 'reply to your thought', one would just need to create an additional notifier object, and inherit from the other class, simple enough... Or is it incorrect in how it's thought out..? I don't think it is, but you tell me.
Quote
I'm suggesting what you've been saying, implement a new column called object_type and add that to the classes.
It took me a long time to bring myself to do without a 'type', and now you're going my way instead... Hold your horses, we might be missing ourselves on our way to the other side... :lol:
Quote
Basically, that way likes can be for thought, post, media, media_comment etc. and similarly mentions can be for thought, post etc. without having multiple notifiers.
But why did you change your mind? I can live with having to rewrite some of my code (the stuff I explained just above isn't implemented yet, I'm only finishing up the e-mail stuff), but I'd like to know if you know what you're doing.. ;)
Quote
Quote
Just... Don't keep moving your mouse wheel more than a couple of times after you reach the end of the page..?
Very hard to do on a trackpad :P
I have no idea how that works, to be honest... Didn't even know they had wheels, uh..?
Quote
Wait, what? Why? That's an abstract class, I'm fairly sure it'll cause a fatal error if you just remove the abstract keyword...
Nope... I'm not an OO specialist, but abstract just means "this is a method that the child MUST declare", right..? Just removing the abstract keyword, and writing your own code in the first place, just turns it into a placeholder, which you can decide to override, if you want, but nothing more...
Quote
even if it doesn't have any abstract function it'll always be an abstract class. It's not supposed to be created as an instance.
I'm not sure why these classes are abstract in the first place, but with my rewrite, Notifier no longer has any abstract methods, meaning it can be a 'regular' class, and I'm not sure what the difference is, so if you could tell me before I commit... Is it okay, if I remove the abstract keyword for it, perchance..?[1]
Quote
If code can be standardised, go for it.
I'm working hard on that, but it's all to make creating notifications easier, really.
Ideally, a "basic" notification wouldn't need any contents in its class (except for loading language files in the constructor), and could simply get away with a Notification::issue call, and the appropriate language strings.
 1. Somehow, yesterday's link reawakened some long gone words in my mind... :lol:
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on June 28th, 2013, 03:08 PM
Quote from Dragooon on June 28th, 2013, 02:53 PM
It's on a case by case basis, it has nothing to do with what's the best behaviour, mostly related to how one would want to be notified if they're to be notified multiple times on that object (handleMultiple)
Oh, speaking about that one... All of the notifiers I've got under my hand, return false for this, so I changed the default value to return false, this way, we don't have to reproduce the method every time, eh eh...
Quote
Basically, in a case of mentions, I always believed any individual post having a mention should be told to the user in a separate notification, hence I chose id_msg as the object. However in the case of Quote, I thought it's enough to be notified once in the topic that you've been quoted and then the guy can catch up with the rest.
I don't have your Quote notifier, so I don't know what it has, and you'll have to rewrite it yourself, I guess...
Quote
I'm not sure what Likes does since I didn't code that one, although I guess it should be on a per-msg basis and use id_msg as the object. The counterpart is stored in $data mostly for generating the complete URL.
So I should use id_msg in id_object by default, right..?
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on June 28th, 2013, 09:15 PM
Notification code is up and fixed, I also fixed the Move thing, which had a broken preview, even in the SVN version.

Oh, and btw... Why is the Move notifier called 'move', when others have a 's' at the end..? (likes, mentions)
We should standardize these, too...

And also strtolower the entire class names, because, well, I'm definitely not into these...
Title: Re: [Plugin] Notifications system (1.0)
Post by: live627 on June 29th, 2013, 01:42 AM
Quote
Oh, and btw... Why is the Move notifier called 'move', when others have a 's' at the end..? (likes, mentions)
I  named  it but  you can change it. My reasoning was that it always notifies on a single topic because one topic was moved at a time.
Title: Re: [Plugin] Notifications system (1.0)
Post by: Nao on June 29th, 2013, 11:47 AM
I'm more suspicious of adding an 's' than removing it, actually... ;)
"It's a move", "it's a like", etc...