Replacing NotificationsStore with custom hooks. (#22741)

* Explicitly type response of notifications endpoint.

* Replacing `NotificationsStore` with custom hooks.

* Mocking `NotificationBadge` in test.

* Disabling linter hint.

* Fixing backend API call when key is not present.

Author: dennisoelkers

graylog2-server/src/main/java/org/graylog2/notifications/Notification.java

@@ -16,18 +16,16 @@
  */
 package org.graylog2.notifications;
 
+import com.fasterxml.jackson.annotation.JsonValue;
 import org.graylog2.cluster.Node;
 import org.graylog2.plugin.database.Persisted;
 import org.joda.time.DateTime;
 
 import javax.annotation.Nullable;
+import java.util.Locale;
 import java.util.Map;
 
 public interface Notification extends Persisted {
-    // Some pre-defined detail keys
-    final String KEY_TITLE = "title";
-    final String KEY_DESCRIPTION = "description";
-
     Notification addType(Type type);
 
     Notification addKey(String key);
@@ -100,10 +98,20 @@ enum Type {
         REMOTE_REINDEX_FINISHED,
         DATA_NODE_VERSION_MISMATCH,
         DATA_TIERING_ROLLOVER_ERROR,
-        DATA_NODE_HEAP_WARNING
+        DATA_NODE_HEAP_WARNING;
+
+        @JsonValue
+        public String json() {
+            return this.name().toLowerCase(Locale.ROOT);
+        }
     }
 
     enum Severity {
-        NORMAL, URGENT
+        NORMAL, URGENT;
+
+        @JsonValue
+        public String json() {
+            return this.name().toLowerCase(Locale.ROOT);
+        }
     }
 }

graylog2-server/src/main/java/org/graylog2/notifications/NotificationImpl.java

@@ -16,6 +16,7 @@
  */
 package org.graylog2.notifications;
 
+import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.google.common.collect.Maps;
 import org.bson.types.ObjectId;
 import org.graylog2.cluster.Node;
@@ -177,6 +178,7 @@ public Notification addNode(String nodeId) {
     }
 
     @Override
+    @JsonIgnore
     public Map<String, Validator> getValidations() {
         return Collections.emptyMap();
     }

graylog2-server/src/main/java/org/graylog2/rest/resources/system/NotificationsResource.java

@@ -17,13 +17,19 @@
 package org.graylog2.rest.resources.system;
 
 import com.codahale.metrics.annotation.Timed;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Lists;
 import io.swagger.annotations.Api;
 import io.swagger.annotations.ApiOperation;
 import io.swagger.annotations.ApiParam;
 import io.swagger.annotations.ApiResponse;
 import io.swagger.annotations.ApiResponses;
+import jakarta.inject.Inject;
+import jakarta.ws.rs.BadRequestException;
+import jakarta.ws.rs.DELETE;
+import jakarta.ws.rs.GET;
+import jakarta.ws.rs.Path;
+import jakarta.ws.rs.PathParam;
+import jakarta.ws.rs.Produces;
+import jakarta.ws.rs.core.MediaType;
 import org.apache.shiro.authz.annotation.RequiresAuthentication;
 import org.graylog2.audit.AuditEventTypes;
 import org.graylog2.audit.jersey.AuditEvent;
@@ -35,20 +41,8 @@
 import org.slf4j.LoggerFactory;
 
 import javax.annotation.Nullable;
-
-import jakarta.inject.Inject;
-
-import jakarta.ws.rs.BadRequestException;
-import jakarta.ws.rs.DELETE;
-import jakarta.ws.rs.GET;
-import jakarta.ws.rs.Path;
-import jakarta.ws.rs.PathParam;
-import jakarta.ws.rs.Produces;
-import jakarta.ws.rs.core.MediaType;
-
 import java.util.List;
 import java.util.Locale;
-import java.util.Map;
 
 import static org.graylog2.shared.rest.documentation.generator.Generator.CLOUD_VISIBLE;
 
@@ -66,29 +60,22 @@ public NotificationsResource(NotificationService notificationService) {
         this.notificationService = notificationService;
     }
 
+    public record NotificationsResponse(int total, List<Notification> notifications) {
+        static NotificationsResponse create(List<Notification> notifications) {
+            return new NotificationsResponse(notifications.size(), notifications);
+        }
+    }
+
     @GET
     @Timed
     @ApiOperation(value = "Get all active notifications")
     @Produces(MediaType.APPLICATION_JSON)
-    public Map<String, Object> listNotifications() {
-        final List<Map<String, Object>> notifications = Lists.newArrayList();
-        for (Notification n : notificationService.all()) {
-            final String notificationType = n.getType().toString().toLowerCase(Locale.ENGLISH);
-            if (!isPermitted(RestPermissions.NOTIFICATIONS_READ, notificationType)) {
-                continue;
-            }
-
-            final Map<String, Object> notification = n.asMap();
-            try {
-                notifications.add(notification);
-            } catch (IllegalArgumentException e) {
-                LOG.warn("There is a notification type we can't handle: [" + n.getType() + "]");
-            }
-        }
+    public NotificationsResponse listNotifications() {
+        final var notifications = notificationService.all().stream()
+                .filter(notification -> isPermitted(RestPermissions.NOTIFICATIONS_READ, notification.getType().toString()))
+                .toList();
 
-        return ImmutableMap.of(
-                "total", notifications.size(),
-                "notifications", notifications);
+        return NotificationsResponse.create(notifications);
     }
 
     @DELETE

graylog2-web-interface/src/components/navigation/NotificationBadge.test.tsx

@@ -15,53 +15,41 @@
  * <http://www.mongodb.com/licensing/server-side-public-license>.
  */
 import * as React from 'react';
-import { render, screen, waitFor, act, within } from 'wrappedTestingLibrary';
+import { render, screen, waitFor, within } from 'wrappedTestingLibrary';
 
-import { MockStore, asMock } from 'helpers/mocking';
-import { NotificationsActions, NotificationsStore } from 'stores/notifications/NotificationsStore';
+import { asMock } from 'helpers/mocking';
+import useNotifications from 'components/notifications/useNotifications';
 
 import NotificationBadge from './NotificationBadge';
 
-jest.mock('stores/notifications/NotificationsStore', () => ({
-  NotificationsActions: { list: jest.fn() },
-  NotificationsStore: MockStore(),
-}));
-
 const BADGE_ID = 'notification-badge';
 
-type NotificationsStoreType = ReturnType<typeof NotificationsStore.getInitialState>;
-
-const setNotificationCount = (count: number) => {
-  asMock(NotificationsStore.getInitialState).mockReturnValue({ total: count } as NotificationsStoreType);
-};
-
-describe('NotificationBadge', () => {
-  beforeAll(() => {
-    jest.useFakeTimers();
-  });
-
-  afterAll(() => {
-    jest.useRealTimers();
-  });
-
-  beforeEach(() => {
-    jest.restoreAllMocks();
-    jest.clearAllMocks();
-  });
-
-  it('triggers update of notifications', async () => {
-    expect(NotificationsActions.list).not.toHaveBeenCalled();
-
-    render(<NotificationBadge />);
-
-    jest.advanceTimersByTime(3000);
-    await waitFor(() => {
-      expect(NotificationsActions.list).toHaveBeenCalled();
-    });
+jest.mock('components/notifications/useNotifications');
+
+const notificationFixture = {
+  id: 'deadbeef',
+  details: {},
+  validations: {},
+  fields: {},
+  severity: 'urgent',
+  type: 'no_input_running',
+  key: 'test',
+  timestamp: '2022-12-12T10:55:55.014Z',
+  node_id: '3fcc3889-18a3-4a0d-821c-0fd560d152e7',
+} as const;
+
+const createNotifications = (count: number) => new Array(count).fill(notificationFixture);
+
+const setNotificationCount = (count: number) =>
+  asMock(useNotifications).mockReturnValue({
+    data: { total: count, notifications: createNotifications(count) },
+    isLoading: false,
   });
 
+describe('NotificationBadge', () => {
   it('renders nothing when there are no notifications', () => {
     setNotificationCount(0);
+
     render(<NotificationBadge />);
 
     expect(screen.queryByTestId(BADGE_ID)).not.toBeInTheDocument();
@@ -81,17 +69,15 @@ describe('NotificationBadge', () => {
   it('updates notification count when triggered by store', async () => {
     setNotificationCount(42);
 
-    render(<NotificationBadge />);
+    const { rerender } = render(<NotificationBadge />);
 
     const badgeBefore = await screen.findByTestId(BADGE_ID);
 
     expect(within(badgeBefore).getByText(42)).toBeInTheDocument();
 
-    const cb = asMock(NotificationsStore.listen).mock.calls[0][0];
+    setNotificationCount(23);
 
-    act(() => {
-      cb({ total: 23 } as NotificationsStoreType);
-    });
+    rerender(<NotificationBadge />);
 
     const badgeAfter = await screen.findByTestId(BADGE_ID);
 

graylog2-web-interface/src/components/navigation/NotificationBadge.tsx

@@ -15,15 +15,13 @@
  * <http://www.mongodb.com/licensing/server-side-public-license>.
  */
 import * as React from 'react';
-import { useEffect } from 'react';
 import styled from 'styled-components';
 
 import { LinkContainer } from 'components/common/router';
 import { Badge, Nav } from 'components/bootstrap';
-import { useStore } from 'stores/connect';
 import Routes from 'routing/Routes';
-import { NotificationsActions, NotificationsStore } from 'stores/notifications/NotificationsStore';
 import { NAV_ITEM_HEIGHT } from 'theme/constants';
+import useNotifications from 'components/notifications/useNotifications';
 
 import InactiveNavItem from './InactiveNavItem';
 
@@ -44,30 +42,20 @@ const StyledInactiveNavItem = styled(InactiveNavItem)`
   }
 `;
 
-const POLL_INTERVAL = 3000;
-
 const NotificationBadge = () => {
-  const total = useStore(NotificationsStore, (notifications) => notifications?.total);
-
-  useEffect(() => {
-    const interval = setInterval(NotificationsActions.list, POLL_INTERVAL);
-
-    return () => {
-      clearInterval(interval);
-    };
-  }, []);
+  const { data, isLoading } = useNotifications();
 
-  return total ? (
+  return isLoading || data.total === 0 ? null : (
     <StyledNav navbar>
       <LinkContainer to={Routes.SYSTEM.OVERVIEW}>
         <StyledInactiveNavItem>
           <Badge bsStyle="danger" data-testid="notification-badge" title="System Notifications">
-            {total}
+            {data.total}
           </Badge>
         </StyledInactiveNavItem>
       </LinkContainer>
     </StyledNav>
-  ) : null;
+  );
 };
 
 export default NotificationBadge;

graylog2-web-interface/src/components/notifications/Notification.test.tsx

@@ -15,12 +15,10 @@
  * <http://www.mongodb.com/licensing/server-side-public-license>.
  */
 import * as React from 'react';
-import { render, act, screen, waitFor } from 'wrappedTestingLibrary';
+import { render, screen } from 'wrappedTestingLibrary';
 
-import MockStore from 'helpers/mocking/StoreMock';
 import asMock from 'helpers/mocking/AsMock';
-import NotificationsFactory from 'logic/notifications/NotificationsFactory';
-import { NotificationsActions, NotificationsStore } from 'stores/notifications/NotificationsStore';
+import useNotificationMessage from 'components/notifications/useNotificationMessage';
 
 import Notification from './Notification';
 
@@ -30,65 +28,29 @@ const notificationMessageFixture = {
     '\n<span>\nThere is a node without any running inputs. This means that you are not receiving any messages from this\nnode at this point in time. This is most probably an indication of an error or misconfiguration.\n         You can click <a href="/system/inputs" target="_blank" rel="noreferrer">here</a> to solve this.\n</span>\n',
 };
 const notificationFixture = {
+  id: 'deadbeef',
+  details: {},
+  validations: {},
+  fields: {},
   severity: 'urgent',
   type: 'no_input_running',
   key: 'test',
   timestamp: '2022-12-12T10:55:55.014Z',
   node_id: '3fcc3889-18a3-4a0d-821c-0fd560d152e7',
-};
+} as const;
 
-jest.mock('stores/notifications/NotificationsStore', () => ({
-  NotificationsStore: MockStore([
-    'getInitialState',
-    jest.fn(() => ({
-      messages: {},
-    })),
-  ]),
-  NotificationsActions: {
-    getInitialState: jest.fn(),
-    getHtmlMessage: jest.fn(() => Promise.resolve(notificationMessageFixture)),
-  },
-}));
+jest.mock('components/notifications/useNotificationMessage');
 
 describe('<Notification>', () => {
-  afterEach(() => {
-    jest.clearAllMocks();
-    jest.useRealTimers();
-  });
-
-  test('should render Notification', async () => {
-    render(<Notification notification={notificationFixture} />);
-
-    await screen.findByText(/loading\.\.\./i);
-
-    await waitFor(() =>
-      expect(NotificationsActions.getHtmlMessage).toHaveBeenCalledWith(
-        notificationFixture.type,
-        notificationFixture.key,
-        NotificationsFactory.getValuesForNotification(notificationFixture),
-      ),
-    );
+  beforeEach(() => {
+    asMock(useNotificationMessage).mockReturnValue(notificationMessageFixture);
   });
 
-  test('should render notification message', async () => {
-    jest.useFakeTimers();
-
-    asMock(NotificationsStore.getInitialState).mockReturnValue({
-      messages: { [`${notificationFixture.type}-${notificationFixture.key}`]: notificationMessageFixture },
-      total: 1,
-      notifications: [notificationFixture],
-    });
-
+  test('it render notification message', async () => {
     render(<Notification notification={notificationFixture} />);
 
-    act(() => {
-      jest.advanceTimersByTime(100);
-    });
-
-    const alert = screen.getByRole('alert', {
+    await screen.findByRole('alert', {
       name: /there is a node without any running inputs/i,
     });
-
-    await waitFor(() => expect(alert).toBeInTheDocument());
   });
 });