Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HAMMER] DialogUser - use DialogData.outputConversion before submit and refresh #1609

Merged
merged 9 commits into from Nov 18, 2019
Merged

[HAMMER] DialogUser - use DialogData.outputConversion before submit and refresh #1609

merged 9 commits into from Nov 18, 2019

Conversation

himdel
Copy link
Contributor

@himdel himdel commented Nov 14, 2019

Hammer version of #1595
With most of #1607 (#1591)
And array-includes shim (originally added in #1566)
And package.json cleanup originally added in #1576


Depends on ManageIQ/ui-components#422 (via #1608, merged)

Right now, when submitting a service dialog,
we simply take the dialogData object, convert to JSON and send to the API.

To allow for output conversions, adding a DialogData.outputConversion method,
which should return the dialogData in a suitable format.

(Right now, all this really means is that it converts Dates to strings before JSON stringification.)

And the same needs to happen in refreshField for dynamic fields.

https://bugzilla.redhat.com/show_bug.cgi?id=1768457

@miq-bot add_label bug

right now, when submitting a service dialog,
we simply take the dialogData object, convert to JSON and send to the API.

To allow for output conversions, adding a DialogData.outputConversion method,
which should return the dialogData in a suitable format.

(Right now, all this really means is that it converts Dates to strings before JSON stringification.)

https://bugzilla.redhat.com/show_bug.cgi?id=1744413
(cherry picked from commit 247665a)
also, we can get rid of the API angular promise to Promise conversion
with the $scope.$apply() removal in dialogUser.ts in ui-components

https://bugzilla.redhat.com/show_bug.cgi?id=1744413
(cherry picked from commit 1ea47e6)
(cherry picked from commit eb98695)
those methods are used by ui-components code, needed in test env too
no .ts or .tsx files in SUI,
the versions of typescript are becoming ancient,
removing.

(manual cherry-pick of 6c82578)

(cherry picked from commit 999ec97)
use eslint-loader instead of standard-loader,
move config to .eslintrc.js and .eslintignore,
use eslint in vet,
remove Standard link from README

(manual cherry-pick of 02ca0e4 - package.json had the tooling still in dependencies, instead of devDependencies)

(cherry picked from commit 959ee94)

and updated yarn.lock
@miq-bot
Copy link
Member

miq-bot commented Nov 14, 2019

Checked commits https://github.com/himdel/manageiq-ui-service/compare/2521dce78e058d5e98e8b9bbaf13dd59f4b1338f~...8be31a755f35dc9532e4c882d39f3082b1d9e59d with ruby 2.5.5, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 🍪

@simaishi simaishi self-assigned this Nov 14, 2019
@simaishi simaishi merged commit 03c29e6 into ManageIQ:hammer Nov 18, 2019
@simaishi simaishi added this to the Sprint 125 Ending Nov 25, 2019 milestone Nov 18, 2019
@himdel himdel deleted the hammer-bz1744413 branch November 18, 2019 14:38
@simaishi
Copy link
Contributor

simaishi commented Nov 18, 2019

@himdel 5.10.z build is now failing: /bin/sh: webpack: command not found

I see webpack moved from dependencies to devDependencies in this PR. The hammer/5.10.z build runs with --production, so I guess webpack is no longer being fetched.

So the question is... should webpack (and possibly other packages) stay in dependencies or do we need to switch to running without --production?

@himdel
Copy link
Contributor Author

himdel commented Nov 19, 2019

Aaah, sorry about this, you should switch to running without --production.

(The difference is that ManageIQ/manageiq-appliance-build#342 removed --production from ivanchuk+ but never made it to hammer)

(yarn available-languages is available in hammer, so, maybe we could just backport that PR.)

@simaishi
Copy link
Contributor

@himdel thanks, I'll backport the appliance-build PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants