Skip to content

Commit

Permalink
feat: check workflow and component parameter names are unique (#490)
Browse files Browse the repository at this point in the history
Co-authored-by: Alessandro Pomponio <[email protected]>
  • Loading branch information
2 people authored and GitHub Enterprise committed Jan 10, 2024
1 parent be2088c commit 9eba7a0
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 15 deletions.
56 changes: 50 additions & 6 deletions src/canvas/components/forms/componentForm.vue
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@
<bx-input
:value="parameter.name"
@input="parameter.name = $event.target.value"
:invalid="parameterNameIsDuplicate(parameter.name)"
validity-message="Parameter names must be unique"
placeholder="parameter name"
colorScheme="light"
/>
Expand Down Expand Up @@ -689,6 +691,9 @@ import "@carbon/web-components/es/components/accordion/index.js";
import St4sdComponent from "@/canvas/classes/St4sdComponent.js";
import { updateNodeLabel } from "@/canvas/functions/updateNodeLabel";
// AP: see comment on parameterNameIsDuplicate
// import { checkParameterNameIsDuplicate } from "@/canvas/functions/validation";
let invalidVariables = {};
export default {
Expand All @@ -698,7 +703,8 @@ export default {
return {
contentSwitcherSelection: ref("config"),
component: new St4sdComponent(),
isError: false,
isNameError: false,
isParameterError: false,
oldName: "",
variableKeys: [],
variableValues: [],
Expand All @@ -716,7 +722,7 @@ export default {
},
methods: {
update() {
if (!this.isError) {
if (!this.isNameError && !this.isParameterError) {
this.setVariables();
let newComponentNode = this.allNodes.find(
(node) => node.id == this.node.id,
Expand All @@ -739,7 +745,7 @@ export default {
}
},
add() {
if (!this.isError) {
if (!this.isNameError && !this.isParameterError) {
this.setVariables();
this.$emit("add", this.component.getComponentDefintion());
}
Expand All @@ -751,15 +757,53 @@ export default {
this.component.signature.name = newName;
this.$emit("nameChanged", newName);
},
// TODO: AP: using this function raises
// [Vue warn]: Maximum recursive updates exceeded.
// This means you have a reactive effect that is mutating its own dependencies and thus recursively triggering itself.
// Possible sources include component template, render function, updated hook or watcher source function.
// in St4sdComponent.js:319 or componentForm.vue:71
// parameterNameIsDuplicate(name) {
// let parameterNames = this.component
// .getParameters()
// .map((param) => param.name);
// this.isParameterError = checkParameterNameIsDuplicate(
// parameterNames,
// name,
// );
// this.$emit("invalid", this.isNameError || this.isParameterError);
// return this.isParameterError;
// },
parameterNameIsDuplicate(name) {
let parametersNames = this.component
.getParameters()
.map((param) => param.name);
if (new Set(parametersNames).size != parametersNames.length) {
let occurrences = 0;
for (let parameter of parametersNames) {
if (parameter == name) {
occurrences++;
if (occurrences > 1) {
this.isParameterError = true;
this.$emit("invalid", true);
return true;
}
}
}
} else {
this.isParameterError = false;
this.$emit("invalid", this.isNameError);
return false;
}
},
onFocusLost(event, item) {
if (item == "") {
event.target.invalid = true;
this.isError = true;
this.isNameError = true;
} else {
this.isError = false;
this.isNameError = false;
event.target.invalid = false;
}
this.$emit("invalid", this.isError);
this.$emit("invalid", this.isNameError);
},
addVariables() {
this.variableKeys.push("");
Expand Down
38 changes: 33 additions & 5 deletions src/canvas/components/forms/workflowForm.vue
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@
placeholder="parameter name"
:value="parameter.name"
@input="parameter.name = $event.target.value"
:invalid="parameterNameIsDuplicate(parameter.name)"
validity-message="Parameter names must be unique"
colorScheme="light"
>
</bx-input>
Expand Down Expand Up @@ -177,12 +179,14 @@ export default {
"stepDeleted",
"workflowAdded",
"nameChanged",
"validityChanged",
],
data() {
return {
workflow: new St4sdWorkflow(),
childrenNodes: [],
isError: false,
isNameError: false,
isParameterError: false,
oldName: "",
removedSteps: [],
};
Expand Down Expand Up @@ -226,7 +230,7 @@ export default {
});
},
add() {
if (!this.isError) {
if (!this.isNameError && !this.isParameterError) {
//Each workflow will generate a workflow node and an input node for that workflow
let { workflowNode, inputNode } = createWorkflowNode(
this.workflow.getName(),
Expand All @@ -237,7 +241,7 @@ export default {
}
},
update() {
if (!this.isError) {
if (!this.isNameError && !this.isParameterError) {
if (this.workflow.areStepsUnique()) {
this.applyStepRemoval();
this.checkStepsChanges();
Expand Down Expand Up @@ -290,14 +294,38 @@ export default {
).label = `${tobeUpdatedNode.label} inputs`;
}
},
// TODO: AP: see warning on parameterNameIsDuplicate in componentForm.vue
parameterNameIsDuplicate(name) {
let parametersNames = this.workflow
.getParameters()
.map((param) => param.name);
if (new Set(parametersNames).size != parametersNames.length) {
let occurrences = 0;
for (let parameter of parametersNames) {
if (parameter == name) {
occurrences++;
if (occurrences > 1) {
this.isParameterError = true;
this.$emit("validityChanged", true);
return true;
}
}
}
} else {
this.isParameterError = false;
this.$emit("validityChanged", this.isNameError);
return false;
}
},
onFocusLost(event, item) {
if (item == "") {
event.target.invalid = true;
this.isError = true;
this.isNameError = true;
} else {
this.isError = false;
this.isNameError = false;
event.target.invalid = false;
}
this.$emit("validityChanged", this.isNameError || this.isParameterError);
},
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,21 @@
<bx-modal-heading>Create workflow</bx-modal-heading>
</bx-modal-header>
<bx-modal-body>
<workflowForm ref="workflowForm" @workflowAdded="workflowAdded" />
<workflowForm
ref="workflowForm"
@workflowAdded="workflowAdded"
@validityChanged="updateIsSubmitButtonDisabled"
/>
</bx-modal-body>
<bx-modal-footer>
<bx-modal-footer-button kind="secondary" data-modal-close
>Cancel</bx-modal-footer-button
>
<bx-modal-footer-button kind="primary" type="submit" @click="addWorkflow"
<bx-modal-footer-button
kind="primary"
type="submit"
:disabled="isSubmitButtonDisabled"
@click="addWorkflow"
>Submit</bx-modal-footer-button
>
</bx-modal-footer>
Expand All @@ -25,7 +33,9 @@ export default {
components: { workflowForm },
emits: ["added"],
data() {
return {};
return {
isSubmitButtonDisabled: true,
};
},
methods: {
addWorkflow() {
Expand All @@ -34,6 +44,9 @@ export default {
workflowAdded(workflowNode, inputNode) {
this.$emit("added", workflowNode, inputNode);
},
updateIsSubmitButtonDisabled(isError) {
this.isSubmitButtonDisabled = isError;
},
},
};
</script>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
@removeParent="removeParent"
@stepDeleted="stepDeleted"
@nameChanged="updateIsTemplateButtonDisabled"
@validityChanged="updateIsSaveButtonDisabled"
/>
<br />
<bx-btn kind="danger" @click="emitDelete">
Expand Down Expand Up @@ -57,7 +58,11 @@
<bx-modal-footer-button kind="secondary" data-modal-close
>Cancel</bx-modal-footer-button
>
<bx-modal-footer-button kind="primary" type="submit" @click="save"
<bx-modal-footer-button
kind="primary"
type="submit"
:disabled="isSaveButtonDisabled"
@click="save"
>Save changes</bx-modal-footer-button
>
</bx-modal-footer>
Expand All @@ -78,6 +83,7 @@ export default {
data() {
return {
isTemplateButtonDisabled: false,
isSaveButtonDisabled: false,
};
},
emits: [
Expand Down Expand Up @@ -130,6 +136,9 @@ export default {
};
this.$emit("updateWorkflowModalNotification", notification);
},
updateIsSaveButtonDisabled(isError) {
this.isSaveButtonDisabled = isError;
},
},
};
</script>
Expand Down
18 changes: 18 additions & 0 deletions src/canvas/functions/validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -281,3 +281,21 @@ export const validateSignature = (node) => {
}
return nodeSignature;
};

export const checkParameterNameIsDuplicate = (parameters, targetParameter) => {
if (new Set(parameters).size == parameters.length) {
return false;
}

let occurrences = 0;
for (let parameter of parameters) {
if (parameter == targetParameter) {
occurrences++;
if (occurrences > 1) {
return true;
}
}
}

return false;
};

0 comments on commit 9eba7a0

Please sign in to comment.