Adds attribute mutation if suspended. #30
Conversation
Also refactored to newer extenders.
| function (Dispatcher $events) { | ||
| $events->subscribe(Listener\AddUserSuspendAttributes::class); | ||
| $events->subscribe(Listener\RevokeAccessFromSuspendedUsers::class); | ||
| (new Extend\Event()) |
There was a problem hiding this comment.
Do we need / want to create whole classes for these new listeners? They seem small / simple enough that they could just be closures, and don't need any DI
Consider this to be a comment / question rather than a "changes request". Both options have merits, although from a modularization perspective I think I prefer separate classes as done here. Just raising this as a potential point of discussion
There was a problem hiding this comment.
I personally do like using invokable classes instead, I'm not a big fan of having logic code in the extend.php file.
|
This does mean the suspended user won't also be able to edit their notification preferences (especially email notifications). |
|
Should suspended users even get email notifications? |
|
I'd say yes, especially if you want to notify them of their suspension/unsuspension. Btw, what I'm referring to above especially is that suspended users not being able to disable email notifications would be an issue. |
|
Hi, waiting for this fix. I want try but seems the fix should be updated for Beta 15 as is not seems to be working. |
In that case, perhaps we could check $user->getDirty(), and if it contains anything other than notification preference changes (or user preference changes at all for that matter, I think we still store all of them together in json rn), only then throw the error? |
|
Yea, and for extensions, how about having an extender to include (or exclude) attributes ? Something like this perhaps ? (new SuspendExtend\User)
->exclude('attribute_name') |
Not a bad idea, but let's wait until L8 and flarum/framework#2544 so we can add it with integration testing. I think that can also probably be done in a separate PR? |
Would be better yes. |
|
Thought about this a bit more. If we add a policy for edit on User, we can restrict editing stuff like bio, avatar, masquerade fields, nickname, etc without infringing on preferences, mark as read, and credentials. I think that's the substantially better option here. |
Fixes flarum/core#2136.
This PR adds two new listeners and refactors some code to new extenders.
I'm aware this is not what has been discussed in the issue. But this is a very sane solution to prevent abuse across other extensions.