From 47dd10607a4452585d2635c8c7aa2b5630642285 Mon Sep 17 00:00:00 2001 From: Martin Hradil Date: Thu, 10 Oct 2019 12:52:31 +0000 Subject: [PATCH 1/8] DialogUser - add DialogData#outputConversion to be called from the ui to get dialogData suitable for API consumption. Right now, on submit, they just JSON.strigify the actual data and send it to the API. Which leaves Date instances to auto conversion, and that breaks Date parsing. Adding an explicit output conversion function and separate ui-classic and ui-service PRs to use it. https://bugzilla.redhat.com/show_bug.cgi?id=1744413 --- src/dialog-user/services/dialogData.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/dialog-user/services/dialogData.ts b/src/dialog-user/services/dialogData.ts index b5a675b82e..00c8dbc850 100644 --- a/src/dialog-user/services/dialogData.ts +++ b/src/dialog-user/services/dialogData.ts @@ -246,4 +246,10 @@ export default class DialogDataService { return invalid; } + + // converts the internal representation into the representation parsed by the API + // currently, this means we convert Date instances in to strings + public outputConversion(dialogData) { + return {...dialogData}; + } } From cb2d0714a12cc773b55d7a55d21b6cd90cbae742 Mon Sep 17 00:00:00 2001 From: Martin Hradil Date: Thu, 17 Oct 2019 13:06:20 +0000 Subject: [PATCH 2/8] dialogUser#setDialogData - save fields and values in DialogData service this is a bit weird structurally, but: * DialogUser is the topmost dialog component, it does call on-user but that's implemented separately in the uis * the uis don't have access to field definitions, only values * the uis do pass those values back to DialogData code, so they can't get pre-converted values without bigger rewrites * thus the uis need a way of obtaining data for submitting separately from on-update * that code needs to be able to access those fields definitions => saving both field definitions and field values in the DialogData instance, for use by DialogData#outputConversion --- src/dialog-user/components/dialog-user/dialogUser.ts | 4 ++++ src/dialog-user/services/dialogData.ts | 1 + 2 files changed, 5 insertions(+) diff --git a/src/dialog-user/components/dialog-user/dialogUser.ts b/src/dialog-user/components/dialog-user/dialogUser.ts index 6926491445..68e05c504d 100644 --- a/src/dialog-user/components/dialog-user/dialogUser.ts +++ b/src/dialog-user/components/dialog-user/dialogUser.ts @@ -82,6 +82,10 @@ export class DialogUserController extends DialogClass implements IDialogs { data: this.dialogValues }; this.onUpdate({ data: outputData }); + this.service.data = { + fields: this.dialogFields, + values: this.dialogValues, + }; } public validateFields() { diff --git a/src/dialog-user/services/dialogData.ts b/src/dialog-user/services/dialogData.ts index 00c8dbc850..5ea985c524 100644 --- a/src/dialog-user/services/dialogData.ts +++ b/src/dialog-user/services/dialogData.ts @@ -3,6 +3,7 @@ import * as angular from 'angular'; import {__} from '../../common/translateFunction'; export default class DialogDataService { + public data: any; /** * Sets up and configures properties for a dialog field From 53d63fc8abcccc53d85d0f9caf4f11ce48f51e55 Mon Sep 17 00:00:00 2001 From: Martin Hradil Date: Thu, 17 Oct 2019 13:11:16 +0000 Subject: [PATCH 3/8] DialogData#outputConversion - convert date to YYYY-MM-DD, datetime to iso datetime with timezone the function is used to convert date before submitting to API, and serializes Date fields back to strings, to prevent a date field from being first converted to utc, submitted to the api, and then cut to 10 characters, we're doing the cutting before such conversion can occur Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1744413 --- src/dialog-user/services/dialogData.ts | 30 ++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/src/dialog-user/services/dialogData.ts b/src/dialog-user/services/dialogData.ts index 5ea985c524..2dd3080e2e 100644 --- a/src/dialog-user/services/dialogData.ts +++ b/src/dialog-user/services/dialogData.ts @@ -1,6 +1,7 @@ import * as _ from 'lodash'; import * as angular from 'angular'; import {__} from '../../common/translateFunction'; +import {sprintf} from 'sprintf-js'; export default class DialogDataService { public data: any; @@ -249,8 +250,33 @@ export default class DialogDataService { } // converts the internal representation into the representation parsed by the API - // currently, this means we convert Date instances in to strings + // currently, this means we convert Date instances to strings public outputConversion(dialogData) { - return {...dialogData}; + const dateString = (date) => { + let y = date.getFullYear(), + m = date.getMonth() + 1, + d = date.getDate(); + return sprintf('%04d-%02d-%02d', y, m, d); + }; + + let out = {...dialogData}; + + Object.values(this.data.fields || {}).forEach(({name, type}) => { + let value = out[name]; + + switch (type) { + case 'DialogFieldDateControl': + // server expects 2019-10-20, anything longer gets cut + // converting first to prevent timezone conversions + out[name] = value && dateString(value); + break; + case 'DialogFieldDateTimeControl': + // explicit conversion to ISO datetime + out[name] = value && value.toISOString(); + break; + }; + }); + + return out; } } From 0621a1329cd2a555ac1c59d2db593a5e83e904d9 Mon Sep 17 00:00:00 2001 From: Martin Hradil Date: Thu, 17 Oct 2019 13:37:45 +0000 Subject: [PATCH 4/8] DialogData#outputConversion spec - test date undergoes no timezone conversions --- src/dialog-user/services/dialogData.spec.ts | 40 +++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/dialog-user/services/dialogData.spec.ts b/src/dialog-user/services/dialogData.spec.ts index 824621854a..c384c840fa 100644 --- a/src/dialog-user/services/dialogData.spec.ts +++ b/src/dialog-user/services/dialogData.spec.ts @@ -465,6 +465,45 @@ describe('DialogDataService test', () => { }); }); + describe('#outputConversion', () => { + beforeEach(() => { + const configuredField = dialogData.setupField({ + name: 'date_1', + type: 'DialogFieldDateControl', + default_value: '2019-10-15', + }); + + dialogData.data = { + fields: { + [configuredField.name]: configuredField, + }, + values: { + [configuredField.name]: configuredField.default_value, + }, + }; + }); + + it('converts Dates to string', () => { + let input = dialogData.data.values; + let output = dialogData.outputConversion(input); + + expect(input === output).toBe(false); // shallow copy + expect(typeof input.date_1).toEqual('object'); // Date + expect(typeof output.date_1).toEqual('string'); + expect(output.date_1).toMatch(/^\d+-\d+-\d+$/); // YYYY-MM-DD + }); + + it('preserves local timezone', () => { + let input = dialogData.data.values; + input.default_value = new Date('2019-10-15T01:23:45+05:00'); + + let output = dialogData.outputConversion(input); + + expect(input.date_1.getUTCDate()).toEqual(14); // UTC is off by one + expect(output.date_1).toEqual('2019-10-15'); // not 2019-10-14 + }); + }); + it('should allow a select list to be sorted', () => { const testDropDown = { values: [ @@ -489,6 +528,7 @@ describe('DialogDataService test', () => { const expectedSortedResult = [[5, 'C'], [1, 'B'], [2, 'A']]; expect(testSortedDescription).toEqual(expectedSortedResult); }); + it('should allow a numeric Description field to be sorted in a dropdown', () => { const testDropDownDescription = { values: [ From d3242a836cdcf524eda7aa466e8b64a0eae0ecbc Mon Sep 17 00:00:00 2001 From: Martin Hradil Date: Thu, 17 Oct 2019 13:40:13 +0000 Subject: [PATCH 5/8] dialogData spec - wrap 2 undescribed `it`s under describe('#updateFieldSortOrder') --- src/dialog-user/services/dialogData.spec.ts | 74 +++++++++++---------- 1 file changed, 38 insertions(+), 36 deletions(-) diff --git a/src/dialog-user/services/dialogData.spec.ts b/src/dialog-user/services/dialogData.spec.ts index c384c840fa..f35dd9233c 100644 --- a/src/dialog-user/services/dialogData.spec.ts +++ b/src/dialog-user/services/dialogData.spec.ts @@ -504,42 +504,44 @@ describe('DialogDataService test', () => { }); }); - it('should allow a select list to be sorted', () => { - const testDropDown = { - values: [ - [1, 'Test'], - [5, 'Test2'], - [2, 'Test5'] - ], - options: { sort_by: 'value', sort_order: 'descending', data_type: 'integer' } - }; - const testSorted = dialogData.updateFieldSortOrder(testDropDown); - const expectedResult = [[5, 'Test2'], [2, 'Test5'], [1, 'Test']]; - expect(testSorted).toEqual(expectedResult); - const testDropDownDescription = { - values: [ - [1, 'B'], - [5, 'C'], - [2, 'A'] - ], - options: { sort_by: 'description', sort_order: 'descending' } - }; - const testSortedDescription = dialogData.updateFieldSortOrder(testDropDownDescription); - const expectedSortedResult = [[5, 'C'], [1, 'B'], [2, 'A']]; - expect(testSortedDescription).toEqual(expectedSortedResult); - }); + describe('#updateFieldSortOrder', () => { + it('should allow a select list to be sorted', () => { + const testDropDown = { + values: [ + [1, 'Test'], + [5, 'Test2'], + [2, 'Test5'] + ], + options: { sort_by: 'value', sort_order: 'descending', data_type: 'integer' } + }; + const testSorted = dialogData.updateFieldSortOrder(testDropDown); + const expectedResult = [[5, 'Test2'], [2, 'Test5'], [1, 'Test']]; + expect(testSorted).toEqual(expectedResult); + const testDropDownDescription = { + values: [ + [1, 'B'], + [5, 'C'], + [2, 'A'] + ], + options: { sort_by: 'description', sort_order: 'descending' } + }; + const testSortedDescription = dialogData.updateFieldSortOrder(testDropDownDescription); + const expectedSortedResult = [[5, 'C'], [1, 'B'], [2, 'A']]; + expect(testSortedDescription).toEqual(expectedSortedResult); + }); - it('should allow a numeric Description field to be sorted in a dropdown', () => { - const testDropDownDescription = { - values: [ - ['zero', '1'], - ['five', '5'], - ['two', '2'] - ], - options: { sort_by: 'description', sort_order: 'descending' } - }; - const testSortedDescription = dialogData.updateFieldSortOrder(testDropDownDescription); - const expectedSortedResult = [['five', '5'], ['two', '2'], ['zero', '1']]; - expect(testSortedDescription).toEqual(expectedSortedResult); + it('should allow a numeric Description field to be sorted in a dropdown', () => { + const testDropDownDescription = { + values: [ + ['zero', '1'], + ['five', '5'], + ['two', '2'] + ], + options: { sort_by: 'description', sort_order: 'descending' } + }; + const testSortedDescription = dialogData.updateFieldSortOrder(testDropDownDescription); + const expectedSortedResult = [['five', '5'], ['two', '2'], ['zero', '1']]; + expect(testSortedDescription).toEqual(expectedSortedResult); + }); }); }); From 12ea0289f411f4a7afdf8b958f154a606a962988 Mon Sep 17 00:00:00 2001 From: Martin Hradil Date: Thu, 17 Oct 2019 13:48:40 +0000 Subject: [PATCH 6/8] dialogData timezone specs - disable spec for now Date always parses times into local time, so the local time != utc difference is different in each timezone. Tried to use timezone-mock for consistent behavior, ERROR [karma]: { inspect: [Function: inspect] } disabling the spec for now you can test by changing system timezone to Europe/Prague --- src/dialog-user/services/dialogData.spec.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/dialog-user/services/dialogData.spec.ts b/src/dialog-user/services/dialogData.spec.ts index f35dd9233c..e4f86f432d 100644 --- a/src/dialog-user/services/dialogData.spec.ts +++ b/src/dialog-user/services/dialogData.spec.ts @@ -1,5 +1,6 @@ import DialogData from './dialogData'; import * as angular from 'angular'; + const dialogField = { 'href': 'http://localhost:3001/api/service_templates/10000000000015/service_dialogs/10000000007060', 'id': 10000000007060, @@ -493,9 +494,12 @@ describe('DialogDataService test', () => { expect(output.date_1).toMatch(/^\d+-\d+-\d+$/); // YYYY-MM-DD }); - it('preserves local timezone', () => { + // this test requires the "local timezone" to be UTC+1 or more + // timezone-mock is not compatible with current karma it seems: + // ERROR [karma]: { inspect: [Function: inspect] } + xit('preserves local timezone', () => { let input = dialogData.data.values; - input.default_value = new Date('2019-10-15T01:23:45+05:00'); + input.default_value = new Date('2019-10-15T00:11:22+01:00'); let output = dialogData.outputConversion(input); From ea26ad56fe0754ba76b7a9e912b73a42199ad607 Mon Sep 17 00:00:00 2001 From: Martin Hradil Date: Tue, 22 Oct 2019 23:17:20 +0000 Subject: [PATCH 7/8] dialogUser - remove $scope.$apply() this was needed because both SUI and ui-classic wrapped their API response promises in a normal Promise, thus escaping the angular digest cycle now that they just use the promise returned by the angular api service, no $scope.$apply() is needed (and indeed triggers the in the middle of $digest cycle error (`$rootScope/inprog?p0=$digest`)) (the corresponding ui changes come in https://github.com/ManageIQ/manageiq-ui-classic/pull/6291 and https://github.com/ManageIQ/manageiq-ui-service/pull/1595) --- src/dialog-user/components/dialog-user/dialogUser.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/dialog-user/components/dialog-user/dialogUser.ts b/src/dialog-user/components/dialog-user/dialogUser.ts index 68e05c504d..4e56e288ac 100644 --- a/src/dialog-user/components/dialog-user/dialogUser.ts +++ b/src/dialog-user/components/dialog-user/dialogUser.ts @@ -27,10 +27,11 @@ export class DialogUserController extends DialogClass implements IDialogs { * @param {Object} DialogData factory. */ - /*@ngInject*/ - constructor(private DialogData: any,private $scope : ng.IScope) { + /* @ngInject */ + constructor(private DialogData, private $q) { super(); } + /** * Runs when component is initialized * @memberof DialogUserController @@ -188,13 +189,12 @@ export class DialogUserController extends DialogClass implements IDialogs { promiseList.push(this.refreshSingleField(field)); }); - Promise.all(promiseList).then((_data) => { + this.$q.all(promiseList).then((_data) => { this.refreshRequestCount -= promiseList.length; if (this.refreshRequestCount === 0) { this.areFieldsBeingRefreshed = false; } this.saveDialogData(); - this.$scope.$apply(); }); } @@ -228,7 +228,6 @@ export class DialogUserController extends DialogClass implements IDialogs { this.dialogFields[field].fieldBeingRefreshed = false; this.saveDialogData(); - this.$scope.$apply(); if (! _.isEmpty(this.fieldAssociations[field])) { this.updateTargetedFieldsFrom(field); From 4f6867cccf461e2d4802527feb3c96f6b8116f38 Mon Sep 17 00:00:00 2001 From: Martin Hradil Date: Thu, 31 Oct 2019 12:45:56 +0000 Subject: [PATCH 8/8] outputConversion - make sure dates are either a converted Date or null --- src/dialog-user/services/dialogData.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dialog-user/services/dialogData.ts b/src/dialog-user/services/dialogData.ts index 2dd3080e2e..257a31cbf3 100644 --- a/src/dialog-user/services/dialogData.ts +++ b/src/dialog-user/services/dialogData.ts @@ -268,11 +268,11 @@ export default class DialogDataService { case 'DialogFieldDateControl': // server expects 2019-10-20, anything longer gets cut // converting first to prevent timezone conversions - out[name] = value && dateString(value); + out[name] = _.isDate(value) ? dateString(value) : null; break; case 'DialogFieldDateTimeControl': // explicit conversion to ISO datetime - out[name] = value && value.toISOString(); + out[name] = _.isDate(value) ? value.toISOString() : null; break; }; });