-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Webhook that only allows Mute/Unmute events should not allow other events to be selected #221
Comments
Several years ago, I made a conscious decision to not clear the checked items when one selects a template that has less supported events. I decided that because if one changes back to the original template, one as to remember what events were previously selected because that information is lost. Since then, the logic in that edit webhook dialog has been completely re-written and is much more advanced. It is also Jquery unit tested! I should be able to support one of the following scenarios:
I'm leaning towards the first option, because that at least shows to the user what the intended configuration will be when they click the save button. Option two is easier to code ;-) |
Thinking about that, I can handle that case too. If a user intentionally modifies the set of checked/unchecked events, then save that to the stored state. Any further changes to template will honour the same rules as above, but the state will have been updated to reflect the user action. If the user wants to completely back out, they can just press It does reveal an interesting bug. What should the REST API or Controller logic do when a user selected a build event that the template does not support? Obviously at the moment, it silently ignores that mis-match and allows the update to be written. This is why you can see these errors being thrown in the log. I feel like an invalid update (PUT) should return 400 bad request when the list of requested events are not supported by the template. |
@matt-richardson Which page URL are you using to edit webhooks? The index page with the blurb on the right, or the edit project page? |
A third option - you could inform the user "new template doesn't support x, y, and z - these have been de-selected." A fourth option could be to do the same, but also have a "revert to previous template" option that puts you back in the previous state. In terms of the REST API / Controller - I think it should return a bad request as you suggest 👍
I was using the edit project page. |
(originally raised over here)
Expected Behavior
A webhook that only allows Mute/Unmute events, eg:
should not allow other events to be selected.
Current Behavior
The webhook is showing all the boxes as ticked, but disabled:
Errors show as
Steps to Reproduce (for bugs)
It appears that this happens when you select a template that allows build events (which auto-ticks them), and then change to one that only allows mute events - the fields get disabled but are still ticked.
Workaround
Selecting a different template (that supports build events), de-selecting the build events, then changing back to the mute/unmute template works.
Your Environment
1.2.3
2023.05.1
The text was updated successfully, but these errors were encountered: