-
Notifications
You must be signed in to change notification settings - Fork 8.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[System Actions] Complete API audit and update APIs to comply with System Actions #172937
Changes from 4 commits
4f51f06
8909cb0
c50e32b
c157663
1057ce6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,13 @@ | |
*/ | ||
import { omit } from 'lodash'; | ||
|
||
import { RuleTypeParams, SanitizedRule, RuleLastRun } from '../../types'; | ||
import { | ||
RuleTypeParams, | ||
SanitizedRule, | ||
RuleLastRun, | ||
RuleActionTypes, | ||
RuleDefaultAction, | ||
} from '../../types'; | ||
|
||
export const rewriteRuleLastRun = (lastRun: RuleLastRun) => { | ||
const { outcomeMsg, outcomeOrder, alertsCount, ...rest } = lastRun; | ||
|
@@ -58,23 +64,37 @@ export const rewriteRule = ({ | |
last_execution_date: executionStatus.lastExecutionDate, | ||
last_duration: executionStatus.lastDuration, | ||
}, | ||
actions: actions.map(({ group, id, actionTypeId, params, frequency, uuid, alertsFilter }) => ({ | ||
group, | ||
id, | ||
params, | ||
connector_type_id: actionTypeId, | ||
...(frequency | ||
? { | ||
frequency: { | ||
summary: frequency.summary, | ||
notify_when: frequency.notifyWhen, | ||
throttle: frequency.throttle, | ||
}, | ||
} | ||
: {}), | ||
...(uuid && { uuid }), | ||
...(alertsFilter && { alerts_filter: alertsFilter }), | ||
})), | ||
actions: actions.map( | ||
({ id, actionTypeId, params, uuid, type, useAlertDataForTemplate, ...action }) => { | ||
if (type === RuleActionTypes.SYSTEM) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's write some unit tests for this as well |
||
return { | ||
...action, | ||
connector_type_id: actionTypeId, | ||
...(typeof useAlertDataForTemplate !== 'undefined' | ||
? { use_alert_data_for_template: useAlertDataForTemplate } | ||
: {}), | ||
}; | ||
} | ||
const { group, frequency, alertsFilter } = action as RuleDefaultAction; | ||
return { | ||
group, | ||
id, | ||
params, | ||
connector_type_id: actionTypeId, | ||
...(frequency | ||
? { | ||
frequency: { | ||
summary: frequency.summary, | ||
notify_when: frequency.notifyWhen, | ||
throttle: frequency.throttle, | ||
}, | ||
} | ||
: {}), | ||
...(uuid && { uuid }), | ||
...(alertsFilter && { alerts_filter: alertsFilter }), | ||
}; | ||
} | ||
), | ||
...(lastRun ? { last_run: rewriteRuleLastRun(lastRun) } : {}), | ||
...(nextRun ? { next_run: nextRun } : {}), | ||
...(apiKeyCreatedByUser !== undefined ? { api_key_created_by_user: apiKeyCreatedByUser } : {}), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,7 +16,7 @@ import { | |
handleDisabledApiKeysError, | ||
actionsSchema, | ||
rewriteRuleLastRun, | ||
rewriteActionsReqWithSystemActions, | ||
rewriteActionsReq, | ||
} from './lib'; | ||
import { | ||
RuleTypeParams, | ||
|
@@ -70,7 +70,7 @@ const rewriteBodyReq = ( | |
data: { | ||
...rest, | ||
notifyWhen, | ||
actions: rewriteActionsReqWithSystemActions(actions, isSystemAction), | ||
actions: rewriteActionsReq(actions, isSystemAction), | ||
}, | ||
}; | ||
}; | ||
|
@@ -146,15 +146,13 @@ export const updateRuleRoute = ( | |
router.handleLegacyErrors( | ||
verifyAccessAndContext(licenseState, async function (context, req, res) { | ||
const rulesClient = (await context.alerting).getRulesClient(); | ||
const actionsClient = (await context.actions).getActionsClient(); | ||
const { isSystemAction } = (await context.actions).getActionsClient(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we have E2E test cases to assert the correct output for these endpoints? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
||
const { id } = req.params; | ||
const rule = req.body; | ||
try { | ||
const alertRes = await rulesClient.update( | ||
rewriteBodyReq({ id, data: rule }, (connectorId: string) => | ||
actionsClient.isSystemAction(connectorId) | ||
) | ||
rewriteBodyReq({ id, data: rule }, isSystemAction) | ||
); | ||
return res.ok({ | ||
body: rewriteBodyRes(alertRes), | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's write some unit tests for this