Unify endpoint data for report data. #2195
Conversation
|
@miq-bot add_label enhancement, GTLs |
|
@miq-bot assign @martinpovolny |
|
@karelhala Cannot apply the following label because they are not recognized: gtls |
|
ping @Ladas : the plan is to unify the naming a bit to make the situation less confusing before we start cleaning up the other problem such as:
|
| end | ||
| end | ||
|
|
||
| options[:parent] = options[:parent] || @parent | ||
| options[:association] = HAS_ASSOCATION[params[:model]] if HAS_ASSOCATION.include?(params[:model]) | ||
| options[:association] = HAS_ASSOCATION[params[:model_name]] if HAS_ASSOCATION.include?(params[:model_name]) |
There was a problem hiding this comment.
I am a bit confused by mixing :association and :model name, those were 2 different things before?
E.g. model_name=ExtManagementSystem and association=:vms with parent_id=1, I would expect we do
ExtManagementSystem.find(1).vms, which list Vms under that ems where :vms model_name can be e.g Vm
We do that call, but we call is as identify_record(parent_id, controller_to_model).send(HAS_ASSOCATION[params[:model_name]]) ?
I think before we were just using :association directly, while having allowed list of associations for each controller. Now we kind of have allowed list of associations per :model_name globally, in HAS_ASSOCATION?
There was a problem hiding this comment.
e.g before if you had controller_name/1/?diplay=all_vms
it could map to this
https://github.com/Ladas/manageiq-ui-classic/blob/36c5a957b6fcad6a89d81bb8238c70ef04a55d1e/app/controllers/mixins/generic_show_mixin.rb#L151
or going to generic, which does https://github.com/Ladas/manageiq-ui-classic/blob/36c5a957b6fcad6a89d81bb8238c70ef04a55d1e/app/controllers/mixins/generic_show_mixin.rb#L139
while expecting to have list of allowed sub-tables per controller like this https://github.com/Ladas/manageiq-ui-classic/blob/35dbd795ba08c3778866245dfce91b4b5d88a76d/app/controllers/network_port_controller.rb#L18
of course the explorer views were more complex and weren't ported to this format :-(
There was a problem hiding this comment.
so to sum what I try to say :-)
- we had allowed list of controller subtables
- we were catching specific sublists in generic_show_mixins, because they could not be generic for some reason (we are not passing all args to UI and back to report data now)
- naming is confusing (but it was confusing before) since get_view takes model as a first arg which is taken from @display , so maybe we need to take this up to @lpichler :-)
There was a problem hiding this comment.
of course the explorer views were more complex and weren't ported to this format :-(

Yes, the explorer views where/are even more mess. I already limited the number of different patterns for nested lists in explorer view but it's still pretty messy. It combines the single and list views deeper in the code and it will need more work to cleanup.
Naming is and was confusing. The methods on the get_view* code path have different names for the same thing on several places and it's not easy to fix.
Let's do small steps that improve the situation. We don't need to fix too much in a single PR.
|
ping @lpichler |
karelhala
force-pushed
the
reportdataRename
branch
2 times, most recently
from
943cf1d to
e271b0e
Compare
model_name for name of model (either current model to be displayed, or association if parent ID is set) parent_id ID of parent so we know from where to fetch assocation. Align tests based on renamed values
karelhala
force-pushed
the
reportdataRename
branch
from
e271b0e to
b1c918e
Compare
|
Checked commit karelhala@b1c918e with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
Merge after ManageIQ/ui-components#162
model_namefor name of model (either current model to be displayed, or association if parent ID is set)parent_idID of parent so we know from where to fetch assocation.