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

Change error message flash of existing label to more informative #1123

Merged
merged 1 commit into from Apr 24, 2017
Merged

Change error message flash of existing label to more informative #1123

merged 1 commit into from Apr 24, 2017

Conversation

zgalor
Copy link
Contributor

@zgalor zgalor commented Apr 23, 2017 •
edited

Added Label and "" for clarity

fix_flash_map_tags3

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

@miq-bot add-label bug

@serenamarie125 @cben @h-kataria PTAL

@miq-bot miq-bot added the bug label Apr 23, 2017
@cben
Copy link
Contributor

cben commented Apr 23, 2017

LGTM.

h-kataria
h-kataria reviewed Apr 23, 2017
View reviewed changes
app/controllers/ops_controller/settings/label_tag_mapping.rb Outdated
@@ -153,7 +153,7 @@ def label_tag_mapping_add(entity, label_name, cat_description)

# UI currently can't allow 2 mappings for same (entity, label).
if Classification.find_by_name(cat_name)
add_flash(_("Mapping for %{entity}, %{label} already exists") %
add_flash(_("Mapping for %{entity}, Label %{label} already exists") %
Copy link
Contributor

Choose a reason for hiding this comment

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

@zgalor would it make sense to have single quotes around %{label} for clarity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@h-kataria I added double quotes to be consistent with
https://github.com/ManageIQ/manageiq-ui-classic/blob/master/app/controllers/ops_controller/settings/label_tag_mapping.rb#L175
and
https://github.com/ManageIQ/manageiq-ui-classic/blob/master/app/controllers/ops_controller/settings/label_tag_mapping.rb#L193

@h-kataria
Copy link
Contributor

@zgalor can you add a link to BZ to commit message if there is one.

@zgalor
Copy link
Contributor Author

zgalor commented Apr 24, 2017 •
edited

@h-kataria thanks,
I updated the description with the BZ

@zgalor zgalor force-pushed the better-error-message-map-tags branch from 2cb1c44 to 068b8fa Compare April 24, 2017 08:07
@miq-bot
Copy link
Member

miq-bot commented Apr 24, 2017

Checked commit zgalor@068b8fa with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks good. ⭐

@mzazrivec mzazrivec self-assigned this Apr 24, 2017
@mzazrivec mzazrivec added this to the Sprint 59 Ending Apr 24, 2017 milestone Apr 24, 2017
@mzazrivec mzazrivec merged commit 03689c2 into ManageIQ:master Apr 24, 2017
simaishi pushed a commit that referenced this pull request Apr 24, 2017
@mzazrivec @simaishi
Merge pull request #1123 from zgalor/better-error-message-map-tags
3737d4b 
Change error message flash of existing label to more informative
(cherry picked from commit 03689c2)
@simaishi
Copy link
Contributor

Fine backport details:

$ git log -1
commit 3737d4b1ae783c192ba51018f8704126b2152277
Author: Milan Zázrivec <mzazrivec@redhat.com>
Date:   Mon Apr 24 10:47:27 2017 +0200

    Merge pull request #1123 from zgalor/better-error-message-map-tags
    
    Change error message flash of existing label to more informative
    (cherry picked from commit 03689c2f213fa0a1d9d74deaefa31b268046f6e9)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@h-kataria h-kataria h-kataria left review comments

Assignees

@mzazrivec mzazrivec

Labels
bug fine/backported
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@zgalor @cben @h-kataria @miq-bot @simaishi @mzazrivec