PR feedback 3

x-pack/plugins/stack_connectors/public/connector_types/servicenow_itsm/servicenow_itsm_params.test.tsx

@@ -337,7 +337,6 @@ describe('ServiceNowITSMParamsFields renders', () => {
 
       expect(editAction.mock.calls[0][1]).toEqual({
         incident: { correlation_id: 'updated correlation id' },
-        comments: [],
       });
     });
   });

x-pack/plugins/stack_connectors/public/connector_types/servicenow_itsm/servicenow_itsm_params.tsx

@@ -99,9 +99,9 @@ const ServiceNowParamsFields: React.FunctionComponent<
       actionParams.subActionParams ??
       ({
         incident: {},
-        comments: [],
+        comments: selectedActionGroupId !== ACTION_GROUP_RECOVERED ? [] : undefined,
       } as unknown as ServiceNowITSMActionParams['subActionParams']),
-    [actionParams.subActionParams]
+    [actionParams.subActionParams, selectedActionGroupId]
   );
 
   const [choices, setChoices] = useState<Fields>(defaultFields);

x-pack/plugins/stack_connectors/server/connector_types/lib/servicenow/service.test.ts

@@ -547,22 +547,6 @@ describe('ServiceNow service', () => {
       });
     });
 
-    test('it should log warning if no incident found', async () => {
-      requestMock.mockImplementationOnce(() => ({
-        status: 404,
-        data: { result: [] },
-      }));
-      const res = await service.getIncidentByCorrelationId('custom_correlation_id');
-
-      expect(requestMock).toHaveBeenCalledTimes(1);
-      expect(logger.warn.mock.calls[0]).toMatchInlineSnapshot(`
-        Array [
-          "No incident found with correlation ID custom_correlation_id.",
-        ]
-      `);
-      expect(res).toBe(null);
-    });
-
     test('it should throw an error', async () => {
       requestMock.mockImplementationOnce(() => {
         throw new Error('An error has occurred');
@@ -1095,21 +1079,26 @@ describe('ServiceNow service', () => {
         expect(requestMock).toHaveBeenCalledTimes(1);
         expect(logger.warn.mock.calls[0]).toMatchInlineSnapshot(`
         Array [
-          "Incident is already closed.",
+          "[ServiceNow][CloseIncident] Incident with correlation_id: null or incidentId: 1 is closed.",
         ]
       `);
       });
 
       test('it should return null if found incident with correlation id is null', async () => {
         requestMock.mockImplementationOnce(() => ({
           data: {
-            result: [null],
+            result: [],
           },
         }));
 
         const res = await service.closeIncident({ incidentId: null, correlationId: 'bar' });
 
         expect(requestMock).toHaveBeenCalledTimes(1);
+        expect(logger.warn.mock.calls[0]).toMatchInlineSnapshot(`
+        Array [
+          "[ServiceNow][CloseIncident] No incident found with correlation_id: bar or incidentId: null.",
+        ]
+      `);
         expect(res).toBeNull();
       });
 
@@ -1176,7 +1165,7 @@ describe('ServiceNow service', () => {
         });
 
         mockIncidentResponse(false);
-        mockImportIncident(true);
+        mockIncidentResponse(true);
         mockIncidentResponse(true);
 
         const res = await service.closeIncident({
@@ -1208,7 +1197,7 @@ describe('ServiceNow service', () => {
           axios,
           logger,
           configurationUtilities,
-          url: 'https://example.com/api/now/v2/table/sn_si_incident/undefined',
+          url: 'https://example.com/api/now/v2/table/sn_si_incident/1',
           method: 'get',
         });
 

x-pack/plugins/stack_connectors/server/connector_types/lib/servicenow/service.ts

@@ -267,10 +267,6 @@ export const createExternalService: ServiceFactory = ({
 
       checkInstance(res);
 
-      if (res.status === 404) {
-        logger.warn(`No incident found with correlation ID ${correlationId}.`);
-      }
-
       const foundIncident = res.data.result[0] ?? null;
 
       return foundIncident;
@@ -292,8 +288,17 @@ export const createExternalService: ServiceFactory = ({
         incidentToBeClosed = await getIncidentByCorrelationId(correlationId);
       }
 
-      if (incidentToBeClosed?.state === 7) {
-        logger.warn(`Incident is already closed.`);
+      if (incidentToBeClosed === null || incidentToBeClosed?.status === 'error') {
+        logger.warn(
+          `[ServiceNow][CloseIncident] No incident found with correlation_id: ${correlationId} or incidentId: ${incidentId}.`
+        );
+        return null;
+      }
+
+      if (incidentToBeClosed.state === 7) {
+        logger.warn(
+          `[ServiceNow][CloseIncident] Incident with correlation_id: ${correlationId} or incidentId: ${incidentId} is closed.`
+        );
 
         return {
           title: incidentToBeClosed.number,
@@ -303,10 +308,6 @@ export const createExternalService: ServiceFactory = ({
         };
       }
 
-      if (incidentToBeClosed === null || incidentToBeClosed?.status === 'error') {
-        return null;
-      }
-
       const closedIncident = await updateIncident({
         incidentId: incidentToBeClosed.sys_id,
         incident: {

x-pack/plugins/stack_connectors/server/connector_types/lib/servicenow/types.ts

@@ -124,7 +124,7 @@ export interface ExternalService {
   checkInstance: (res: AxiosResponse) => void;
   getApplicationInformation: () => Promise<GetApplicationInfoResponse>;
   checkIfApplicationIsInstalled: () => Promise<void>;
-  getIncidentByCorrelationId: (correlationId: string) => Promise<ServiceNowIncident>;
+  getIncidentByCorrelationId: (correlationId: string) => Promise<ServiceNowIncident | null>;
 }
 
 export type PushToServiceApiParams = ExecutorSubActionPushParams;