Address comments, clean up -- Round 1

x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/action.ts

@@ -4,10 +4,8 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-// import { Immutable } from '../../../../../common/types';
 import { AlertListData } from './types';
 
-// type ServerReturnedAlertsData = Immutable<{
 interface ServerReturnedAlertsData {
   type: 'serverReturnedAlertsData';
   payload: AlertListData;

x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/middleware.ts

@@ -4,35 +4,19 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-// import { AlertData, ImmutableArray } from '../../../../../common/types';
 import qs from 'querystring';
+import { HttpFetchQuery } from 'src/core/public';
 import { AppAction } from '../action';
 import { MiddlewareFactory } from '../../types';
 
 export const alertMiddlewareFactory: MiddlewareFactory = coreStart => {
-  interface Params {
-    paging_properties: object[];
-  }
-
-  const params: Params = {
-    paging_properties: [],
-  };
-
   const qp = qs.parse(window.location.search.slice(1));
-  if ('page_size' in qp && typeof qp.page_size === 'string') {
-    const pageSize: number = parseInt(qp.page_size, 10);
-    params.paging_properties.push({ page_size: pageSize });
-  }
-  if ('page_index' in qp && typeof qp.page_index === 'string') {
-    const pageIndex: number = parseInt(qp.page_index, 10);
-    params.paging_properties.push({ page_index: pageIndex });
-  }
 
   return api => next => async (action: AppAction) => {
     next(action);
     if (action.type === 'userNavigatedToPage' && action.payload === 'alertsPage') {
-      const response = await coreStart.http.post('/api/endpoint/alerts', {
-        body: JSON.stringify(params),
+      const response = await coreStart.http.get('/api/endpoint/alerts', {
+        query: qp as HttpFetchQuery,
       });
       api.dispatch({ type: 'serverReturnedAlertsData', payload: response });
     }

x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/types.ts

@@ -6,7 +6,6 @@
 
 import { AlertData } from '../../../../../common/types';
 
-// FIXME: temporary until server defined `interface` is moved to a module we can reference
 export interface AlertListData {
   alerts: AlertData[];
   request_page_size: number;

x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx

@@ -8,6 +8,7 @@ import { memo, useState, useMemo } from 'react';
 import React from 'react';
 import { EuiDataGrid } from '@elastic/eui';
 import { useSelector } from 'react-redux';
+import { i18n } from '@kbn/i18n';
 import * as selectors from '../../store/selectors';
 import { usePageId } from '../use_page_id';
 
@@ -40,7 +41,12 @@ export const AlertIndex = memo(() => {
       const row = json[rowIndex];
 
       if (columnId === 'alert_type') {
-        return 'Malicious File';
+        return i18n.translate(
+          'xpack.endpoint.application.endpoint.alerts.alertType.maliciousFileDescription',
+          {
+            defaultMessage: 'Malicious File',
+          }
+        );
       } else if (columnId === 'event_type') {
         return row.event.action;
       } else if (columnId === 'os') {

x-pack/plugins/endpoint/server/routes/alerts.test.ts

@@ -113,14 +113,8 @@ describe('test alerts route', () => {
     const mockRequest = httpServerMock.createKibanaRequest({
       method: 'post',
       body: {
-        paging_properties: [
-          {
-            page_size: 20,
-          },
-          {
-            page_index: 2,
-          },
-        ],
+        page_size: 20,
+        page_index: 2,
       },
     });
     mockScopedClient.callAsCurrentUser.mockImplementationOnce(() =>
@@ -188,4 +182,5 @@ describe('test alerts route', () => {
     expect(alertResultList.request_page_index).toEqual(40);
     expect(alertResultList.request_page_size).toEqual(20);
   });
+  // TODO: add tests for track_total_hits
 });

x-pack/plugins/endpoint/server/routes/alerts.ts

@@ -16,14 +16,14 @@ import { EndpointAppContext } from '../types';
 const ALERTS_ROUTE = '/api/endpoint/alerts';
 
 export function registerAlertRoutes(router: IRouter, endpointAppContext: EndpointAppContext) {
-  const alertsHandler: RequestHandler<any, any, any> = async (ctx, req, res) => {
+  const alertsHandler: RequestHandler<unknown, unknown, unknown> = async (ctx, req, res) => {
     try {
       const queryParams = await kibanaRequestToAlertListQuery(req, endpointAppContext);
       const response = (await ctx.core.elasticsearch.dataClient.callAsCurrentUser(
         'search',
         queryParams
       )) as SearchResponse<AlertData>;
-      return res.ok({ body: mapToAlertResultList(queryParams, response) });
+      return res.ok({ body: mapToAlertResultList(endpointAppContext, queryParams, response) });
     } catch (err) {
       return res.internalError({ body: err });
     }
@@ -32,7 +32,12 @@ export function registerAlertRoutes(router: IRouter, endpointAppContext: Endpoin
   router.get(
     {
       path: ALERTS_ROUTE,
-      validate: {},
+      validate: {
+        query: schema.object({
+          page_size: schema.number({ defaultValue: 10, min: 1, max: 10000 }),
+          page_index: schema.number({ defaultValue: 0, min: 0 }),
+        }),
+      },
       options: { authRequired: true },
     },
     alertsHandler
@@ -42,24 +47,10 @@ export function registerAlertRoutes(router: IRouter, endpointAppContext: Endpoin
     {
       path: ALERTS_ROUTE,
       validate: {
-        body: schema.nullable(
-          schema.object({
-            paging_properties: schema.arrayOf(
-              // FIXME: This structure is weird, but it will change when the following bug is fixed:
-              // https://github.com/elastic/kibana/issues/54843
-              schema.oneOf([
-                // the number of results to return for this request per page
-                schema.object({
-                  page_size: schema.number({ defaultValue: 10, min: 1, max: 10000 }),
-                }),
-                // the index of the page to return
-                schema.object({
-                  page_index: schema.number({ defaultValue: 0, min: 0 }),
-                }),
-              ])
-            ),
-          })
-        ),
+        body: schema.object({
+          page_size: schema.number({ defaultValue: 10, min: 1, max: 10000 }),
+          page_index: schema.number({ defaultValue: 0, min: 0 }),
+        }),
       },
       options: { authRequired: true },
     },
@@ -68,6 +59,7 @@ export function registerAlertRoutes(router: IRouter, endpointAppContext: Endpoin
 }
 
 function mapToAlertResultList(
+  endpointAppContext: EndpointAppContext,
   queryParams: Record<string, any>,
   searchResponse: SearchResponse<AlertData>
 ): AlertResultList {
@@ -79,8 +71,7 @@ function mapToAlertResultList(
   let totalNumberOfAlerts: number = 0;
   let totalIsLowerBound: boolean = false;
 
-  // HACK: because of mismatch in elasticsearch type versions
-  // https://www.elastic.co/guide/en/elasticsearch/reference/current/breaking-changes-7.0.html#hits-total-now-object-search-response
+  // We handle 2 separate schemas for the response below, due to: https://github.com/elastic/kibana/issues/56694
   if (typeof searchResponse?.hits?.total === 'object') {
     const total: Total = searchResponse?.hits?.total as Total;
     totalNumberOfAlerts = total?.value || 0;
@@ -90,22 +81,16 @@ function mapToAlertResultList(
   }
 
   if (totalIsLowerBound) {
-    // TODO: handle this
+    // This shouldn't happen, as we always try to fetch enough hits to satisfy the current request and the next page.
+    endpointAppContext.logFactory
+      .get('endpoint')
+      .warn('WARNING: Total hits not counted accurately. Pagination numbers may be inaccurate.');
   }
 
-  if (searchResponse.hits.hits.length > 0) {
-    return {
-      request_page_size: queryParams.size,
-      request_page_index: queryParams.from,
-      alerts: searchResponse.hits.hits.map(entry => entry._source),
-      total: totalNumberOfAlerts,
-    };
-  } else {
-    return {
-      request_page_size: queryParams.size,
-      request_page_index: queryParams.from,
-      total: totalNumberOfAlerts,
-      alerts: [],
-    };
-  }
+  return {
+    request_page_size: queryParams.size,
+    request_page_index: queryParams.from,
+    alerts: searchResponse?.hits?.hits?.map(entry => entry._source),
+    total: totalNumberOfAlerts,
+  };
 }

x-pack/plugins/endpoint/server/services/endpoint/alert_query_builders.test.ts

@@ -29,11 +29,13 @@ describe('test query builder', () => {
               },
             },
           ],
+          track_total_hits: 10000,
         },
         from: 0,
         size: 10,
         index: 'my-index',
       } as Record<string, any>);
     });
+    // TODO: add tests for calculating track_total_hits
   });
 });

x-pack/plugins/endpoint/server/services/endpoint/alert_query_builders.ts

@@ -3,7 +3,6 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-import qs from 'querystring';
 import { KibanaRequest } from 'kibana/server';
 import { EndpointAppConstants } from '../../../common/types';
 import { EndpointAppContext } from '../../types';
@@ -13,8 +12,16 @@ export const kibanaRequestToAlertListQuery = async (
   endpointAppContext: EndpointAppContext
 ): Promise<Record<string, any>> => {
   const pagingProperties = await getPagingProperties(request, endpointAppContext);
+
+  // Calculate minimum total hits set to indicate there's a next page
+  const DEFAULT_TOTAL_HITS = 10000;
+
+  const fromIdx = pagingProperties.pageIndex * pagingProperties.pageSize;
+  const totalHitsMin = Math.max(fromIdx + pagingProperties.pageSize * 2, DEFAULT_TOTAL_HITS);
+
   return {
     body: {
+      track_total_hits: totalHitsMin,
       query: {
         match_all: {},
       },
@@ -26,7 +33,7 @@ export const kibanaRequestToAlertListQuery = async (
         },
       ],
     },
-    from: pagingProperties.pageIndex * pagingProperties.pageSize,
+    from: fromIdx,
     size: pagingProperties.pageSize,
     index: EndpointAppConstants.ALERT_INDEX_NAME,
   };
@@ -37,25 +44,14 @@ async function getPagingProperties(
   endpointAppContext: EndpointAppContext
 ) {
   const config = await endpointAppContext.config();
-  let pagingProperties: { page_size?: number; page_index?: number } = {};
+  const pagingProperties: { page_size?: number; page_index?: number } = {};
 
   if (request?.route?.method === 'get') {
-    if (typeof request?.url?.query === 'string') {
-      const qp = qs.parse(request.url.query);
-      pagingProperties.page_size = Number(qp.page_size);
-      pagingProperties.page_index = Number(qp.page_index);
-    } else if (request?.url?.query) {
-      pagingProperties = request.url.query;
-    }
+    pagingProperties.page_index = request.query?.page_index;
+    pagingProperties.page_size = request.query?.page_size;
   } else {
-    if (request?.body?.paging_properties) {
-      for (const property of request.body.paging_properties) {
-        Object.assign(
-          pagingProperties,
-          ...Object.keys(property).map(key => ({ [key]: property[key] }))
-        );
-      }
-    }
+    pagingProperties.page_index = request.body?.page_index;
+    pagingProperties.page_size = request.body?.page_size;
   }
 
   return {