Skip to content

Commit

Permalink
Do not fetch ML Jobs if StepRuleDescription is not rendering ML Data
Browse files Browse the repository at this point in the history
We were incorrectly invoking the useSecurityJobs hook any time the
StepRuleDescription component was rendered, regardless of whether we
actually needed that data.

This moves the useSecurityJobs hook into the component that uses it,
MlJobDescription. If we end up having multiple of these on the page we
can evaluate caching/sharing this data somehow, but for now this
prevents:

* 3x3=9 unnecessary ML calls on the Rule Create page.
* 1x3=3 unnecessary ML calls on Rule Details
* 1x3=3 unnecessary ML Calls on the Rule Edit page.
  • Loading branch information
rylnd committed Sep 3, 2020
1 parent deeaa48 commit aac72c9
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import {
buildRuleTypeDescription,
buildThresholdDescription,
} from './helpers';
import { useSecurityJobs } from '../../../../common/components/ml_popover/hooks/use_security_jobs';
import { buildMlJobDescription } from './ml_job_description';
import { buildActionsDescription } from './actions_description';
import { buildThrottleDescription } from './throttle_description';
Expand Down Expand Up @@ -67,7 +66,6 @@ export const StepRuleDescriptionComponent = <T,>({
}: StepRuleDescriptionProps<T>) => {
const kibana = useKibana();
const [filterManager] = useState<FilterManager>(new FilterManager(kibana.services.uiSettings));
const { jobs } = useSecurityJobs(false);

const keys = Object.keys(schema);
const listItems = keys.reduce((acc: ListItems[], key: string) => {
Expand All @@ -76,8 +74,7 @@ export const StepRuleDescriptionComponent = <T,>({
...acc,
buildMlJobDescription(
get(key, data) as string,
(get(key, schema) as { label: string }).label,
jobs
(get(key, schema) as { label: string }).label
),
];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jest.mock('../../../../common/lib/kibana');

describe('MlJobDescription', () => {
it('renders correctly', () => {
const wrapper = shallow(<MlJobDescription job={mockOpenedJob} />);
const wrapper = shallow(<MlJobDescription jobId={'myJobId'} />);

expect(wrapper.find('[data-test-subj="machineLearningJobId"]')).toHaveLength(1);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { EuiBadge, EuiIcon, EuiLink, EuiToolTip } from '@elastic/eui';

import { MlSummaryJob } from '../../../../../../ml/public';
import { isJobStarted } from '../../../../../common/machine_learning/helpers';
import { useSecurityJobs } from '../../../../common/components/ml_popover/hooks/use_security_jobs';
import { useKibana } from '../../../../common/lib/kibana';
import { ListItems } from './types';
import { ML_JOB_STARTED, ML_JOB_STOPPED } from './translations';
Expand Down Expand Up @@ -69,35 +70,33 @@ const Wrapper = styled.div`
overflow: hidden;
`;

const MlJobDescriptionComponent: React.FC<{ job: MlSummaryJob }> = ({ job }) => {
const MlJobDescriptionComponent: React.FC<{ jobId: string }> = ({ jobId }) => {
const { jobs } = useSecurityJobs(false);
const jobUrl = useKibana().services.application.getUrlForApp(
`ml#/jobs?mlManagement=(jobId:${encodeURI(job.id)})`
`ml#/jobs?mlManagement=(jobId:${encodeURI(jobId)})`
);
const job = jobs.find(({ id }) => id === jobId);

return (
const jobIdSpan = <span data-test-subj="machineLearningJobId">{jobId}</span>;

return job != null ? (
<Wrapper>
<div>
<JobLink data-test-subj="machineLearningJobId" href={jobUrl} target="_blank">
{job.id}
<JobLink href={jobUrl} target="_blank">
{jobIdSpan}
</JobLink>
<AuditIcon message={job.auditMessage} />
</div>
<JobStatusBadge job={job} />
</Wrapper>
) : (
jobIdSpan
);
};

export const MlJobDescription = React.memo(MlJobDescriptionComponent);

export const buildMlJobDescription = (
jobId: string,
label: string,
jobs: MlSummaryJob[]
): ListItems => {
const job = jobs.find(({ id }) => id === jobId);

return {
title: label,
description: job ? <MlJobDescription job={job} /> : jobId,
};
};
export const buildMlJobDescription = (jobId: string, label: string): ListItems => ({
title: label,
description: <MlJobDescription jobId={jobId} />,
});

0 comments on commit aac72c9

Please sign in to comment.