Skip to content
This repository has been archived by the owner on Aug 9, 2022. It is now read-only.

Improve logging and error handling; Fix edit report bug; Fix header/footer rendering #123

Merged
merged 13 commits into from
Oct 15, 2020
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions kibana-reports/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
"@types/showdown": "^1.9.3",
"babel-polyfill": "^6.26.0",
"cron-validator": "^1.1.1",
"delay": "^4.4.0",
"elastic-builder": "^2.7.1",
"jquery": "^3.5.0",
"json-2-csv": "^3.7.6",
Expand Down
9 changes: 6 additions & 3 deletions kibana-reports/public/components/main/main_utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,12 @@ export const generateReport = async (metadata, httpClient) => {
},
})
.then(async (response) => {
const fileFormat = extractFileFormat(response['filename']);
const fileName = response['filename'];
await readStreamToFile(await response['data'], fileFormat, fileName);
// for emailing a report, this API response doesn't have response body
if (response) {
const fileFormat = extractFileFormat(response['filename']);
const fileName = response['filename'];
await readStreamToFile(await response['data'], fileFormat, fileName);
}
status = true;
})
.catch((error) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -305,18 +305,6 @@ export function ReportDefinitionDetails(props) {
updatedReportDefinition.status = 'Active';
}

// update report definition, delete extraneous schedule params
if (
updatedReportDefinition.trigger.trigger_params.schedule_type ===
'Recurring'
) {
delete updatedReportDefinition.trigger.trigger_params.schedule.cron;
} else if (
updatedReportDefinition.trigger.trigger_params.schedule_type ===
'Cron based'
) {
delete updatedReportDefinition.trigger.trigger_params.schedule.interval;
}
const { httpClient } = props;
httpClient
.put(`../api/reporting/reportDefinitions/${reportDefinitionId}`, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,6 @@ export function CreateReport(props) {
core_params: {
base_url: '',
report_format: '',
header: '',
footer: '',
time_duration: '',
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,13 @@ import {
EMAIL_RECIPIENT_OPTIONS,
} from './delivery_constants';
import ReactMDE from 'react-mde';
import Showdown from 'showdown';
import { ReportDeliveryProps } from './delivery';
import { ChannelSchemaType } from 'server/model';
import {
ChannelSchemaType,
DeliverySchemaType,
KibanaUserSchemaType,
} from 'server/model';
import { converter } from '../utils';

const INSERT_PLACEHOLDER_OPTIONS = [
{
Expand Down Expand Up @@ -62,14 +66,6 @@ const INSERT_PLACEHOLDER_OPTIONS = [
},
];

const converter = new Showdown.Converter({
tables: true,
simplifiedAutoLink: true,
strikethrough: true,
tasklists: true,
noHeaderId: true,
});

export const EmailDelivery = (props: ReportDeliveryProps) => {
const {
edit,
Expand Down Expand Up @@ -176,38 +172,49 @@ export const EmailDelivery = (props: ReportDeliveryProps) => {
const showPlaceholder =
emailFormat === 'Embedded HTML' ? null : <InsertPlaceholderPopover />;

const defaultEditDeliveryParams = (deliveryParams: ChannelSchemaType) => {
const defaultEditDeliveryParams = (delivery: DeliverySchemaType) => {
//TODO: need better handle?
delete reportDefinitionRequest.delivery.delivery_params.kibana_recipients;
if (delivery.delivery_type === 'Kibana user') {
//@ts-ignore
const kibanaUserParams: KibanaUserSchemaType = delivery.delivery_params;
const { kibana_recipients } = kibanaUserParams;
defaultCreateDeliveryParams();
delete reportDefinitionRequest.delivery.delivery_params.kibana_recipients;
} else {
//@ts-ignore
const emailParams: ChannelSchemaType = delivery.delivery_params;
const { recipients, title, textDescription, email_format } = emailParams;

deliveryParams.recipients.map((emailRecipient) =>
handleCreateEmailRecipient(emailRecipient, emailRecipients)
);
setEmailSubject(deliveryParams.title);
reportDefinitionRequest.delivery.delivery_params.title =
deliveryParams.title;
handleEmailBody(deliveryParams.textDescription);
handleEmailFormat(deliveryParams.email_format);
recipients.map((emailRecipient) =>
handleCreateEmailRecipient(emailRecipient, emailRecipients)
);
setEmailSubject(title);
reportDefinitionRequest.delivery.delivery_params.title = title;
handleEmailBody(textDescription);
handleEmailFormat(email_format);
}
};

const defaultCreateDeliveryParams = () => {
reportDefinitionRequest.delivery.delivery_params = {
recipients: emailRecipients.map((option) => option.label),
title: emailSubject,
email_format: emailFormat,
//TODO: need better render
textDescription: emailBody,
htmlDescription: converter.makeHtml(emailBody),
};
};

useEffect(() => {
if (edit) {
httpClientProps
.get(`../api/reporting/reportDefinitions/${editDefinitionId}`)
.then(async (response) => {
defaultEditDeliveryParams(
response.report_definition.delivery.delivery_params
);
defaultEditDeliveryParams(response.report_definition.delivery);
});
} else {
reportDefinitionRequest.delivery.delivery_params = {
recipients: emailRecipients.map((option) => option.label),
title: emailSubject,
email_format: emailFormat,
//TODO: need better render
textDescription: emailBody,
htmlDescription: converter.makeHtml(emailBody),
};
defaultCreateDeliveryParams();
}
}, []);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
import { ReportSettings } from '../report_settings';
import { ReportDelivery } from '../delivery';
import { ReportTrigger } from '../report_trigger';
import { ReportDefinitionSchemaType } from 'server/model';

export function EditReportDefinition(props) {
const [toasts, setToasts] = useState([]);
Expand All @@ -51,6 +52,7 @@ export function EditReportDefinition(props) {
};

const reportDefinitionId = props['match']['params']['reportDefinitionId'];
let curReportDefinition: ReportDefinitionSchemaType;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

np: can we spell out the entire word -> currentReportDefinition vs curReportDefinition

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will change

let editReportDefinitionRequest = {
report_params: {
report_name: '',
Expand All @@ -59,8 +61,6 @@ export function EditReportDefinition(props) {
core_params: {
base_url: '',
report_format: '',
header: '',
footer: '',
time_duration: '',
},
},
Expand All @@ -71,21 +71,25 @@ export function EditReportDefinition(props) {
trigger: {
trigger_type: '',
},
time_created: 0,
last_updated: 0,
status: '',
};

let timeRange = {
timeFrom: new Date(),
timeTo: new Date(),
};

const editReportDefinition = async (metadata) => {
const callUpdateAPI = async (metadata) => {
const { httpClient } = props;

httpClient
.put(`../api/reporting/reportDefinitions/${reportDefinitionId}`, {
body: JSON.stringify(metadata),
params: reportDefinitionId.toString(),
})
.then(async (response) => {
.then(async () => {
window.location.assign(`opendistro_kibana_reports#/`);
})
.catch((error) => {
Expand All @@ -94,22 +98,61 @@ export function EditReportDefinition(props) {
});
};

const editReportDefinition = async (metadata) => {
const { httpClient } = props;
/*
we check if this edit update the trigger type from Schedule to On demand.
If so, need to first delete the curReportDefinition along with the scheduled job first, by calling the delete
report definition API
*/
const {
trigger: { trigger_type: triggerType },
} = curReportDefinition;
if (
triggerType !== 'On demand' &&
metadata.trigger.trigger_type === 'On demand'
) {
httpClient
.delete(`../api/reporting/reportDefinitions/${reportDefinitionId}`)
.then(async () => {
await callUpdateAPI(metadata);
})
.catch((error) => {
console.log('error when deleting report definition:', error);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

np: add a toast for when this error is triggered

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will add

});
} else {
await callUpdateAPI(metadata);
}
};

useEffect(() => {
const { httpClient } = props;
httpClient
.get(`../api/reporting/reportDefinitions/${reportDefinitionId}`)
.then((response) => {
curReportDefinition = response.report_definition;
const {
time_created: timeCreated,
status,
last_updated: lastUpdated,
report_params: { report_name: reportName },
} = curReportDefinition;
// configure non-editable fields
editReportDefinitionRequest.time_created = timeCreated;
editReportDefinitionRequest.last_updated = lastUpdated;
editReportDefinitionRequest.status = status;

props.setBreadcrumbs([
{
text: 'Reporting',
href: '#',
},
{
text: `Report definition details: ${response.report_definition.report_params.report_name}`,
text: `Report definition details: ${reportName}`,
href: `#/report_definition_details/${reportDefinitionId}`,
},
{
text: `Edit report definition: ${response.report_definition.report_params.report_name}`,
text: `Edit report definition: ${reportName}`,
},
]);
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ import {
getDashboardOptions,
} from './report_settings_helpers';
import { TimeRangeSelect } from './time_range';
import { converter } from '../utils';
import { ReportDefinitionSchemaType } from 'server/model';

type ReportSettingProps = {
edit: boolean;
Expand Down Expand Up @@ -170,13 +172,6 @@ export function ReportSettings(props: ReportSettingProps) {
reportDefinitionRequest.report_params.core_params.report_format = e.toString();
};

const converter = new Showdown.Converter({
tables: true,
simplifiedAutoLink: true,
strikethrough: true,
tasklists: true,
});

const PDFandPNGFileFormats = () => {
return (
<div>
Expand Down Expand Up @@ -209,12 +204,16 @@ export function ReportSettings(props: ReportSettingProps) {

const handleHeader = (e) => {
setHeader(e);
reportDefinitionRequest.report_params.core_params.header = e;
reportDefinitionRequest.report_params.core_params.header = converter.makeHtml(
e
);
};

const handleFooter = (e) => {
setFooter(e);
reportDefinitionRequest.report_params.core_params.footer = e;
reportDefinitionRequest.report_params.core_params.footer = converter.makeHtml(
e
);
};

const handleCheckboxHeaderFooter = (optionId) => {
Expand Down Expand Up @@ -269,25 +268,24 @@ export function ReportSettings(props: ReportSettingProps) {
httpClientProps
.get(`../api/reporting/reportDefinitions/${editDefinitionId}`)
.then(async (response: {}) => {
const reportDefinition: ReportDefinitionSchemaType =
response.report_definition;
const {
report_params: {
core_params: { header, footer },
},
} = reportDefinition;
// set header/footer default
if (
response.report_definition.report_params.core_params.header != ''
) {
if (header) {
checkboxIdSelectHeaderFooter.header = true;
if (!unmounted) {
setHeader(
response.report_definition.report_params.core_params.header
);
setHeader(converter.makeMarkdown(header));
}
}
if (
response.report_definition.report_params.core_params.footer != ''
) {
if (footer) {
checkboxIdSelectHeaderFooter.footer = true;
if (!unmounted) {
setFooter(
response.report_definition.report_params.core_params.footer
);
setFooter(converter.makeMarkdown(header));
}
}
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,6 @@ import {
} from '@elastic/eui';
import moment, { Moment } from 'moment';
import { reportDefinitionParams } from '../create/create_report_definition';
import {
AVAILABLE_MONITOR_OPTIONS,
AVAILABLE_TRIGGER_OPTIONS,
} from './report_trigger_test_data';
import {
SCHEDULE_RECURRING_OPTIONS,
INTERVAL_TIME_PERIODS,
Expand Down Expand Up @@ -93,8 +89,6 @@ export function ReportTrigger(props: ReportTriggerProps) {
const [monthlyDaySelect, setMonthlyDaySelect] = useState(
MONTHLY_DAY_SELECT_OPTIONS[0].value
);
const [monitor, setMonitor] = useState(AVAILABLE_MONITOR_OPTIONS[0].value);
const [trigger, setTrigger] = useState(AVAILABLE_TRIGGER_OPTIONS[0].value);

const handleReportTriggerType = (e: string) => {
setReportTriggerType(e);
Expand Down Expand Up @@ -404,7 +398,7 @@ export function ReportTrigger(props: ReportTriggerProps) {
}
>
<EuiFieldText
placeholder={'Ex: 0 0 12 * * ? (Fire at 12:00 PM (noon) every day)'}
placeholder={'Ex: 0 12 * * * (Fire at 12:00 PM (noon) every day)'}
value={cronExpression}
onChange={handleCronExpression}
/>
Expand Down
Loading