MyBBStuff / MyAlerts

A simple notification/alert system for MyBB similar to IPB and XenForo's core implementations of user notifications.
http://www.euantor.com/myalerts/
47 stars 33 forks source link

incorrect error when user doesn't have proper permissions to view thread #121

Closed andrewjs18 closed 8 months ago

andrewjs18 commented 9 years ago

Hi,

If a user receives an alert for a post they were tagged in from the MentionMe plugin when they don't have the proper permissions to view said thread/post, it displays an incorrect error, IMHO:

136 may 20

Thinking about it though, shouldn't MyAlerts block all alerts if a user doesn't have the proper permissions to view the alert in the first place? The last thing we'd want is for a mod/admin to start a thread discussing a user and the user starts getting alerts that someone has been talking about them in a thread that they can't view.

euantorano commented 9 years ago

Hi,

This is supposed to be managed by the plugin that integrates with MyAlerts. There's no way of knowing what an alert is related to (eg: post, thread, forum) unless it's a core alert type so it's difficult to check permissions.

On 20 May 2015, at 07:08, andrewjs18 notifications@github.com wrote:

Hi,

If a user receives an alert for a post they were tagged in from the MentionMe plugin when they don't have the proper permissions to view said thread/post, it displays an incorrect error, IMHO:

Thinking about it though, shouldn't MyAlerts block all alerts if a user doesn't have the proper permissions to view the alert in the first place?

— Reply to this email directly or view it on GitHub.

andrewjs18 commented 9 years ago

Ah, I wasn't sure if this was an issue with MyAlerts or MentionMe. Thanks for the reply..

how's the update going?

WhiteNeo commented 9 years ago

Indeed, if you alert someone and the post or thread was removed then alert appears because this is done, but if you alert someone with mentions is more dificult to set an extra query to get forumpermissions and then select only users with permissions, then set on array and finaally do the job, the charge and ammount of unnecesary sentences can be more than you think.

If you named someone on a private forum, then this user have a knowledge he was named on forum maybe for a ban or inadecuate use of forum, or because it'a a goo user, maybe you can inform of this or won't but the mention is there and username is there, so why don't you remove the mention maanuaally of this user or use a mention if you don't want to that user receive an alert for that it's so weird to use an alert to not use or you can disable alerts for this user, or something like that, because there are no sence to use it if you don't want to use by forum :dancer:

andrewjs18 commented 9 years ago

well, hopefully Euan will get 2.0 released as stable within the next few weeks or so. once that's done, I hope Devilshakerz can come up with a good solution to the permissions stuff once he integrates dvz mentions with myalerts 2.0

Sama34 commented 9 years ago

Unsure if this is about the dev 2.0 version, but it does stores the type ID (rep, pm, etc) and alert ID (tid, pid, pmid, etc) for each alert: https://github.com/MyBBStuff/MyAlerts/blob/develop/inc/plugins/myalerts.php#L62

So it should be possible to filter out items based off permissions for core alert types. For plugins like MnetionMe/DVZ Mentions a hook should be offered to equally be able to filter out items within the query from plugin alert types.

Of course the third-party plugin can simply do the work but it will feel incorrect to me, IMO.

euantorano commented 9 years ago

Yes, that's what I mean Omar. MyAlerts can't do the actual checks, but it should be easy for developers to hook in and do them.

On 27 May 2015, at 06:48, Omar Gonzalez notifications@github.com wrote:

Unsure if this is about the dev 2.0 version, but it does stores the type ID (rep, pm, etc) and alert ID (tid, pid, pmid, etc) for each alert: https://github.com/MyBBStuff/MyAlerts/blob/develop/inc/plugins/myalerts.php#L62

So it should be possible to filter out items based off permissions for core alert types. For plugins like MnetionMe/DVZ Mentions a hook should be offered to equally be able to filter out items within the query from plugin alert types.

Of course the plugin can simply do the work but it will feel incorrect to me, IMO.

— Reply to this email directly or view it on GitHub.

andrewjs18 commented 9 years ago

I believe dvz mentions might be more efficient than mentionme...1 file vs a bunch. @Sama34, could you assist Devilshakerz with adding myalerts support to his plugin?

Sama34 commented 9 years ago

Honestly I lack the time to check how 2.0 works since that is what I most likely will use once I install this in my forum. For MyAlerts 1.6 it was pretty easy though.

andrewjs18 commented 9 years ago

@euantorano is it similar to add support between the two versions?

euantorano commented 9 years ago

It's slightly different, but should be easier for 2.0.

On 27 May 2015, at 07:17, andrewjs18 notifications@github.com wrote:

@euantorano is it similar to add support between the two versions?

— Reply to this email directly or view it on GitHub.

andrewjs18 commented 9 years ago

ok, good. this should make it easy to be added to dvz mentions when myalerts 2.0 is released as stable. =)

euantorano commented 9 years ago

I'm going to postpone this to v2.1.0 and look at better permissions checking then.

andrewjs18 commented 9 years ago

no problem!

lairdshaw commented 8 months ago

I'm going to postpone this to v2.1.0 and look at better permissions checking then.

Some improved permission checking was introduced a while back by #238, and much more recently in the yet-to-be-merged #325. Is there anything more to do, or can this issue (which otherwise refers to third-party plugins) be closed?

euantorano commented 8 months ago

I think this can be closed personally. On 13 Mar 2024, at 09:18, Laird @.***> wrote:

I'm going to postpone this to v2.1.0 and look at better permissions checking then.

Some improved permission checking was introduced a while back by #238, and much more recently in the yet-to-be-merged #325. Is there anything more to do, or can this issue (which otherwise refers to third-party plugins) be closed?

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

lairdshaw commented 8 months ago

Done again! We're whittling down the remaining milestone issues nicely.

While I have your attention: I've created a bunch of PRs, which I welcome you (and anyone else who's interested) to look over and comment on or critique if you want to. I understand though that your time is limited, so if nobody has any comment or objection after a week or two, I'll simply merge them myself. Let me know whether you want me to hold off for longer than a week or two. (There is probably at least one more PR to come based on the remaining milestones, to which the same applies).

euantorano commented 8 months ago

Thanks, I’ll try give them a look over this week and post any feedback that I think of. On 13 Mar 2024, at 09:59, Laird @.***> wrote: Done again! We're whittling down the remaining milestone issues nicely. While I have your attention: I've created a bunch of PRs, which I welcome you (and anyone else who's interested) to look over and comment on or critique if you want to. I understand though that your time is limited, so if nobody has any comment or objection after a week or two, I'll simply merge them myself. Let me know whether you want me to hold off for longer than a week or two. (There is probably at least one more PR to come based on the remaining milestones, to which the same applies).

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>