Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import React from 'react';

import { shallow } from 'enzyme';

import { SuggestedCurationsCallout } from '../../engine_overview/components/suggested_curations_callout';
import {
AnalyticsCards,
AnalyticsChart,
Expand Down Expand Up @@ -40,6 +41,7 @@ describe('Analytics overview', () => {
});
const wrapper = shallow(<Analytics />);

expect(wrapper.find(SuggestedCurationsCallout)).toHaveLength(1);
expect(wrapper.find(AnalyticsCards)).toHaveLength(1);
expect(wrapper.find(AnalyticsChart)).toHaveLength(1);
expect(wrapper.find(AnalyticsSection)).toHaveLength(2);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
import { DataPanel } from '../../data_panel';
import { generateEnginePath } from '../../engine';

import { SuggestedCurationsCallout } from '../../engine_overview/components/suggested_curations_callout';
import { AnalyticsLayout } from '../analytics_layout';
import { AnalyticsSection, AnalyticsTable, RecentQueriesTable } from '../components';
import {
Expand Down Expand Up @@ -60,6 +61,7 @@ export const Analytics: React.FC = () => {

return (
<AnalyticsLayout isAnalyticsView title={ANALYTICS_TITLE}>
<SuggestedCurationsCallout />
<EuiFlexGroup alignItems="center">
<EuiFlexItem grow={1}>
<AnalyticsCards
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import '../../../../__mocks__/react_router';

jest.mock('../../../../shared/use_local_storage', () => ({
useLocalStorage: jest.fn(),
}));

import React from 'react';

import { useLocation } from 'react-router-dom';

import { shallow } from 'enzyme';

import { EuiButtonEmpty, EuiCallOut } from '@elastic/eui';

import { EuiButtonTo } from '../../../../shared/react_router_helpers';
import { useLocalStorage } from '../../../../shared/use_local_storage';

import { SuggestionsCallout } from './suggestions_callout';

const props = {
title: 'Title',
description: 'A description.',
buttonTo: '/suggestions',
};

const now = '2021-01-01T00:30:00Z';
const tenMinutesAgo = '2021-01-01T00:20:00Z';
const twentyMinutesAgo = '2021-01-01T00:10:00Z';

describe('SuggestionsCallout', () => {
const mockSetLastDismissedTimestamp = jest.fn();
const setMockLastDismissedTimestamp = (lastDismissedTimestamp: string) => {
(useLocalStorage as jest.Mock).mockImplementation(() => [
lastDismissedTimestamp,
mockSetLastDismissedTimestamp,
]);
};

beforeEach(() => {
jest.clearAllMocks();
setMockLastDismissedTimestamp(tenMinutesAgo);
(useLocation as jest.Mock).mockImplementationOnce(() => ({
pathname: '/engines/some-engine',
}));
});

it('renders a callout with a link to the suggestions', () => {
const wrapper = shallow(<SuggestionsCallout {...props} lastUpdatedTimestamp={now} />);

expect(wrapper.find(EuiCallOut));
expect(wrapper.find(EuiButtonTo).prop('to')).toEqual('/suggestions');
});

it('is empty is it was updated before it was last dismissed', () => {
const wrapper = shallow(
<SuggestionsCallout {...props} lastUpdatedTimestamp={twentyMinutesAgo} />
);

expect(wrapper.isEmptyRender()).toBe(true);
});

it('clicking the dismiss button updates the timestamp in local storage', () => {
jest.spyOn(global.Date.prototype, 'toISOString').mockImplementation(() => now);

const wrapper = shallow(<SuggestionsCallout {...props} lastUpdatedTimestamp={now} />);
wrapper.find(EuiButtonEmpty).simulate('click');

expect(mockSetLastDismissedTimestamp).toHaveBeenCalledWith(now);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import React from 'react';

import { useLocation } from 'react-router-dom';

import {
EuiButtonEmpty,
EuiCallOut,
EuiFlexGroup,
EuiFlexItem,
EuiSpacer,
EuiText,
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';

import { LightbulbIcon } from '../../../../shared/icons';
import { EuiButtonTo } from '../../../../shared/react_router_helpers';
import { useLocalStorage } from '../../../../shared/use_local_storage';

interface SuggestionsCalloutProps {
title: string;
description: string;
buttonTo: string;
lastUpdatedTimestamp: string; // ISO string like '2021-10-04T18:53:02.784Z'
}

export const SuggestionsCallout: React.FC<SuggestionsCalloutProps> = ({
title,
description,
buttonTo,
lastUpdatedTimestamp,
}) => {
const { pathname } = useLocation();

const [lastDismissedTimestamp, setLastDismissedTimestamp] = useLocalStorage<string>(
`suggestions-callout--${pathname}`,
new Date(0).toISOString()
);

if (new Date(lastDismissedTimestamp) >= new Date(lastUpdatedTimestamp)) {
return null;
}

return (
<>
<EuiCallOut color="success" iconType={LightbulbIcon} title={title}>
<EuiText size="s">
<p>{description}</p>
</EuiText>
<EuiSpacer size="m" />
<EuiFlexGroup gutterSize="s">
<EuiFlexItem grow={false}>
<EuiButtonTo to={buttonTo} color="success" fill size="s">
{i18n.translate(
'xpack.enterpriseSearch.appSearch.engine.curations.suggestionsCallout.reviewSuggestionsButtonLabel',
{ defaultMessage: 'Review suggestions' }
)}
</EuiButtonTo>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiButtonEmpty
color="success"
iconType="eyeClosed"
size="s"
onClick={() => {
setLastDismissedTimestamp(new Date().toISOString());
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

An ISO timestamp is of form 2021-10-04T22:53:20.588Z. This matches the style of the timestamp we get back from the API. We've used both ISO timestamps and unix timestamps interchangeably in our codebase so far.

}}
>
{i18n.translate(
'xpack.enterpriseSearch.appSearch.engine.curations.suggestionsCallout.hideForNowLabel',
{ defaultMessage: 'Hide this for now' }
)}
</EuiButtonEmpty>
</EuiFlexItem>
</EuiFlexGroup>
</EuiCallOut>
<EuiSpacer />
</>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { CurationLogic } from './curation_logic';

import { ManualCuration } from './manual_curation';
import { AddResultFlyout } from './results';
import { SuggestedDocumentsCallout } from './suggested_documents_callout';

describe('ManualCuration', () => {
const values = {
Expand Down Expand Up @@ -50,6 +51,12 @@ describe('ManualCuration', () => {
]);
});

it('contains a suggested documents callout', () => {
const wrapper = shallow(<ManualCuration />);

expect(wrapper.find(SuggestedDocumentsCallout)).toHaveLength(1);
});

it('renders the add result flyout when open', () => {
setMockValues({ ...values, isFlyoutOpen: true });
const wrapper = shallow(<ManualCuration />);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { CurationLogic } from './curation_logic';
import { PromotedDocuments, OrganicDocuments, HiddenDocuments } from './documents';
import { ActiveQuerySelect, ManageQueriesModal } from './queries';
import { AddResultLogic, AddResultFlyout } from './results';
import { SuggestedDocumentsCallout } from './suggested_documents_callout';

export const ManualCuration: React.FC = () => {
const { curationId } = useParams() as { curationId: string };
Expand All @@ -46,6 +47,7 @@ export const ManualCuration: React.FC = () => {
}}
isLoading={dataLoading}
>
<SuggestedDocumentsCallout />
<EuiFlexGroup alignItems="flexEnd" gutterSize="xl" responsive={false}>
<EuiFlexItem>
<ActiveQuerySelect />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import '../../../__mocks__/engine_logic.mock';

import { setMockValues } from '../../../../__mocks__/kea_logic';

import React from 'react';

import { shallow } from 'enzyme';
import { set } from 'lodash/fp';

import { SuggestionsCallout } from '../components/suggestions_callout';

import { SuggestedDocumentsCallout } from './suggested_documents_callout';

const MOCK_VALUES = {
// CurationLogic
curation: {
suggestion: {
status: 'pending',
updated_at: '2021-01-01T00:30:00Z',
},
queries: ['some query'],
},
};

describe('SuggestedDocumentsCallout', () => {
beforeEach(() => {
jest.clearAllMocks();
setMockValues(MOCK_VALUES);
});

it('renders', () => {
const wrapper = shallow(<SuggestedDocumentsCallout />);

expect(wrapper.is(SuggestionsCallout));
});

it('is empty when the suggested is undefined', () => {
setMockValues({ ...MOCK_VALUES, curation: {} });

const wrapper = shallow(<SuggestedDocumentsCallout />);

expect(wrapper.isEmptyRender()).toBe(true);
});

it('is empty when curation status is not pending', () => {
const values = set('curation.suggestion.status', 'applied', MOCK_VALUES);
setMockValues(values);
const wrapper = shallow(<SuggestedDocumentsCallout />);

expect(wrapper.isEmptyRender()).toBe(true);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import React from 'react';

import { useValues } from 'kea';

import { i18n } from '@kbn/i18n';

import { ENGINE_CURATION_SUGGESTION_PATH } from '../../../routes';
import { generateEnginePath } from '../../engine';

import { SuggestionsCallout } from '../components/suggestions_callout';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice, I like how these share a base callout component.


import { CurationLogic } from '.';

export const SuggestedDocumentsCallout: React.FC = () => {
const {
curation: { suggestion, queries },
} = useValues(CurationLogic);

if (typeof suggestion === 'undefined' || suggestion.status !== 'pending') {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just curious why you check for undefined rather than falsy.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would be nice to have statuses in an enum or at constants.

Copy link
Copy Markdown
Contributor Author

@byronhulcher byronhulcher Oct 5, 2021

Choose a reason for hiding this comment

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

Just curious why you check for undefined rather than falsy.

To be honest, I default to checking specifically for undefined so I don't have to remember whether false/0/""/{} etc are a valid value for the variable or not.

Would be nice to have statuses in an enum or at constants.

This field uses a union to enforce specific values

  status: 'pending' | 'applied' | 'automated' | 'rejected' | 'disabled';

VSCode and tsc will complain if you use a value here that's not one of these strings. And I can still hover over stuggestion.status and it lists all the valid strings. In fact its kind of more useful than enums because for an SuggestionStatus enum it would just say status: SuggestionStatus.

When I used enums on Crawler there was some pushback against that and towards using unions. I dunno which is better tbh. For what its worth I think using a union still enforces a single "source of truth" for valid values, and I think its unlikely we'd change these valid values (that would be a breaking API change) so I'm not super worried about having to search/replace this. Anyway tsc would let us know which values we forgot to update.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

OK, maybe adding the strings to a constants file at least. We can deal with that in a different PR if we want.

return null;
}

return (
<SuggestionsCallout
title={i18n.translate(
'xpack.enterpriseSearch.appSearch.engine.curation.suggestedDocumentsCallout.title',
{ defaultMessage: 'New suggested documents for this query' }
)}
description={i18n.translate(
'xpack.enterpriseSearch.appSearch.engine.curation.suggestedDocumentsCallout.description',
{
defaultMessage:
"Based on your engine's analytics, there are new suggested document promotions ready to review.",
}
)}
buttonTo={generateEnginePath(ENGINE_CURATION_SUGGESTION_PATH, {
query: queries[0],
})}
lastUpdatedTimestamp={suggestion.updated_at}
/>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,21 @@ export interface Engine {
};
}

interface CurationSuggestionDetails {
count: number;
pending: number;
applied: number;
automated: number;
rejected: number;
disabled: number;
last_updated: string;
}

interface SearchRelevanceSuggestionDetails {
count: number;
curation: CurationSuggestionDetails;
}

export interface EngineDetails extends Engine {
created_at: string;
document_count: number;
Expand All @@ -38,6 +53,7 @@ export interface EngineDetails extends Engine {
isMeta: boolean;
engine_count?: number;
includedEngines?: EngineDetails[];
search_relevance_suggestions?: SearchRelevanceSuggestionDetails;
}

interface ResultField {
Expand Down
Loading