NMS-20118: Event Notifications tab - #8710
Conversation
|
@marshallmassengill review requested — stacks on #8709 (only the top commit is new here). |
2e6363a to
e4746e4
Compare
84b1a5e to
83c527e
Compare
marshallmassengill
left a comment
marshallmassengill
left a comment
There was a problem hiding this comment.
Mostly ok but a couple things:
Deleting the last event notification corrupts the running config. notifications.xsd requires at least one notification element. removeNotification drops the entry from memory and then calls saveCurrent(), which marshals with a schema attached. Memory ends up with zero notifications, notifications.xml still has one, the client gets a 500, and NotificationFactory.update() only reloads when the file mtime changes, which it didn't. notifd serves an empty notification config until OpenNMS restarts. The PR's validation can't catch this because DELETE has no body.
status is missing from validateEventNotification. replaceNotification overwrites the live object field by field through setters that assert non-empty, in this order: writeable, name, description, uei, rule, destinationPath, noticeQueue, textMessage, subject, numericMessage, status, varbind, parameters, then save. A PUT omitting status therefore passes validation, then throws inside setStatus after five fields have already been written onto the live object. Nothing saves, nothing rolls back, and the next successful save of any other notification persists the half-updated entry. POST has the same exposure via addNotification.
|
All addressed: deleting the last notification now 400s with a pointer to turn it off instead; validation covers status (and parameter names/values, which the in-place chain can also die on after clearing the live entry's parameters); and the comment now describes what addNotification and replaceNotification actually do. Tests cover the delete-last guard and the missing-status atomicity on both POST and PUT. |
3e6c8ec to
27d3ec9
Compare
marshallmassengill
left a comment
marshallmassengill
left a comment
There was a problem hiding this comment.
The delete guard's message still never reaches the user. notificationConfigService.ts:141 discards err.response.data and Delete is not disabled at length === 1, so the guard works but reads as an unexplained failure.
TAB_LOADERS['event-notifications'] returns literal true, and this PR makes it the default tab. All three siblings have this now, so it belongs in the base rather than fixed three times.
e62bc00 to
6e5951f
Compare
Replaces the event-notifications placeholder with the tab content: the notification list with per-notification on/off toggles, an add/edit dialog with a searchable UEI picker fed by the event configuration, and delete with confirmation. The /rest/notification-config event-notifications endpoints wrap the notifications.xml factory the legacy wizard uses. Deleting the last event notification is rejected (notifications.xsd requires at least one, and the schema-validated save would diverge memory from the file until a restart), and validation covers every field whose asserting setter could kill the factory's in-place update chain, including status and parameter names/values.
…ification Deleting an event notification now surfaces the server's reason instead of a generic failure, so the "last event notification cannot be deleted" guard reads as an explanation rather than an unexplained error. The Delete button is disabled when only one notification remains, with a title pointing the user at turning it off instead.
The event-notifications tab loader now returns every([...]).every(Boolean) instead of a literal true: getEventNotifications returns null on failure and the store reports a boolean, so a failed fetch retries instead of latching the default tab on "No event notifications configured." Rebased onto the updated base. Adds a store test pinning the failure contract.
Migrate the Event Notifications tab — its table and the add/edit dialog — onto the Onms-* seam wrappers so the page passes the no-direct-primevue lint rule. iftalabel stays on PrimeVue (no wrapper). The UEI autocomplete drops its custom two-line suggestion slot, which OnmsAutoComplete does not forward; suggestions show the UEI via optionLabel.
6e5951f to
b513ea2
Compare
2 participants
NMS-20118 (NMS-20100 epic): adds the Event Notifications tab to the Configure Notifications dialog. Stacked on #8709, so this PR shows only its own commit.