From 6c25b37e235a90608a8f3654863a040d25b457a6 Mon Sep 17 00:00:00 2001 From: guerler Date: Tue, 25 Jan 2022 21:47:41 -0500 Subject: [PATCH 1/5] Initialize error attribute in input model --- client/src/components/Form/FormDisplay.vue | 3 +++ client/src/components/Tool/ToolForm.vue | 2 -- client/src/components/Workflow/Run/model.js | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/client/src/components/Form/FormDisplay.vue b/client/src/components/Form/FormDisplay.vue index 846641bd15d..6a0c2fb22bf 100644 --- a/client/src/components/Form/FormDisplay.vue +++ b/client/src/components/Form/FormDisplay.vue @@ -146,6 +146,9 @@ export default { this.onChange(true); }, onCloneInputs() { + visitInputs(this.inputs, (input) => { + input.error = null; + }); this.formInputs = JSON.parse(JSON.stringify(this.inputs)); this.onCreateIndex(); }, diff --git a/client/src/components/Tool/ToolForm.vue b/client/src/components/Tool/ToolForm.vue index 3f254711b95..1bdcf6e9da4 100644 --- a/client/src/components/Tool/ToolForm.vue +++ b/client/src/components/Tool/ToolForm.vue @@ -142,7 +142,6 @@ export default { showSuccess: false, showError: false, showExecuting: false, - error: null, formConfig: {}, formData: {}, remapAllowed: false, @@ -159,7 +158,6 @@ export default { jobResponse: {}, validationInternal: null, validationScrollTo: null, - validationErrors: null, currentVersion: this.version, }; }, diff --git a/client/src/components/Workflow/Run/model.js b/client/src/components/Workflow/Run/model.js index 8edbd49914b..fe1eeab994f 100644 --- a/client/src/components/Workflow/Run/model.js +++ b/client/src/components/Workflow/Run/model.js @@ -135,7 +135,7 @@ export class WorkflowRunModel { // select fields are shown for dynamic fields if all putative data inputs are available, // or if an explicit reference is specified as data_ref and available - _.each(this.steps, (step, i) => { + _.each(this.steps, (step) => { if (step.step_type == "tool") { var data_resolved = true; visitInputs(step.inputs, (input, name, context) => { From e5f79a2f9fa7a789f51bdd88a1f10d97f54e728f Mon Sep 17 00:00:00 2001 From: guerler Date: Wed, 26 Jan 2022 00:34:13 -0500 Subject: [PATCH 2/5] Add test case to ensure that error attribute of inputs is properly initialized --- .../src/components/Form/FormDisplay.test.js | 48 +++++++++++++++++++ client/src/components/Form/FormDisplay.vue | 5 +- 2 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 client/src/components/Form/FormDisplay.test.js diff --git a/client/src/components/Form/FormDisplay.test.js b/client/src/components/Form/FormDisplay.test.js new file mode 100644 index 00000000000..85a6a3ddb13 --- /dev/null +++ b/client/src/components/Form/FormDisplay.test.js @@ -0,0 +1,48 @@ +import { mount } from "@vue/test-utils"; +import { getLocalVue } from "jest/helpers"; +import Vue from "vue"; +import FormDisplay from "./FormDisplay"; + +const localVue = getLocalVue(); + +describe("FormDisplay", () => { + let wrapper; + let propsData; + + beforeEach(() => { + propsData = { + id: "input", + inputs: [ + { + name: "name", + type: "text", + value: "value", + help: "help", + }, + ], + errors: {}, + prefix: "", + sustainRepeats: false, + sustainConditionals: false, + collapsedEnableText: "Enable", + collapsedDisableText: "Disable", + collapsedEnableIcon: "fa fa-caret-square-o-down", + collapsedDisableIcon: "fa fa-caret-square-o-up", + validationScrollTo: [], + replaceParams: {}, + }; + wrapper = mount(FormDisplay, { + propsData, + localVue, + stubs: {}, + }); + }); + + it("props", async () => { + await wrapper.setProps({ + validationScrollTo: ["name", "error_message"], + }); + const error = wrapper.find(".ui-form-error-text"); + expect(error.text()).toEqual("error_message"); + }); +}); diff --git a/client/src/components/Form/FormDisplay.vue b/client/src/components/Form/FormDisplay.vue index 6a0c2fb22bf..574813f9a10 100644 --- a/client/src/components/Form/FormDisplay.vue +++ b/client/src/components/Form/FormDisplay.vue @@ -181,7 +181,10 @@ export default { const elementId = getElementId(inputId); const element = this.$el.querySelector(`#${elementId}`); if (element) { - document.querySelector(".center-panel").scrollTo(0, this.getOffsetTop(element)); + const centerPanel = document.querySelector(".center-panel"); + if (centerPanel) { + centerPanel.scrollTo(0, this.getOffsetTop(element)); + } } } } From 5189ea58ad14264eaa61f741908e0349b97ac8f6 Mon Sep 17 00:00:00 2001 From: guerler Date: Wed, 26 Jan 2022 01:05:38 -0500 Subject: [PATCH 3/5] Add additional error detecting and form display tests --- .../src/components/Form/FormDisplay.test.js | 24 ++++++++++++++----- client/src/components/Form/FormDisplay.vue | 3 +-- client/src/components/Form/utilities.js | 9 ++++--- client/src/components/Form/utilities.test.js | 19 ++++++++++++++- 4 files changed, 41 insertions(+), 14 deletions(-) diff --git a/client/src/components/Form/FormDisplay.test.js b/client/src/components/Form/FormDisplay.test.js index 85a6a3ddb13..ee2af4b9323 100644 --- a/client/src/components/Form/FormDisplay.test.js +++ b/client/src/components/Form/FormDisplay.test.js @@ -1,6 +1,5 @@ import { mount } from "@vue/test-utils"; import { getLocalVue } from "jest/helpers"; -import Vue from "vue"; import FormDisplay from "./FormDisplay"; const localVue = getLocalVue(); @@ -21,15 +20,15 @@ describe("FormDisplay", () => { }, ], errors: {}, + validationScrollTo: [], + replaceParams: {}, prefix: "", sustainRepeats: false, sustainConditionals: false, collapsedEnableText: "Enable", collapsedDisableText: "Disable", - collapsedEnableIcon: "fa fa-caret-square-o-down", - collapsedDisableIcon: "fa fa-caret-square-o-up", - validationScrollTo: [], - replaceParams: {}, + collapsedEnableIcon: "collapsedEnableIcon", + collapsedDisableIcon: "collapsedDisableIcon", }; wrapper = mount(FormDisplay, { propsData, @@ -38,11 +37,24 @@ describe("FormDisplay", () => { }); }); - it("props", async () => { + it("error highlighting", async () => { await wrapper.setProps({ validationScrollTo: ["name", "error_message"], }); const error = wrapper.find(".ui-form-error-text"); expect(error.text()).toEqual("error_message"); + await wrapper.setProps({ + errors: { name: "error_message_2" }, + }); + expect(error.text()).toEqual("error_message_2"); + }); + + it("parameter replacement", async () => { + const input = wrapper.find("[id='field-name']"); + expect(input.element.value).toEqual("value"); + await wrapper.setProps({ + replaceParams: { name: "replaced" }, + }); + expect(input.element.value).toEqual("replaced"); }); }); diff --git a/client/src/components/Form/FormDisplay.vue b/client/src/components/Form/FormDisplay.vue index 574813f9a10..f953da722c7 100644 --- a/client/src/components/Form/FormDisplay.vue +++ b/client/src/components/Form/FormDisplay.vue @@ -9,7 +9,6 @@ :collapsed-enable-icon="collapsedEnableIcon" :collapsed-disable-text="collapsedDisableText" :collapsed-disable-icon="collapsedDisableIcon" - :errors="errors" :on-change="onChange" :on-change-form="onChangeForm" /> @@ -108,7 +107,7 @@ export default { errors() { this.resetError(); if (this.errors) { - const errorMessages = matchErrors(this.errors, this.formIndex); + const errorMessages = matchErrors(this.formIndex, this.errors); for (const inputId in errorMessages) { this.setError(inputId, errorMessages[inputId]); } diff --git a/client/src/components/Form/utilities.js b/client/src/components/Form/utilities.js index 71d697b3cbe..4359868b70f 100644 --- a/client/src/components/Form/utilities.js +++ b/client/src/components/Form/utilities.js @@ -72,16 +72,15 @@ export function matchCase(input, value) { } /** Match server validation response to highlight errors - * @param{dict} response - Nested dictionary with error messages * @param{dict} index - Index of input elements + * @param{dict} response - Nested dictionary with error messages */ -export function matchErrors(response, index) { +export function matchErrors(index, response) { var result = {}; function search(id, head) { if (typeof head === "string") { - var input_id = index[id]; - if (input_id) { - result[input_id] = head; + if (index[id]) { + result[id] = head; } } else { for (var i in head) { diff --git a/client/src/components/Form/utilities.test.js b/client/src/components/Form/utilities.test.js index 1b0bbd8645c..783b11629a8 100644 --- a/client/src/components/Form/utilities.test.js +++ b/client/src/components/Form/utilities.test.js @@ -1,4 +1,4 @@ -import { visitInputs, validateInputs, matchCase } from "./utilities"; +import { visitInputs, validateInputs, matchCase, matchErrors } from "./utilities"; import toolModel from "./test-data/tool"; function visitInputsString(inputs) { @@ -117,4 +117,21 @@ describe("form component utilities", () => { result = validateInputs(index, values); expect(JSON.stringify(result)).toEqual('["input_c","Please provide data for this input."]'); }); + + it("test error matching", () => { + const index = { + input_a: {}, + input_b_0: {}, + "input_c|input_d": {}, + }; + const values = { + input_a: "error_a", + input_b: ["error_b"], + input_c: { input_d: "error_d" }, + }; + const result = matchErrors(index, values); + expect(result["input_a"]).toEqual("error_a"); + expect(result["input_b_0"]).toEqual("error_b"); + expect(result["input_c|input_d"]).toEqual("error_d"); + }); }); From 5f3c0e894f0ef52086e1d0447b385659b5ca083e Mon Sep 17 00:00:00 2001 From: guerler Date: Wed, 26 Jan 2022 09:24:08 -0500 Subject: [PATCH 4/5] Add conditional tests and value replacement test to form display component --- .../src/components/Form/FormDisplay.test.js | 66 ++++++++++++++++--- 1 file changed, 57 insertions(+), 9 deletions(-) diff --git a/client/src/components/Form/FormDisplay.test.js b/client/src/components/Form/FormDisplay.test.js index ee2af4b9323..b149fe8ca80 100644 --- a/client/src/components/Form/FormDisplay.test.js +++ b/client/src/components/Form/FormDisplay.test.js @@ -13,10 +13,37 @@ describe("FormDisplay", () => { id: "input", inputs: [ { - name: "name", + name: "text_name", + value: "text_value", + help: "text_help", type: "text", - value: "value", - help: "help", + }, + { + type: "conditional", + name: "conditional_section", + test_param: { + name: "conditional_bool", + label: "conditional_bool_label", + type: "boolean", + value: "true", + help: "", + }, + cases: [ + { + value: "true", + inputs: [ + { + name: "conditional_leaf", + value: "conditional_leaf_value", + type: "text", + }, + ], + }, + { + value: "false", + inputs: [], + }, + ], }, ], errors: {}, @@ -39,22 +66,43 @@ describe("FormDisplay", () => { it("error highlighting", async () => { await wrapper.setProps({ - validationScrollTo: ["name", "error_message"], + validationScrollTo: ["text_name", "error_message"], }); const error = wrapper.find(".ui-form-error-text"); expect(error.text()).toEqual("error_message"); await wrapper.setProps({ - errors: { name: "error_message_2" }, + errors: { text_name: "error_message_2" }, }); expect(error.text()).toEqual("error_message_2"); }); it("parameter replacement", async () => { - const input = wrapper.find("[id='field-name']"); - expect(input.element.value).toEqual("value"); + const textInput = wrapper.find("[id='field-text_name']"); + const conditionalInput = wrapper.find("[id='field-conditional_section|conditional_leaf']"); + expect(textInput.element.value).toEqual("text_value"); + expect(conditionalInput.element.value).toEqual("conditional_leaf_value"); await wrapper.setProps({ - replaceParams: { name: "replaced" }, + replaceParams: { + text_name: "replaced", + "conditional_section|conditional_leaf": "conditional_leaf_value_new", + }, }); - expect(input.element.value).toEqual("replaced"); + expect(textInput.element.value).toEqual("replaced"); + expect(conditionalInput.element.value).toEqual("conditional_leaf_value_new"); + }); + + it("conditional switch", async () => { + const conditionalBool = wrapper.find("[type='checkbox']"); + await conditionalBool.setChecked(false); + const conditionalInputUnchecked = wrapper.findAll("[id='field-conditional_section|conditional_leaf']"); + expect(conditionalInputUnchecked.length).toEqual(0); + await conditionalBool.setChecked(true); + const conditionalInputChecked = wrapper.findAll("[id='field-conditional_section|conditional_leaf']"); + expect(conditionalInputChecked.length).toEqual(1); + await wrapper.setProps({ + sustainConditionals: true, + }); + const conditionalBoolDisabled = wrapper.findAll("[type='checkbox']"); + expect(conditionalBoolDisabled.length).toEqual(0); }); }); From 1a3004c13139960a1052f667e4de2d4eed947305 Mon Sep 17 00:00:00 2001 From: guerler Date: Thu, 27 Jan 2022 22:06:45 -0500 Subject: [PATCH 5/5] Use vue set to avoid changing the incoming input array prop --- client/src/components/Form/FormDisplay.vue | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/client/src/components/Form/FormDisplay.vue b/client/src/components/Form/FormDisplay.vue index f953da722c7..708e863d6e9 100644 --- a/client/src/components/Form/FormDisplay.vue +++ b/client/src/components/Form/FormDisplay.vue @@ -14,6 +14,7 @@