umbraco / Umbraco-CMS

Umbraco is a free and open source .NET content management system helping you deliver delightful digital experiences.
https://umbraco.com
MIT License
4.48k stars 2.69k forks source link

Make internal SavingUserGroup and SavedUserGroup events in UserService public #7720

Closed nathanwoulfe closed 4 years ago

nathanwoulfe commented 4 years ago

Umbraco.Core/Serviced/Implement/UserService raises a bunch of events when users and/or groups are modified, however SavingUserGroup and SavedUserGroup are both internal.

Wondering if there's a reason for this? If there's not, I'll make em public.

Example use case: in Plumber, I want to add a caching layer around approval groups to avoid db load. I can cache my native groups, but am also adding a feature to allow using Umbraco groups in the workflow, so need to know when these are changed so that I can invalidate cache...

Shazwazza commented 4 years ago

Can't remember, but don't think they couldn't be public

nul800sebastiaan commented 4 years ago

Seems lovely to open those up! 👍

umbrabot commented 4 years ago

Hi @nathanwoulfe,

We're writing to let you know that we've added the Up For Grabs label to your issue. We feel that this issue is ideal to flag for a community member to work on it. Once flagged here, folk looking for issues to work on will know to look at yours. Of course, please feel free work on this yourself ;-). If there are any changes to this status, we'll be sure to let you know.

For more information about issues and states, have a look at this blog post

Thanks muchly, from your friendly PR team bot :-)

nathanwoulfe commented 4 years ago

Had a poke back through history - https://github.com/umbraco/Umbraco-CMS/blob/2aa0dfb2c5a682d74eca8858f26bcfe563515a3f/src/Umbraco.Core/Services/UserService.cs has both internal and public versions of the same events, with TODO re renaming in v8. I think what might have happened was the internal events were renamed, and the public versions removed, rather than removing the internal. Should be fine to expose em, will do so.

nul800sebastiaan commented 4 years ago

Fixed in https://github.com/umbraco/Umbraco-CMS/pull/7725