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

Display Generic Object Associations correctly #2420

Conversation

AparnaKarve
Copy link
Contributor

The prior implementation of displaying GO Associations assumed that the Association name would be based off a valid model. For e.g. services or vms.
This assumption should not be made with Generic Objects since the association name could pretty much be anything.

The screenshot below displays a nested list of GO associations of type 'cp' (short for Cloud Providers)
(I have purposely used a non-standard Association name to demonstrate that this fix can display a GO association with any name)
The nested display list url generated in this case would be -
http://localhost:3000/generic_object/show/10000000000003?display=cp

screen shot 2017-10-16 at 2 41 32 pm

screen shot 2017-10-16 at 2 41 43 pm

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

@miq-bot
Copy link
Member

miq-bot commented Oct 16, 2017

Checked commits AparnaKarve/manageiq-ui-classic@4bb33fb~...338fa01 with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0
4 files checked, 2 offenses detected

app/controllers/generic_object_controller.rb

spec/controllers/generic_object_controller_spec.rb

@martinpovolny
Copy link
Member

@AparnaKarve : is this still needed?

@AparnaKarve
Copy link
Contributor Author

@martinpovolny I will find out today

@AparnaKarve
Copy link
Contributor Author

AparnaKarve commented Nov 1, 2017

@martinpovolny I have created a new PR - #2596

As far as the GTL related code goes, the only change that had to be retained from this PR, was the dynamic creation of create_display_methods, which requires an association value to be passed, in order to render the List successfully.
Please take a look at #2596

options[:association] = HAS_ASSOCATION[params[:model]]
elsif options[:parent].kind_of?(GenericObject)
options.merge!(generate_options(params[:model]))
end
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why this is needed. All options that are given to the GTL component are passwed throught to the /report_data.

Meaning we should not need to add any new exceptions here.

Actually I plan to remove the remaning exceptions and generalize this path.

Why don't you pass all the necessary options when rendering the grid?

Copy link
Member

Choose a reason for hiding this comment

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

btw: even if we wanted to add th the exception here, I already wrote that generate_options is far too generic name in the context of the HUGE ApplicationController.

@AparnaKarve
Copy link
Contributor Author

@martinpovolny I'm closing this in favor of #2596

@AparnaKarve AparnaKarve closed this Nov 2, 2017
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.

None yet

4 participants