DialogUser - use DialogData.outputConversion before submit and refresh #422
Conversation
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
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
… 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
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
|
(For dynamic fields, I'm partly blocked by ManageIQ/manageiq-automation_engine#354 (comment)) EDIT: but ManageIQ/manageiq#19407 works :) |
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 ManageIQ/manageiq-ui-classic#6291 and ManageIQ/manageiq-ui-service#1595)
|
@miq-bot remove_label wip |
himdel
changed the title
miq-bot
changed the title
|
@eclarizio could you please..? :) (If you're going to test this, you also need ManageIQ/manageiq#19407 in core.) |
Merged. @eclarizio : can you review this one, pls? |
|
Cc @tinaafitz maybe? :) |
|
.. Or @d-m-u :) Sorry for spamming multiple people :) |
|
Hi @himdel @martinpovolny , I'm sorry for the delayed response, @eclarizio is on PTO. I'll see who else is available to review. |
There was a problem hiding this comment.
Sorry for the delayed review, as @tinaafitz mentioned I was out on PTO.
Looks good and makes sense. I wish we could get some form of timezone-mock included so we can have a test proving that it works for timezones UTC+.
Is there a cleaner way to write the out[name] = value && dateString(value)? At first glance, it looks like this is going to be setting either a true or false to out[name] but because javascript simply returns the result of the 2nd expression if the 1st is true, we get the desired output. It just looks very strange. Can we simply use _.isDate? _.isDate(value) ? dateString(value) : value.
What are your thoughts? Obviously the way I'm proposing is a bit more verbose and it does the same thing, so I'm going to approve anyway.
Yeah, I think the strings should now always go through the whole conversion process, Updating :) |
|
Checked commits https://github.com/himdel/ui-components/compare/47dd10607a4452585d2635c8c7aa2b5630642285~...4f6867cccf461e2d4802527feb3c96f6b8116f38 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0 |
DialogUser - use DialogData.outputConversion before submit and refresh (cherry picked from commit 4c6d61d) https://bugzilla.redhat.com/show_bug.cgi?id=1768456
|
Ivanchuk backport details: |
DialogUser - use DialogData.outputConversion before submit and refresh (cherry picked from commit 4c6d61d) https://bugzilla.redhat.com/show_bug.cgi?id=1768457
|
Hammer backport details: |
(This won't affect anything without the changes in ManageIQ/manageiq-ui-service#1595 and ManageIQ/manageiq-ui-classic#6291, merge this one first.)
Problem:
date fields need to be a Date instance, for the date/datetime input to work
there is no explicit conversion from Date to string before submitting to the API
that Date objects gets serialized by converting to a UTC datetime string,
which gets cut to 10 charecters on the backend,
and in UTC+N timezones, that leads to "2019-10-17 0:00 local" being submitted as "2019-10-16T(24-N):00:00Z" and cut to "2019-10-16" which is off by one.
Solution:
provide a
DialogDate#outputConversionfunction which will return dialog data in a format suitable for API submission,make the UIs use it (separate PRs)
Currently that function only converts date and datetime fields, and makes sure to return a shallow copy of the data.
(A small wiring hack (the dialogUser.ts change) was needed for the function to be able to access field definitions.)
The UI PRs make sure
outputConversionis used for both submitting the dialog and for refreshing individual fields.Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1744413