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

feat(ui): differentiate analytics events for list and details pages #1845

Merged
merged 2 commits into from
Nov 9, 2020
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
130 changes: 65 additions & 65 deletions ui/.betterer.results

Large diffs are not rendered by default.

24 changes: 16 additions & 8 deletions ui/src/app/base/components/ActionForm/ActionForm.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { Spinner, Strip } from "@canonical/react-components";
import type { ReactNode } from "react";
import React, { useState } from "react";
import React, { useEffect, useState } from "react";

import type { TSFixMe } from "app/base/types";
import type { AnalyticsEvent, TSFixMe } from "app/base/types";
import { useProcessing } from "app/base/hooks";
import { formatErrors } from "app/utils";
import FormikForm from "app/base/components/FormikForm";
Expand Down Expand Up @@ -99,6 +99,7 @@ type Props = {
loaded?: boolean;
loading?: boolean;
modelName: string;
onSaveAnalytics?: AnalyticsEvent;
onSubmit: (...args: unknown[]) => void;
onSuccess?: () => void;
processingCount?: number;
Expand All @@ -120,6 +121,7 @@ const ActionForm = ({
loaded = true,
loading,
modelName,
onSaveAnalytics,
onSubmit,
onSuccess,
processingCount,
Expand All @@ -128,19 +130,28 @@ const ActionForm = ({
validationSchema,
}: Props): JSX.Element => {
const [processing, setProcessing] = useState(false);
const [saved, setSaved] = useState(false);
const formattedErrors = formatErrors(errors);

useProcessing(
processingCount,
() => {
setProcessing(false);
setSaved(true);
onSuccess && onSuccess();
clearSelectedAction && clearSelectedAction();
},
errors && Object.keys(errors).length > 0,
() => setProcessing(false)
);

// Clearing the selected action is moved into its own effect so that `saved`
// can be set before the component unmounts. This triggers analytics being sent.
useEffect(() => {
if (saved && clearSelectedAction) {
clearSelectedAction();
}
}, [clearSelectedAction, saved]);

if (loaded) {
return (
<FormikForm
Expand All @@ -154,15 +165,12 @@ const ActionForm = ({
initialValues={initialValues}
loading={loading}
onCancel={clearSelectedAction}
onSaveAnalytics={{
action: actionName,
category: "Take action form",
label: getLabel(modelName, actionName),
}}
onSaveAnalytics={onSaveAnalytics}
onSubmit={(...args) => {
onSubmit(...args);
setProcessing(true);
}}
saved={saved}
saving={processing}
savingLabel={`${getLabel(
modelName,
Expand Down
6 changes: 6 additions & 0 deletions ui/src/app/base/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,9 @@ export type Sort = {
export type RouteParams = {
id: string;
};

export type AnalyticsEvent = {
action: string;
category: string;
label: string;
};
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,11 @@ const ComposeForm = ({ setSelectedAction }: Props): JSX.Element => {
zone: `${zones[0]?.id}` || "",
}}
modelName="machine"
onSaveAnalytics={{
action: "Submit",
category: "KVM details action form",
label: "Compose",
}}
onSubmit={(values: ComposeFormValues) => {
// Remove any errors before dispatching compose action.
dispatch(cleanup());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ type Props = {
const DeleteForm = ({ setSelectedAction }: Props): JSX.Element => {
const dispatch = useDispatch();
const { id } = useParams<RouteParams>();
const activePod = useSelector(podSelectors.active);
const errors = useSelector(podSelectors.errors);
const selectedKVMIDs = useSelector(podSelectors.selectedKVMs).map(
(kvm) => kvm.id
Expand All @@ -29,6 +30,11 @@ const DeleteForm = ({ setSelectedAction }: Props): JSX.Element => {
clearSelectedAction={() => setSelectedAction(null)}
errors={errors}
modelName="KVM"
onSaveAnalytics={{
action: "Submit",
category: `KVM ${activePod ? "details" : "list"} action form`,
label: "Delete",
}}
onSubmit={() => {
kvmsToDelete.forEach((kvmID) => {
dispatch(podActions.delete(kvmID));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ type Props = {
const RefreshForm = ({ setSelectedAction }: Props): JSX.Element | null => {
const dispatch = useDispatch();
const { id } = useParams<RouteParams>();
const activePod = useSelector(podSelectors.active);
const errors = useSelector(podSelectors.errors);
const selectedKVMIDs = useSelector(podSelectors.selectedKVMs).map(
(kvm) => kvm.id
Expand All @@ -30,6 +31,11 @@ const RefreshForm = ({ setSelectedAction }: Props): JSX.Element | null => {
clearSelectedAction={() => setSelectedAction(null)}
errors={errors}
modelName="KVM"
onSaveAnalytics={{
action: "Submit",
category: `KVM ${activePod ? "details" : "list"} action form`,
label: "Refresh",
}}
onSubmit={() => {
kvmsToRefresh.forEach((podID) => {
dispatch(podActions.refresh(podID));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,16 @@ import React from "react";
import type { RootState } from "app/store/root/types";

import {
scripts as scriptsFactory,
machineAction as machineActionFactory,
machine as machineFactory,
generalState as generalStateFactory,
machine as machineFactory,
machineAction as machineActionFactory,
machineActionsState as machineActionsStateFactory,
machineState as machineStateFactory,
scriptsState as scriptsStateFactory,
rootState as rootStateFactory,
machineStatus as machineStatusFactory,
machineStatuses as machineStatusesFactory,
machineActionsState as machineActionsStateFactory,
rootState as rootStateFactory,
scripts as scriptsFactory,
scriptsState as scriptsStateFactory,
} from "testing/factories";
import CommissionForm from "./CommissionForm";

Expand All @@ -31,7 +31,7 @@ describe("CommissionForm", () => {
data: [
machineActionFactory({
name: "commission",
sentence: "commission",
title: "Commission",
}),
],
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ type Props = {

export const CommissionForm = ({ setSelectedAction }: Props): JSX.Element => {
const dispatch = useDispatch();
const activeMachine = useSelector(machineSelectors.active);
const errors = useSelector(machineSelectors.errors);
const scriptsLoaded = useSelector(scriptSelectors.loaded);
const commissioningScripts = useSelector(scriptSelectors.commissioning);
Expand Down Expand Up @@ -119,6 +120,11 @@ export const CommissionForm = ({ setSelectedAction }: Props): JSX.Element => {
}}
loaded={scriptsLoaded}
modelName="machine"
onSaveAnalytics={{
action: "Submit",
category: `Machine ${activeMachine ? "details" : "list"} action form`,
label: "Commission",
}}
onSubmit={(values: CommissionFormValues) => {
const {
enableSSH,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@ import {
configState as configStateFactory,
defaultMinHweKernelState as defaultMinHweKerelStateFactory,
generalState as generalStateFactory,
osInfoState as osInfoStateFactory,
rootState as rootStateFactory,
machine as machineFactory,
machineAction as machineActionFactory,
machineActionsState as machineActionsStateFactory,
machineState as machineStateFactory,
machineStatus as machineStatusFactory,
osInfoState as osInfoStateFactory,
rootState as rootStateFactory,
} from "testing/factories";

const mockStore = configureStore();
Expand Down Expand Up @@ -48,6 +50,14 @@ describe("DeployForm", () => {
data: "ga-18.04",
loaded: true,
}),
machineActions: machineActionsStateFactory({
data: [
machineActionFactory({
name: "deploy",
title: "Deploy",
}),
],
}),
osInfo: osInfoStateFactory({
data: {
osystems: [
Expand Down Expand Up @@ -338,10 +348,13 @@ describe("DeployForm", () => {
]);
});

it("sends an event if cloud-init is set", () => {
it("sends an analytics event with cloud-init user data set", () => {
const state = { ...initialState };
state.machine.selected = ["abc123"];
const useSendMock = jest.spyOn(hooks, "useSendAnalytics");
const mockSendAnalytics = jest.fn();
const mockUseSendAnalytics = (hooks.useSendAnalytics = jest.fn(
() => mockSendAnalytics
));
const store = mockStore(state);
const wrapper = mount(
<Provider store={store}>
Expand All @@ -352,6 +365,7 @@ describe("DeployForm", () => {
</MemoryRouter>
</Provider>
);

act(() =>
wrapper.find("Formik").props().onSubmit({
includeUserData: true,
Expand All @@ -362,6 +376,13 @@ describe("DeployForm", () => {
userData: "test script",
})
);
expect(useSendMock).toHaveBeenCalled();

expect(mockSendAnalytics).toHaveBeenCalled();
expect(mockSendAnalytics.mock.calls[0]).toEqual([
"Machine list deploy form",
"Has cloud-init config",
"Cloud-init user data",
]);
mockUseSendAnalytics.mockRestore();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ type Props = {

export const DeployForm = ({ setSelectedAction }: Props): JSX.Element => {
const dispatch = useDispatch();
const activeMachine = useSelector(machineSelectors.active);
const errors = useSelector(machineSelectors.errors);
const defaultMinHweKernel = useSelector(
generalSelectors.defaultMinHweKernel.get
Expand All @@ -52,6 +53,7 @@ export const DeployForm = ({ setSelectedAction }: Props): JSX.Element => {
generalSelectors.defaultMinHweKernel.loaded
);
const osInfoLoaded = useSelector(generalSelectors.osInfo.loaded);
const sendAnalytics = useSendAnalytics();
const { machinesToAction, processingCount } = useMachineActionForm("deploy");

useEffect(() => {
Expand All @@ -77,8 +79,6 @@ export const DeployForm = ({ setSelectedAction }: Props): JSX.Element => {
initialRelease = default_release;
}

const sendAnalytics = useSendAnalytics();

return (
<ActionForm
actionName="deploy"
Expand All @@ -95,6 +95,11 @@ export const DeployForm = ({ setSelectedAction }: Props): JSX.Element => {
}}
loaded={defaultMinHweKernelLoaded && osInfoLoaded}
modelName="machine"
onSaveAnalytics={{
action: "Submit",
category: `Machine ${activeMachine ? "details" : "list"} action form`,
label: "Deploy",
}}
onSubmit={(values: DeployFormValues) => {
const hasUserData =
values.includeUserData && values.userData && values.userData !== "";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ import { MemoryRouter } from "react-router-dom";
import React from "react";
import { Provider } from "react-redux";

import { machine as machineFactory } from "testing/factories";
import {
generalState as generalStateFactory,
machine as machineFactory,
} from "testing/factories";
import { TSFixMe } from "app/base/types";
import DeployForm from "../../DeployForm";

Expand Down Expand Up @@ -61,7 +64,7 @@ describe("DeployFormFields", () => {
loaded: true,
loading: false,
},
general: {
general: generalStateFactory({
defaultMinHweKernel: {
data: "",
errors: {},
Expand Down Expand Up @@ -106,7 +109,7 @@ describe("DeployFormFields", () => {
loaded: true,
loading: false,
},
},
}),
machine: {
errors: {},
loading: false,
Expand Down Expand Up @@ -147,11 +150,7 @@ describe("DeployFormFields", () => {
wrapper = mount(
<Provider store={store}>
<MemoryRouter initialEntries={[{ pathname: "/" }]}>
<DeployForm
processing={false}
setProcessing={jest.fn()}
setSelectedAction={jest.fn()}
/>
<DeployForm setSelectedAction={jest.fn()} />
</MemoryRouter>
</Provider>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const fieldlessActions = [

export const FieldlessForm = ({ selectedAction, setSelectedAction }) => {
const dispatch = useDispatch();
const activeMachine = useSelector(machineSelectors.active);
const errors = useSelector(machineSelectors.errors);
const { machinesToAction, processingCount } = useMachineActionForm(
selectedAction.name
Expand All @@ -41,6 +42,11 @@ export const FieldlessForm = ({ selectedAction, setSelectedAction }) => {
clearSelectedAction={() => setSelectedAction(null, true)}
errors={errors}
modelName="machine"
onSaveAnalytics={{
action: "Submit",
category: `Machine ${activeMachine ? "details" : "list"} action form`,
label: selectedAction.name,
}}
onSubmit={() => {
if (fieldlessActions.includes(selectedAction.name)) {
const actionMethod = kebabToCamelCase(selectedAction.name);
Expand Down
Loading