MyAlerts v3
#1
I've been meaning to revisit MyAlerts and tidy it up a bit and iron out some of the long standing bugs in the code. I started that process today, and wanted to create this development thread to get some feedback from users about the changes I'm making for the plugin.

Changes

Not all of these are implemented yet, but they're planned.
  • Simpler code structure: This will break third party plugins integrating with MyAlerts (which is why this is v3, not just v2.1 or something like that). It also means the code is easier to read and follow, and hopefully less prone to bugs.
  • Mark alerts as read when viewing associated items. For instance, if you get an alert for a PM and read the PM, the alert is also marked as read.
  • Send alerts for PMs sent by the MyBB system, not just by users. The code now uses the "datahandler_pm_insert_end" hook for PM alerts.
  • Show only unread alerts in the modal box that opens when you click the count in the header.
  • Background polling via AJAX for new alerts, updating the count in the header, and updating the page title with the unread count - some users have requested an option for an audible noise when there's a new alert in the past - do people still want this?
  • Better de-duplication of alerts, preventing getting multiple alerts for one event.
If anybody has any (sensible) suggestions for other changes or improvements they'd like to see, now is the time to let me know. I can't guarantee I'll get time to implement all suggestions, but I'll do my best to implement what I can.

I also can't make any promises about when v3 might be finished, but so far I've got alerts for buddy requests and PMs working.

Development is currently taking place in a separate repository, which can be found here: https://github.com/MyBBStuff/myalerts-redux

I'm planning on creating actual releases for v3 when it's ready, and looking after it a bit better (and putting it on the mods site too).
Reply
#2
Suggestions.


1. Follow MyBB standards for structure. /alerts.php /inc/functions_alerts.php / inc/class_alerts.php / inc/plugins/myalerts.php and the module in acp and the lang file too. This may also make it easier to make it a 1.10x built-in feature.

2. Not sure you should hook into the reading of PMs to mark an unread alert but I suppose that's fine. I'm iffy on the alarm audible sound but consider the option or at least make it so it's easy to implement via plugin or small code change.

3. Ajax polling is definitely a must imho. Have a setting for the time with maybe default 10 seconds.

4. Have the alerts table in innodb at least but consider memory too.

5. For reps there is no extra_details. At least have it say whether the rep was Nuetral, Negative or Positive. It doesn't have to give the reputation though. Also I haven't tested but does the alert show for an updated rep?

6. Make sure all the elements have CSS definitions editable in alerts.css. There are some that are missing.

7. Alerts curently work from the users table. I think that's iffy too. I'd rather have a separate table that gets a query. User table already has a ton of columns and adjustments to it can be bothersome.

8. Add ability for group permissions. So that you can allow/disallow certain groups from using it.

9. It appears you can get alerts on forums the member does not have permission to access. Example is if you move a thread to a forum the member can't view. They still get alerts.

10. I noticed in some of the code you reuse hook names. Unsure if that is causing some bugs with plugins but it might and it's probably not the best way to do that. I wish MyBB would never use the same name. Hooks for plugins should be named similar to the plugin for obvious uniqueness.

I think that's enough for now. Only took me 5 minutes to come up with all that off the top of my head. I'm sure there is more.
Reply
#3
1. I personally find it easier to have plugin files all inside inc/plugins as it makes removing a plugin far easier. Having files scattered about the filesystem makes life more difficult when trying to purge a plugin. If you look at the new repository I'm working on, I have simplified the structure a lot, but most things are still inside the inc/plugins folder.

2. Yeah, it should definitely b an option for the sound. I'm not sure on it either, but people have expressed interest for it in the past.

4. The current re-worked code doesn't set a table engine when creating the table, so it'll default to the default table engine for your database. I have no idea why the old code specified the engine.

5. I can certainly look at that. The problem I've found with rep and buddylist so far is that you have to duplicate some queries due to the way that some of the hooks work in MyBB without details being global or passed to the hook.

6. For the most part I strive to re-use MyBB CSS classes, but any that I do add I can always add as comments at least to the custom CSS file.

7. It used to be a separate table. The reason for putting it in the users tbale was to save a query (rather than 2 global queries to get disabled alert types and number of alerts, it uses 1 to get the numebr of alerts as the disabled alerts are already loaded into "$mybb->user"). I can change that I guess, but it's worth knowing the reasoning behind it.

8. Sure, a simple permission for "can use alerts" should be easy to add.

9. Yep, this needs improving for sure.

10. I'll have to check the hooks, there are probably some redundant ones there anyway that can be removed.
Reply
#4
Quote:1. I personally find it easier to have plugin files all inside inc/plugins as it makes removing a plugin far easier. Having files scattered about the filesystem makes life more difficult when trying to purge a plugin. If you look at the new repository I'm working on, I have simplified the structure a lot, but most things are still inside the inc/plugins folder.

I think it's rare that plugins get removed once they are installed into a forum. You as a developer probably do it much more often than a forum admin would. I agree that it's easier to remove if it's all there but imho it's not common structure. If all plugin authors were either forced or agreed to such an implementation I think that would suffice. But I'm probably just arguing a personal preference and something that's not very important in the end.

Quote:5. I can certainly look at that. The problem I've found with rep and buddylist so far is that you have to duplicate some queries due to the way that some of the hooks work in MyBB without details being global or passed to the hook.

Someone on the dev team should fix that (wink wink). But also maybe you can look into using a different hook. I gotta think with reps at least it goes to a function that knows what the input was. I haven't spent huge time looking into your code.

Quote:7. It used to be a separate table. The reason for putting it in the users tbale was to save a query (rather than 2 global queries to get disabled alert types and number of alerts, it uses 1 to get the number of alerts as the disabled alerts are already loaded into "$mybb->user"). I can change that I guess, but it's worth knowing the reasoning behind it.

I understand the reasoning. But for larger boards any changes to posts or users table can be a problem. So let's say you update or expand the plugin having it inside users table can be an issue. Let's say I extend the plugin, now I have to do a huge query into myalerts_disabled_alert_types column. Like if I want to have it disabled for all users. On a note my users table is 600k entries and 400Mb in size.

btw, I probably sound like this is all about me. I know my case is special and I'm not making demands. Only expressing my view and wishes and providing feedback from my perspective.

EDIT: Also a member mentioned that avatars show in the notifications even if you have the Display Users Avatars disabled. You should fix that imho.
Reply
#5
Regarding rep and buddylist: you'd think it would be a function, but unfortunately not. Buddylist is especially messy. We can change it in the core, but MyAlerts aims to be compatible with most versions of 1.8, so it'd need to continue to duplicate queries and have a version check to handle new versions sanely. It's not out of the question, just a bit of a pain.

I'll have a look at moving the settings into a separate table. That's how they were stored originally anyway, and it offers greater flexibility for the future.

Regarding avatars, I'll definitely look into that as I hadn't thought about that setting.
Reply
#6
Add a support to set default settings for alerts - now you can only enable or disable alerts in ACP, but you cannot set the default state of te specific alert type - for example enable alert for a new PM but leave it OFF for users by default.
[MyBB 1.8 Czech translation] [MyBB 1.8 plugins]: Prune old PMs + optimize DB plugin --- Thank you/like system
Reply
#7
(2018-04-02, 04:29 PM)Eldenroot Wrote: Add a support to set default settings for alerts - now you can only enable or disable alerts in ACP, but you cannot set the default state of te specific alert type - for example enable alert for a new PM but leave it OFF for users by default.

How would that be expected to work with users aready registered on the board before the plugin is installed? Should they receive all enabled alerts?
Reply
#8
(2018-04-02, 05:38 PM)Euan T Wrote:
(2018-04-02, 04:29 PM)Eldenroot Wrote: Add a support to set default settings for alerts - now you can only enable or disable alerts in ACP, but you cannot set the default state of te specific alert type - for example enable alert for a new PM but leave it OFF for users by default.

How would that be expected to work with users aready registered on the board before the plugin is installed? Should they receive all enabled alerts?

It depends... it would be the easiest way
[MyBB 1.8 Czech translation] [MyBB 1.8 plugins]: Prune old PMs + optimize DB plugin --- Thank you/like system
Reply
#9
Making it use default classes instead of how it is now should make it easier for adaptations to design.
Reply
#10
I do feel that we in MyAlerts v3,
- the quote functionality should be modified as I have seen in some forums that users with spaces/special characters not getting Alerts. This should be looked into MyAlerts v3

- Live Alerts feature to be shown on all pages just like in Xenforo alerts, I feel that should become core for MyAlerts. Very nice feature to have

overall I am very much satisfied with MyAlerts Smile
Reply


Forum Jump:


Users browsing this thread: 1 Guest(s)