From 8d9755161c82a1464e0ca60a97d028f47e89ace9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 14 Mar 2016 14:42:54 +0100 Subject: [PATCH 01/22] Add HalTransform for API errors --- frontend/app/components/api/api-v3/api-v3.config.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/frontend/app/components/api/api-v3/api-v3.config.ts b/frontend/app/components/api/api-v3/api-v3.config.ts index af17aed1f4..c55e015568 100644 --- a/frontend/app/components/api/api-v3/api-v3.config.ts +++ b/frontend/app/components/api/api-v3/api-v3.config.ts @@ -42,6 +42,10 @@ function apiV3Config(apiV3, halTransform) { return data; }); + + apiV3.setErrorInterceptor(response => { + response.data = halTransform(response.data); + }); } angular From 41e6a97fab09c34c2c7f0f1c3bcc201ded22b47a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 14 Mar 2016 14:44:07 +0100 Subject: [PATCH 02/22] Add column for attributes when in error --- .../components/query/query-service.service.ts | 4 ++ .../wp-edit-field/wp-edit-field.directive.ts | 39 ++++++++++++------- .../wp-edit/wp-edit-form.directive.ts | 33 ++++++++++------ 3 files changed, 49 insertions(+), 27 deletions(-) diff --git a/frontend/app/components/query/query-service.service.ts b/frontend/app/components/query/query-service.service.ts index 60bb263f2a..98556f8265 100644 --- a/frontend/app/components/query/query-service.service.ts +++ b/frontend/app/components/query/query-service.service.ts @@ -213,6 +213,10 @@ function QueryService(Query, return this.getQuery().getSelectedColumns(); }, + getSelectedColumnNames: function() { + return this.getSelectedColumns().map(column => column.name); + }, + setSelectedColumns: function(selectedColumnNames) { query.dirty = true; var currentColumns = this.getSelectedColumns(); diff --git a/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.ts b/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.ts index 3ef3c10828..82f9c305c6 100644 --- a/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.ts +++ b/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.ts @@ -32,13 +32,13 @@ import {Field} from "./wp-edit-field.module"; export class WorkPackageEditFieldController { - public formCtrl: WorkPackageEditFormController; + public formCtrl:WorkPackageEditFormController; public fieldName:string; public field:Field; protected _active:boolean = false; - constructor(protected wpEditField:WorkPackageEditFieldService) { + constructor(protected wpEditField:WorkPackageEditFieldService, protected QueryService) { } public get workPackage() { @@ -50,9 +50,19 @@ export class WorkPackageEditFieldController { } public submit() { - this.formCtrl.updateWorkPackage().then(() => { - this.deactivate(); - }); + this.formCtrl.updateWorkPackage() + .then(() => this.deactivate()) + .catch(missingFields => { + var selected = this.QueryService.getSelectedColumnNames(); + missingFields.map(field => { + var name = field.details.attribute; + if (selected.indexOf(name) === -1) { + selected.push(name); + } + }) + + this.QueryService.setSelectedColumns(selected); + }); } public activate() { @@ -70,7 +80,7 @@ export class WorkPackageEditFieldController { } protected setupField():ng.IPromise { - return this.formCtrl.loadSchema().then(schema => { + return this.formCtrl.loadSchema().then(schema => { this.field = this.wpEditField.getField( this.workPackage, this.fieldName, schema[this.fieldName]); }); @@ -81,18 +91,17 @@ export class WorkPackageEditFieldController { // the method is to be removed. private get isSupportedField():boolean { return ['subject', - 'priority', - 'type', - 'status', - 'assignee', - 'responsible', - 'version', - 'category'].indexOf(this.fieldName) !== -1 + 'priority', + 'type', + 'status', + 'assignee', + 'responsible', + 'version', + 'category'].indexOf(this.fieldName) !== -1 } } -function wpEditFieldLink(scope, element, attrs, controllers: - [WorkPackageEditFormController, WorkPackageEditFieldController]) { +function wpEditFieldLink(scope, element, attrs, controllers:[WorkPackageEditFormController, WorkPackageEditFieldController]) { controllers[1].formCtrl = controllers[0]; diff --git a/frontend/app/components/wp-edit/wp-edit-form.directive.ts b/frontend/app/components/wp-edit/wp-edit-form.directive.ts index 4f5694a49c..380d6d3d8e 100644 --- a/frontend/app/components/wp-edit/wp-edit-form.directive.ts +++ b/frontend/app/components/wp-edit/wp-edit-form.directive.ts @@ -40,21 +40,30 @@ export class WorkPackageEditFormController { var deferred = this.$q.defer(); this.workPackage.save() - .then(() => { - deferred.resolve(); - }) - .catch(error => { - if (error && error.data && error.data.message) { - this.NotificationsService.addError(error.data.message); - } else { - this.NotificationsService.addError("An internal error has occcurred."); - } - - return deferred.reject(); - }); + .then(() => { + deferred.resolve(); + }) + .catch((error) => this.handleSubmissionErrors(error, deferred)); return deferred.promise; } + + private handleSubmissionErrors(error:any, deferred:any) { + if (!error.data) { + this.NotificationsService.addError("An internal error has occcurred."); + return deferred.reject([]); + } + + error = error.data; + this.NotificationsService.addError(error.message); + + // Process single API errors + if (error.details) { + return deferred.reject([error]); + } + + return deferred.reject(error.errors || []); + } } function wpEditForm() { From 3043fcae79c927fc517c96eccfab7bb514ea3195 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 14 Mar 2016 16:08:29 +0100 Subject: [PATCH 03/22] Add very basic error styling --- .../content/_work_packages_table_edit.sass | 44 +++++++++++++++++++ app/assets/stylesheets/default.css.sass | 1 + .../wp-edit-field.directive.html | 4 +- .../wp-edit-field/wp-edit-field.directive.ts | 17 ++----- .../wp-edit/wp-edit-form.directive.ts | 27 +++++++++--- 5 files changed, 74 insertions(+), 19 deletions(-) create mode 100644 app/assets/stylesheets/content/_work_packages_table_edit.sass diff --git a/app/assets/stylesheets/content/_work_packages_table_edit.sass b/app/assets/stylesheets/content/_work_packages_table_edit.sass new file mode 100644 index 0000000000..f38beccf83 --- /dev/null +++ b/app/assets/stylesheets/content/_work_packages_table_edit.sass @@ -0,0 +1,44 @@ +//-- copyright +// OpenProject is a project management system. +// Copyright (C) 2012-2015 the OpenProject Foundation (OPF) +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See doc/COPYRIGHT.rdoc for more details. +//++ + +.wp-edit-field + border: 1px solid transparent + + &:hover + border-color: #eee + + &.-active:hover + border-color: transparent + + &.-error + background: rgb(254, 208, 209) + border-color: $inplace-edit--color--very-dark + + &:hover + border-color: lighten($inplace-edit--color--very-dark, 10%) + diff --git a/app/assets/stylesheets/default.css.sass b/app/assets/stylesheets/default.css.sass index ff5bd18313..db1afbf4d0 100644 --- a/app/assets/stylesheets/default.css.sass +++ b/app/assets/stylesheets/default.css.sass @@ -69,6 +69,7 @@ @import content/simple_filters @import content/advanced_filters @import content/work_packages_table +@import content/work_packages_table_edit @import content/attributes_key_value @import content/attributes_group @import content/attributes_table diff --git a/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.html b/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.html index 9423766990..bda8e602f6 100644 --- a/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.html +++ b/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.html @@ -1,4 +1,6 @@ -
+
diff --git a/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.ts b/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.ts index 82f9c305c6..197161470e 100644 --- a/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.ts +++ b/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.ts @@ -35,10 +35,11 @@ export class WorkPackageEditFieldController { public formCtrl:WorkPackageEditFormController; public fieldName:string; public field:Field; + public errorenous:boolean; protected _active:boolean = false; - constructor(protected wpEditField:WorkPackageEditFieldService, protected QueryService) { + constructor(protected wpEditField:WorkPackageEditFieldService) { } public get workPackage() { @@ -51,18 +52,7 @@ export class WorkPackageEditFieldController { public submit() { this.formCtrl.updateWorkPackage() - .then(() => this.deactivate()) - .catch(missingFields => { - var selected = this.QueryService.getSelectedColumnNames(); - missingFields.map(field => { - var name = field.details.attribute; - if (selected.indexOf(name) === -1) { - selected.push(name); - } - }) - - this.QueryService.setSelectedColumns(selected); - }); + .then(() => this.deactivate()); } public activate() { @@ -104,6 +94,7 @@ export class WorkPackageEditFieldController { function wpEditFieldLink(scope, element, attrs, controllers:[WorkPackageEditFormController, WorkPackageEditFieldController]) { controllers[1].formCtrl = controllers[0]; + controllers[1].formCtrl.fields[scope.vm.fieldName] = scope.vm; element.click(event => { event.stopImmediatePropagation(); diff --git a/frontend/app/components/wp-edit/wp-edit-form.directive.ts b/frontend/app/components/wp-edit/wp-edit-form.directive.ts index 380d6d3d8e..9b4d4b6186 100644 --- a/frontend/app/components/wp-edit/wp-edit-form.directive.ts +++ b/frontend/app/components/wp-edit/wp-edit-form.directive.ts @@ -28,8 +28,9 @@ export class WorkPackageEditFormController { public workPackage; + public fields = {}; - constructor(protected NotificationsService, protected $q) { + constructor(protected NotificationsService, protected $q, protected QueryService, protected $timeout) { } public loadSchema() { @@ -58,11 +59,27 @@ export class WorkPackageEditFormController { this.NotificationsService.addError(error.message); // Process single API errors - if (error.details) { - return deferred.reject([error]); - } + this.handleErrorenousColumns(error.details ? [error.details] : error.errors); + return deferred.reject(); + } + + private handleErrorenousColumns(columns:any[]) { + var selected = this.QueryService.getSelectedColumnNames(); + var active = _.find(this.fields, (f:any) => f.active); + columns.reverse().map(field => { + var name = field.details.attribute; + + if (selected.indexOf(name) === -1) { + selected.splice(selected.indexOf(active.fieldName) + 1, 0, name); + } + }); - return deferred.reject(error.errors || []); + this.QueryService.setSelectedColumns(selected); + this.$timeout(_ => { + angular.forEach(columns, (field) => { + this.fields[field.details.attribute].errorenous = true; + }); + }); } } From 08761bcd75d90fd429a9bc790a89f810fb66626c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 14 Mar 2016 17:00:45 +0100 Subject: [PATCH 04/22] Add separate ErrorResource --- .../hal-transform/hal-transform.config.ts | 5 +- .../api/api-v3/hal/error-resource.service.ts | 68 +++++++++++++++++++ .../wp-edit/wp-edit-form.directive.ts | 30 ++++---- 3 files changed, 85 insertions(+), 18 deletions(-) create mode 100644 frontend/app/components/api/api-v3/hal/error-resource.service.ts diff --git a/frontend/app/components/api/api-v3/hal-transform/hal-transform.config.ts b/frontend/app/components/api/api-v3/hal-transform/hal-transform.config.ts index e0c73d912b..4671048664 100644 --- a/frontend/app/components/api/api-v3/hal-transform/hal-transform.config.ts +++ b/frontend/app/components/api/api-v3/hal-transform/hal-transform.config.ts @@ -26,11 +26,12 @@ // See doc/COPYRIGHT.rdoc for more details. // ++ -function halTransformConfig(halTransformTypes, HalResource, WorkPackageResource, CollectionResource) { +function halTransformConfig(halTransformTypes, HalResource, WorkPackageResource, CollectionResource, ErrorResource) { angular.extend(halTransformTypes, { 'default': HalResource, WorkPackage: WorkPackageResource, - Collection: CollectionResource + Collection: CollectionResource, + Error: ErrorResource }); } diff --git a/frontend/app/components/api/api-v3/hal/error-resource.service.ts b/frontend/app/components/api/api-v3/hal/error-resource.service.ts new file mode 100644 index 0000000000..b81e253eba --- /dev/null +++ b/frontend/app/components/api/api-v3/hal/error-resource.service.ts @@ -0,0 +1,68 @@ +//-- copyright +// OpenProject is a project management system. +// Copyright (C) 2012-2015 the OpenProject Foundation (OPF) +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See doc/COPYRIGHT.rdoc for more details. +//++ + +function errorResource(HalResource:typeof op.HalResource, NotificationsService:any) { + class ErrorResource extends HalResource { + public errors:any[]; + public message:string; + public details:any; + public errorIdentifier:string; + + public get errorMessages():string[] { + if (this.isMultiErrorMessage()) { + return this.errors.map(error => error.message); + } else { + return [this.message]; + } + } + + public isMultiErrorMessage() { + return this.errorIdentifier === 'urn:openproject-org:api:v3:errors:MultipleErrors'; + } + + public showErrorNotification() { + var messages = this.errorMessages; + if (messages.length > 1) { + NotificationsService.addError('', messages); + } else { + NotificationsService.addError(messages[0]); + } + } + + public getInvolvedColumns():string[] { + var columns = this.details ? [this.details] : this.errors; + return columns.map(field => field.details.attribute); + } + } + + return ErrorResource; +} + +angular + .module('openproject.api') + .factory('ErrorResource', errorResource); diff --git a/frontend/app/components/wp-edit/wp-edit-form.directive.ts b/frontend/app/components/wp-edit/wp-edit-form.directive.ts index 9b4d4b6186..8477916d7e 100644 --- a/frontend/app/components/wp-edit/wp-edit-form.directive.ts +++ b/frontend/app/components/wp-edit/wp-edit-form.directive.ts @@ -44,31 +44,29 @@ export class WorkPackageEditFormController { .then(() => { deferred.resolve(); }) - .catch((error) => this.handleSubmissionErrors(error, deferred)); + .catch((error) => { + if (!error.data) { + this.NotificationsService.addError("An internal error has occcurred."); + return deferred.reject([]); + } + + error.data.showErrorNotification(); + this.handleSubmissionErrors(error.data, deferred) + }); return deferred.promise; } private handleSubmissionErrors(error:any, deferred:any) { - if (!error.data) { - this.NotificationsService.addError("An internal error has occcurred."); - return deferred.reject([]); - } - - error = error.data; - this.NotificationsService.addError(error.message); - // Process single API errors - this.handleErrorenousColumns(error.details ? [error.details] : error.errors); + this.handleErrorenousColumns(error.getInvolvedColumns()); return deferred.reject(); } - private handleErrorenousColumns(columns:any[]) { + private handleErrorenousColumns(columns:string[]) { var selected = this.QueryService.getSelectedColumnNames(); var active = _.find(this.fields, (f:any) => f.active); - columns.reverse().map(field => { - var name = field.details.attribute; - + columns.reverse().map(name => { if (selected.indexOf(name) === -1) { selected.splice(selected.indexOf(active.fieldName) + 1, 0, name); } @@ -76,8 +74,8 @@ export class WorkPackageEditFormController { this.QueryService.setSelectedColumns(selected); this.$timeout(_ => { - angular.forEach(columns, (field) => { - this.fields[field.details.attribute].errorenous = true; + angular.forEach(columns, (name) => { + this.fields[name].errorenous = true; }); }); } From 8dbafd3fb56ddadf72f97e0ad94c97562083fb7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Tue, 15 Mar 2016 09:05:09 +0100 Subject: [PATCH 05/22] Properly handle no-longer erroneous fields --- .../app/components/api/api-v3/hal/error-resource.service.ts | 2 +- frontend/app/components/wp-edit/wp-edit-form.directive.ts | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/frontend/app/components/api/api-v3/hal/error-resource.service.ts b/frontend/app/components/api/api-v3/hal/error-resource.service.ts index b81e253eba..a5d3314fea 100644 --- a/frontend/app/components/api/api-v3/hal/error-resource.service.ts +++ b/frontend/app/components/api/api-v3/hal/error-resource.service.ts @@ -55,7 +55,7 @@ function errorResource(HalResource:typeof op.HalResource, NotificationsService:a } public getInvolvedColumns():string[] { - var columns = this.details ? [this.details] : this.errors; + var columns = this.details ? [{ details: this.details }] : this.errors; return columns.map(field => field.details.attribute); } } diff --git a/frontend/app/components/wp-edit/wp-edit-form.directive.ts b/frontend/app/components/wp-edit/wp-edit-form.directive.ts index 8477916d7e..04daadd96b 100644 --- a/frontend/app/components/wp-edit/wp-edit-form.directive.ts +++ b/frontend/app/components/wp-edit/wp-edit-form.directive.ts @@ -58,6 +58,7 @@ export class WorkPackageEditFormController { } private handleSubmissionErrors(error:any, deferred:any) { + // Process single API errors this.handleErrorenousColumns(error.getInvolvedColumns()); return deferred.reject(); @@ -74,8 +75,8 @@ export class WorkPackageEditFormController { this.QueryService.setSelectedColumns(selected); this.$timeout(_ => { - angular.forEach(columns, (name) => { - this.fields[name].errorenous = true; + angular.forEach(this.fields, (field) => { + field.errorenous = columns.indexOf(field.fieldName) !== -1; }); }); } From 6c6bd49aa3e1126cd0c98fe530f78ea99e4888d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Tue, 15 Mar 2016 10:10:06 +0100 Subject: [PATCH 06/22] Add class states to outer td element Mark the td element with `-editable` when the wp-edit field is editable. --- .../content/_work_packages_table_edit.sass | 9 +++++++++ .../wp-edit-field/wp-edit-field.directive.ts | 18 +++++++++++++++++- .../wp-edit/wp-edit-form.directive.ts | 2 +- .../wp-table/wp-table.directive.html | 4 ++-- .../wp-table/wp-td/wp-td.directive.html | 5 ++++- 5 files changed, 33 insertions(+), 5 deletions(-) diff --git a/app/assets/stylesheets/content/_work_packages_table_edit.sass b/app/assets/stylesheets/content/_work_packages_table_edit.sass index f38beccf83..e04a9c432f 100644 --- a/app/assets/stylesheets/content/_work_packages_table_edit.sass +++ b/app/assets/stylesheets/content/_work_packages_table_edit.sass @@ -42,3 +42,12 @@ &:hover border-color: lighten($inplace-edit--color--very-dark, 10%) + + +.wp-table--cell + + &.-editable .wp-table--cell-span + cursor: pointer + + &:not(.-editable) .wp-table--cell-span + cursor: not-allowed diff --git a/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.ts b/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.ts index 197161470e..7dc057f5e3 100644 --- a/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.ts +++ b/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.ts @@ -39,7 +39,7 @@ export class WorkPackageEditFieldController { protected _active:boolean = false; - constructor(protected wpEditField:WorkPackageEditFieldService) { + constructor(protected wpEditField:WorkPackageEditFieldService, protected $element) { } public get workPackage() { @@ -69,6 +69,15 @@ export class WorkPackageEditFieldController { return this._active = false; } + public setErrorState(error = true) { + this.errorenous = error; + if (error) { + this.$element.addClass('-error'); + } else { + this.$element.removeClass('-error'); + } + } + protected setupField():ng.IPromise { return this.formCtrl.loadSchema().then(schema => { this.field = this.wpEditField.getField( @@ -99,6 +108,13 @@ function wpEditFieldLink(scope, element, attrs, controllers:[WorkPackageEditForm element.click(event => { event.stopImmediatePropagation(); }); + + // Mark the td field if it is inline-editable + if (scope.vm.isEditable) { + element.addClass('-editable'); + } + + element.addClass(scope.vm.fieldName); } function wpEditField() { diff --git a/frontend/app/components/wp-edit/wp-edit-form.directive.ts b/frontend/app/components/wp-edit/wp-edit-form.directive.ts index 04daadd96b..eb4a1dc72a 100644 --- a/frontend/app/components/wp-edit/wp-edit-form.directive.ts +++ b/frontend/app/components/wp-edit/wp-edit-form.directive.ts @@ -76,7 +76,7 @@ export class WorkPackageEditFormController { this.QueryService.setSelectedColumns(selected); this.$timeout(_ => { angular.forEach(this.fields, (field) => { - field.errorenous = columns.indexOf(field.fieldName) !== -1; + field.setErrorState(columns.indexOf(field.fieldName) !== -1); }); }); } diff --git a/frontend/app/components/wp-table/wp-table.directive.html b/frontend/app/components/wp-table/wp-table.directive.html index 99233d18e4..4345a8a712 100644 --- a/frontend/app/components/wp-table/wp-table.directive.html +++ b/frontend/app/components/wp-table/wp-table.directive.html @@ -111,10 +111,10 @@ + class="wp-table--cell" + ng-class="{ '-short': column.name == 'id' }"> + From 4f14459a61af301386bbdcc61d4607324d9cd20b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Tue, 15 Mar 2016 11:49:11 +0100 Subject: [PATCH 07/22] Reset error state when submission successful --- frontend/app/components/wp-edit/wp-edit-form.directive.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/frontend/app/components/wp-edit/wp-edit-form.directive.ts b/frontend/app/components/wp-edit/wp-edit-form.directive.ts index eb4a1dc72a..0209b43308 100644 --- a/frontend/app/components/wp-edit/wp-edit-form.directive.ts +++ b/frontend/app/components/wp-edit/wp-edit-form.directive.ts @@ -42,6 +42,7 @@ export class WorkPackageEditFormController { this.workPackage.save() .then(() => { + angular.forEach(this.fields, field => field.setErrorState(false)); deferred.resolve(); }) .catch((error) => { From e128ff9a9cf4c110c6ec54f1a6fe0b1767e6e3e9 Mon Sep 17 00:00:00 2001 From: Henriette Dinger Date: Tue, 15 Mar 2016 12:47:54 +0100 Subject: [PATCH 08/22] Use styling of inplaceEditing for table edit --- .../content/_in_place_editing.sass | 18 +++++++--- .../content/_work_packages_table_edit.sass | 34 +++++++++++-------- 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/app/assets/stylesheets/content/_in_place_editing.sass b/app/assets/stylesheets/content/_in_place_editing.sass index 3c2e73c6c3..e1905627b8 100644 --- a/app/assets/stylesheets/content/_in_place_editing.sass +++ b/app/assets/stylesheets/content/_in_place_editing.sass @@ -160,7 +160,8 @@ @include grid-block overflow: visible -.inplace-edit--read-value +.inplace-edit--read-value, +.wp-edit-field @include grid-block padding: 0.375rem @@ -224,7 +225,8 @@ a.inplace-edit--icon-wrapper // need to specify the a explicitly as otherwise // the default class will win a.inplace-editing--trigger-link, -.inplace-editing--trigger-link +.inplace-editing--trigger-link, +td.-editable @include grid-block color: $body-font-color font-weight: inherit @@ -237,8 +239,13 @@ a.inplace-editing--trigger-link, text-decoration: none color: $body-font-color - .inplace-editing--container + .inplace-editing--container, + .wp-edit-field border-color: $inplace-edit--border-color + + &.-active + border-color: transparent + .inplace-edit--icon-wrapper visibility: visible @@ -247,9 +254,10 @@ a.inplace-editing--trigger-link, .inplace-edit--read-value display: block -.inplace-editing--container +.inplace-editing--container, +.wp-edit-field @include grid-block - border-color: #fff + border-color: transparent border-style: solid border-radius: 2px border-width: 1px diff --git a/app/assets/stylesheets/content/_work_packages_table_edit.sass b/app/assets/stylesheets/content/_work_packages_table_edit.sass index e04a9c432f..7aa31e02c5 100644 --- a/app/assets/stylesheets/content/_work_packages_table_edit.sass +++ b/app/assets/stylesheets/content/_work_packages_table_edit.sass @@ -26,28 +26,34 @@ // See doc/COPYRIGHT.rdoc for more details. //++ -.wp-edit-field - border: 1px solid transparent +.-editable + .wp-edit-field + &.-active + padding: 0 - &:hover - border-color: #eee + &.-error + background: $nm-color-error-background + border-color: $nm-color-error-border - &.-active:hover - border-color: transparent - - &.-error - background: rgb(254, 208, 209) - border-color: $inplace-edit--color--very-dark - - &:hover - border-color: lighten($inplace-edit--color--very-dark, 10%) + &:hover + border-color: lighten($nm-color-error-border, 10%) + form + width: 100% + &:hover .wp-edit-field.-error:hover + border-color: $nm-color-error-border .wp-table--cell - &.-editable .wp-table--cell-span cursor: pointer &:not(.-editable) .wp-table--cell-span cursor: not-allowed + + +.work-package-table--container .generic-table tbody + td.-editable + padding-top: 0 + padding-bottom: 0 + display: table-cell From 9a51f103007b4709ff926830afc5d4d7bd57faf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Tue, 15 Mar 2016 13:09:45 +0100 Subject: [PATCH 09/22] Write feature spec --- .../inline_edit/edit_work_packages_spec.rb | 116 ++++++++++++++++++ spec/support/pages/work_packages_table.rb | 13 +- .../work_packages/inline_edit_field.rb | 85 +++++++++++++ 3 files changed, 210 insertions(+), 4 deletions(-) create mode 100644 spec/features/work_packages/inline_edit/edit_work_packages_spec.rb create mode 100644 spec/support/work_packages/inline_edit_field.rb diff --git a/spec/features/work_packages/inline_edit/edit_work_packages_spec.rb b/spec/features/work_packages/inline_edit/edit_work_packages_spec.rb new file mode 100644 index 0000000000..8fdedbf5bc --- /dev/null +++ b/spec/features/work_packages/inline_edit/edit_work_packages_spec.rb @@ -0,0 +1,116 @@ +require 'spec_helper' + +describe 'Inline editing work packages', js: true do + let(:dev_role) do + FactoryGirl.create :role, + permissions: [:view_work_packages, + :add_work_packages] + end + let(:dev) do + FactoryGirl.create :user, + firstname: 'Dev', + lastname: 'Guy', + member_in_project: project, + member_through_role: dev_role + end + let(:manager_role) do + FactoryGirl.create :role, + permissions: [:view_work_packages, + :edit_work_packages] + end + let(:manager) do + FactoryGirl.create :user, + firstname: 'Manager', + lastname: 'Guy', + member_in_project: project, + member_through_role: manager_role + end + let(:type) { FactoryGirl.create :type } + let(:type2) { FactoryGirl.create :type } + let(:project) { FactoryGirl.create(:project, types: [type, type2]) } + let(:work_package) { FactoryGirl.create(:work_package, author: dev, project: project, type: type, subject: 'Foobar') } + + let(:wp_table) { Pages::WorkPackagesTable.new(project) } + let(:fields) { InlineEditField.new(wp_table, work_package) } + + + let(:priority2) { FactoryGirl.create :priority } + let(:status2) { FactoryGirl.create :status } + let(:workflow) do + FactoryGirl.create :workflow, + type_id: type2.id, + old_status: work_package.status, + new_status: status2, + role: manager_role + end + let(:version) { FactoryGirl.create :version, project: project } + let(:category) { FactoryGirl.create :category, project: project } + + before do + login_as(manager) + + manager + dev + priority2 + workflow + + wp_table.visit! + wp_table.expect_work_package_listed(work_package) + end + + it 'allows updating and seeing the results' do + subject_field = wp_table.edit_field(work_package, :subject) + subject_field.expect_text('Foobar') + + subject_field.activate! + + subject_field.set_value('New subject!') + + expect(UpdateWorkPackageService).to receive(:new).and_call_original + subject_field.save! + subject_field.expect_text('New subject!') + + work_package.reload + expect(work_package.subject).to eq('New subject!') + end + + it 'allows to edit multiple fields' do + subject_field = wp_table.edit_field(work_package, :subject) + priority_field = wp_table.edit_field(work_package, :priority) + + subject_field.activate! + priority_field.activate! + + + subject_field.set_value('Other subject') + priority_field.set_value(priority2.name) + + expect(UpdateWorkPackageService).to receive(:new).and_call_original + priority_field.save! + + subject_field.expect_text('Other subject!') + priority_field.expect_text(priority2.name) + + work_package.reload + expect(work_package.subject).to eq('Other subject!') + expect(work_package.priority.id).to eq(priority2.id) + end + + it 'provides error handling' do + subject_field = wp_table.edit_field(work_package, :subject) + subject_field.expect_text('Foobar') + + subject_field.activate! + + subject_field.set_value('') + + expect(UpdateWorkPackageService).to receive(:new).and_call_original + subject_field.save! + wait_until_requests_completed! + subject_field.expect_error + subject_field.expect_text('-') + + work_package.reload + expect(work_package.subject).to eq('Foobar') + end +end diff --git a/spec/support/pages/work_packages_table.rb b/spec/support/pages/work_packages_table.rb index 45a26f133a..480f63d87a 100644 --- a/spec/support/pages/work_packages_table.rb +++ b/spec/support/pages/work_packages_table.rb @@ -27,6 +27,7 @@ #++ require 'support/pages/page' +require 'support/work_packages/inline_edit_field' module Pages class WorkPackagesTable < Page @@ -59,6 +60,14 @@ module Pages FullWorkPackage.new(work_package) end + def row(work_package) + table_container.find("#work-package-#{work_package.id}") + end + + def edit_field(work_package, attribute) + InlineEditField.new(work_package, attribute) + end + private def path @@ -68,9 +77,5 @@ module Pages def table_container find('#content .work-package-table--container') end - - def row(work_package) - table_container.find("#work-package-#{work_package.id}") - end end end diff --git a/spec/support/work_packages/inline_edit_field.rb b/spec/support/work_packages/inline_edit_field.rb new file mode 100644 index 0000000000..458ff3bbc4 --- /dev/null +++ b/spec/support/work_packages/inline_edit_field.rb @@ -0,0 +1,85 @@ +class InlineEditField + include Capybara::DSL + include RSpec::Matchers + + attr_reader :work_package, :attribute, :element, :selector + + def initialize(work_package, attribute, field_type: nil) + @work_package = work_package + @attribute = attribute + @field_type = field_type + + @selector = "#work-package-#{work_package.id} .#{attribute}" + @element = page.find(selector) + end + + def expect_text(text) + expect(page).to have_selector(selector, text: text) + end + + ## + # Activate the field and check it opened correctly + def activate! + edit_field.click + expect_active! + end + + ## + # Set or select the given value. + # For fields of type select, will check for an option with that value. + def set_value(content) + if field_type == 'select' + input_field.find(:option, content) + else + input_field.set(content) + end + end + + def expect_error + expect(page).to have_selector("#{field_selector}.-error") + end + + def expect_active! + expect(edit_field).to have_selector(field_type) + end + + def expect_inactive! + expect(edit_field).not_to have_selector(field_type) + end + + def save! + input_field.native.send_keys(:return) + reset_field + end + + def edit_field + @edit_field ||= @element.find('.wp-edit-field') + end + + def input_field + @input_field ||= edit_field.find(field_type) + end + + private + + def field_selector + "#{selector} .wp-edit-field" + end + + ## + # Reset the input field e.g., after saving + def reset_field + @input_field = nil + end + + def field_type + @field_type ||= begin + case attribute + when :assignee, :priority, :status + :select + else + :input + end.to_s + end + end +end From eebb92ef557e83b2f0604dede1d18161bc53e7f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 14 Mar 2016 14:42:54 +0100 Subject: [PATCH 10/22] Add HalTransform for API errors --- frontend/app/components/api/api-v3/api-v3.config.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/frontend/app/components/api/api-v3/api-v3.config.ts b/frontend/app/components/api/api-v3/api-v3.config.ts index af17aed1f4..c55e015568 100644 --- a/frontend/app/components/api/api-v3/api-v3.config.ts +++ b/frontend/app/components/api/api-v3/api-v3.config.ts @@ -42,6 +42,10 @@ function apiV3Config(apiV3, halTransform) { return data; }); + + apiV3.setErrorInterceptor(response => { + response.data = halTransform(response.data); + }); } angular From 560da7b09b2a733ef168474d601fa2f7ce878ba3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 14 Mar 2016 14:44:07 +0100 Subject: [PATCH 11/22] Add column for attributes when in error --- .../components/query/query-service.service.ts | 4 +++ .../wp-edit-field/wp-edit-field.directive.ts | 25 +++++++++----- .../wp-edit/wp-edit-form.directive.ts | 33 ++++++++++++------- 3 files changed, 42 insertions(+), 20 deletions(-) diff --git a/frontend/app/components/query/query-service.service.ts b/frontend/app/components/query/query-service.service.ts index 60bb263f2a..98556f8265 100644 --- a/frontend/app/components/query/query-service.service.ts +++ b/frontend/app/components/query/query-service.service.ts @@ -213,6 +213,10 @@ function QueryService(Query, return this.getQuery().getSelectedColumns(); }, + getSelectedColumnNames: function() { + return this.getSelectedColumns().map(column => column.name); + }, + setSelectedColumns: function(selectedColumnNames) { query.dirty = true; var currentColumns = this.getSelectedColumns(); diff --git a/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.ts b/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.ts index b302956564..5c289c2053 100644 --- a/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.ts +++ b/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.ts @@ -32,13 +32,13 @@ import {Field} from "./wp-edit-field.module"; export class WorkPackageEditFieldController { - public formCtrl: WorkPackageEditFormController; + public formCtrl:WorkPackageEditFormController; public fieldName:string; public field:Field; protected _active:boolean = false; - constructor(protected wpEditField:WorkPackageEditFieldService) { + constructor(protected wpEditField:WorkPackageEditFieldService, protected QueryService) { } public get workPackage() { @@ -50,9 +50,19 @@ export class WorkPackageEditFieldController { } public submit() { - this.formCtrl.updateWorkPackage().then(() => { - this.deactivate(); - }); + this.formCtrl.updateWorkPackage() + .then(() => this.deactivate()) + .catch(missingFields => { + var selected = this.QueryService.getSelectedColumnNames(); + missingFields.map(field => { + var name = field.details.attribute; + if (selected.indexOf(name) === -1) { + selected.push(name); + } + }) + + this.QueryService.setSelectedColumns(selected); + }); } public activate() { @@ -70,15 +80,14 @@ export class WorkPackageEditFieldController { } protected setupField():ng.IPromise { - return this.formCtrl.loadSchema().then(schema => { + return this.formCtrl.loadSchema().then(schema => { this.field = this.wpEditField.getField( this.workPackage, this.fieldName, schema[this.fieldName]); }); } } -function wpEditFieldLink(scope, element, attrs, controllers: - [WorkPackageEditFormController, WorkPackageEditFieldController]) { +function wpEditFieldLink(scope, element, attrs, controllers:[WorkPackageEditFormController, WorkPackageEditFieldController]) { controllers[1].formCtrl = controllers[0]; diff --git a/frontend/app/components/wp-edit/wp-edit-form.directive.ts b/frontend/app/components/wp-edit/wp-edit-form.directive.ts index 4f5694a49c..380d6d3d8e 100644 --- a/frontend/app/components/wp-edit/wp-edit-form.directive.ts +++ b/frontend/app/components/wp-edit/wp-edit-form.directive.ts @@ -40,21 +40,30 @@ export class WorkPackageEditFormController { var deferred = this.$q.defer(); this.workPackage.save() - .then(() => { - deferred.resolve(); - }) - .catch(error => { - if (error && error.data && error.data.message) { - this.NotificationsService.addError(error.data.message); - } else { - this.NotificationsService.addError("An internal error has occcurred."); - } - - return deferred.reject(); - }); + .then(() => { + deferred.resolve(); + }) + .catch((error) => this.handleSubmissionErrors(error, deferred)); return deferred.promise; } + + private handleSubmissionErrors(error:any, deferred:any) { + if (!error.data) { + this.NotificationsService.addError("An internal error has occcurred."); + return deferred.reject([]); + } + + error = error.data; + this.NotificationsService.addError(error.message); + + // Process single API errors + if (error.details) { + return deferred.reject([error]); + } + + return deferred.reject(error.errors || []); + } } function wpEditForm() { From cc315c21946b1a98d4ebef7dd315e345f51aee2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 14 Mar 2016 16:08:29 +0100 Subject: [PATCH 12/22] Add very basic error styling --- .../content/_work_packages_table_edit.sass | 44 +++++++++++++++++++ app/assets/stylesheets/default.css.sass | 1 + .../wp-edit-field.directive.html | 4 +- .../wp-edit-field/wp-edit-field.directive.ts | 17 ++----- .../wp-edit/wp-edit-form.directive.ts | 27 +++++++++--- 5 files changed, 74 insertions(+), 19 deletions(-) create mode 100644 app/assets/stylesheets/content/_work_packages_table_edit.sass diff --git a/app/assets/stylesheets/content/_work_packages_table_edit.sass b/app/assets/stylesheets/content/_work_packages_table_edit.sass new file mode 100644 index 0000000000..f38beccf83 --- /dev/null +++ b/app/assets/stylesheets/content/_work_packages_table_edit.sass @@ -0,0 +1,44 @@ +//-- copyright +// OpenProject is a project management system. +// Copyright (C) 2012-2015 the OpenProject Foundation (OPF) +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See doc/COPYRIGHT.rdoc for more details. +//++ + +.wp-edit-field + border: 1px solid transparent + + &:hover + border-color: #eee + + &.-active:hover + border-color: transparent + + &.-error + background: rgb(254, 208, 209) + border-color: $inplace-edit--color--very-dark + + &:hover + border-color: lighten($inplace-edit--color--very-dark, 10%) + diff --git a/app/assets/stylesheets/default.css.sass b/app/assets/stylesheets/default.css.sass index ff5bd18313..db1afbf4d0 100644 --- a/app/assets/stylesheets/default.css.sass +++ b/app/assets/stylesheets/default.css.sass @@ -69,6 +69,7 @@ @import content/simple_filters @import content/advanced_filters @import content/work_packages_table +@import content/work_packages_table_edit @import content/attributes_key_value @import content/attributes_group @import content/attributes_table diff --git a/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.html b/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.html index 3240a426c1..460f169f06 100644 --- a/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.html +++ b/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.html @@ -1,4 +1,6 @@ -
+<<<<<<< HEAD +
diff --git a/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.ts b/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.ts index 5c289c2053..68b18cc3d3 100644 --- a/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.ts +++ b/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.ts @@ -35,10 +35,11 @@ export class WorkPackageEditFieldController { public formCtrl:WorkPackageEditFormController; public fieldName:string; public field:Field; + public errorenous:boolean; protected _active:boolean = false; - constructor(protected wpEditField:WorkPackageEditFieldService, protected QueryService) { + constructor(protected wpEditField:WorkPackageEditFieldService) { } public get workPackage() { @@ -51,18 +52,7 @@ export class WorkPackageEditFieldController { public submit() { this.formCtrl.updateWorkPackage() - .then(() => this.deactivate()) - .catch(missingFields => { - var selected = this.QueryService.getSelectedColumnNames(); - missingFields.map(field => { - var name = field.details.attribute; - if (selected.indexOf(name) === -1) { - selected.push(name); - } - }) - - this.QueryService.setSelectedColumns(selected); - }); + .then(() => this.deactivate()); } public activate() { @@ -90,6 +80,7 @@ export class WorkPackageEditFieldController { function wpEditFieldLink(scope, element, attrs, controllers:[WorkPackageEditFormController, WorkPackageEditFieldController]) { controllers[1].formCtrl = controllers[0]; + controllers[1].formCtrl.fields[scope.vm.fieldName] = scope.vm; element.click(event => { event.stopImmediatePropagation(); diff --git a/frontend/app/components/wp-edit/wp-edit-form.directive.ts b/frontend/app/components/wp-edit/wp-edit-form.directive.ts index 380d6d3d8e..9b4d4b6186 100644 --- a/frontend/app/components/wp-edit/wp-edit-form.directive.ts +++ b/frontend/app/components/wp-edit/wp-edit-form.directive.ts @@ -28,8 +28,9 @@ export class WorkPackageEditFormController { public workPackage; + public fields = {}; - constructor(protected NotificationsService, protected $q) { + constructor(protected NotificationsService, protected $q, protected QueryService, protected $timeout) { } public loadSchema() { @@ -58,11 +59,27 @@ export class WorkPackageEditFormController { this.NotificationsService.addError(error.message); // Process single API errors - if (error.details) { - return deferred.reject([error]); - } + this.handleErrorenousColumns(error.details ? [error.details] : error.errors); + return deferred.reject(); + } + + private handleErrorenousColumns(columns:any[]) { + var selected = this.QueryService.getSelectedColumnNames(); + var active = _.find(this.fields, (f:any) => f.active); + columns.reverse().map(field => { + var name = field.details.attribute; + + if (selected.indexOf(name) === -1) { + selected.splice(selected.indexOf(active.fieldName) + 1, 0, name); + } + }); - return deferred.reject(error.errors || []); + this.QueryService.setSelectedColumns(selected); + this.$timeout(_ => { + angular.forEach(columns, (field) => { + this.fields[field.details.attribute].errorenous = true; + }); + }); } } From c872a321246ec2ac3b88efb76edb2f514a0543f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 14 Mar 2016 17:00:45 +0100 Subject: [PATCH 13/22] Add separate ErrorResource --- .../hal-transform/hal-transform.config.ts | 5 +- .../api/api-v3/hal/error-resource.service.ts | 68 +++++++++++++++++++ .../wp-edit/wp-edit-form.directive.ts | 30 ++++---- 3 files changed, 85 insertions(+), 18 deletions(-) create mode 100644 frontend/app/components/api/api-v3/hal/error-resource.service.ts diff --git a/frontend/app/components/api/api-v3/hal-transform/hal-transform.config.ts b/frontend/app/components/api/api-v3/hal-transform/hal-transform.config.ts index e0c73d912b..4671048664 100644 --- a/frontend/app/components/api/api-v3/hal-transform/hal-transform.config.ts +++ b/frontend/app/components/api/api-v3/hal-transform/hal-transform.config.ts @@ -26,11 +26,12 @@ // See doc/COPYRIGHT.rdoc for more details. // ++ -function halTransformConfig(halTransformTypes, HalResource, WorkPackageResource, CollectionResource) { +function halTransformConfig(halTransformTypes, HalResource, WorkPackageResource, CollectionResource, ErrorResource) { angular.extend(halTransformTypes, { 'default': HalResource, WorkPackage: WorkPackageResource, - Collection: CollectionResource + Collection: CollectionResource, + Error: ErrorResource }); } diff --git a/frontend/app/components/api/api-v3/hal/error-resource.service.ts b/frontend/app/components/api/api-v3/hal/error-resource.service.ts new file mode 100644 index 0000000000..b81e253eba --- /dev/null +++ b/frontend/app/components/api/api-v3/hal/error-resource.service.ts @@ -0,0 +1,68 @@ +//-- copyright +// OpenProject is a project management system. +// Copyright (C) 2012-2015 the OpenProject Foundation (OPF) +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See doc/COPYRIGHT.rdoc for more details. +//++ + +function errorResource(HalResource:typeof op.HalResource, NotificationsService:any) { + class ErrorResource extends HalResource { + public errors:any[]; + public message:string; + public details:any; + public errorIdentifier:string; + + public get errorMessages():string[] { + if (this.isMultiErrorMessage()) { + return this.errors.map(error => error.message); + } else { + return [this.message]; + } + } + + public isMultiErrorMessage() { + return this.errorIdentifier === 'urn:openproject-org:api:v3:errors:MultipleErrors'; + } + + public showErrorNotification() { + var messages = this.errorMessages; + if (messages.length > 1) { + NotificationsService.addError('', messages); + } else { + NotificationsService.addError(messages[0]); + } + } + + public getInvolvedColumns():string[] { + var columns = this.details ? [this.details] : this.errors; + return columns.map(field => field.details.attribute); + } + } + + return ErrorResource; +} + +angular + .module('openproject.api') + .factory('ErrorResource', errorResource); diff --git a/frontend/app/components/wp-edit/wp-edit-form.directive.ts b/frontend/app/components/wp-edit/wp-edit-form.directive.ts index 9b4d4b6186..8477916d7e 100644 --- a/frontend/app/components/wp-edit/wp-edit-form.directive.ts +++ b/frontend/app/components/wp-edit/wp-edit-form.directive.ts @@ -44,31 +44,29 @@ export class WorkPackageEditFormController { .then(() => { deferred.resolve(); }) - .catch((error) => this.handleSubmissionErrors(error, deferred)); + .catch((error) => { + if (!error.data) { + this.NotificationsService.addError("An internal error has occcurred."); + return deferred.reject([]); + } + + error.data.showErrorNotification(); + this.handleSubmissionErrors(error.data, deferred) + }); return deferred.promise; } private handleSubmissionErrors(error:any, deferred:any) { - if (!error.data) { - this.NotificationsService.addError("An internal error has occcurred."); - return deferred.reject([]); - } - - error = error.data; - this.NotificationsService.addError(error.message); - // Process single API errors - this.handleErrorenousColumns(error.details ? [error.details] : error.errors); + this.handleErrorenousColumns(error.getInvolvedColumns()); return deferred.reject(); } - private handleErrorenousColumns(columns:any[]) { + private handleErrorenousColumns(columns:string[]) { var selected = this.QueryService.getSelectedColumnNames(); var active = _.find(this.fields, (f:any) => f.active); - columns.reverse().map(field => { - var name = field.details.attribute; - + columns.reverse().map(name => { if (selected.indexOf(name) === -1) { selected.splice(selected.indexOf(active.fieldName) + 1, 0, name); } @@ -76,8 +74,8 @@ export class WorkPackageEditFormController { this.QueryService.setSelectedColumns(selected); this.$timeout(_ => { - angular.forEach(columns, (field) => { - this.fields[field.details.attribute].errorenous = true; + angular.forEach(columns, (name) => { + this.fields[name].errorenous = true; }); }); } From a56b38d658d376c8ad50bac952f9de9ed3aff047 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Tue, 15 Mar 2016 09:05:09 +0100 Subject: [PATCH 14/22] Properly handle no-longer erroneous fields --- .../app/components/api/api-v3/hal/error-resource.service.ts | 2 +- frontend/app/components/wp-edit/wp-edit-form.directive.ts | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/frontend/app/components/api/api-v3/hal/error-resource.service.ts b/frontend/app/components/api/api-v3/hal/error-resource.service.ts index b81e253eba..a5d3314fea 100644 --- a/frontend/app/components/api/api-v3/hal/error-resource.service.ts +++ b/frontend/app/components/api/api-v3/hal/error-resource.service.ts @@ -55,7 +55,7 @@ function errorResource(HalResource:typeof op.HalResource, NotificationsService:a } public getInvolvedColumns():string[] { - var columns = this.details ? [this.details] : this.errors; + var columns = this.details ? [{ details: this.details }] : this.errors; return columns.map(field => field.details.attribute); } } diff --git a/frontend/app/components/wp-edit/wp-edit-form.directive.ts b/frontend/app/components/wp-edit/wp-edit-form.directive.ts index 8477916d7e..04daadd96b 100644 --- a/frontend/app/components/wp-edit/wp-edit-form.directive.ts +++ b/frontend/app/components/wp-edit/wp-edit-form.directive.ts @@ -58,6 +58,7 @@ export class WorkPackageEditFormController { } private handleSubmissionErrors(error:any, deferred:any) { + // Process single API errors this.handleErrorenousColumns(error.getInvolvedColumns()); return deferred.reject(); @@ -74,8 +75,8 @@ export class WorkPackageEditFormController { this.QueryService.setSelectedColumns(selected); this.$timeout(_ => { - angular.forEach(columns, (name) => { - this.fields[name].errorenous = true; + angular.forEach(this.fields, (field) => { + field.errorenous = columns.indexOf(field.fieldName) !== -1; }); }); } From ec3f48e6a6f4a5c8b419592d2efc8eaf4fc96292 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Tue, 15 Mar 2016 10:10:06 +0100 Subject: [PATCH 15/22] Add class states to outer td element Mark the td element with `-editable` when the wp-edit field is editable. --- .../content/_work_packages_table_edit.sass | 9 +++++++++ .../wp-edit-field/wp-edit-field.directive.ts | 18 +++++++++++++++++- .../wp-edit/wp-edit-form.directive.ts | 2 +- .../wp-table/wp-table.directive.html | 4 ++-- .../wp-table/wp-td/wp-td.directive.html | 5 ++++- 5 files changed, 33 insertions(+), 5 deletions(-) diff --git a/app/assets/stylesheets/content/_work_packages_table_edit.sass b/app/assets/stylesheets/content/_work_packages_table_edit.sass index f38beccf83..e04a9c432f 100644 --- a/app/assets/stylesheets/content/_work_packages_table_edit.sass +++ b/app/assets/stylesheets/content/_work_packages_table_edit.sass @@ -42,3 +42,12 @@ &:hover border-color: lighten($inplace-edit--color--very-dark, 10%) + + +.wp-table--cell + + &.-editable .wp-table--cell-span + cursor: pointer + + &:not(.-editable) .wp-table--cell-span + cursor: not-allowed diff --git a/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.ts b/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.ts index 68b18cc3d3..517f34e338 100644 --- a/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.ts +++ b/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.ts @@ -39,7 +39,7 @@ export class WorkPackageEditFieldController { protected _active:boolean = false; - constructor(protected wpEditField:WorkPackageEditFieldService) { + constructor(protected wpEditField:WorkPackageEditFieldService, protected $element) { } public get workPackage() { @@ -69,6 +69,15 @@ export class WorkPackageEditFieldController { return this._active = false; } + public setErrorState(error = true) { + this.errorenous = error; + if (error) { + this.$element.addClass('-error'); + } else { + this.$element.removeClass('-error'); + } + } + protected setupField():ng.IPromise { return this.formCtrl.loadSchema().then(schema => { this.field = this.wpEditField.getField( @@ -85,6 +94,13 @@ function wpEditFieldLink(scope, element, attrs, controllers:[WorkPackageEditForm element.click(event => { event.stopImmediatePropagation(); }); + + // Mark the td field if it is inline-editable + if (scope.vm.isEditable) { + element.addClass('-editable'); + } + + element.addClass(scope.vm.fieldName); } function wpEditField() { diff --git a/frontend/app/components/wp-edit/wp-edit-form.directive.ts b/frontend/app/components/wp-edit/wp-edit-form.directive.ts index 04daadd96b..eb4a1dc72a 100644 --- a/frontend/app/components/wp-edit/wp-edit-form.directive.ts +++ b/frontend/app/components/wp-edit/wp-edit-form.directive.ts @@ -76,7 +76,7 @@ export class WorkPackageEditFormController { this.QueryService.setSelectedColumns(selected); this.$timeout(_ => { angular.forEach(this.fields, (field) => { - field.errorenous = columns.indexOf(field.fieldName) !== -1; + field.setErrorState(columns.indexOf(field.fieldName) !== -1); }); }); } diff --git a/frontend/app/components/wp-table/wp-table.directive.html b/frontend/app/components/wp-table/wp-table.directive.html index 99233d18e4..4345a8a712 100644 --- a/frontend/app/components/wp-table/wp-table.directive.html +++ b/frontend/app/components/wp-table/wp-table.directive.html @@ -111,10 +111,10 @@ + class="wp-table--cell" + ng-class="{ '-short': column.name == 'id' }"> + From 51b467ee15493c662f31f9a94aaa64599c0d5430 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Tue, 15 Mar 2016 11:49:11 +0100 Subject: [PATCH 16/22] Reset error state when submission successful --- frontend/app/components/wp-edit/wp-edit-form.directive.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/frontend/app/components/wp-edit/wp-edit-form.directive.ts b/frontend/app/components/wp-edit/wp-edit-form.directive.ts index eb4a1dc72a..0209b43308 100644 --- a/frontend/app/components/wp-edit/wp-edit-form.directive.ts +++ b/frontend/app/components/wp-edit/wp-edit-form.directive.ts @@ -42,6 +42,7 @@ export class WorkPackageEditFormController { this.workPackage.save() .then(() => { + angular.forEach(this.fields, field => field.setErrorState(false)); deferred.resolve(); }) .catch((error) => { From eb20be3db0c21b1cee10d62c77723f5163be7632 Mon Sep 17 00:00:00 2001 From: Henriette Dinger Date: Tue, 15 Mar 2016 12:47:54 +0100 Subject: [PATCH 17/22] Use styling of inplaceEditing for table edit --- .../content/_in_place_editing.sass | 18 +++++++--- .../content/_work_packages_table_edit.sass | 34 +++++++++++-------- 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/app/assets/stylesheets/content/_in_place_editing.sass b/app/assets/stylesheets/content/_in_place_editing.sass index 3c2e73c6c3..e1905627b8 100644 --- a/app/assets/stylesheets/content/_in_place_editing.sass +++ b/app/assets/stylesheets/content/_in_place_editing.sass @@ -160,7 +160,8 @@ @include grid-block overflow: visible -.inplace-edit--read-value +.inplace-edit--read-value, +.wp-edit-field @include grid-block padding: 0.375rem @@ -224,7 +225,8 @@ a.inplace-edit--icon-wrapper // need to specify the a explicitly as otherwise // the default class will win a.inplace-editing--trigger-link, -.inplace-editing--trigger-link +.inplace-editing--trigger-link, +td.-editable @include grid-block color: $body-font-color font-weight: inherit @@ -237,8 +239,13 @@ a.inplace-editing--trigger-link, text-decoration: none color: $body-font-color - .inplace-editing--container + .inplace-editing--container, + .wp-edit-field border-color: $inplace-edit--border-color + + &.-active + border-color: transparent + .inplace-edit--icon-wrapper visibility: visible @@ -247,9 +254,10 @@ a.inplace-editing--trigger-link, .inplace-edit--read-value display: block -.inplace-editing--container +.inplace-editing--container, +.wp-edit-field @include grid-block - border-color: #fff + border-color: transparent border-style: solid border-radius: 2px border-width: 1px diff --git a/app/assets/stylesheets/content/_work_packages_table_edit.sass b/app/assets/stylesheets/content/_work_packages_table_edit.sass index e04a9c432f..7aa31e02c5 100644 --- a/app/assets/stylesheets/content/_work_packages_table_edit.sass +++ b/app/assets/stylesheets/content/_work_packages_table_edit.sass @@ -26,28 +26,34 @@ // See doc/COPYRIGHT.rdoc for more details. //++ -.wp-edit-field - border: 1px solid transparent +.-editable + .wp-edit-field + &.-active + padding: 0 - &:hover - border-color: #eee + &.-error + background: $nm-color-error-background + border-color: $nm-color-error-border - &.-active:hover - border-color: transparent - - &.-error - background: rgb(254, 208, 209) - border-color: $inplace-edit--color--very-dark - - &:hover - border-color: lighten($inplace-edit--color--very-dark, 10%) + &:hover + border-color: lighten($nm-color-error-border, 10%) + form + width: 100% + &:hover .wp-edit-field.-error:hover + border-color: $nm-color-error-border .wp-table--cell - &.-editable .wp-table--cell-span cursor: pointer &:not(.-editable) .wp-table--cell-span cursor: not-allowed + + +.work-package-table--container .generic-table tbody + td.-editable + padding-top: 0 + padding-bottom: 0 + display: table-cell From 75717c2d0db8e68bb5936c7483bddb093bd91318 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Tue, 15 Mar 2016 13:17:27 +0100 Subject: [PATCH 18/22] Let cursor appear over whole line --- .../stylesheets/content/_work_packages_table_edit.sass | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/assets/stylesheets/content/_work_packages_table_edit.sass b/app/assets/stylesheets/content/_work_packages_table_edit.sass index 7aa31e02c5..6cceef8ea6 100644 --- a/app/assets/stylesheets/content/_work_packages_table_edit.sass +++ b/app/assets/stylesheets/content/_work_packages_table_edit.sass @@ -45,12 +45,10 @@ border-color: $nm-color-error-border .wp-table--cell - &.-editable .wp-table--cell-span - cursor: pointer - - &:not(.-editable) .wp-table--cell-span - cursor: not-allowed + cursor: not-allowed + &.-editable + cursor: pointer .work-package-table--container .generic-table tbody td.-editable From 0631162702ce39e461b103a8ccb8164a3eec6a20 Mon Sep 17 00:00:00 2001 From: Henriette Dinger Date: Tue, 15 Mar 2016 14:00:01 +0100 Subject: [PATCH 19/22] Fix bug with column width and avoid overflow of text --- .../content/_work_packages_table_edit.sass | 14 ++++++++++---- .../wp-edit-field/wp-edit-field.directive.html | 3 +-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/app/assets/stylesheets/content/_work_packages_table_edit.sass b/app/assets/stylesheets/content/_work_packages_table_edit.sass index 6cceef8ea6..b713127242 100644 --- a/app/assets/stylesheets/content/_work_packages_table_edit.sass +++ b/app/assets/stylesheets/content/_work_packages_table_edit.sass @@ -51,7 +51,13 @@ cursor: pointer .work-package-table--container .generic-table tbody - td.-editable - padding-top: 0 - padding-bottom: 0 - display: table-cell + td + .wp-edit-field .-hidden-overflow + overflow: hidden + text-overflow: ellipsis + &.-editable + padding-top: 0 + padding-bottom: 0 + display: table-cell + width: auto + diff --git a/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.html b/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.html index 460f169f06..93a682aeb0 100644 --- a/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.html +++ b/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.html @@ -1,4 +1,3 @@ -<<<<<<< HEAD
- +
From 3fa34c38c53ae0634ec1f8756964d550ea09d360 Mon Sep 17 00:00:00 2001 From: Henriette Dinger Date: Wed, 16 Mar 2016 12:56:37 +0100 Subject: [PATCH 20/22] Introduce -small class for smaller inplace editing fields --- .../content/_in_place_editing.sass | 21 +++++++++++++++++++ .../wp-edit-field.directive.html | 2 +- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/content/_in_place_editing.sass b/app/assets/stylesheets/content/_in_place_editing.sass index e1905627b8..f861b36ce3 100644 --- a/app/assets/stylesheets/content/_in_place_editing.sass +++ b/app/assets/stylesheets/content/_in_place_editing.sass @@ -380,3 +380,24 @@ td.-editable i vertical-align: text-top + + +.inplace-edit.-small .inplace-edit--read, +.inplace-edit.-small .inplace-edit--write, +.wp-edit-field.-small + height: 30px + padding-bottom: 3px + padding-top: 3px + + .inplace-edit--read-value + padding-top: 0 + padding-bottom: 0 + + select, input + height: 28px + padding-top: 0 + padding-bottom: 0 + line-height: normal + +.inplace-edit.-small + margin-top: 3px diff --git a/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.html b/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.html index 93a682aeb0..80ac74a5d2 100644 --- a/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.html +++ b/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.html @@ -1,4 +1,4 @@ -
From dd3e6d312104cef587991fe33abc638308b0e99d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Wed, 16 Mar 2016 14:09:21 +0100 Subject: [PATCH 21/22] Fix integration test after correct blur'ing --- .../inline_edit/edit_work_packages_spec.rb | 13 +++++-------- spec/support/work_packages/inline_edit_field.rb | 4 ++-- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/spec/features/work_packages/inline_edit/edit_work_packages_spec.rb b/spec/features/work_packages/inline_edit/edit_work_packages_spec.rb index ad5238e320..39e82031bb 100644 --- a/spec/features/work_packages/inline_edit/edit_work_packages_spec.rb +++ b/spec/features/work_packages/inline_edit/edit_work_packages_spec.rb @@ -79,18 +79,17 @@ describe 'Inline editing work packages', js: true do expect(work_package.subject).to eq('New subject!') end - it 'allows to edit multiple fields' do + it 'allows to subsequently edit multiple fields' do subject_field = wp_table.edit_field(work_package, :subject) priority_field = wp_table.edit_field(work_package, :priority) + expect(UpdateWorkPackageService).to receive(:new).and_call_original subject_field.activate! - priority_field.activate! + subject_field.set_value('Other subject!') - subject_field.set_value('Other subject') + priority_field.activate! priority_field.set_value(priority2.name) - - expect(UpdateWorkPackageService).to receive(:new).and_call_original - priority_field.save! + priority_field.expect_inactive! subject_field.expect_text('Other subject!') priority_field.expect_text(priority2.name) @@ -110,9 +109,7 @@ describe 'Inline editing work packages', js: true do expect(UpdateWorkPackageService).to receive(:new).and_call_original subject_field.save! - wait_until_requests_completed! subject_field.expect_error - subject_field.expect_text('-') work_package.reload expect(work_package.subject).to eq('Foobar') diff --git a/spec/support/work_packages/inline_edit_field.rb b/spec/support/work_packages/inline_edit_field.rb index 458ff3bbc4..d3f5361ff8 100644 --- a/spec/support/work_packages/inline_edit_field.rb +++ b/spec/support/work_packages/inline_edit_field.rb @@ -29,7 +29,7 @@ class InlineEditField # For fields of type select, will check for an option with that value. def set_value(content) if field_type == 'select' - input_field.find(:option, content) + input_field.find(:option, content).select_option else input_field.set(content) end @@ -44,7 +44,7 @@ class InlineEditField end def expect_inactive! - expect(edit_field).not_to have_selector(field_type) + expect(edit_field).to have_no_selector(field_type) end def save! From b34b28b632910a08fa16faa893f4395373e93f0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Fri, 18 Mar 2016 11:55:02 +0100 Subject: [PATCH 22/22] Load schema to determine writable state --- .../wp-edit-field/wp-edit-field.directive.ts | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.ts b/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.ts index 517f34e338..474ffe5a20 100644 --- a/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.ts +++ b/frontend/app/components/wp-edit/wp-edit-field/wp-edit-field.directive.ts @@ -71,11 +71,7 @@ export class WorkPackageEditFieldController { public setErrorState(error = true) { this.errorenous = error; - if (error) { - this.$element.addClass('-error'); - } else { - this.$element.removeClass('-error'); - } + this.$element.toggleClass('-error', error) } protected setupField():ng.IPromise { @@ -96,9 +92,12 @@ function wpEditFieldLink(scope, element, attrs, controllers:[WorkPackageEditForm }); // Mark the td field if it is inline-editable - if (scope.vm.isEditable) { - element.addClass('-editable'); - } + // We're resolving the non-form schema here since its loaded anyway for the table + scope.vm.formCtrl.loadSchema().then(schema => { + if (schema[scope.vm.fieldName].writable) { + element.addClass('-editable'); + } + }) element.addClass(scope.vm.fieldName); }