Add comments to methods in IWebHookService#15151
Conversation
|
Hi there @bjarnef, thank you for this contribution! 👍 While we wait for one of the Core Collaborators team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:
Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution. If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request. Thanks, from your friendly Umbraco GitHub bot 🤖 🙂 |
|
Hmm I considered this when doing the original implementation, but I'm not really sure where it would be useful to get multiple by ids 🤔 If you can think of a use-case that would use this, then I am open to implement it! 💪 |
|
LGTM 🚀 |
|
@Zeegaan the multiple by ids, was because I looked into the management API in v14, which had an |
|
No hopefully we should not need those for webhooks 🙈 |
|
Do we need |
|
For now no, its used for an Audit trail, but we don't need that for Webhooks. This should be more in line with `ILanguageService`` from v14 👍 |
Prerequisites
Description
Added comments to methods in new
IWebHookService.Not sure if create, update and delete should have
int userId = Constants.Security.SuperUserIdparam like many others services, e.g.IDataTypeService:Umbraco-CMS/src/Umbraco.Core/Services/IDataTypeService.cs
Line 75 in a2a2680
which is used for
Audit():Umbraco-CMS/src/Umbraco.Core/Services/DataTypeService.cs
Line 515 in a2a2680
while looking into supporting Webhooks in management API, I noticed other services pass in
CurrentUserKey(_backOfficeSecurityAccessor), so maybe we need this later anyway?https://github.com/umbraco/Umbraco-CMS/pull/15147/files#diff-33a00493a31cf8daad754645a3ba59e063c24b6aa7d9a89fc24a3226237f6a6eR35