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.

Sign up for GitHub

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

Jump to bottom

Show flash message when "Submit" button is pressed. #3198

Merged
merged 3 commits into from Jan 10, 2018
Merged

Show flash message when "Submit" button is pressed. #3198

merged 3 commits into from Jan 10, 2018

Conversation

h-kataria
Copy link
Contributor

Removed code to show flash message while options are being selected, should only show flash message for missing options after submit button is pressed.

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

@h-kataria h-kataria removed the wip label Jan 9, 2018
@h-kataria h-kataria changed the title [WIP] - Show flash message when "Submit" button is pressed. Show flash message when "Submit" button is pressed. Jan 9, 2018
@h-kataria
Copy link
Contributor Author

@lgalis can you please test/review.

@martinpovolny changed reverse_merge on both lines here to reverse_merge!, having reverse_merge was not setting any default options in @sb https://github.com/ManageIQ/manageiq-ui-classic/pull/3198/files#diff-2b25dd7aade4c0a9fe7816de49ce6a79L213 and https://github.com/ManageIQ/manageiq-ui-classic/pull/3198/files#diff-2b25dd7aade4c0a9fe7816de49ce6a79L218

@h-kataria h-kataria force-pushed the planning_options_flash_message_fix branch 2 times, most recently from 9e8101d to 9d621a1 Compare January 9, 2018 15:16
@h-kataria
Show flash message when "Submit" button is pressed.
5c6f1ee 
Removed code to show flash message while options are being selected, should only show flash message for missing options after submit button is pressed.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1531462
@h-kataria
Fixed to set all default options when screen loads or Reset is pressed
77efd28 
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1531462
@h-kataria h-kataria force-pushed the planning_options_flash_message_fix branch from 9d621a1 to 77efd28 Compare January 9, 2018 16:33
lgalis
lgalis approved these changes Jan 9, 2018
View reviewed changes
Copy link
Contributor

@lgalis lgalis left a comment •
edited

Choose a reason for hiding this comment

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

Looks good, tested in the UI.

I did find an unrelated issue - the VM is not cleared when the target type changes - we could make that change as part of this PR or a separate one.

@h-kataria
Copy link
Contributor Author

h-kataria commented Jan 9, 2018 •
edited

@lgalis addressed your comment, please re-review.

@lgalis
Copy link
Contributor

lgalis commented Jan 10, 2018 •
edited

@h-kataria looks good, the only small thing, also not related to your changes -the dropdown list for vms ( @sb[:vms] also needs to be cleared when changing the filter_type, except for 'all vms'

@h-kataria h-kataria force-pushed the planning_options_flash_message_fix branch from 4e86f2d to 148b4fa Compare January 10, 2018 14:44
@h-kataria
Copy link
Contributor Author

@lgalis addressed your latest comment.

@h-kataria h-kataria force-pushed the planning_options_flash_message_fix branch from 148b4fa to 4a37036 Compare January 10, 2018 18:25
@miq-bot
Copy link
Member

miq-bot commented Jan 10, 2018

Checked commits h-kataria/manageiq-ui-classic@5c6f1ee~...4a37036 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 👍

@lgalis
Copy link
Contributor

lgalis commented Jan 10, 2018

@h-kataria - looks good, tested in the UI

@dclarizio dclarizio merged commit 86854e0 into ManageIQ:master Jan 10, 2018
@dclarizio dclarizio added this to the Sprint 77 Ending Jan 15, 2018 milestone Jan 10, 2018
simaishi pushed a commit that referenced this pull request Jan 11, 2018
@simaishi
Merge pull request #3198 from h-kataria/planning_options_flash_messag…
6ad0477 
…e_fix

Show flash message when "Submit" button is pressed.
(cherry picked from commit 86854e0)

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

Gaprindashvili backport details:

$ git log -1
commit 6ad04772c9544b20feaf04330cc98d783631d02c
Author: Dan Clarizio <dclarizi@redhat.com>
Date:   Wed Jan 10 12:56:31 2018 -0800

    Merge pull request #3198 from h-kataria/planning_options_flash_message_fix
    
    Show flash message when "Submit" button is pressed.
    (cherry picked from commit 86854e0f296ee27c01aa524cbf3c6ea16d03319a)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1533514

@h-kataria h-kataria deleted the planning_options_flash_message_fix branch January 12, 2018 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lgalis lgalis lgalis approved these changes

@martinpovolny martinpovolny Awaiting requested review from martinpovolny

Assignees

@dclarizio dclarizio

Labels
blocker bug gaprindashvili/backported optimize
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@h-kataria @lgalis @miq-bot @simaishi @dclarizio