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

bulk update separation #356

Merged
merged 9 commits into from
Apr 17, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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
86 changes: 41 additions & 45 deletions public/components/custom_panels/home.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { StaticContext } from 'react-router';
import { Route, RouteComponentProps, Switch } from 'react-router-dom';
import { map, mergeMap, tap, toArray } from 'rxjs/operators';
import { concat, from, Observable, of } from 'rxjs';
import { useDispatch } from 'react-redux';
import PPLService from '../../services/requests/ppl';
import DSLService from '../../services/requests/dsl';
import { CoreStart, SavedObjectsStart } from '../../../../../src/core/public';
Expand Down Expand Up @@ -39,8 +40,7 @@ import { SavedObject } from '../../../../../src/core/types';
import { CustomPanelViewSO } from './custom_panel_view_so';
import { coreRefs } from '../../framework/core_refs';
import { CustomPanelType } from '../../../common/types/custom_panels';
import { fetchPanels } from './redux/panel_slice';
import { useDispatch } from 'react-redux';
import { deletePanel, fetchPanels } from './redux/panel_slice';

// import { ObjectFetcher } from '../common/objectFetcher';

Expand Down Expand Up @@ -80,7 +80,7 @@ export const Home = ({
const [start, setStart] = useState<ShortDate>('');
const [end, setEnd] = useState<ShortDate>('');

const dispatch = useDispatch()
const dispatch = useDispatch();

const setToast = (title: string, color = 'success', text?: ReactChild, side?: string) => {
if (!text) text = '';
Expand Down Expand Up @@ -138,7 +138,6 @@ export const Home = ({

const isUuid = (id) => !!id.match(uuidRx);


const fetchSavedObjectPanel = async (id: string) => {
const soPanel = await coreRefs.savedObjectsClient?.get(CUSTOM_PANELS_SAVED_OBJECT_TYPE, id);
return savedObjectToCustomPanel(soPanel);
Expand Down Expand Up @@ -167,19 +166,15 @@ export const Home = ({

try {
// const panelToClone = await fetchPanelfn(clonedCustomPanelId)

// const newPanel: PanelType = {
// ...panelToClone,
// title: clonedCustomPanelName,
// dateCreated: new Date().getTime(),
// dateModified: new Date().getTime()
// }

// const clonedPanel: CustomPanelType = await coreRefs.savedObjectsClient!.create(
// CUSTOM_PANELS_SAVED_OBJECT_TYPE, newPanel, { id: panelToClone.id }
// )


// setcustomPanelData((prevCustomPanelData) => {
// const newPanelData = [
// ...prevCustomPanelData,
Expand Down Expand Up @@ -222,44 +217,46 @@ export const Home = ({

// Deletes multiple existing Operational Panels
const deleteCustomPanelList = (customPanelIdList: string[], toastMessage: string) => {
// Promise.all([
// deletePanelSO(customPanelIdList),
// deletePanels(customPanelIdList)
// ]).then((res) => {
// setcustomPanelData((prevCustomPanelData) => {
// return prevCustomPanelData.filter(
// (customPanel) => !customPanelIdList.includes(customPanel.id)
// );
// });
// setToast(toastMessage);
// })
// .catch((err) => {
// setToast(
// 'Error deleting Operational Panels, please make sure you have the correct permission.',
// 'danger'
// );
// console.error(err.body.message);
// });
Promise.all([deletePanelSO(customPanelIdList), deletePanels(customPanelIdList)])
.then((res) => {
// setcustomPanelData((prevCustomPanelData) => {
// return prevCustomPanelData.filter(
// (customPanel) => !customPanelIdList.includes(customPanel.id)
// );
// });
// setToast(toastMessage);
})
.catch((err) => {
setToast(
'Error deleting Operational Panels, please make sure you have the correct permission.',
'danger'
);
console.error(err.body.message);
});
};

// Deletes an existing Operational Panel
const deleteCustomPanel = async (customPanelId: string, customPanelName: string) => {
// return http
// .delete(`${CUSTOM_PANELS_API_PREFIX}/panels/` + customPanelId)
// .then((res) => {
// setcustomPanelData((prevCustomPanelData) => {
// return prevCustomPanelData.filter((customPanel) => customPanel.id !== customPanelId);
// });
// setToast(`Operational Panel "${customPanelName}" successfully deleted!`);
// return res;
// })
// .catch((err) => {
// setToast(
// 'Error deleting Operational Panel, please make sure you have the correct permission.',
// 'danger'
// );
// console.error(err.body.message);
// });
return http
.delete(`${CUSTOM_PANELS_API_PREFIX}/panels/` + customPanelId)
.then((res) => {
dispatch(fetchPanels());
setToast(`Operational Panel "${customPanelName}" successfully deleted!`);
return res;
})
.catch((err) => {
setToast(
'Error deleting Operational Panel, please make sure you have the correct permission.',
'danger'
);
console.error(err.body.message);
});
};

// Deletes an existing SO Operational Panel
const deleteCustomPanelSO = async (customPanelId: string, customPanelName: string) => {
dispatch(deletePanel(customPanelId));
// TODO: toast here
};

const addSamplePanels = async () => {
Expand Down Expand Up @@ -301,8 +298,7 @@ export const Home = ({
}),
})
.then((res) => {
dispatch(fetchPanels())
// setcustomPanelData([...customPanelData, ...res.demoPanelsData]);
dispatch(fetchPanels());
});
setToast(`Sample panels successfully added.`);
} catch (err: any) {
Expand Down Expand Up @@ -354,7 +350,7 @@ export const Home = ({
chrome={chrome}
parentBreadcrumbs={parentBreadcrumbs}
cloneCustomPanel={cloneCustomPanel}
deleteCustomPanel={deleteCustomPanel}
deleteCustomPanel={deleteCustomPanelSO}
Copy link
Contributor

Choose a reason for hiding this comment

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

delete savedobject should be in panel_slice. Therefore, we should collapse a reference and just call dispatch(deletePanel) (or whatever) directly from <CustomPanelViewSO>. Dont' pass it in.

We're trying to eliminate all prop-drilling, especially functions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can modify this in a follow up PR

setToast={setToast}
onEditClick={onEditClick}
page="operationalPanels"
Expand Down
45 changes: 21 additions & 24 deletions public/components/custom_panels/redux/panel_slice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,7 @@ export const panelReducer = panelSlice.reducer;

export const selectPanel = (rootState): CustomPanelType => rootState.customPanel.panel;

export const selectPanelList = (rootState): CustomPanelType[] => {
// console.log('selectPanelList', { rootState, panelList: rootState.customPanel.panelList });
return rootState.customPanel.panelList;
};
export const selectPanelList = (rootState): CustomPanelType[] => rootState.customPanel.panelList;

// export const selectPanelList = createSelector(
// rootState => { console.log("selectPanelList", { rootState }); return rootState.customPanel.panelList },
Expand Down Expand Up @@ -90,16 +87,18 @@ const fetchCustomPanels = async () => {
const panels$: Observable<CustomPanelListType> = concat(
fetchSavedObjectPanels$(),
fetchObservabilityPanels$()
).pipe(map((res) => {
console.log("fetchCustomPanels", res);
return res as CustomPanelListType
}));
).pipe(
map((res) => {
console.log('fetchCustomPanels', res);
return res as CustomPanelListType;
})
);

return panels$.pipe(toArray()).toPromise();
};

export const fetchPanels = () => async (dispatch, getState) => {
const panels = await fetchCustomPanels()
const panels = await fetchCustomPanels();
console.log('fetchPanels', { panels });
dispatch(setPanelList(panels));
};
Expand All @@ -112,31 +111,27 @@ export const fetchPanel = (id) => async (dispatch, getState) => {

export const fetchVisualization = () => (dispatch, getState) => {};

const updateLegacyPanel = (panel: CustomPanelType) => coreRefs.http!
.post(`${CUSTOM_PANELS_API_PREFIX}/panels/update`, {
const updateLegacyPanel = (panel: CustomPanelType) =>
coreRefs.http!.post(`${CUSTOM_PANELS_API_PREFIX}/panels/update`, {
body: JSON.stringify({ panelId: panel.id, panel: panel as PanelType }),
});

const updateSavedObjectPanel = (panel: CustomPanelType) => savedObjectPanelsClient.update(panel);


const uuidRx = /^[0-9a-fA-F]{8}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{12}$/;

const isUuid = (id) => !!id.match(uuidRx);


export const updatePanel = (panel: CustomPanelType) => async (dispatch, getState) => {
try {
if (isUuid(panel.id))
await updateSavedObjectPanel(panel)
else
await updateLegacyPanel(panel)
if (isUuid(panel.id)) await updateSavedObjectPanel(panel);
else await updateLegacyPanel(panel);

dispatch(setPanel(panel));
const panelList = getState().customPanel.panelList.map((p) => (p.id === panel.id ? panel : p));
dispatch(setPanelList(panelList));
} catch (err) {
console.log("Error updating panel", { err, panel })
console.log('Error updating panel', { err, panel });
}
};

Expand All @@ -152,7 +147,6 @@ export const createPanel = (panel) => async (dispatch, getState) => {
dispatch(setPanelList([...panelList, newPanel]));
};


const saveRenamedPanel = async (id, name) => {
const renamePanelObject = {
panelId: id,
Expand All @@ -174,17 +168,20 @@ const saveRenamedPanelSO = async (id, name) => {
};

// Renames an existing CustomPanel
export const renameCustomPanel = (editedCustomPanelName: string, id: string) => async (dispatch, getState) => {
console.log("renameCustomPanel dispatched", { editedCustomPanelName, id })
export const renameCustomPanel = (editedCustomPanelName: string, id: string) => async (
dispatch,
getState
) => {
console.log('renameCustomPanel dispatched', { editedCustomPanelName, id });

if (!isNameValid(editedCustomPanelName)) {
console.log('Invalid Custom Panel name', 'danger');
return Promise.reject();
}

const panel = getState().customPanel.panelList.find(p => p.id === id)
const updatedPanel = { ...panel, title: editedCustomPanelName }
dispatch(updatePanel(updatedPanel))
const panel = getState().customPanel.panelList.find((p) => p.id === id);
const updatedPanel = { ...panel, title: editedCustomPanelName };
dispatch(updatePanel(updatedPanel));

// try {
// // await savePanelFn(editedCustomPanelId, editedCustomPanelName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ export const SavePanel = ({
setSubType,
isSaveAsMetricEnabled,
}: ISavedPanelProps) => {
const [options, setOptions] = useState([]);
const [checked, setChecked] = useState(false);
const [svpnlError, setSvpnlError] = useState(null);

Expand All @@ -62,20 +61,6 @@ export const SavePanel = ({
dispatch(fetchPanels());
}, []);

const getCustomPabnelList = async (svobj: SavedObjects) => {
const optionRes = await svobj
.fetchCustomPanels()
.then((res: any) => {
return res;
})
.catch((error: any) => setSvpnlError(error));
setOptions(optionRes?.panels || []);
};

useEffect(() => {
getCustomPabnelList(savedObjects);
}, []);

const onToggleChange = (e: { target: { checked: React.SetStateAction<boolean> } }) => {
setChecked(e.target.checked);
if (e.target.checked) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,12 @@ export class SaveAsCurrentVisualization extends SavedQuerySaver {
}

addToPanel({ selectedPanels, saveTitle, notifications, visId }) {
const uuidRx = /^[0-9a-fA-F]{8}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{12}$/;
const soPanels = selectedPanels.filter((id) => id.panel.id.match(uuidRx));
Copy link
Member

Choose a reason for hiding this comment

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

duplicate and unused variable?

const opsPanels = selectedPanels.filter((id) => !id.panel.id.match(uuidRx));
Copy link
Member

Choose a reason for hiding this comment

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

  • use uuidRx.test(id.panel.id)
  • id variable name is not accurate
  • i'm not sure if the logic should be here, e.g. what happens to panels in .observability index? save_as_current_vis don't support them anymore?
  • not sure if it helps but you can check .observability and .kibana visualizations handling here as reference, for example bulk delete
    /**
    * Delete a list of objects. Assumes object is osd visualization if id is a
    * UUID. Rest and failed ids will then be deleted by PPL client.
    *
    * @param params - SavedObjectsDeleteBulkParams
    * @returns SavedObjectsDeleteResponse
    */
    static async deleteBulk(
    params: SavedObjectsDeleteBulkParams
    ): Promise<SavedObjectsDeleteResponse> {
    const idMap = params.objectIdList.reduce((prev, id) => {
    const type = OSDSavedObjectClient.extractType(id);
    const key = type === '' ? 'non_osd' : type;
    return { ...prev, [key]: [...(prev[key] || []), id] };
    }, {} as { [k in 'non_osd' | ObservabilitySavedObjectsType]: string[] });

this.panelClient
.updateBulk({
selectedCustomPanels: selectedPanels,
selectedCustomPanels: opsPanels,
savedVisualizationId: visId,
})
.then((res: any) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,12 @@ export class SaveAsNewVisualization extends SavedQuerySaver {
}

addToPanel({ selectedPanels, saveTitle, notifications, visId }) {
const uuidRx = /^[0-9a-fA-F]{8}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{12}$/;
Copy link
Member

Choose a reason for hiding this comment

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

similar comments and probably define regex globally

const soPanels = selectedPanels.filter((id) => id.panel.id.match(uuidRx));
const opsPanels = selectedPanels.filter((id) => !id.panel.id.match(uuidRx));
this.panelClient
.updateBulk({
selectedCustomPanels: selectedPanels,
selectedCustomPanels: opsPanels,
savedVisualizationId: visId,
})
.then((res: any) => {
Expand Down