-
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
[Workplace Search] Convert Overview & Security pages to new page template #102444
Changes from all commits
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 |
---|---|---|
|
@@ -5,61 +5,48 @@ | |
* 2.0. | ||
*/ | ||
|
||
import '../../../__mocks__/shallow_useeffect.mock'; | ||
import './__mocks__/overview_logic.mock'; | ||
import { mockActions, setMockValues } from './__mocks__'; | ||
|
||
import React from 'react'; | ||
|
||
import { shallow, mount } from 'enzyme'; | ||
|
||
import { Loading } from '../../../shared/loading'; | ||
import { ViewContentHeader } from '../../components/shared/view_content_header'; | ||
import { shallow } from 'enzyme'; | ||
|
||
import { OnboardingSteps } from './onboarding_steps'; | ||
import { OrganizationStats } from './organization_stats'; | ||
import { Overview } from './overview'; | ||
import { RecentActivity } from './recent_activity'; | ||
|
||
describe('Overview', () => { | ||
describe('non-happy-path states', () => { | ||
it('isLoading', () => { | ||
Comment on lines
-24
to
-25
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. Mostly just indenting changes in this file due to deleting the happy/non-happy path blocks - I recommend viewing the diff with whitespace changes off. Also as a heads up, I'm removing most |
||
const wrapper = shallow(<Overview />); | ||
it('calls initialize function', async () => { | ||
shallow(<Overview />); | ||
|
||
expect(wrapper.find(Loading)).toHaveLength(1); | ||
}); | ||
expect(mockActions.initializeOverview).toHaveBeenCalled(); | ||
}); | ||
|
||
describe('happy-path states', () => { | ||
it('calls initialize function', async () => { | ||
mount(<Overview />); | ||
|
||
expect(mockActions.initializeOverview).toHaveBeenCalled(); | ||
}); | ||
it('renders onboarding state', () => { | ||
setMockValues({ dataLoading: false }); | ||
const wrapper = shallow(<Overview />); | ||
|
||
it('renders onboarding state', () => { | ||
setMockValues({ dataLoading: false }); | ||
const wrapper = shallow(<Overview />); | ||
expect(wrapper.find(OnboardingSteps)).toHaveLength(1); | ||
expect(wrapper.find(OrganizationStats)).toHaveLength(1); | ||
expect(wrapper.find(RecentActivity)).toHaveLength(1); | ||
}); | ||
|
||
expect(wrapper.find(ViewContentHeader)).toHaveLength(1); | ||
expect(wrapper.find(OnboardingSteps)).toHaveLength(1); | ||
expect(wrapper.find(OrganizationStats)).toHaveLength(1); | ||
expect(wrapper.find(RecentActivity)).toHaveLength(1); | ||
it('renders when onboarding complete', () => { | ||
setMockValues({ | ||
dataLoading: false, | ||
hasUsers: true, | ||
hasOrgSources: true, | ||
isOldAccount: true, | ||
organization: { | ||
name: 'foo', | ||
defaultOrgName: 'bar', | ||
}, | ||
}); | ||
const wrapper = shallow(<Overview />); | ||
|
||
it('renders when onboarding complete', () => { | ||
setMockValues({ | ||
dataLoading: false, | ||
hasUsers: true, | ||
hasOrgSources: true, | ||
isOldAccount: true, | ||
organization: { | ||
name: 'foo', | ||
defaultOrgName: 'bar', | ||
}, | ||
}); | ||
const wrapper = shallow(<Overview />); | ||
|
||
expect(wrapper.find(OnboardingSteps)).toHaveLength(0); | ||
}); | ||
expect(wrapper.find(OnboardingSteps)).toHaveLength(0); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,11 +14,8 @@ import { shallow } from 'enzyme'; | |
|
||
import { EuiSwitch, EuiConfirmModal } from '@elastic/eui'; | ||
|
||
import { SetWorkplaceSearchChrome as SetPageChrome } from '../../../shared/kibana_chrome'; | ||
|
||
import { Loading } from '../../../shared/loading'; | ||
import { UnsavedChangesPrompt } from '../../../shared/unsaved_changes_prompt'; | ||
import { ViewContentHeader } from '../../components/shared/view_content_header'; | ||
import { getPageHeaderActions } from '../../../test_helpers'; | ||
|
||
import { Security } from './security'; | ||
|
||
|
@@ -59,9 +56,7 @@ describe('Security', () => { | |
setMockValues({ ...mockValues, hasPlatinumLicense: false }); | ||
const wrapper = shallow(<Security />); | ||
|
||
expect(wrapper.find(SetPageChrome)).toHaveLength(1); | ||
expect(wrapper.find(UnsavedChangesPrompt)).toHaveLength(1); | ||
expect(wrapper.find(ViewContentHeader)).toHaveLength(1); | ||
expect(wrapper.find(EuiSwitch).prop('disabled')).toEqual(true); | ||
}); | ||
|
||
|
@@ -71,13 +66,6 @@ describe('Security', () => { | |
expect(wrapper.find(EuiSwitch).prop('disabled')).toEqual(false); | ||
}); | ||
|
||
it('returns Loading when loading', () => { | ||
setMockValues({ ...mockValues, dataLoading: true }); | ||
const wrapper = shallow(<Security />); | ||
|
||
expect(wrapper.find(Loading)).toHaveLength(1); | ||
}); | ||
|
||
it('handles switch click', () => { | ||
const wrapper = shallow(<Security />); | ||
|
||
|
@@ -92,8 +80,8 @@ describe('Security', () => { | |
setMockValues({ ...mockValues, unsavedChanges: true }); | ||
const wrapper = shallow(<Security />); | ||
|
||
const header = wrapper.find(ViewContentHeader).dive(); | ||
header.find('[data-test-subj="SaveSettingsButton"]').prop('onClick')!({} as any); | ||
const headerActions = getPageHeaderActions(wrapper); | ||
headerActions.find('[data-test-subj="SaveSettingsButton"]').prop('onClick')!({} as any); | ||
Comment on lines
+83
to
+84
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. new test_helpers already paying off in spades |
||
const modal = wrapper.find(EuiConfirmModal); | ||
modal.prop('onConfirm')!({} as any); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,13 +22,10 @@ import { | |
EuiConfirmModal, | ||
} from '@elastic/eui'; | ||
|
||
import { FlashMessages } from '../../../shared/flash_messages'; | ||
import { SetWorkplaceSearchChrome as SetPageChrome } from '../../../shared/kibana_chrome'; | ||
import { LicensingLogic } from '../../../shared/licensing'; | ||
import { Loading } from '../../../shared/loading'; | ||
import { UnsavedChangesPrompt } from '../../../shared/unsaved_changes_prompt'; | ||
import { WorkplaceSearchPageTemplate } from '../../components/layout'; | ||
import { LicenseCallout } from '../../components/shared/license_callout'; | ||
import { ViewContentHeader } from '../../components/shared/view_content_header'; | ||
import { | ||
SECURITY_UNSAVED_CHANGES_MESSAGE, | ||
RESET_BUTTON, | ||
|
@@ -72,44 +69,24 @@ export const Security: React.FC = () => { | |
initializeSourceRestrictions(); | ||
}, []); | ||
|
||
if (dataLoading) return <Loading />; | ||
|
||
const savePrivateSources = () => { | ||
saveSourceRestrictions(); | ||
hideConfirmModal(); | ||
}; | ||
|
||
const headerActions = ( | ||
<EuiFlexGroup alignItems="center" justifyContent="flexStart" gutterSize="m"> | ||
<EuiFlexItem grow={false}> | ||
<EuiButtonEmpty disabled={!unsavedChanges} onClick={resetState}> | ||
{RESET_BUTTON} | ||
</EuiButtonEmpty> | ||
</EuiFlexItem> | ||
<EuiFlexItem> | ||
<EuiButton | ||
disabled={!hasPlatinumLicense || !unsavedChanges} | ||
onClick={showConfirmModal} | ||
fill | ||
data-test-subj="SaveSettingsButton" | ||
> | ||
{SAVE_SETTINGS_BUTTON} | ||
</EuiButton> | ||
</EuiFlexItem> | ||
</EuiFlexGroup> | ||
); | ||
|
||
const header = ( | ||
<> | ||
<ViewContentHeader | ||
title={PRIVATE_SOURCES} | ||
alignItems="flexStart" | ||
description={PRIVATE_SOURCES_DESCRIPTION} | ||
action={headerActions} | ||
/> | ||
<EuiSpacer /> | ||
</> | ||
); | ||
const headerActions = [ | ||
<EuiButtonEmpty disabled={!unsavedChanges || dataLoading} onClick={resetState}> | ||
{RESET_BUTTON} | ||
</EuiButtonEmpty>, | ||
<EuiButton | ||
disabled={!hasPlatinumLicense || !unsavedChanges || dataLoading} | ||
onClick={showConfirmModal} | ||
fill | ||
data-test-subj="SaveSettingsButton" | ||
> | ||
{SAVE_SETTINGS_BUTTON} | ||
</EuiButton>, | ||
]; | ||
|
||
const allSourcesToggle = ( | ||
<EuiPanel | ||
|
@@ -180,20 +157,25 @@ export const Security: React.FC = () => { | |
); | ||
|
||
return ( | ||
<> | ||
<SetPageChrome trail={[NAV.SECURITY]} /> | ||
<FlashMessages /> | ||
<WorkplaceSearchPageTemplate | ||
pageChrome={[NAV.SECURITY]} | ||
pageHeader={{ | ||
pageTitle: PRIVATE_SOURCES, | ||
description: PRIVATE_SOURCES_DESCRIPTION, | ||
rightSideItems: headerActions, | ||
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. nit (not for this PR): What do you think about renaming 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. This is an EuiPageTemplate prop, we don't control it! https://elastic.github.io/eui/#/layout/page and https://elastic.github.io/eui/#/layout/page-header for reference 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. Sorry still waking up so I just realized I didn't fully answer you. I totally agree with you on the rtl thing - I like 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. Ah, didn't know that! Well, ok, then. 🤷 |
||
}} | ||
isLoading={dataLoading} | ||
> | ||
<UnsavedChangesPrompt | ||
hasUnsavedChanges={unsavedChanges} | ||
messageText={SECURITY_UNSAVED_CHANGES_MESSAGE} | ||
/> | ||
{header} | ||
<EuiPanel color="subdued" hasBorder={false}> | ||
{allSourcesToggle} | ||
{!hasPlatinumLicense && platinumLicenseCallout} | ||
{sourceTables} | ||
</EuiPanel> | ||
{confirmModalVisible && confirmModal} | ||
</> | ||
</WorkplaceSearchPageTemplate> | ||
); | ||
}; |
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.
This error state isn't going away long term - I'll handle 404 & 500 states in the final commit/pass that cleans up all the layouts and so forth. Basically it's going to get handled in 1 place at the top-level going forward like how App Search manages it.