From 729c96de3a4829f218de574c88626f6f97095ea4 Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 24 Sep 2018 14:27:19 -0400 Subject: [PATCH 1/8] new project Vue component is responsible for form validation --- js/components/forms/new_project.js | 30 ++++++++++++++++++++++ templates/fragments/edit_project_form.html | 5 +++- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/js/components/forms/new_project.js b/js/components/forms/new_project.js index 634c49ff..3b48cbf6 100644 --- a/js/components/forms/new_project.js +++ b/js/components/forms/new_project.js @@ -32,6 +32,7 @@ export default { ).map(createEnvironment) return { + showError: false, environments, name, } @@ -41,6 +42,12 @@ export default { this.$root.$on('onEnvironmentAdded', this.addEnvironment) }, + updated: function() { + if (this.environmentsHaveNames()) { + this.showError = false + } + }, + methods: { addEnvironment: function (event) { this.environments.push(createEnvironment("")) @@ -50,6 +57,29 @@ export default { if (this.environments.length > 1) { this.environments.splice(index, 1) } + }, + + environmentsHaveNames: function () { + return this.environments.every((e) => e.name !== "") + }, + + validateAndOpenModal: function (modalName) { + const textInputs = this.$children.reduce((previous, newVal) => { + // display textInput error if it is not valid + if (!newVal.showValid) { + newVal.showError = true + } + + return newVal.showValid && previous + }, true) + + const isValid = textInputs && this.environmentsHaveNames() + + if (isValid) { + this.openModal(modalName) + } else { + this.showError = true + } } } } diff --git a/templates/fragments/edit_project_form.html b/templates/fragments/edit_project_form.html index 7fbba8b1..3c76a73d 100644 --- a/templates/fragments/edit_project_form.html +++ b/templates/fragments/edit_project_form.html @@ -47,6 +47,9 @@ {{ Alert(error, level="error") }} {% endfor %} {% endif %} +
+ {{ Alert("Provide at least one environment name.", level="error") }} +
@@ -76,7 +79,7 @@
- +
From 97f3816627dd4a66b2cddfa3498a160ac951bed1 Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 24 Sep 2018 14:50:54 -0400 Subject: [PATCH 2/8] new project Vue component handles redundant env name validation --- js/components/forms/new_project.js | 31 +++++++++++++++++----- templates/fragments/edit_project_form.html | 10 +++---- 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/js/components/forms/new_project.js b/js/components/forms/new_project.js index 3b48cbf6..6201b3f5 100644 --- a/js/components/forms/new_project.js +++ b/js/components/forms/new_project.js @@ -32,7 +32,8 @@ export default { ).map(createEnvironment) return { - showError: false, + showMissingError: false, + showUniqueError: false, environments, name, } @@ -44,7 +45,11 @@ export default { updated: function() { if (this.environmentsHaveNames()) { - this.showError = false + this.showMissingError = false + } + + if (this.envNamesAreUnique()) { + this.showUniqueError = false } }, @@ -63,8 +68,13 @@ export default { return this.environments.every((e) => e.name !== "") }, + envNamesAreUnique: function () { + const names = this.environments.map((e) => e.name) + return [...new Set(names)].length == this.environments.length + }, + validateAndOpenModal: function (modalName) { - const textInputs = this.$children.reduce((previous, newVal) => { + const childrenValid = this.$children.reduce((previous, newVal) => { // display textInput error if it is not valid if (!newVal.showValid) { newVal.showError = true @@ -73,12 +83,19 @@ export default { return newVal.showValid && previous }, true) - const isValid = textInputs && this.environmentsHaveNames() + const hasNames = this.environmentsHaveNames() + const uniqNames = this.envNamesAreUnique() - if (isValid) { + if (!hasNames) { + this.showMissingError = true + } + + if (!uniqNames) { + this.showUniqueError = true + } + + if (childrenValid && hasNames && uniqNames) { this.openModal(modalName) - } else { - this.showError = true } } } diff --git a/templates/fragments/edit_project_form.html b/templates/fragments/edit_project_form.html index 3c76a73d..6cc46e0a 100644 --- a/templates/fragments/edit_project_form.html +++ b/templates/fragments/edit_project_form.html @@ -42,14 +42,12 @@ - {% if form.environment_names.errors %} - {% for error in form.environment_names.errors %} - {{ Alert(error, level="error") }} - {% endfor %} - {% endif %} -
+
{{ Alert("Provide at least one environment name.", level="error") }}
+
+ {{ Alert("Environment names must be unique.", level="error") }} +
From 95afcc69ddc0ea616b581084e9d98723a1a8efc7 Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 24 Sep 2018 15:00:01 -0400 Subject: [PATCH 3/8] tidy new project component a little --- js/components/forms/new_project.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/js/components/forms/new_project.js b/js/components/forms/new_project.js index 6201b3f5..994716fd 100644 --- a/js/components/forms/new_project.js +++ b/js/components/forms/new_project.js @@ -74,7 +74,7 @@ export default { }, validateAndOpenModal: function (modalName) { - const childrenValid = this.$children.reduce((previous, newVal) => { + let isValid = this.$children.reduce((previous, newVal) => { // display textInput error if it is not valid if (!newVal.showValid) { newVal.showError = true @@ -83,18 +83,17 @@ export default { return newVal.showValid && previous }, true) - const hasNames = this.environmentsHaveNames() - const uniqNames = this.envNamesAreUnique() - - if (!hasNames) { + if (!this.environmentsHaveNames()) { + isValid = false this.showMissingError = true } - if (!uniqNames) { + if (!this.envNamesAreUnique()) { + isValid = false this.showUniqueError = true } - if (childrenValid && hasNames && uniqNames) { + if (isValid) { this.openModal(modalName) } } From 033ee4e36e2af50e5255b59e2606ebb49b1bef0e Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 25 Sep 2018 10:09:01 -0400 Subject: [PATCH 4/8] refactor for more abstract validation handling in new project Vue component --- js/components/forms/new_project.js | 55 ++++++++++++++-------- templates/fragments/edit_project_form.html | 7 ++- 2 files changed, 41 insertions(+), 21 deletions(-) diff --git a/js/components/forms/new_project.js b/js/components/forms/new_project.js index 994716fd..fcab3823 100644 --- a/js/components/forms/new_project.js +++ b/js/components/forms/new_project.js @@ -3,6 +3,12 @@ import textinput from '../text_input' const createEnvironment = (name) => ({ name }) +const VALIDATIONS = { + showMissingError: "hasEnvironments", + showUniqueError: "envNamesAreUnique", + showEmptyError: "environmentsHaveNames", +} + export default { name: 'new-project', @@ -31,9 +37,13 @@ export default { : [""] ).map(createEnvironment) + let errors = {} + Object.keys(VALIDATIONS).map((key) => { + errors[key] = false + }) + return { - showMissingError: false, - showUniqueError: false, + errors, environments, name, } @@ -44,13 +54,12 @@ export default { }, updated: function() { - if (this.environmentsHaveNames()) { - this.showMissingError = false - } - - if (this.envNamesAreUnique()) { - this.showUniqueError = false - } + Object.keys(VALIDATIONS).map((errName) => { + const func = VALIDATIONS[errName] + if (this[func]()) { + this.errors[errName] = false + } + }) }, methods: { @@ -64,8 +73,18 @@ export default { } }, + hasEnvironments: function () { + return this.environments.length > 0 && this.environments.some((e) => e.name !== "") + }, + environmentsHaveNames: function () { - return this.environments.every((e) => e.name !== "") + if (this.environments.length > 1) { + // only want to display this error if we have multiple envs and one or + // more do not have names + return this.environments.every((e) => e.name !== "") + } else { + return true + } }, envNamesAreUnique: function () { @@ -83,15 +102,13 @@ export default { return newVal.showValid && previous }, true) - if (!this.environmentsHaveNames()) { - isValid = false - this.showMissingError = true - } - - if (!this.envNamesAreUnique()) { - isValid = false - this.showUniqueError = true - } + Object.keys(VALIDATIONS).map((errName) => { + const func = VALIDATIONS[errName] + if (!this[func]()) { + isValid = false + this.errors[errName] = true + } + }) if (isValid) { this.openModal(modalName) diff --git a/templates/fragments/edit_project_form.html b/templates/fragments/edit_project_form.html index 6cc46e0a..59f90607 100644 --- a/templates/fragments/edit_project_form.html +++ b/templates/fragments/edit_project_form.html @@ -42,12 +42,15 @@
-
+
{{ Alert("Provide at least one environment name.", level="error") }}
-
+
{{ Alert("Environment names must be unique.", level="error") }}
+
+ {{ Alert("Environment names cannot be empty.", level="error") }} +
From e78c535659225fe7bc8b6488fa670f36deb4ef78 Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 25 Sep 2018 10:44:19 -0400 Subject: [PATCH 5/8] refactor form component validation into class --- js/components/forms/new_project.js | 56 ++++++++++++++++++++---------- 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/js/components/forms/new_project.js b/js/components/forms/new_project.js index fcab3823..c4aa3642 100644 --- a/js/components/forms/new_project.js +++ b/js/components/forms/new_project.js @@ -9,6 +9,39 @@ const VALIDATIONS = { showEmptyError: "environmentsHaveNames", } +class Validator { + constructor(validations) { + this.validations = validations + } + + errorList() { + let errors = {} + this.map((key) => { + errors[key] = false + }) + return errors + } + + map(callback) { + Object.keys(this.validations).map((k) => callback(k)) + } + + update(object) { + this.map((errName) => { + if (object[this.validations[errName]]()) { + object.errors[errName] = false + } + }) + } + + validate(object) { + this.map((errName) => { + object.errors[errName] = !object[this.validations[errName]]() + }) + return Object.values(object.errors).every(e => e) + } +} + export default { name: 'new-project', @@ -37,13 +70,11 @@ export default { : [""] ).map(createEnvironment) - let errors = {} - Object.keys(VALIDATIONS).map((key) => { - errors[key] = false - }) + const validator = new Validator(VALIDATIONS) return { - errors, + validator, + errors: validator.errorList(), environments, name, } @@ -54,12 +85,7 @@ export default { }, updated: function() { - Object.keys(VALIDATIONS).map((errName) => { - const func = VALIDATIONS[errName] - if (this[func]()) { - this.errors[errName] = false - } - }) + this.validator.update(this) }, methods: { @@ -102,13 +128,7 @@ export default { return newVal.showValid && previous }, true) - Object.keys(VALIDATIONS).map((errName) => { - const func = VALIDATIONS[errName] - if (!this[func]()) { - isValid = false - this.errors[errName] = true - } - }) + isValid = this.validator.validate(this) && isValid if (isValid) { this.openModal(modalName) From 7ae201655ab1e877d01d97e443570c3514cdf14c Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 25 Sep 2018 12:02:59 -0400 Subject: [PATCH 6/8] refactor new project component form validation --- js/components/forms/new_project.js | 64 ++++++---------------- templates/components/alert.html | 8 ++- templates/fragments/edit_project_form.html | 11 +--- 3 files changed, 24 insertions(+), 59 deletions(-) diff --git a/js/components/forms/new_project.js b/js/components/forms/new_project.js index c4aa3642..480592b4 100644 --- a/js/components/forms/new_project.js +++ b/js/components/forms/new_project.js @@ -3,45 +3,6 @@ import textinput from '../text_input' const createEnvironment = (name) => ({ name }) -const VALIDATIONS = { - showMissingError: "hasEnvironments", - showUniqueError: "envNamesAreUnique", - showEmptyError: "environmentsHaveNames", -} - -class Validator { - constructor(validations) { - this.validations = validations - } - - errorList() { - let errors = {} - this.map((key) => { - errors[key] = false - }) - return errors - } - - map(callback) { - Object.keys(this.validations).map((k) => callback(k)) - } - - update(object) { - this.map((errName) => { - if (object[this.validations[errName]]()) { - object.errors[errName] = false - } - }) - } - - validate(object) { - this.map((errName) => { - object.errors[errName] = !object[this.validations[errName]]() - }) - return Object.values(object.errors).every(e => e) - } -} - export default { name: 'new-project', @@ -70,11 +31,13 @@ export default { : [""] ).map(createEnvironment) - const validator = new Validator(VALIDATIONS) - return { - validator, - errors: validator.errorList(), + validations: [ + {func: "hasEnvironments", message: "Provide at least one environment name."}, + {func: "envNamesAreUnique", message: "Environment names must be unique."}, + {func: "environmentsHaveNames", message: "Environment names cannot be empty."}, + ], + errors: [], environments, name, } @@ -84,10 +47,6 @@ export default { this.$root.$on('onEnvironmentAdded', this.addEnvironment) }, - updated: function() { - this.validator.update(this) - }, - methods: { addEnvironment: function (event) { this.environments.push(createEnvironment("")) @@ -99,6 +58,14 @@ export default { } }, + validate: function() { + this.errors = this.validations.map((validation) => { + if (!this[validation.func]()) { + return validation.message + } + }).filter(Boolean) + }, + hasEnvironments: function () { return this.environments.length > 0 && this.environments.some((e) => e.name !== "") }, @@ -128,7 +95,8 @@ export default { return newVal.showValid && previous }, true) - isValid = this.validator.validate(this) && isValid + this.validate() + isValid = this.errors.length == 0 && isValid if (isValid) { this.openModal(modalName) diff --git a/templates/components/alert.html b/templates/components/alert.html index 1fa86489..d170e2bb 100644 --- a/templates/components/alert.html +++ b/templates/components/alert.html @@ -1,6 +1,6 @@ {% from "components/icon.html" import Icon %} -{% macro Alert(title, message=None, actions=None, level='info', fragment=None) -%} +{% macro Alert(title, message=None, actions=None, level='info', fragment=None, vue_template=False) -%} {% set role = 'alertdialog' if actions else 'alert' %} {% set levels = { 'warning': { @@ -25,7 +25,11 @@ {{ Icon(levels.get(level).get('icon'), classes='alert__icon icon--large') }}
-

{{title}}

+ {% if vue_template %} +

+ {% else %} +

{{title}}

+ {% endif %} {% if message %}
{{ message | safe }}
diff --git a/templates/fragments/edit_project_form.html b/templates/fragments/edit_project_form.html index 59f90607..006ceb24 100644 --- a/templates/fragments/edit_project_form.html +++ b/templates/fragments/edit_project_form.html @@ -42,14 +42,8 @@
-
- {{ Alert("Provide at least one environment name.", level="error") }} -
-
- {{ Alert("Environment names must be unique.", level="error") }} -
-
- {{ Alert("Environment names cannot be empty.", level="error") }} +
+ {{ Alert(message=None, level="error", vue_template=True) }}
@@ -78,7 +72,6 @@
-
From 774df938c464b4f09a6c53983e23aabfb03d9e0d Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 25 Sep 2018 14:34:55 -0400 Subject: [PATCH 7/8] cleanup, temp fix for PT bug #160768940 --- js/components/forms/new_project.js | 8 ++++---- templates/fragments/edit_project_form.html | 6 ++++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/js/components/forms/new_project.js b/js/components/forms/new_project.js index 480592b4..9a0cf6f9 100644 --- a/js/components/forms/new_project.js +++ b/js/components/forms/new_project.js @@ -33,9 +33,9 @@ export default { return { validations: [ - {func: "hasEnvironments", message: "Provide at least one environment name."}, - {func: "envNamesAreUnique", message: "Environment names must be unique."}, - {func: "environmentsHaveNames", message: "Environment names cannot be empty."}, + {func: this.hasEnvironments, message: "Provide at least one environment name."}, + {func: this.envNamesAreUnique, message: "Environment names must be unique."}, + {func: this.environmentsHaveNames, message: "Environment names cannot be empty."}, ], errors: [], environments, @@ -60,7 +60,7 @@ export default { validate: function() { this.errors = this.validations.map((validation) => { - if (!this[validation.func]()) { + if (!validation.func()) { return validation.message } }).filter(Boolean) diff --git a/templates/fragments/edit_project_form.html b/templates/fragments/edit_project_form.html index 006ceb24..e6b4977e 100644 --- a/templates/fragments/edit_project_form.html +++ b/templates/fragments/edit_project_form.html @@ -42,8 +42,10 @@
-
- {{ Alert(message=None, level="error", vue_template=True) }} +
{# this extra div prevents this bug: https://www.pivotaltracker.com/story/show/160768940 #} +
+ {{ Alert(message=None, level="error", vue_template=True) }} +
From 5eef5917e7babfc83c898f9f57b37922a5b688ad Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 25 Sep 2018 14:44:50 -0400 Subject: [PATCH 8/8] do not use spread operator --- js/components/forms/new_project.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/components/forms/new_project.js b/js/components/forms/new_project.js index 9a0cf6f9..aaa555d5 100644 --- a/js/components/forms/new_project.js +++ b/js/components/forms/new_project.js @@ -82,7 +82,7 @@ export default { envNamesAreUnique: function () { const names = this.environments.map((e) => e.name) - return [...new Set(names)].length == this.environments.length + return names.every((n, index) => names.indexOf(n) === index) }, validateAndOpenModal: function (modalName) {