[pve-devel] [PATCH v3 many 00/66] fix #4156: introduce new notification system

[Dominik Csapak]

gave the series a quick spin, review of the gui patches comes later ;)

a few high level comments from a user perspective:

* the node fencing/replication edit windows always shows the warning that it shouldn't be
   disabled, that should imo only be there if i select 'never' ?
   (conversely, the package update window never shows the warning...)
* when we have this, we could remove the pacakge notify line from the datacenter options, no?
* imho having "Notification Targets" directly below "Notifications" is a bit redundant
   (and wasting space), but it's just a minor thing
* the filter 'mode' is also not exposed on the gui (intentionally?)
* also the mode is not quite clear since only one filter can be active per target?
   (or is it meant when there are multiple filter by means of notification groups?)
* what is a filter without a minimum severity? what does it do?
   (does it make sense to allow such filters in the first place?)
* the backup edit window is rather tall at this point, and if we assume a minimum
   screen size of 1280x720 there is not quite enough space when you subtract
   the (typical) os bar and browser window decoration, maybe a 'notification'
   tab would be better (we already have some tabs there anyway)
* i found one bug, but not quite sure yet where it comes from exactly,
   putting in emojis into a field (e.g. a comment or author) it's accepted,
   but editing a different entry fails with:

--->8---
could not serialize configuration: writing 'notifications.cfg' failed: detected unexpected control 
character in section 'testgroup' key 'comment' (500)
---8<---

not sure where the utf-8 info gets lost. (or we could limit all fields to ascii?)
such a notification target still works AFAICT (but if set as e.g. the author it's
probably the wrong value)

(i used 😀 as a test)



otherwise works AFAICT