Feature/14285 migrate event definitions page to new bulk friendly design #14502

moesterheld · 2023-01-24T17:14:20Z

Migrate Event Definitions and Event Notifications to new Entity List

closes #14285

Description

added paginated endpoints for event definitions and notifications, deprecated old endpoints
changed entity list in ui to new component

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Refactoring (non-breaking change)
Breaking change (fix or feature that would cause existing functionality to change)

…stResponse

…ge-to-new-bulk-friendly-design

patrickmann · 2023-02-16T12:09:39Z

The UI doesn't adjust horizontally the way I'd expect. I can never get rid of the horizontal scroll bar, even in maximized view, because the more button is pushed out too far.

Also: System notification events is always shown as disabled. Even after enabling it and seeing a confirmation pop-up that it was successfully enabled.

patrickmann · 2023-02-24T08:55:31Z

System Notification Events is still showing as disabled - should always be enabled. Note that this is not a scheduled event, so you can't use that as the conditional.

ousmaneo · 2023-02-24T09:19:13Z

Hi @patrickmann, thank you, I fixed that.

patrickmann · 2023-02-24T08:57:20Z

changelog/unreleased/issue-14285.toml

@@ -0,0 +1,5 @@
+type = "c" # One of: a(dded), c(hanged), d(eprecated), r(emoved), f(ixed), s(ecurity)
+message = "Changed event definitions and notifications to new paginated list and entity list ui component."


Still think the CL should mention bulk operations, since that is the new feature.

patrickmann · 2023-02-24T09:33:57Z

graylog2-server/src/main/java/org/graylog/events/rest/EventDefinitionsResource.java

+    @Path("/bulk_schedule")
+    @Consumes(MediaType.APPLICATION_JSON)
+    @Timed
+    @ApiOperation(value = "Enable a bulk of event definitions", response = BulkOperationResponse.class)


"Bulk" sounds strange in this context - "group" or "selection" or "multiple" would be better.

i used bulk to makes this consistent with bulk_delete used here and in streams.

That's fine. I just mean the string "Enable a bulk of event definitions". Bulk as a noun refers to a homogeneous mass, rather than a grouping of distinct elements.
If we want to stick with it, then I prefer e.g. "Enable event definitions in bulk"

patrickmann · 2023-02-24T09:34:31Z

graylog2-server/src/main/java/org/graylog/events/rest/EventDefinitionsResource.java

+    @Path("/bulk_unschedule")
+    @Consumes(MediaType.APPLICATION_JSON)
+    @Timed
+    @ApiOperation(value = "Disable a bulk of event definitions", response = BulkOperationResponse.class)


Replace "bulk" - see above

patrickmann · 2023-02-24T11:08:46Z

I can perform bulk disable on System Notification Events definition and get a success confirmation, even though it does not get disabled. For consistency, it should show a warning like it does for an attempted bulk delete.

…ge-to-new-bulk-friendly-design

moesterheld · 2023-02-24T15:15:18Z

adjusted the swagger docs and added additional validation for disabling events in the backend

patrickmann · 2023-02-24T15:44:19Z

Attempting to delete the System Event Definition is blocked with an appropriate error message.
But it still shows up as successfully deleted in the recent activity log. :)

grotlue · 2023-02-24T16:07:42Z

Attempting to delete the System Event Definition is blocked with an appropriate error message. But it still shows up as successfully deleted in the recent activity log. :)

I think we should create a separate issue for handling this. When selecting multiple Event Definition and the System Event Definition, it shows the error message "One of the event definitions could not be deleted" which is expected, but in this case, it looks like something is broken - which is not. I think it's not blocking but would be nice to handle this case in the future.

grotlue

Checked it again and looks good now! The build is still red though.

…ge-to-new-bulk-friendly-design

patrickmann

I opened #14786 for the inconsistencies related to deleting System Notification event definition.
The rest looks good.

On save, the UI is passing back the "scheduled=false" param from the context. There's several ways we could fix this, but it's probably easiest to not throw an exception, but just ignore the unschedule call. This probably got broken with #14502

* Fix saving of system notification event definition On save, the UI is passing back the "scheduled=false" param from the context. There's several ways we could fix this, but it's probably easiest to not throw an exception, but just ignore the unschedule call. This probably got broken with #14502 * changelog

On save, the UI is passing back the "scheduled=false" param from the context. There's several ways we could fix this, but it's probably easiest to not throw an exception, but just ignore the unschedule call. This probably got broken with #14502 cherry-picked from d1b77ed

* Fix saving of system notification event definition On save, the UI is passing back the "scheduled=false" param from the context. There's several ways we could fix this, but it's probably easiest to not throw an exception, but just ignore the unschedule call. This probably got broken with #14502 * changelog (cherry picked from commit d1b77ed)

On save, the UI is passing back the "scheduled=false" param from the context. There's several ways we could fix this, but it's probably easiest to not throw an exception, but just ignore the unschedule call. This probably got broken with #14502 cherry-picked from d1b77ed

moesterheld linked an issue Jan 24, 2023 that may be closed by this pull request

Migrate event definitions page to new bulk-friendly design #14285

Closed

ousmaneo force-pushed the feature/14285-migrate-event-definitions-page-to-new-bulk-friendly-design branch from 84bdaaa to 116fca0 Compare January 27, 2023 11:14

moesterheld and others added 17 commits February 6, 2023 10:09

feat: add paginated endpoint with sort parameter returning new PageLi…

79ecf4f

…stResponse

doc: adjust swagger doc

6074b65

feat: add scheduler context to event definition dto

86bdf52

feat: add PageListResponse for notifications

23595a0

fix: remove nested attributes for the time being

6a957a6

update types

521955b

add Status and Scheduler components

96a54d6

add eventDefinition datatable action

1064eb0

add paginated eventDefinition and hook

fabf18d

move EventDefinitionsContainer to ts and use EntityDataTable

f66c813

add bulk delete action

c59c560

fix types

94a9205

doc: make scheduler property read-only

632f78f

doc: add changelog

ad76cc1

fix linter

c939b9f

fix status size

04c87f5

fix sorting order

ead564b

moesterheld force-pushed the feature/14285-migrate-event-definitions-page-to-new-bulk-friendly-design branch from bce953a to ead564b Compare February 6, 2023 09:10

moesterheld and others added 3 commits February 6, 2023 11:05

feat: bulk schedule/unschedule

ea00fd2

implement delete, enable and disable bulk action

43d8b00

add BulkActions test

e9f1a3e

ousmaneo requested a review from grotlue February 13, 2023 07:30

ousmaneo added the ready-for-review label Feb 14, 2023

ousmaneo requested a review from a team February 15, 2023 11:47

patrickmann requested review from patrickmann and removed request for a team February 16, 2023 10:46

Merge branch 'master' into feature/14285-migrate-event-definitions-pa…

e6b8d66

…ge-to-new-bulk-friendly-design

ousmaneo requested review from grotlue and patrickmann February 23, 2023 16:09

show SYSTEM_EVENT_DEFINITION_TYPE as enabled

720bcd0

ousmaneo force-pushed the feature/14285-migrate-event-definitions-page-to-new-bulk-friendly-design branch from a845172 to 720bcd0 Compare February 24, 2023 09:38

patrickmann requested changes Feb 24, 2023

View reviewed changes

moesterheld and others added 4 commits February 24, 2023 12:23

update changelog

22c6bf0

doc: adjust swagger doc wording

371bb92

Merge branch 'master' into feature/14285-migrate-event-definitions-pa…

bc05073

…ge-to-new-bulk-friendly-design

fix: exception if trying to disable system notification events

185307c

grotlue approved these changes Feb 24, 2023

View reviewed changes

Merge branch 'master' into feature/14285-migrate-event-definitions-pa…

0a9adc6

…ge-to-new-bulk-friendly-design

patrickmann mentioned this pull request Feb 27, 2023

Better UI messaging when attempting to delete System Notification event definition #14786

Closed

patrickmann approved these changes Feb 27, 2023

View reviewed changes

patrickmann merged commit 6554a6f into master Feb 27, 2023

patrickmann deleted the feature/14285-migrate-event-definitions-page-to-new-bulk-friendly-design branch February 27, 2023 08:59

mpfz0r mentioned this pull request Nov 22, 2023

Fix saving of system notification event definition #17435

Merged

mpfz0r mentioned this pull request Nov 23, 2023

Fix saving of system notification event definition (5.1) #17451

Merged

mpfz0r mentioned this pull request Nov 23, 2023

Fix saving of system notification event definition (5.2) #17452

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/14285 migrate event definitions page to new bulk friendly design #14502

Feature/14285 migrate event definitions page to new bulk friendly design #14502

moesterheld commented Jan 24, 2023 •

edited

patrickmann commented Feb 16, 2023 •

edited

patrickmann commented Feb 24, 2023

ousmaneo commented Feb 24, 2023

patrickmann Feb 24, 2023

patrickmann Feb 24, 2023

moesterheld Feb 24, 2023

patrickmann Feb 24, 2023

patrickmann Feb 24, 2023

patrickmann commented Feb 24, 2023 •

edited

moesterheld commented Feb 24, 2023

patrickmann commented Feb 24, 2023

grotlue commented Feb 24, 2023

grotlue left a comment

patrickmann left a comment

		@@ -0,0 +1,5 @@
		type = "c" # One of: a(dded), c(hanged), d(eprecated), r(emoved), f(ixed), s(ecurity)
		message = "Changed event definitions and notifications to new paginated list and entity list ui component."

Feature/14285 migrate event definitions page to new bulk friendly design #14502

Feature/14285 migrate event definitions page to new bulk friendly design #14502

Conversation

moesterheld commented Jan 24, 2023 • edited

Description

Types of changes

patrickmann commented Feb 16, 2023 • edited

patrickmann commented Feb 24, 2023

ousmaneo commented Feb 24, 2023

patrickmann Feb 24, 2023

Choose a reason for hiding this comment

patrickmann Feb 24, 2023

Choose a reason for hiding this comment

moesterheld Feb 24, 2023

Choose a reason for hiding this comment

patrickmann Feb 24, 2023

Choose a reason for hiding this comment

patrickmann Feb 24, 2023

Choose a reason for hiding this comment

patrickmann commented Feb 24, 2023 • edited

moesterheld commented Feb 24, 2023

patrickmann commented Feb 24, 2023

grotlue commented Feb 24, 2023

grotlue left a comment

Choose a reason for hiding this comment

patrickmann left a comment

Choose a reason for hiding this comment

moesterheld commented Jan 24, 2023 •

edited

patrickmann commented Feb 16, 2023 •

edited

patrickmann commented Feb 24, 2023 •

edited