Closed josephting closed 2 years ago
Looks good, just awaiting confirmation if we want this from the team (I voted yes).
Just thinking about the setting itself though, we'll either need to write a migration (if you have any values, enable the notifications, OR enable them by default (they would be disabled by default due to a lack of an value anyway).
Thanks for taking a look into this. Yea, I was worried about existing settings too. If this is a feature you guys want to add, I'll look deeper into a proper migration of existing settings so just let me know here.
We talked over it briefly, and decided that we'll add it, but it should be enabled by default, as it would be how it's now and it wouldn't send notifications anyway unless they put something in the input field anyway.
You're right. It's a lot simpler too. Changed default to true.
Add checkboxes to each of the notification type below to easily selectively disable them without having to delete the numbers and refill them later to reenable them.