Skip to content

Commit 894e62f

Browse files
doakalexidoakalexi
committed
More updates from pr comments
1 parent dfeaf86 commit 894e62f

10 files changed

Lines changed: 171 additions & 33 deletions

File tree

‎x-pack/platform/plugins/private/reporting/server/lib/tasks/run_report.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,8 @@ export abstract class RunReportTask<TaskParams extends ReportTaskParamsType>
133133

134134
protected abstract notify(
135135
report: SavedReport,
136+
taskInstance: ConcreteTaskInstance,
136137
output: TaskRunResult,
137-
runAt: Date,
138138
byteSize: number,
139139
reportSO?: SavedObject<ScheduledReportType>,
140140
spaceId?: string
@@ -570,8 +570,8 @@ export abstract class RunReportTask<TaskParams extends ReportTaskParamsType>
570570

571571
await this.notify(
572572
report,
573+
taskInstance,
573574
output,
574-
taskInstance.runAt,
575575
byteSize,
576576
reportSO,
577577
task.payload.spaceId

‎x-pack/platform/plugins/private/reporting/server/lib/tasks/run_scheduled_report.test.ts‎

Lines changed: 85 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -135,9 +135,6 @@ describe('Run Scheduled Report Task', () => {
135135
mockReporting = await createMockReportingCore(configType);
136136

137137
soClient = await mockReporting.getInternalSoClient();
138-
soClient.get = jest.fn().mockImplementation(async () => {
139-
return reportSO;
140-
});
141138

142139
mockReporting.getExportTypesRegistry().register({
143140
id: 'test1',
@@ -153,6 +150,9 @@ describe('Run Scheduled Report Task', () => {
153150
});
154151

155152
beforeEach(async () => {
153+
soClient.get = jest.fn().mockImplementation(async () => {
154+
return reportSO;
155+
});
156156
reportStore = await mockReporting.getStore();
157157
reportStore.addReport = jest.fn().mockImplementation(async () => {
158158
return savedReport;
@@ -468,12 +468,57 @@ describe('Run Scheduled Report Task', () => {
468468
});
469469
const mockTaskManager = taskManagerMock.createStart();
470470
await task.init(mockTaskManager, emailNotificationService);
471-
const runAt = new Date('2025-06-04T00:00:00Z');
471+
const taskInstance = {
472+
id: 'task-id',
473+
runAt: new Date('2025-06-04T00:00:00Z'),
474+
params: { id: 'report-so-id', jobtype: 'test1' },
475+
};
476+
const byteSize = 2097152; // 2MB
477+
const output = { content_type: 'application/pdf' };
478+
479+
// @ts-expect-error
480+
await task.notify(savedReport, taskInstance, output, byteSize, reportSO, 'default');
481+
expect(soClient.get).not.toHaveBeenCalled();
482+
expect(emailNotificationService.notify).toHaveBeenCalledWith({
483+
contentType: 'application/pdf',
484+
emailParams: {
485+
bcc: ['test2@test.com'],
486+
cc: undefined,
487+
spaceId: 'default',
488+
subject: 'Test Report [2025-06-04T00:00:00.000Z] scheduled report',
489+
to: ['test1@test.com'],
490+
},
491+
id: '290357209345723095',
492+
index: '.reporting-fantastic',
493+
jobType: 'test1',
494+
relatedObject: {
495+
id: 'report-so-id',
496+
namespace: 'default',
497+
type: 'scheduled-report',
498+
},
499+
reporting: mockReporting,
500+
});
501+
});
502+
503+
it("gets the report SO if it's not defined", async () => {
504+
const task = new RunScheduledReportTask({
505+
reporting: mockReporting,
506+
config: configType,
507+
logger,
508+
});
509+
const mockTaskManager = taskManagerMock.createStart();
510+
await task.init(mockTaskManager, emailNotificationService);
511+
const taskInstance = {
512+
id: 'task-id',
513+
runAt: new Date('2025-06-04T00:00:00Z'),
514+
params: { id: 'report-so-id', jobtype: 'test1' },
515+
};
472516
const byteSize = 2097152; // 2MB
473517
const output = { content_type: 'application/pdf' };
474518

475519
// @ts-expect-error
476-
await task.notify(savedReport, output, runAt, byteSize, reportSO, 'default');
520+
await task.notify(savedReport, taskInstance, output, byteSize, undefined, 'default');
521+
expect(soClient.get).toHaveBeenCalled();
477522
expect(emailNotificationService.notify).toHaveBeenCalledWith({
478523
contentType: 'application/pdf',
479524
emailParams: {
@@ -486,6 +531,11 @@ describe('Run Scheduled Report Task', () => {
486531
id: '290357209345723095',
487532
index: '.reporting-fantastic',
488533
jobType: 'test1',
534+
relatedObject: {
535+
id: 'report-so-id',
536+
namespace: 'default',
537+
type: 'scheduled-report',
538+
},
489539
reporting: mockReporting,
490540
});
491541
});
@@ -498,19 +548,24 @@ describe('Run Scheduled Report Task', () => {
498548
});
499549
const mockTaskManager = taskManagerMock.createStart();
500550
await task.init(mockTaskManager, emailNotificationService);
501-
const runAt = new Date('2025-06-04T00:00:00Z');
551+
const taskInstance = {
552+
id: 'task-id',
553+
runAt: new Date('2025-06-04T00:00:00Z'),
554+
params: { id: 'report-so-id', jobtype: 'test1' },
555+
};
502556
const byteSize = 11534336; // 11MB
503557
const output = { content_type: 'application/pdf' };
504558

505559
// @ts-expect-error
506-
await task.notify(savedReport, output, runAt, byteSize, reportSO, 'default');
560+
await task.notify(savedReport, taskInstance, output, byteSize, reportSO, 'default');
507561
expect(emailNotificationService.notify).not.toHaveBeenCalled();
508562
expect(logger.warn).toHaveBeenCalledWith(
509-
'Error sending scheduled report: The report is larger than the 10MB limit.'
563+
'Error sending notification for scheduled report: The report is larger than the 10MB limit.'
510564
);
511565
expect(reportStore.setReportWarning).toHaveBeenCalledWith(savedReport, {
512566
output: { content_type: 'application/pdf', size: 11534336 },
513-
warning: 'Error sending scheduled report: The report is larger than the 10MB limit.',
567+
warning:
568+
'Error sending notification for scheduled report: The report is larger than the 10MB limit.',
514569
});
515570
});
516571

@@ -522,20 +577,24 @@ describe('Run Scheduled Report Task', () => {
522577
});
523578
const mockTaskManager = taskManagerMock.createStart();
524579
await task.init(mockTaskManager);
525-
const runAt = new Date('2025-06-04T00:00:00Z');
580+
const taskInstance = {
581+
id: 'task-id',
582+
runAt: new Date('2025-06-04T00:00:00Z'),
583+
params: { id: 'report-so-id', jobtype: 'test1' },
584+
};
526585
const byteSize = 2097152; // 2MB
527586
const output = { content_type: 'application/pdf' };
528587

529588
// @ts-expect-error
530-
await task.notify(savedReport, output, runAt, byteSize, reportSO, 'default');
589+
await task.notify(savedReport, taskInstance, output, byteSize, reportSO, 'default');
531590
expect(emailNotificationService.notify).not.toHaveBeenCalled();
532591
expect(logger.warn).toHaveBeenCalledWith(
533-
'Error sending scheduled report: Reporting notification service has not been initialized.'
592+
'Error sending notification for scheduled report: Reporting notification service has not been initialized.'
534593
);
535594
expect(reportStore.setReportWarning).toHaveBeenCalledWith(savedReport, {
536595
output: { content_type: 'application/pdf', size: 2097152 },
537596
warning:
538-
'Error sending scheduled report: Reporting notification service has not been initialized.',
597+
'Error sending notification for scheduled report: Reporting notification service has not been initialized.',
539598
});
540599
});
541600

@@ -550,12 +609,16 @@ describe('Run Scheduled Report Task', () => {
550609
});
551610
const mockTaskManager = taskManagerMock.createStart();
552611
await task.init(mockTaskManager, emailNotificationService);
553-
const runAt = new Date('2025-06-04T00:00:00Z');
612+
const taskInstance = {
613+
id: 'task-id',
614+
runAt: new Date('2025-06-04T00:00:00Z'),
615+
params: { id: 'report-so-id', jobtype: 'test1' },
616+
};
554617
const byteSize = 2097152; // 2MB
555618
const output = { content_type: 'application/pdf' };
556619

557620
// @ts-expect-error
558-
await task.notify(savedReport, output, runAt, byteSize, reportSO, 'default');
621+
await task.notify(savedReport, taskInstance, output, byteSize, reportSO, 'default');
559622
expect(emailNotificationService.notify).toHaveBeenCalledWith({
560623
contentType: 'application/pdf',
561624
emailParams: {
@@ -568,14 +631,19 @@ describe('Run Scheduled Report Task', () => {
568631
id: '290357209345723095',
569632
index: '.reporting-fantastic',
570633
jobType: 'test1',
634+
relatedObject: {
635+
id: 'report-so-id',
636+
namespace: 'default',
637+
type: 'scheduled-report',
638+
},
571639
reporting: mockReporting,
572640
});
573641
expect(logger.warn).toHaveBeenCalledWith(
574-
'Error sending scheduled report: This is a test error!'
642+
'Error sending notification for scheduled report: This is a test error!'
575643
);
576644
expect(reportStore.setReportWarning).toHaveBeenCalledWith(savedReport, {
577645
output: { content_type: 'application/pdf', size: 2097152 },
578-
warning: 'Error sending scheduled report: This is a test error!',
646+
warning: 'Error sending notification for scheduled report: This is a test error!',
579647
});
580648
});
581649
});

‎x-pack/platform/plugins/private/reporting/server/lib/tasks/run_scheduled_report.ts‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,8 @@ export class RunScheduledReportTask extends RunReportTask<ScheduledReportTaskPar
9494

9595
protected async notify(
9696
report: SavedReport,
97+
taskInstance: ConcreteTaskInstance,
9798
output: TaskRunResult,
98-
runAt: Date,
9999
byteSize: number,
100100
reportSO?: SavedObject<ScheduledReportType>,
101101
spaceId?: string
@@ -108,8 +108,20 @@ export class RunScheduledReportTask extends RunReportTask<ScheduledReportTaskPar
108108
throw new Error('Reporting notification service has not been initialized.');
109109
}
110110

111-
if (reportSO && reportSO.attributes.notification && reportSO.attributes.notification.email) {
112-
const { email } = reportSO.attributes.notification;
111+
const { runAt, params } = taskInstance;
112+
const task = params as ScheduledReportTaskParams;
113+
if (!reportSO) {
114+
const internalSoClient = await this.opts.reporting.getInternalSoClient();
115+
reportSO = await internalSoClient.get<ScheduledReportType>(
116+
SCHEDULED_REPORT_SAVED_OBJECT_TYPE,
117+
task.id,
118+
{ namespace: spaceId }
119+
);
120+
}
121+
122+
const { notification } = reportSO.attributes;
123+
if (notification && notification.email) {
124+
const email = notification.email;
113125
const title = reportSO.attributes.title;
114126

115127
await this.emailNotificationService.notify({
@@ -118,6 +130,11 @@ export class RunScheduledReportTask extends RunReportTask<ScheduledReportTaskPar
118130
id: report._id,
119131
jobType: report.jobtype,
120132
contentType: output.content_type,
133+
relatedObject: {
134+
id: reportSO.id,
135+
type: reportSO.type,
136+
namespace: spaceId,
137+
},
121138
emailParams: {
122139
to: email.to,
123140
cc: email.cc,
@@ -128,7 +145,7 @@ export class RunScheduledReportTask extends RunReportTask<ScheduledReportTaskPar
128145
});
129146
}
130147
} catch (error) {
131-
const message = `Error sending scheduled report: ${error.message}`;
148+
const message = `Error sending notification for scheduled report: ${error.message}`;
132149
await this.saveExecutionWarning(
133150
report,
134151
{

‎x-pack/platform/plugins/private/reporting/server/services/notifications/email_notification_service.test.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,11 @@ describe('EmailNotificationService', () => {
5151
id: '1234',
5252
jobType: 'pdf',
5353
contentType: 'test-content-type',
54+
relatedObject: {
55+
id: 'report-so-id',
56+
type: 'scheduled-report',
57+
namespace: 'space1',
58+
},
5459
emailParams: {
5560
to: ['test@test.com'],
5661
subject: 'Scheduled report for 04/18/2025',
@@ -72,4 +77,30 @@ describe('EmailNotificationService', () => {
7277
to: ['test@test.com'],
7378
});
7479
});
80+
81+
it('throws an error when the email service is not available', async () => {
82+
notifications.isEmailServiceAvailable.mockReturnValue(false);
83+
84+
await expect(
85+
emailNotificationService.notify({
86+
reporting: mockCore,
87+
index: '.reporting-test-1234',
88+
id: '1234',
89+
jobType: 'pdf',
90+
contentType: 'test-content-type',
91+
relatedObject: {
92+
id: 'report-so-id',
93+
type: 'scheduled-report',
94+
namespace: 'space1',
95+
},
96+
emailParams: {
97+
to: ['test@test.com'],
98+
subject: 'Scheduled report for 04/18/2025',
99+
spaceId: 'space1',
100+
},
101+
})
102+
).rejects.toThrowErrorMatchingInlineSnapshot(`"Email notification service is not available"`);
103+
104+
expect(notifications.getEmailService().sendAttachmentEmail).not.toHaveBeenCalled();
105+
});
75106
});

‎x-pack/platform/plugins/private/reporting/server/services/notifications/email_notification_service.ts‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,19 @@ export class EmailNotificationService implements NotificationService {
5454
];
5555
}
5656

57-
public async notify({ reporting, index, id, contentType, jobType, emailParams }: NotifyArgs) {
57+
public async notify({
58+
reporting,
59+
index,
60+
id,
61+
contentType,
62+
jobType,
63+
relatedObject,
64+
emailParams,
65+
}: NotifyArgs) {
66+
if (!this.notifications.isEmailServiceAvailable()) {
67+
throw new Error('Email notification service is not available');
68+
}
69+
5870
const attachments = await this.getAttachments(reporting, index, id, jobType, contentType);
5971
const { to, bcc, cc, subject, spaceId } = emailParams;
6072
const message = "Here's your report!";
@@ -66,6 +78,9 @@ export class EmailNotificationService implements NotificationService {
6678
message,
6779
attachments,
6880
spaceId: spaceId ?? DEFAULT_SPACE_ID,
81+
context: {
82+
relatedObjects: [relatedObject],
83+
},
6984
});
7085
}
7186
}

‎x-pack/platform/plugins/private/reporting/server/services/notifications/types.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
* 2.0; you may not use this file except in compliance with the Elastic License
55
* 2.0.
66
*/
7+
import { RelatedSavedObject } from '@kbn/notifications-plugin/server/services/types';
78
import { ReportingCore } from '../..';
89

910
export interface NotifyArgs {
@@ -12,6 +13,7 @@ export interface NotifyArgs {
1213
id: string;
1314
contentType?: string | null;
1415
jobType: string;
16+
relatedObject: RelatedSavedObject;
1517
emailParams: {
1618
to?: string[];
1719
bcc?: string[];

‎x-pack/platform/plugins/shared/stack_connectors/server/connector_types/email/send_email.test.ts‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,6 @@ describe('send_email module', () => {
9090
expect(sendMailMock.mock.calls[0]).toMatchInlineSnapshot(`
9191
Array [
9292
Object {
93-
"attachments": Array [],
9493
"bcc": Array [],
9594
"cc": Array [
9695
"bob@example.com",
@@ -140,7 +139,6 @@ describe('send_email module', () => {
140139
expect(sendMailMock.mock.calls[0]).toMatchInlineSnapshot(`
141140
Array [
142141
Object {
143-
"attachments": Array [],
144142
"bcc": Array [],
145143
"cc": Array [
146144
"bob@example.com",
@@ -471,7 +469,6 @@ describe('send_email module', () => {
471469
expect(sendMailMock.mock.calls[0]).toMatchInlineSnapshot(`
472470
Array [
473471
Object {
474-
"attachments": Array [],
475472
"bcc": Array [],
476473
"cc": Array [
477474
"bob@example.com",
@@ -525,7 +522,6 @@ describe('send_email module', () => {
525522
expect(sendMailMock.mock.calls[0]).toMatchInlineSnapshot(`
526523
Array [
527524
Object {
528-
"attachments": Array [],
529525
"bcc": Array [],
530526
"cc": Array [
531527
"bob@example.com",
@@ -581,7 +577,6 @@ describe('send_email module', () => {
581577
expect(sendMailMock.mock.calls[0]).toMatchInlineSnapshot(`
582578
Array [
583579
Object {
584-
"attachments": Array [],
585580
"bcc": Array [],
586581
"cc": Array [
587582
"bob@example.com",

0 commit comments

Comments
 (0)