Skip to content
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

Fixes an issue when loading unknown datasource causes page to crash #1947

Closed
Closed
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
2a2d757
Adds a separate handler for unknown datasource
DarshitChanpura May 9, 2024
9d6aa1d
Fixes linter errors
DarshitChanpura May 9, 2024
60d9910
Merge branch 'main' into unknown-ds-fix
DarshitChanpura May 9, 2024
a45e592
Merge remote-tracking branch 'upstream/main' into unknown-ds-fix
DarshitChanpura May 10, 2024
2775a8c
Changes button text
DarshitChanpura May 10, 2024
3e05fc4
Remove button and adds text to keep better user experience
DarshitChanpura May 10, 2024
ad30c1d
Adds @opensearch-project/oui dependency
DarshitChanpura May 10, 2024
891e1bf
Removes @opensearch-project/oui dependency due to compilation failures
DarshitChanpura May 11, 2024
62f28a9
Fixes unit tests
DarshitChanpura May 11, 2024
e6321b8
Fixes remainder unit tests
DarshitChanpura May 11, 2024
2b37fa5
Adds unit tests to expand code coverage
DarshitChanpura May 11, 2024
7a26e9f
Adds couple more unit tests
DarshitChanpura May 11, 2024
05d8d55
Merge branch 'main' into unknown-ds-fix
DarshitChanpura May 13, 2024
6e8284d
Fixes a call to dataSource id causing test failures
DarshitChanpura May 13, 2024
7d3193c
Merge remote-tracking branch 'origin/unknown-ds-fix' into unknown-ds-fix
DarshitChanpura May 13, 2024
ac06227
Merge branch 'main' into unknown-ds-fix
DarshitChanpura May 15, 2024
76880f8
Removes console logs
DarshitChanpura May 17, 2024
1e314a7
Merge remote-tracking branch 'origin/unknown-ds-fix' into unknown-ds-fix
DarshitChanpura May 17, 2024
a5b09ee
Merge branch 'main' into unknown-ds-fix
DarshitChanpura May 20, 2024
773087f
Fixes unit test broken due to recent commit @a202d48 in main
DarshitChanpura May 21, 2024
86c0b27
Merge remote-tracking branch 'upstream/main' into unknown-ds-fix
DarshitChanpura Jun 4, 2024
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 @@ -38,6 +38,7 @@ import { setCrossPageToast } from '../../utils/storage-utils';
import { SecurityPluginTopNavMenu } from '../../top-nav-menu';
import { DataSourceContext } from '../../app-router';
import { getClusterInfo } from '../../../../utils/datasource-utils';
import { UnknownDataSourcePage } from '../../unknown-datasource';

interface AuditLoggingEditSettingProps extends AppDependencies {
setting: 'general' | 'compliance';
Expand Down Expand Up @@ -76,7 +77,7 @@ export function AuditLoggingEditSettings(props: AuditLoggingEditSettingProps) {
};

fetchConfig();
}, [props.coreStart.http, dataSource.id]);
}, [props.coreStart.http, dataSource]);
DarshitChanpura marked this conversation as resolved.
Show resolved Hide resolved

const renderSaveAndCancel = () => {
return (
Expand Down Expand Up @@ -248,6 +249,10 @@ export function AuditLoggingEditSettings(props: AuditLoggingEditSettingProps) {
content = renderComplianceSetting();
}

if (dataSourceEnabled && dataSource === undefined) {
return <UnknownDataSourcePage {...props} />;
}

return (
<div className="panel-restrict-width">
<SecurityPluginTopNavMenu
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import { ViewSettingGroup } from './view-setting-group';
import { DocLinks } from '../../constants';
import { DataSourceContext } from '../../app-router';
import { SecurityPluginTopNavMenu } from '../../top-nav-menu';
import { UnknownDataSourcePage } from '../../unknown-datasource';

interface AuditLoggingProps extends AppDependencies {
fromType: string;
Expand Down Expand Up @@ -135,8 +136,11 @@ export function renderComplianceSettings(config: AuditLoggingSettings) {
}

export function AuditLogging(props: AuditLoggingProps) {
const dataSourceEnabled = !!props.depsStart.dataSource?.dataSourceEnabled;
console.log(props.depsStart);
const [configuration, setConfiguration] = React.useState<AuditLoggingSettings>({});
const { dataSource, setDataSource } = useContext(DataSourceContext)!;
console.log(dataSource);

const onSwitchChange = async () => {
try {
Expand All @@ -163,7 +167,7 @@ export function AuditLogging(props: AuditLoggingProps) {
};

fetchData();
}, [props.coreStart.http, props.fromType, dataSource.id]);
}, [props.coreStart.http, props.fromType, dataSource]);

const statusPanel = renderStatusPanel(onSwitchChange, configuration.enabled || false);

Expand Down Expand Up @@ -229,6 +233,10 @@ export function AuditLogging(props: AuditLoggingProps) {
);
}

if (dataSourceEnabled && dataSource === undefined) {
return <UnknownDataSourcePage {...props} />;
}

return (
<div className="panel-restrict-width">
<SecurityPluginTopNavMenu
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Audit logs edit Render unable to access dataSource when enabled and inaccessible 1`] = `
<Memo()
coreStart={
Object {
"http": 1,
}
}
depsStart={
Object {
"dataSource": Object {
"dataSourceEnabled": true,
},
}
}
navigation={Object {}}
/>
`;
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ exports[`Audit logs render when AuditLoggingSettings.enabled is true 1`] = `
}
}
dataSourcePickerReadOnly={false}
depsStart={Object {}}
navigation={Object {}}
selectedDataSource={
Object {
Expand Down Expand Up @@ -644,3 +645,21 @@ exports[`Audit logs render when AuditLoggingSettings.enabled is true 1`] = `
</EuiPanel>
</div>
`;

exports[`Audit logs Render unable to access dataSource when enabled and inaccessible 1`] = `
<Memo()
coreStart={
Object {
"http": 1,
}
}
depsStart={
Object {
"dataSource": Object {
"dataSourceEnabled": true,
},
}
}
navigation={Object {}}
/>
`;
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,14 @@ describe('Audit logs edit', () => {
jest.spyOn(React, 'useState').mockImplementation((initialValue) => [initialValue, setState]);
});

afterEach(() => {
React.useContext.mockRestore();
React.useContext.mockReturnValue({
dataSource: { id: 'test' },
setDataSource: jest.fn(),
});
});

it('Render edit general settings', (done) => {
jest.spyOn(React, 'useEffect').mockImplementationOnce((f) => f());

Expand Down Expand Up @@ -172,4 +180,24 @@ describe('Audit logs edit', () => {
expect(mockAuditLoggingUtils.updateAuditLogging).toBeCalled();
expect(window.location.hash).toBe(buildHashUrl(ResourceType.auditLogging));
});

it('Render unable to access dataSource when enabled and inaccessible', () => {
React.useContext.mockImplementation(() => ({
dataSource: undefined,
setDataSource: jest.fn(),
}));
const depsStart = {
dataSource: {
dataSourceEnabled: true,
},
};
const component = shallow(
<AuditLoggingEditSettings
coreStart={mockCoreStart as any}
depsStart={depsStart as any}
navigation={{} as any}
/>
);
expect(component).toMatchSnapshot();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,21 @@ describe('Audit logs', () => {
http: 1,
};

const mockDepsStart = {};

beforeEach(() => {
jest.spyOn(React, 'useState').mockImplementation((initialValue) => [initialValue, setState]);
jest.spyOn(React, 'useEffect').mockImplementationOnce((f) => f());
});

afterEach(() => {
React.useContext.mockRestore();
React.useContext.mockReturnValue({
dataSource: { id: 'test' },
setDataSource: jest.fn(),
});
});

it('Render disabled', () => {
const mockAuditLoggingData = {
enabled: false,
Expand All @@ -54,7 +64,11 @@ describe('Audit logs', () => {
mockAuditLoggingUtils.getAuditLogging = jest.fn().mockReturnValue(mockAuditLoggingData);

const component = shallow(
<AuditLogging coreStart={mockCoreStart as any} navigation={{} as any} />
<AuditLogging
coreStart={mockCoreStart as any}
depsStart={mockDepsStart as any}
navigation={{} as any}
/>
);

const switchFound = component.find(EuiSwitch);
Expand All @@ -72,7 +86,13 @@ describe('Audit logs', () => {

mockAuditLoggingUtils.getAuditLogging = jest.fn().mockReturnValue(mockAuditLoggingData);

shallow(<AuditLogging coreStart={mockCoreStart as any} navigation={{} as any} />);
shallow(
<AuditLogging
coreStart={mockCoreStart as any}
depsStart={mockDepsStart as any}
navigation={{} as any}
/>
);

process.nextTick(() => {
expect(mockAuditLoggingUtils.getAuditLogging).toHaveBeenCalledTimes(1);
Expand All @@ -90,7 +110,13 @@ describe('Audit logs', () => {
throw Error();
});

shallow(<AuditLogging coreStart={mockCoreStart as any} navigation={{} as any} />);
shallow(
<AuditLogging
coreStart={mockCoreStart as any}
depsStart={mockDepsStart as any}
navigation={{} as any}
/>
);

process.nextTick(() => {
expect(mockAuditLoggingUtils.getAuditLogging).toHaveBeenCalledTimes(1);
Expand All @@ -116,7 +142,11 @@ describe('Audit logs', () => {

it('audit logging switch change', () => {
const component = shallow(
<AuditLogging coreStart={mockCoreStart as any} navigation={{} as any} />
<AuditLogging
coreStart={mockCoreStart as any}
depsStart={mockDepsStart as any}
navigation={{} as any}
/>
);
component.find('[data-test-subj="audit-logging-enabled-switch"]').simulate('change');
expect(mockAuditLoggingUtils.updateAuditLogging).toHaveBeenCalledTimes(1);
Expand All @@ -130,7 +160,11 @@ describe('Audit logs', () => {
throw Error();
});
const component = shallow(
<AuditLogging coreStart={mockCoreStart as any} navigation={{} as any} />
<AuditLogging
coreStart={mockCoreStart as any}
depsStart={mockDepsStart as any}
navigation={{} as any}
/>
);
component.find('[data-test-subj="audit-logging-enabled-switch"]').simulate('change');

Expand All @@ -142,7 +176,11 @@ describe('Audit logs', () => {
const auditLoggingSettings = { enabled: true };
jest.spyOn(React, 'useState').mockImplementation(() => [auditLoggingSettings, setState]);
const component = shallow(
<AuditLogging coreStart={mockCoreStart as any} navigation={{} as any} />
<AuditLogging
coreStart={mockCoreStart as any}
depsStart={mockDepsStart as any}
navigation={{} as any}
/>
);
expect(component).toMatchSnapshot();
});
Expand All @@ -151,7 +189,11 @@ describe('Audit logs', () => {
const auditLoggingSettings = { enabled: true };
jest.spyOn(React, 'useState').mockImplementation(() => [auditLoggingSettings, setState]);
const component = shallow(
<AuditLogging coreStart={mockCoreStart as any} navigation={{} as any} />
<AuditLogging
coreStart={mockCoreStart as any}
depsStart={mockDepsStart as any}
navigation={{} as any}
/>
);
component.find('[data-test-subj="general-settings-configure"]').simulate('click');
expect(window.location.hash).toBe(
Expand All @@ -163,11 +205,35 @@ describe('Audit logs', () => {
const auditLoggingSettings = { enabled: true };
jest.spyOn(React, 'useState').mockImplementation(() => [auditLoggingSettings, setState]);
const component = shallow(
<AuditLogging coreStart={mockCoreStart as any} navigation={{} as any} />
<AuditLogging
coreStart={mockCoreStart as any}
depsStart={mockDepsStart as any}
navigation={{} as any}
/>
);
component.find('[data-test-subj="compliance-settings-configure"]').simulate('click');
expect(window.location.hash).toBe(
buildHashUrl(ResourceType.auditLogging) + SUB_URL_FOR_COMPLIANCE_SETTINGS_EDIT
);
});

it('Render unable to access dataSource when enabled and inaccessible', () => {
React.useContext.mockImplementation(() => ({
dataSource: undefined,
setDataSource: jest.fn(),
}));
const depsStart = {
dataSource: {
dataSourceEnabled: true,
},
};
const component = shallow(
<AuditLogging
coreStart={mockCoreStart as any}
depsStart={depsStart as any}
navigation={{} as any}
/>
);
expect(component).toMatchSnapshot();
});
});
16 changes: 15 additions & 1 deletion public/apps/configuration/panels/auth-view/auth-view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,13 @@
import { InstructionView } from './instruction-view';
import { DataSourceContext } from '../../app-router';
import { SecurityPluginTopNavMenu } from '../../top-nav-menu';
import { UnknownDataSourcePage } from '../../unknown-datasource';

export function AuthView(props: AppDependencies) {
const [authentication, setAuthentication] = React.useState([]);
const [authorization, setAuthorization] = React.useState([]);
const [loading, setLoading] = useState(false);
const dataSourceEnabled = !!props.depsStart.dataSource?.dataSourceEnabled;
const { dataSource, setDataSource } = useContext(DataSourceContext)!;

React.useEffect(() => {
Expand All @@ -48,9 +50,18 @@
};

fetchData();
}, [props.coreStart.http, dataSource.id]);
}, [props.coreStart.http, dataSource]);

if (isEmpty(authentication)) {
console.log(dataSource);
console.log(dataSourceEnabled);
if (dataSourceEnabled && dataSource === undefined) {
return (
<>
<UnknownDataSourcePage {...props} />
</>
);
}
return (
<>
<SecurityPluginTopNavMenu
Expand All @@ -64,6 +75,9 @@
);
}

if (dataSourceEnabled && dataSource === undefined) {
return <UnknownDataSourcePage {...props} />;

Check warning on line 79 in public/apps/configuration/panels/auth-view/auth-view.tsx

View check run for this annotation

Codecov / codecov/patch

public/apps/configuration/panels/auth-view/auth-view.tsx#L79

Added line #L79 was not covered by tests
}
return (
<>
<SecurityPluginTopNavMenu
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Auth view Render unable to access dataSource when enabled and inaccessible 1`] = `
<Fragment>
<Memo()
coreStart={
Object {
"http": 1,
}
}
depsStart={
Object {
"dataSource": Object {
"dataSourceEnabled": true,
},
}
}
navigation={Object {}}
/>
</Fragment>
`;

exports[`Auth view Render unable to access dataSource when enabled and inaccessible: Empty Authentication 1`] = `
<Fragment>
<Memo()
coreStart={
Object {
"http": 1,
}
}
depsStart={
Object {
"dataSource": Object {
"dataSourceEnabled": true,
},
}
}
navigation={Object {}}
/>
</Fragment>
`;
Loading
Loading