V13: Webhook corrections by bjarnef · Pull Request #15077 · umbraco/Umbraco-CMS

bjarnef · 2023-10-31T15:01:42Z

Prerequisites

I have added steps to test this contribution in the description below

Description

Some corrections for #15050

Added webhook icon (it need to be a "solid" icon using fill and not stroke/outline). https://www.svgrepo.com/svg/503141/webhook (the icon may be updated in the new backoffice)
Localize tree.
Datalist with common headers for webhook events.
Modified content type picker to not allow folder (entity container) selection - maybe also exclude element types?
Promises to load events and webhook in overview and show loading indicator + sync of tree node.
Changed buttons to <button> elements.
Stop event bubbling when clicking delete button in overview to not open edit overlay at the same time.
Removed the globe icon since I didn't think it had any value and not to confuse with languages.
Some formatting and cleanup.

Furthermore I think editing webhook shouldn't modify model directly, but only if actually saved and not update in background, e.g. when typing URL or toggle enabled state.

I also noticed when unchecking the selected events, it reset to original selection. Maybe selecting Content or Media events should filter the others, but just disable them?

github-actions · 2023-10-31T15:01:57Z

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:

It's clear what problem this is solving, there's a connected issue or a description of what the changes do and how to test them
The automated tests all pass (see "Checks" tab on this PR)
The level of security for this contribution is the same or improved
The level of performance for this contribution is the same or improved
Avoids creating breaking changes; note that behavioral changes might also be perceived as breaking
If this is a new feature, Umbraco HQ provided guidance on the implementation beforehand
💡 The contribution looks original and the contributor is presumably allowed to share it

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 🤖 🙂

bjarnef · 2023-10-31T15:03:40Z

@Zeegaan I have updated ~~a few~~ lots of things.

bjarnef · 2023-10-31T22:49:14Z

Similar to content type picker in MNTP configuration, so folder selection is disabled.

Umbraco-CMS/src/Umbraco.Web.UI.Client/src/views/prevalueeditors/treesourcetypepicker.controller.js

Lines 37 to 74 in b1b48ec

    
           function add() { 
        
               if (!currentItemType) { 
        
                   return; 
        
               } 
        
               var editor = { 
        
                   multiPicker: true, 
        
                   filterCssClass: "not-allowed not-published", 
        
                   filter: function (item) { 
        
                       // filter out folders (containers), element types (for content) and already selected items 
        
                       return item.nodeType === "container" || item.metaData.isElement || !!_.findWhere(vm.itemTypes, { udi: item.udi }); 
        
                   }, 
        
                   submit: function (model) { 
        
                       var newItemTypes = _.map(model.selection, 
        
                           function(selected) { 
        
                               return _.findWhere(allItemTypes, { udi: selected.udi }); 
        
                           }); 
        
                       vm.itemTypes = _.uniq(_.union(vm.itemTypes, newItemTypes)); 
        
                       updateModel(); 
        
                       editorService.close(); 
        
                   }, 
        
                   close: function() { 
        
                       editorService.close(); 
        
                   } 
        
               }; 
        
               switch (currentItemType) { 
        
                   case "content": 
        
                       editorService.contentTypePicker(editor); 
        
                       break; 
        
                   case "media": 
        
                       editorService.mediaTypePicker(editor); 
        
                       break; 
        
                   case "member": 
        
                       editorService.memberTypePicker(editor); 
        
                       break; 
        
               } 
        
           }

Like this PR #15034 regarding contentTypeEditor is would be great if the contentTypePicker handled different entities. The contentTypePicker does however open document types, but we could probably avoid a breaking change either with another parameter or pass in entity type to editor arguments with fallback to documentType, but it would be great if it is consistent with contentTypeEditor.

bjarnef · 2023-10-31T23:04:12Z

src/Umbraco.Web.UI.Client/src/views/webhooks/logs.html

-                    size="xxs"
-                    label="Details">
-        </umb-button>
-      </td>


It had a column too much and there shouldn't be a delete webhook button in log entry :)

Zeegaan · 2023-11-01T08:12:09Z

src/Umbraco.Web.UI.Client/src/views/webhooks/overlays/edit.controller.js


    function clearContentType(contentTypeKey) {
-      if (Array.isArray($scope.model.webhook.contentTypeKeys)) {
+      if (Utilities.isArray($scope.model.webhook.contentTypeKeys)) {


Just for my own knowledge, what is the difference between using Utilities & Array ? 🤔

Not much, it is just a wrapper we use much elsewhere in codebase and some of the methods have replaced the angular functions.

Umbraco-CMS/src/Umbraco.Web.UI.Client/src/utilities.js

Line 26 in b1b48ec

const isArray = val => Array.isArray(val) || val instanceof Array;

Most of the time there won't be a difference.
Not sure if Array.isArray(val) could return a different result than val instanceof Array

I tried with [], "", null, undefined, NaN .. all returned same result.

bjarnef · 2023-11-01T08:36:22Z

src/Umbraco.Web.UI.Client/src/views/webhooks/overlays/edit.controller.js

+        case "member":
+          editorService.memberTypePicker(editor);
+          break;
+      }


It would be great if we could use this here: #15080

Would it make sense the support webhooks for Member events @Zeegaan ?

It is in the pipeline for rc-2 (but no promises 😛), so I think it's okay 👍

At least it never opens member type picker for now and if #15080 gets merged, we can eventually refactor this part, but anyway it should still work 😎

bjarnef · 2023-11-01T08:52:56Z

src/Umbraco.Web.UI.Client/src/views/webhooks/overlays/details.html

      <div class="flx-g0 flx-s0" style="flex-basis: 40px;">
        <umb-checkmark checked="true" size="m" style="cursor: default"></umb-checkmark>
-        <i class="icon-wrong umb-checkmark umb-checkmark--m" style="cursor: default;" ng-if="model.webhookLogEntry.response.isSuccess === false"></i>
+        <umb-icon icon="icon-wrong" class="umb-checkmark umb-checkmark--m" style="cursor: default;" ng-if="model.webhookLogEntry.response.isSuccess === false"></umb-icon>


I didn't have any logs, but we should use <umb-icon> instead of the <i> element :)

Zeegaan · 2023-11-01T12:07:56Z

Seems to be a bug with already existing webhooks, when trying to change events, it seems like unselecting the last one i causing issues 🤣
Heres my showing it off:

bjarnef · 2023-11-01T12:33:48Z

Seems to be a bug with already existing webhooks, when trying to change events, it seems like unselecting the last one i causing issues 🤣 Heres my showing it off:

Yes, that's what I mentioned in the description :)

I also noticed when unchecking the selected events, it reset to original selection. Maybe selecting Content or Media events should filter the others, but just disable them?

Zeegaan · 2023-11-01T14:03:45Z

Aha totally missed that!
Yes, perhaps the filtering should just disable them I guess 🤷

bjarnef · 2023-11-01T14:25:47Z

Aha totally missed that! Yes, perhaps the filtering should just disable them I guess 🤷

We can have a look in another PR 👍

Zeegaan · 2023-11-01T15:55:00Z

Looks good, tests good 🚀

* Update icons * Update tree headers * Cleanup and change icon name * Use button element instead * Disable button instead * Fix overlay title * Simplify labels * Add datalist for common headers * Use Utilties function * Events in plural form * Cleanup and formatting * Formatting * More formatting * Stop event bubbling when clicking delete button * Sync tree node and show loading indicator * Add webhook icon * Remove globe icon to not confuse with languages * Update logs * Remove extra column with delete button which shouldn't be there * Use umb-icon and update titles * Use content type picker (cherry picked from commit 29be27b)

bjarnef added 8 commits October 31, 2023 15:21

Update icons

a519262

Update tree headers

13791c7

Cleanup and change icon name

bc4218c

Use button element instead

3571c0f

Disable button instead

07a2e7c

Fix overlay title

8ff0857

Simplify labels

954a81a

Add datalist for common headers

efb6c30

bjarnef marked this pull request as draft October 31, 2023 15:01

nul800sebastiaan added the hacktoberfest-accepted label Oct 31, 2023

bjarnef added 9 commits October 31, 2023 20:08

Use Utilties function

064c10d

Events in plural form

8cca662

Cleanup and formatting

506d546

Formatting

ac021f8

More formatting

194ddae

Stop event bubbling when clicking delete button

5ebe052

Sync tree node and show loading indicator

3db2ca9

Add webhook icon

28e5542

Remove globe icon to not confuse with languages

11dd837

bjarnef changed the title ~~Feature/webhook corrections~~ Webhook corrections Oct 31, 2023

bjarnef marked this pull request as ready for review October 31, 2023 21:38

bjarnef changed the title ~~Webhook corrections~~ V13: Webhook corrections Oct 31, 2023

bjarnef added 4 commits October 31, 2023 23:04

Update logs

eb8169c

Remove extra column with delete button which shouldn't be there

834ff6e

Use umb-icon and update titles

f7c6c82

Use content type picker

5aa9044

bjarnef commented Oct 31, 2023

View reviewed changes

Zeegaan reviewed Nov 1, 2023

View reviewed changes

bjarnef commented Nov 1, 2023

View reviewed changes

Zeegaan merged commit 29be27b into umbraco:v13/dev Nov 1, 2023

Zeegaan added hacktoberfest/contrib-2023 release/12.3.0 release/13.0.0 and removed release/12.3.0 labels Nov 1, 2023

bjarnef deleted the feature/webhook-corrections branch November 1, 2023 15:56

Zeegaan mentioned this pull request Nov 2, 2023

V13 RC: Deleting a webhook opens up webhook side bar #15094

Closed

This was referenced Nov 6, 2023

Feature: Webhooks workspace umbraco/Umbraco.CMS.Backoffice#962

Merged

V13: Add eventype to webhookevents #15157

Merged

nul800sebastiaan added community/pr type/improvement labels Dec 5, 2023

Conversation

bjarnef commented Oct 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Prerequisites

Description

Uh oh!

github-actions bot commented Oct 31, 2023

Uh oh!

bjarnef commented Oct 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bjarnef commented Oct 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zeegaan commented Nov 1, 2023

Uh oh!

bjarnef commented Nov 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Zeegaan commented Nov 1, 2023

Uh oh!

bjarnef commented Nov 1, 2023

Uh oh!

Zeegaan commented Nov 1, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bjarnef commented Oct 31, 2023 •

edited

Loading

bjarnef commented Oct 31, 2023 •

edited

Loading

bjarnef commented Oct 31, 2023 •

edited

Loading

bjarnef commented Nov 1, 2023 •

edited

Loading