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

dialog-user: Move tagging fix from #412 to setDefaultValue #414

Merged
merged 1 commit into from
Sep 4, 2019
Merged

dialog-user: Move tagging fix from #412 to setDefaultValue #414

merged 1 commit into from
Sep 4, 2019

Conversation

himdel
Copy link
Contributor

@himdel himdel commented Sep 4, 2019

(follow-up to #412)

The dialogField component should not be changing default_value,
the dialogData service is the place where default_value gets set.

Otherwise, validations from dialog-user and from dialog-field will work with different values, which can cause invalid validations or validation loops.

(docs - https://github.com/ManageIQ/manageiq-ui-classic/wiki/Dialogs#dialog-user-tidbits)

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

Cc @romanblanco @h-kataria

The dialogField component should not be changing default_value,
the dialogData service is the place where default_value gets set.

https://bugzilla.redhat.com/show_bug.cgi?id=1729379
@miq-bot
Copy link
Member

miq-bot commented Sep 4, 2019

Checked commit https://github.com/himdel/ui-components/commit/403703b65ff20b3225ba5e35ab06f747718cbfc6 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 🍰

Copy link
Member

@romanblanco romanblanco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍, thank you for the correction @himdel

@h-kataria h-kataria self-assigned this Sep 4, 2019
@h-kataria h-kataria added this to the Sprint 120 Ending Sep 16, 2019 milestone Sep 4, 2019
@h-kataria h-kataria merged commit b224a50 into ManageIQ:master Sep 4, 2019
simaishi pushed a commit that referenced this pull request Sep 4, 2019
dialog-user: Move tagging fix from #412 to setDefaultValue
(cherry picked from commit b224a50)

https://bugzilla.redhat.com/show_bug.cgi?id=1749061
@simaishi
Copy link

simaishi commented Sep 4, 2019

Hammer backport details:

$ git log -1
commit d934145909d7973395c70b568980a5200b632b37
Author: Harpreet Kataria <hkataria@redhat.com>
Date:   Wed Sep 4 15:00:12 2019 -0400

    Merge pull request #414 from himdel/fix412
    
    dialog-user: Move tagging fix from #412 to setDefaultValue
    (cherry picked from commit b224a5079f8e86de77e8ebc7c5cdace16b993355)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1749061

@romanblanco
Copy link
Member

@himdel realizing now, that we don't allow setting a default value for Tag Control in Dialog Editor, the test (defaultValue === undefined) seems to be unnecessary to me, and I feel like we can just set the defaul_value on 0 every time.

@simaishi
Copy link

simaishi commented Nov 1, 2019

Ivanchuk backport details:

$ git log -1
commit 32c18f92db1a30f15f33e02aaf50d11e7f984541
Author: Harpreet Kataria <hkataria@redhat.com>
Date:   Wed Sep 4 15:00:12 2019 -0400

    Merge pull request #414 from himdel/fix412
    
    dialog-user: Move tagging fix from #412 to setDefaultValue
    (cherry picked from commit b224a5079f8e86de77e8ebc7c5cdace16b993355)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1767836

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

Successfully merging this pull request may close these issues.

5 participants