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

Fix "Storage Pods" string in GTL settings #8845

Merged

Conversation

mzazrivec
Copy link
Contributor

@mzazrivec mzazrivec commented May 20, 2016

storage_pod is incorrect table name, correct should be: container_group

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

storage_pod is incorrect table name, correct should be: container_group

https://bugzilla.redhat.com/show_bug.cgi?id=1337558
@simon3z
Copy link
Contributor

simon3z commented May 20, 2016

@lgalis can you review this? If I am not mistaken it was introduced in #8004.

Also, @mzazrivec @lgalis I see that storage_pod was used in many more parts of the code in #8004, it's worth reviewing if we need other changes (if you haven't done it already).

Can anyone please add a screenshot too? Thanks.

@mzazrivec
Copy link
Contributor Author

@simon3z This is the only occurrence in our code where we call ui_lookup() with
the argument :tables => storage_pod (or :tables => storage_pods).

This is incorrect, since:

  1. there is no such table in our schema and the record storage_pod is not present in
    our catalog of tables (and pretty, human readable strings). What this means that
    this part would not be translated into other locales.
  2. the part of ui, where we're using this construct is about settings preferred GTL type for
    a page showing container groups
  3. we do have a record for container_groups in our catalog and is already translated.

This PR is about fixing just this.

@simon3z
Copy link
Contributor

simon3z commented May 20, 2016

@simon3z This is the only occurrence in our code where we call ui_lookup() with
the argument :tables => storage_pod (or :tables => storage_pods).

OK 👍

@chessbyte I think this should be assigned to @dclarizio

@miq-bot assign @dclarizio

@miq-bot miq-bot assigned dclarizio and unassigned simon3z May 20, 2016
@miq-bot
Copy link
Member

miq-bot commented May 20, 2016

Checked commit mzazrivec@9a76852 with ruby 2.2.3, rubocop 0.37.2, and haml-lint 0.16.1
1 file checked, 0 offenses detected
Everything looks good. 🍰

@dclarizio dclarizio merged commit 64f6e5a into ManageIQ:master May 20, 2016
@dclarizio dclarizio added this to the Sprint 41 Ending May 30, 2016 milestone May 20, 2016
chessbyte pushed a commit that referenced this pull request May 20, 2016
…l_settings

Fix "Storage Pods" string in GTL settings
(cherry picked from commit 64f6e5a)
@mzazrivec mzazrivec deleted the fix_storage_pods_string_in_gtl_settings branch May 24, 2016 11:45
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