[RAM] fix logger bug (#146139)

Resolves: #146097 ## Summary In this PR I'm fixing a bug for bulkDelete endpoint. It loggers this error message: `[2022-11-22T15:19:31.182+01:00][ERROR][plugins.alerting] Failure to delete schedules for underlying tasks:` even if we do not have any failed to delete tasks. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
elastic · Nov 24, 2022 · ad8373c · ad8373c
1 parent 89afc44
commit ad8373c
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 19 deletions.
diff --git a/x-pack/plugins/alerting/server/rules_client/rules_client.ts b/x-pack/plugins/alerting/server/rules_client/rules_client.ts
@@ -2117,14 +2117,20 @@ export class RulesClient {
             taskIdsFailedToBeDeleted.push(status.id);
           }
         });
-        this.logger.debug(
-          `Successfully deleted schedules for underlying tasks: ${taskIdsSuccessfullyDeleted.join(
-            ', '
-          )}`
-        );
-        this.logger.error(
-          `Failure to delete schedules for underlying tasks: ${taskIdsFailedToBeDeleted.join(', ')}`
-        );
+        if (taskIdsSuccessfullyDeleted.length) {
+          this.logger.debug(
+            `Successfully deleted schedules for underlying tasks: ${taskIdsSuccessfullyDeleted.join(
+              ', '
+            )}`
+          );
+        }
+        if (taskIdsFailedToBeDeleted.length) {
+          this.logger.error(
+            `Failure to delete schedules for underlying tasks: ${taskIdsFailedToBeDeleted.join(
+              ', '
+            )}`
+          );
+        }
       } catch (error) {
         this.logger.error(
           `Failure to delete schedules for underlying tasks: ${taskIdsToDelete.join(

diff --git a/x-pack/plugins/alerting/server/rules_client/tests/bulk_delete.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/bulk_delete.test.ts
@@ -390,19 +390,16 @@ describe('bulkDelete', () => {
         ],
       }));
 
-      const result = await rulesClient.bulkDeleteRules({ filter: 'fake_filter' });
+      await rulesClient.bulkDeleteRules({ filter: 'fake_filter' });
 
+      expect(logger.debug).toBeCalledTimes(1);
       expect(logger.debug).toBeCalledWith(
         'Successfully deleted schedules for underlying tasks: taskId1'
       );
+      expect(logger.error).toBeCalledTimes(1);
       expect(logger.error).toBeCalledWith(
         'Failure to delete schedules for underlying tasks: taskId2'
       );
-      expect(result).toStrictEqual({
-        errors: [],
-        total: 2,
-        taskIdsFailedToBeDeleted: ['taskId2'],
-      });
     });
 
     test('should not throw an error if taskManager throw an error', async () => {
@@ -417,17 +414,44 @@ describe('bulkDelete', () => {
         throw new Error('UPS');
       });
 
-      const result = await rulesClient.bulkDeleteRules({ filter: 'fake_filter' });
+      await rulesClient.bulkDeleteRules({ filter: 'fake_filter' });
 
       expect(logger.error).toBeCalledTimes(1);
       expect(logger.error).toBeCalledWith(
         'Failure to delete schedules for underlying tasks: taskId1, taskId2. TaskManager bulkRemoveIfExist failed with Error: UPS'
       );
-      expect(result).toStrictEqual({
-        errors: [],
-        taskIdsFailedToBeDeleted: [],
-        total: 2,
+    });
+
+    test('should not call logger.error if all tasks successfully deleted', async () => {
+      mockCreatePointInTimeFinderAsInternalUser();
+      unsecuredSavedObjectsClient.bulkDelete.mockResolvedValue({
+        statuses: [
+          { id: 'id1', type: 'alert', success: true },
+          { id: 'id2', type: 'alert', success: true },
+        ],
       });
+      taskManager.bulkRemoveIfExist.mockImplementation(async () => ({
+        statuses: [
+          {
+            id: 'taskId1',
+            type: 'alert',
+            success: true,
+          },
+          {
+            id: 'taskId2',
+            type: 'alert',
+            success: true,
+          },
+        ],
+      }));
+
+      await rulesClient.bulkDeleteRules({ filter: 'fake_filter' });
+
+      expect(logger.debug).toBeCalledTimes(1);
+      expect(logger.debug).toBeCalledWith(
+        'Successfully deleted schedules for underlying tasks: taskId1, taskId2'
+      );
+      expect(logger.error).toBeCalledTimes(0);
     });
   });