Skip to content

Commit

Permalink
[Workplace Search] Fix Source Settings bug (#90242)
Browse files Browse the repository at this point in the history
* Remove comment

Verified that this works as expected

* Replaces usage from SourceLogic to AddSourceLogic

* Remove unused duplicate code

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
scottybollinger and kibanamachine authored Feb 4, 2021
1 parent 2955d65 commit 284842d
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 102 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -48,30 +48,31 @@ import { ViewContentHeader } from '../../../components/shared/view_content_heade

import { SourceDataItem } from '../../../types';
import { AppLogic } from '../../../app_logic';
import { AddSourceLogic } from '../components/add_source/add_source_logic';
import { staticSourceData } from '../source_data';

import { SourceLogic } from '../source_logic';

export const SourceSettings: React.FC = () => {
const {
updateContentSource,
removeContentSource,
resetSourceState,
getSourceConfigData,
} = useActions(SourceLogic);
const { updateContentSource, removeContentSource, resetSourceState } = useActions(SourceLogic);
const { getSourceConfigData } = useActions(AddSourceLogic);

const {
contentSource: { name, id, serviceType },
buttonLoading,
sourceConfigData: { configuredFields },
} = useValues(SourceLogic);

const {
sourceConfigData: { configuredFields },
} = useValues(AddSourceLogic);

const { isOrganization } = useValues(AppLogic);

useEffect(() => {
getSourceConfigData(serviceType);
return resetSourceState;
}, []);

const {
configuration: { isPublicKey },
editPath,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,7 @@ jest.mock('../../app_logic', () => ({
AppLogic: { values: { isOrganization: true } },
}));

import {
fullContentSources,
sourceConfigData,
contentItems,
} from '../../__mocks__/content_sources.mock';
import { fullContentSources, contentItems } from '../../__mocks__/content_sources.mock';
import { meta } from '../../__mocks__/meta.mock';

import { DEFAULT_META } from '../../../shared/constants';
Expand All @@ -46,7 +42,6 @@ describe('SourceLogic', () => {
const defaultValues = {
contentSource: {},
contentItems: [],
sourceConfigData: {},
dataLoading: true,
sectionLoading: true,
buttonLoading: false,
Expand Down Expand Up @@ -88,13 +83,6 @@ describe('SourceLogic', () => {
expect(setSuccessMessage).toHaveBeenCalled();
});

it('setSourceConfigData', () => {
SourceLogic.actions.setSourceConfigData(sourceConfigData);

expect(SourceLogic.values.sourceConfigData).toEqual(sourceConfigData);
expect(SourceLogic.values.dataLoading).toEqual(false);
});

it('setSearchResults', () => {
SourceLogic.actions.setSearchResults(searchServerResponse);

Expand Down Expand Up @@ -402,40 +390,6 @@ describe('SourceLogic', () => {
});
});

describe('getSourceConfigData', () => {
const serviceType = 'github';

it('calls API and sets values', async () => {
AppLogic.values.isOrganization = true;

const setSourceConfigDataSpy = jest.spyOn(SourceLogic.actions, 'setSourceConfigData');
const promise = Promise.resolve(contentSource);
http.get.mockReturnValue(promise);
SourceLogic.actions.getSourceConfigData(serviceType);

expect(http.get).toHaveBeenCalledWith(
`/api/workplace_search/org/settings/connectors/${serviceType}`
);
await promise;
expect(setSourceConfigDataSpy).toHaveBeenCalled();
});

it('handles error', async () => {
const error = {
response: {
error: 'this is an error',
status: 400,
},
};
const promise = Promise.reject(error);
http.get.mockReturnValue(promise);
SourceLogic.actions.getSourceConfigData(serviceType);
await expectedAsyncError(promise);

expect(flashAPIErrors).toHaveBeenCalledWith(error);
});
});

it('resetSourceState', () => {
SourceLogic.actions.resetSourceState();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import { ContentSourceFullData, Meta, DocumentSummaryItem, SourceContentItem } f
export interface SourceActions {
onInitializeSource(contentSource: ContentSourceFullData): ContentSourceFullData;
onUpdateSourceName(name: string): string;
setSourceConfigData(sourceConfigData: SourceConfigData): SourceConfigData;
setSearchResults(searchResultsResponse: SearchResultsResponse): SearchResultsResponse;
initializeFederatedSummary(sourceId: string): { sourceId: string };
onUpdateSummary(summary: DocumentSummaryItem[]): DocumentSummaryItem[];
Expand All @@ -41,28 +40,9 @@ export interface SourceActions {
resetSourceState(): void;
removeContentSource(sourceId: string): { sourceId: string };
initializeSource(sourceId: string): { sourceId: string };
getSourceConfigData(serviceType: string): { serviceType: string };
setButtonNotLoading(): void;
}

interface SourceConfigData {
serviceType: string;
name: string;
configured: boolean;
categories: string[];
needsPermissions?: boolean;
privateSourcesEnabled: boolean;
configuredFields: {
publicKey: string;
privateKey: string;
consumerKey: string;
baseUrl?: string;
clientId?: string;
clientSecret?: string;
};
accountContextOnly?: boolean;
}

interface SourceValues {
contentSource: ContentSourceFullData;
dataLoading: boolean;
Expand All @@ -71,7 +51,6 @@ interface SourceValues {
contentItems: SourceContentItem[];
contentMeta: Meta;
contentFilterValue: string;
sourceConfigData: SourceConfigData;
}

interface SearchResultsResponse {
Expand All @@ -84,7 +63,6 @@ export const SourceLogic = kea<MakeLogicType<SourceValues, SourceActions>>({
actions: {
onInitializeSource: (contentSource: ContentSourceFullData) => contentSource,
onUpdateSourceName: (name: string) => name,
setSourceConfigData: (sourceConfigData: SourceConfigData) => sourceConfigData,
onUpdateSummary: (summary: object[]) => summary,
setSearchResults: (searchResultsResponse: SearchResultsResponse) => searchResultsResponse,
setContentFilterValue: (contentFilterValue: string) => contentFilterValue,
Expand All @@ -96,7 +74,6 @@ export const SourceLogic = kea<MakeLogicType<SourceValues, SourceActions>>({
removeContentSource: (sourceId: string) => ({
sourceId,
}),
getSourceConfigData: (serviceType: string) => ({ serviceType }),
resetSourceState: () => true,
setButtonNotLoading: () => false,
},
Expand All @@ -115,25 +92,17 @@ export const SourceLogic = kea<MakeLogicType<SourceValues, SourceActions>>({
}),
},
],
sourceConfigData: [
{} as SourceConfigData,
{
setSourceConfigData: (_, sourceConfigData) => sourceConfigData,
},
],
dataLoading: [
true,
{
onInitializeSource: () => false,
setSourceConfigData: () => false,
resetSourceState: () => false,
},
],
buttonLoading: [
false,
{
setButtonNotLoading: () => false,
setSourceConfigData: () => false,
resetSourceState: () => false,
removeContentSource: () => true,
},
Expand Down Expand Up @@ -181,7 +150,6 @@ export const SourceLogic = kea<MakeLogicType<SourceValues, SourceActions>>({
actions.initializeFederatedSummary(sourceId);
}
} catch (e) {
// TODO: Verify this works once components are there. Not sure if the catch gives a status code.
if (e.response.status === 404) {
KibanaLogic.values.navigateToUrl(NOT_FOUND_PATH);
} else {
Expand Down Expand Up @@ -260,16 +228,6 @@ export const SourceLogic = kea<MakeLogicType<SourceValues, SourceActions>>({
actions.setButtonNotLoading();
}
},
getSourceConfigData: async ({ serviceType }) => {
const route = `/api/workplace_search/org/settings/connectors/${serviceType}`;

try {
const response = await HttpLogic.values.http.get(route);
actions.setSourceConfigData(response);
} catch (e) {
flashAPIErrors(e);
}
},
onUpdateSourceName: (name: string) => {
setSuccessMessage(
i18n.translate(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import { EuiConfirmModal, EuiOverlayMask } from '@elastic/eui';
import { Loading } from '../../../../shared/loading';
import { SourceDataItem } from '../../../types';
import { staticSourceData } from '../../content_sources/source_data';
import { SourceLogic } from '../../content_sources/source_logic';
import { AddSourceLogic } from '../../content_sources/components/add_source/add_source_logic';

import { AddSourceHeader } from '../../content_sources/components/add_source/add_source_header';
Expand All @@ -31,18 +30,18 @@ export const SourceConfig: React.FC<SourceConfigProps> = ({ sourceIndex }) => {
const [confirmModalVisible, setConfirmModalVisibility] = useState(false);
const { configuration, serviceType } = staticSourceData[sourceIndex] as SourceDataItem;
const { deleteSourceConfig } = useActions(SettingsLogic);
const { getSourceConfigData } = useActions(SourceLogic);
const { saveSourceConfig } = useActions(AddSourceLogic);
const { saveSourceConfig, getSourceConfigData } = useActions(AddSourceLogic);
const {
sourceConfigData: { name, categories },
dataLoading: sourceDataLoading,
} = useValues(SourceLogic);
dataLoading,
} = useValues(AddSourceLogic);

useEffect(() => {
getSourceConfigData(serviceType);
}, []);

if (sourceDataLoading) return <Loading />;
if (dataLoading) return <Loading />;

const hideConfirmModal = () => setConfirmModalVisibility(false);
const showConfirmModal = () => setConfirmModalVisibility(true);
const saveUpdatedConfig = () => saveSourceConfig(true);
Expand Down

0 comments on commit 284842d

Please sign in to comment.