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

mrc-2755 Move error reporting related mutations out of root mutations #611

Merged
merged 16 commits into from Nov 17, 2021
Merged
4 changes: 4 additions & 0 deletions NEWS.md
@@ -1,3 +1,7 @@
# hint 1.70.1

* Move error reporting related mutations out of root mutations

# hint 1.70.0

* adds basic data exploration app with loading spinner
Expand Down
5 changes: 3 additions & 2 deletions src/app/static/src/app/components/ErrorReport.vue
Expand Up @@ -102,13 +102,14 @@
import {StepDescription, StepperState} from "../store/stepper/stepper";
import {ProjectsState} from "../store/projects/projects"
import Modal from "./Modal.vue";
import {ErrorReportManualDetails} from "../types";
import ErrorAlert from "./ErrorAlert.vue";
import {ErrorReportManualDetails} from "../store/root/actions";
import {VTooltip} from 'v-tooltip';
import i18next from "i18next";
import {RootState} from "../root";
import {Language} from "../store/translations/locales";
import {Error} from "../generated";
import { ErrorsState } from "../store/errors/errors";


interface Methods {
Expand Down Expand Up @@ -171,7 +172,7 @@
this.section = newVal
}
},
errorReportError: mapStateProp<RootState, Error | null>(null, (state: RootState) => state.errorReportError),
errorReportError: mapStateProp<ErrorsState, Error | null>("errors", (state: ErrorsState) => state.errorReportError),
isGuest: mapGetterByName(null, "isGuest"),
projectName: mapStateProp<ProjectsState, string | undefined>("projects", state => state.currentProject?.name),
steps: mapStateProp<StepperState, StepDescription[]>("stepper", state => state.steps),
Expand Down
2 changes: 1 addition & 1 deletion src/app/static/src/app/hintVersion.ts
@@ -1 +1 @@
export const currentHintVersion = "1.69.0";
export const currentHintVersion = "1.70.1";
7 changes: 1 addition & 6 deletions src/app/static/src/app/root.ts
Expand Up @@ -33,7 +33,6 @@ import {currentHintVersion} from "./hintVersion";
import {ModelRunMutation, ModelRunUpdates} from "./store/modelRun/mutations";
import {adr, ADRState, initialADRState} from "./store/adr/adr";
import {adrUpload, ADRUploadState, initialADRUploadState} from "./store/adrUpload/adrUpload";
import {Error} from "./generated";

import {
downloadResults,
Expand Down Expand Up @@ -64,9 +63,7 @@ export interface RootState extends DataExplorationState {
errors: ErrorsState,
projects: ProjectsState
currentUser: string,
downloadResults: DownloadResultsState,
errorReportError: Error | null
errorReportSuccess: boolean
downloadResults: DownloadResultsState
}

export interface ReadyState {
Expand Down Expand Up @@ -135,8 +132,6 @@ export const emptyState = (): RootState => {
language: Language.en,
version: currentHintVersion,
updatingLanguage: false,
errorReportError: null,
errorReportSuccess: false,
hintrVersion: initialHintrVersionState(),
adr: initialADRState(),
genericChart: initialGenericChartState(),
Expand Down
Expand Up @@ -38,8 +38,6 @@ export interface DataExplorationState extends TranslatableState {
errors: ErrorsState,
currentUser: string,
updatingLanguage: boolean,
errorReportError: Error | null
errorReportSuccess: boolean,
dataExplorationMode: boolean
}

Expand All @@ -48,8 +46,6 @@ export const initialDataExplorationState = (): DataExplorationState => {
language: Language.en,
version: currentHintVersion,
updatingLanguage: false,
errorReportError: null,
errorReportSuccess: false,
hintrVersion: initialHintrVersionState(),
adr: initialADRState(),
genericChart: initialGenericChartState(),
Expand Down
8 changes: 6 additions & 2 deletions src/app/static/src/app/store/errors/errors.ts
Expand Up @@ -4,12 +4,16 @@ import {mutations} from "./mutations";
import {DataExplorationState} from "../dataExploration/dataExploration";

export interface ErrorsState {
errors: Error[]
errors: Error[],
errorReportError: Error | null
errorReportSuccess: boolean
}

export const initialErrorsState = (): ErrorsState => {
return {
errors: []
errors: [],
errorReportError: null,
errorReportSuccess: false,
}
};

Expand Down
18 changes: 15 additions & 3 deletions src/app/static/src/app/store/errors/mutations.ts
Expand Up @@ -5,7 +5,9 @@ import {ErrorsState} from "./errors";

export enum ErrorsMutation {
ErrorAdded = "ErrorAdded",
DismissAll = "DismissAll"
DismissAll = "DismissAll",
ErrorReportError = "ErrorReportError",
ErrorReportSuccess = "ErrorReportSuccess"
}

export const mutations: MutationTree<ErrorsState> = {
Expand All @@ -14,7 +16,17 @@ export const mutations: MutationTree<ErrorsState> = {
state.errors.push(action.payload);
},

[ErrorsMutation.DismissAll](state: ErrorsState, action: PayloadWithType<null>) {
[ErrorsMutation.DismissAll](state: ErrorsState) {
state.errors = [];
}
},

[ErrorsMutation.ErrorReportError](state: ErrorsState, action: PayloadWithType<Error>) {
state.errorReportError = action.payload;
state.errorReportSuccess = false;
},

[ErrorsMutation.ErrorReportSuccess](state: ErrorsState) {
state.errorReportSuccess = true;
state.errorReportError = null;
},
};
29 changes: 7 additions & 22 deletions src/app/static/src/app/store/root/actions.ts
Expand Up @@ -2,10 +2,12 @@ import {ActionContext, ActionTree} from "vuex";
import {RootState} from "../../root";
import {StepDescription} from "../stepper/stepper";
import {RootMutation} from "./mutations";
import {ErrorsMutation} from "../errors/mutations";
import {LanguageActions} from "../language/language";
import {changeLanguage} from "../language/actions";
import i18next from "i18next";
import {api} from "../../apiService";
import {ErrorReportManualDetails} from "../../types";
import {VersionInfo} from "../../generated";
import {currentHintVersion} from "../../hintVersion";
import {LanguageMutation} from "../language/mutations";
Expand All @@ -14,24 +16,7 @@ import {LanguageMutation} from "../language/mutations";
export interface RootActions extends LanguageActions<RootState> {
validate: (store: ActionContext<RootState, RootState>) => void;
generateErrorReport: (store: ActionContext<RootState, RootState>,
payload: ErrorReportManualDetails) => void;
}

export interface ErrorReportManualDetails {
section: string,
description: string,
stepsToReproduce: string,
email: string
}

export interface ErrorReport extends ErrorReportManualDetails {
country: string,
projectName: string | undefined,
browserAgent: string,
timeStamp: string,
jobId: string,
versions: VersionInfo,
errors: Error[]
payload: ErrorReportManualDetails) => void;
}

export const actions: ActionTree<RootState, RootState> & RootActions = {
Expand Down Expand Up @@ -118,12 +103,12 @@ export const actions: ActionTree<RootState, RootState> & RootActions = {
errors: getters.errors
}

await api<RootMutation, RootMutation>(context)
.withSuccess(RootMutation.ErrorReportSuccess)
.withError(RootMutation.ErrorReportError)
await api<ErrorsMutation, ErrorsMutation>(context)
.withSuccess(`errors/${ErrorsMutation.ErrorReportSuccess}` as ErrorsMutation, true)
.withError(`errors/${ErrorsMutation.ErrorReportError}` as ErrorsMutation, true)
.postAndReturn("error-report", data)
.then(() => {
if (rootState.projects.currentProject && !rootState.errorReportError) {
if (rootState.projects.currentProject && !rootState.errors.errorReportError) {
dispatch("projects/cloneProject",
{
emails: ["naomi-support@imperial.ac.uk"],
Expand Down
18 changes: 1 addition & 17 deletions src/app/static/src/app/store/root/mutations.ts
Expand Up @@ -17,17 +17,14 @@ import {initialModelCalibrateState} from "../modelCalibrate/modelCalibrate";
import {initialADRUploadState} from "../adrUpload/adrUpload";
import {initialDownloadResultsState} from "../downloadResults/downloadResults";
import {initialGenericChartState} from "../genericChart/genericChart";
import {Error} from "../../generated";

export enum RootMutation {
Reset = "Reset",
ResetSelectedDataType = "ResetSelectedDataType",
ResetOptions = "ResetOptions",
ResetOutputs = "ResetOutputs",
SetProject = "SetProject",
ResetDownload = "ResetDownload",
ErrorReportError = "ErrorReportError",
ErrorReportSuccess = "ErrorReportSuccess"
ResetDownload = "ResetDownload"
}

export const mutations: MutationTree<RootState> = {
Expand All @@ -40,8 +37,6 @@ export const mutations: MutationTree<RootState> = {
const resetState: RootState = {
version: state.version,
hintrVersion: state.hintrVersion,
errorReportError: null,
errorReportSuccess: false,
language: state.language,
updatingLanguage: false,
adr: state.adr,
Expand Down Expand Up @@ -156,17 +151,6 @@ export const mutations: MutationTree<RootState> = {
Object.assign(state.adrUpload, initialADRUploadState());
Object.assign(state.downloadResults, initialDownloadResultsState());
},

[RootMutation.ErrorReportError](state: RootState, action: PayloadWithType<Error>) {
state.errorReportError = action.payload;
state.errorReportSuccess = false;
},

[RootMutation.ErrorReportSuccess](state: RootState) {
state.errorReportSuccess = true;
state.errorReportError = null
},

...languageMutations

};
19 changes: 18 additions & 1 deletion src/app/static/src/app/types.ts
@@ -1,5 +1,5 @@
import {Payload} from "vuex";
import {FilterOption, Error, DownloadStatusResponse, DownloadSubmitResponse, Warning} from "./generated";
import {FilterOption, Error, DownloadStatusResponse, DownloadSubmitResponse, Warning, VersionInfo} from "./generated";
import {Language} from "./store/translations/locales";

export interface PayloadWithType<T> extends Payload {
Expand Down Expand Up @@ -266,6 +266,23 @@ export interface StepWarnings {
modelCalibrate: Warning[]
}

export interface ErrorReportManualDetails {
section: string,
description: string,
stepsToReproduce: string,
email: string
}

export interface ErrorReport extends ErrorReportManualDetails {
country: string,
projectName: string | undefined,
browserAgent: string,
timeStamp: string,
jobId: string,
versions: VersionInfo,
errors: Error[]
}

export interface TranslatableState {
language: Language
updatingLanguage: boolean
Expand Down
14 changes: 10 additions & 4 deletions src/app/static/src/tests/components/errorReport.test.ts
Expand Up @@ -3,7 +3,7 @@ import {shallowMount, mount, createLocalVue} from '@vue/test-utils';
import ErrorReport from "../../app/components/ErrorReport.vue";
import Modal from "../../app/components/Modal.vue";
import Vuex from "vuex";
import {mockProjectsState, mockRootState, mockStepperState} from "../mocks";
import {mockErrorsState, mockProjectsState, mockRootState, mockStepperState} from "../mocks";
import registerTranslations from "../../app/store/translations/registerTranslations";
import {StepperState} from "../../app/store/stepper/stepper";
import {ProjectsState} from "../../app/store/projects/projects";
Expand All @@ -12,6 +12,7 @@ import VueRouter from "vue-router";
import {Language} from "../../app/store/translations/locales";
import ErrorAlert from "../../app/components/ErrorAlert.vue";
import {RootState} from "../../app/root";
import { ErrorsState } from "../../app/store/errors/errors";

describe("Error report component", () => {

Expand All @@ -20,7 +21,8 @@ describe("Error report component", () => {
const createStore = (stepperState: Partial<StepperState> = {},
projectsState: Partial<ProjectsState> = {},
rootState: Partial<RootState> = {},
isGuest = false) => {
isGuest = false,
errorsState: Partial<ErrorsState> ={}) => {
const store = new Vuex.Store({
state: mockRootState(rootState),
modules: {
Expand All @@ -31,7 +33,11 @@ describe("Error report component", () => {
projects: {
namespaced: true,
state: mockProjectsState(projectsState)
}
},
errors: {
namespaced: true,
state: mockErrorsState(errorsState)
},
},
actions: {
generateErrorReport
Expand Down Expand Up @@ -591,7 +597,7 @@ describe("Error report component", () => {
propsData: {
open: true
},
store: createStore({}, {}, {errorReportError})
store: createStore({}, {}, {}, false, {errorReportError})
});
wrapper.setData({showFeedback: true});
expectTranslated(wrapper.find("#report-error"),
Expand Down
31 changes: 26 additions & 5 deletions src/app/static/src/tests/errors/mutations.test.ts
@@ -1,20 +1,41 @@
import {mutations} from "../../app/store/errors/mutations";
import {ErrorsState} from "../../app/store/errors/errors";
import {mockError} from "../mocks";
import {Error} from "../../app/generated";
import {mockError, mockErrorsState} from "../mocks";

describe("Errors mutations", () => {
it("ErrorAdded adds error", () => {
const firstError = mockError("first error");
const secondError = mockError("second error");
const testState = {errors: [firstError]};
const testState = {
errors: [firstError],
errorReportError: null,
errorReportSuccess: false
};
mutations.ErrorAdded(testState, {type: mutations.ErrorAdded, payload: secondError});
expect(testState.errors).toStrictEqual([firstError, secondError]);
});

it("DismissAll clears errors", () => {
const testState = {errors: [mockError("First Error")]};
const testState = {
errors: [mockError("First Error")],
errorReportError: null,
errorReportSuccess: false
};
mutations.DismissAll(testState, null);
expect(testState.errors).toStrictEqual([]);
});

it("can set ErrorReportError", () => {
const error = mockError("Error");
const state = mockErrorsState();
mutations.ErrorReportError(state, {payload: error});
expect(state.errorReportError).toBe(error);
expect(state.errorReportSuccess).toBe(false);
});

it("can set ErrorReportSuccess", () => {
const state = mockErrorsState();
mutations.ErrorReportSuccess(state);
expect(state.errorReportSuccess).toBe(true);
expect(state.errorReportError).toBe(null);
});
});