search

ManageIQ / manageiq-ui-classic

Classic UI of ManageIQ
Apache License 2.0
50 stars 357 forks source link

Deduplicate app/controllers/vm_common.rb by using MoreShowMixins #375

Open martinpovolny opened 7 years ago

martinpovolny commented 7 years ago

For some of the methods the code is the same.

I see a problem with 'show_timeline':

1) seems that the URL generated there for breadrumb is wrong -- it contains db that comes from get_rec_cls and that returns a class. 1) some of the show_timeline methods call find_by_id_filtered that should be probably called everywhere instead 1) there are a bunch of alianses (such as :image_timeline,, some are used from toolbars) 1) there's extra code to deal with @explorer being true

warning: if inside a module A you use:

included do
  include Mixins::MoreShowMixins
end

and then try to redefine a method from MixingsMoreShowMixins, you'll fail as the method from the included mixin will get called, not the one in module A.

When done, look at app/controllers/container_controller.rb it also has it's implementation of the methods from MoreShowMixins but slightly differently arranged.

JPrause commented 5 years ago

@martinpovolny is this still a valid issue? If yes, lease remove the stale label. If not can you close. If there's no update by next week, I'll be closing this issue.

JPrause commented 5 years ago

Closing issue. If you feel the issue needs to remain open, please either reopen or let me know and it will be reopened. @miq-bot close_issue

JPrause commented 5 years ago

@miq-bot remove_label stale