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

Show acknowledgement when saving replication settings #4714

Conversation

yrudman
Copy link
Contributor

@yrudman yrudman commented Sep 27, 2018

This PR is follow-up to #4628

Issue: When queuing saving replication settings there is no confirmation after user clicking on Save button and it is not clear if task queued or not.

After:
screen shot 2018-09-27 at 3 50 48 pm

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

@miq-bot add-label bug, hammer/yes

…t of operation (success of error message) would not be shown as flash message and usr would need to to go to MyTask screen to see if operation succesfull or failed.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1633653
@yrudman
Copy link
Contributor Author

yrudman commented Sep 28, 2018

\cc @gtanzillo
This UX change (to show acknowledgment immediately and pointing user to MyTask screen to check result of operation) was discussed with @bascar and approved

Copy link
Member

@gtanzillo gtanzillo left a comment

Choose a reason for hiding this comment

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

LGTM 👍

add_flash(_("Error during replication configuration save: %{message}") % {:message => task.message }, :error)
end
MiqTask.generic_action_with_callback(task_opts, queue_opts)
add_flash(_("Replication configuration save initiated. Check status of task '#{task_opts[:action]}' on MyTasks screen"))
Copy link
Contributor

Choose a reason for hiding this comment

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

#{task_opts[:action]} inside _() will not work. You have to use _("один %{two} три") % {:two => два} here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you @mzazrivec for review and nice explanation :)

@mzazrivec mzazrivec self-assigned this Oct 1, 2018
@miq-bot
Copy link
Member

miq-bot commented Oct 1, 2018

Checked commits yrudman/manageiq-ui-classic@0330e31~...c04af58 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. 🍰

@mzazrivec mzazrivec added this to the Sprint 96 Ending Oct 8, 2018 milestone Oct 2, 2018
@mzazrivec mzazrivec merged commit 883df6d into ManageIQ:master Oct 2, 2018
@yrudman yrudman deleted the show-confirmation-when-replication-setup-queued branch October 2, 2018 13:38
simaishi pushed a commit that referenced this pull request Oct 2, 2018
…ion-setup-queued

Show acknowledgement when saving replication settings

(cherry picked from commit 883df6d)

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

simaishi commented Oct 2, 2018

Hammer backport details:

$ git log -1
commit 031b2f93d93a9d877fb9404a44c4bb9046d36ca6
Author: Milan Zázrivec <mzazrivec@redhat.com>
Date:   Tue Oct 2 15:22:09 2018 +0200

    Merge pull request #4714 from yrudman/show-confirmation-when-replication-setup-queued
    
    Show acknowledgement when saving replication settings
    
    (cherry picked from commit 883df6dd5c7b79281e99bbb818e17cc70977e363)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1633653

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