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

Remove method "name" from physical_server #41

Merged
merged 1 commit into from
Apr 11, 2017
Merged

Remove method "name" from physical_server #41

merged 1 commit into from
Apr 11, 2017

Conversation

walteraa
Copy link
Member

@walteraa walteraa commented Apr 7, 2017

The method "name" in physical server class is causing inconsistency when we are trying show a Physical server's name in the UI and in the REST API as well.

The method was removed to solve this issue.

The method named "name" was causing inconsistency when A physical server is showing in UI and in RES API.
@miq-bot
Copy link
Member

miq-bot commented Apr 7, 2017

Checked commit https://github.com/lenovo/manageiq-providers-lenovo/commit/ba6a699b1d6904d83a9259e167d6354802d84061 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks good. 🍪

@juliancheal
Copy link
Member

Hmm, I thought name was needed in the model for ManageIQ. Maybe @Fryguy knows why, I'm trying to find out where.

This is another reason why we need to add more tests :)

@rodneyhbrown7
Copy link
Contributor

walter is there a reason we can't set the name method to return the information that we want it to return?

@Fryguy
Copy link
Member

Fryguy commented Apr 11, 2017

I don't know why you would need a name described like this.

@walteraa
Copy link
Member Author

walteraa commented Apr 11, 2017

@juliancheal @rodneyhbrown7 @Fryguy The problem is that when we try to use physical_server_object.name if there is a method named name in ManageIQ::Providers::Lenovo::PhysicalInfraManager::PhysicalServer, this method will be called instead of the property name which reffering to the column name in the physical_servers table and is where the real Physical Server name value is saved, then will always return the content defined in the method, wich in this case is the string "physical_server"

@juliancheal
Copy link
Member

@Fryguy ah ok. Must have been a mistake on my part

@juliancheal
Copy link
Member

@miq-bot add_label fine/yes

@juliancheal juliancheal merged commit 8553890 into ManageIQ:master Apr 11, 2017
simaishi pushed a commit that referenced this pull request Apr 11, 2017
Remove method "name" from physical_server
(cherry picked from commit 8553890)
@simaishi
Copy link
Contributor

Fine backport details:

$ git log -1
commit 1fe3cb8f7659d6527d6b14c433414b5a50710b72
Author: Julian Cheal <[email protected]>
Date:   Tue Apr 11 16:00:10 2017 +0100

    Merge pull request #41 from lenovo/fix_physical_server_name
    
    Remove method "name" from physical_server
    (cherry picked from commit 85538905bdbf6bc3d7c18165dfce18acf79704e9)

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.

6 participants