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 copy and paste button for automation simulation #9308

Merged

Conversation

liu-samuel
Copy link
Contributor

Old:
Screenshot 2024-11-11 at 3 09 17 PM
Screenshot 2024-11-11 at 3 09 08 PM

New:
Screenshot 2024-11-11 at 3 07 50 PM
Screenshot 2024-11-11 at 3 08 26 PM

@liu-samuel liu-samuel requested a review from a team as a code owner November 11, 2024 20:09
@@ -1,12 +1,2 @@
class ApplicationHelper::Toolbar::MiqAeToolsSimulateCenter < ApplicationHelper::Toolbar::Basic
Copy link
Member

Choose a reason for hiding this comment

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

If there's nothing left in this toolbar, is it possible to delete the toolbar?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried doing that, but the application was saying it expected the class definition for some reason. I was trying to find where that was happening but wasn't able to. Do you know where that might be?

Copy link
Member

@Fryguy Fryguy Nov 11, 2024

Choose a reason for hiding this comment

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

Looks like

class_name = 'ApplicationHelper::Toolbar::' + ActiveSupport::Inflector.camelize(tb_name.sub(/_tb$/, ''))

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 really understand how it gets there though - do you have a stack track when it fails when you delete it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's no stack trace in my terminal, just this error message shows up on my UI
Screenshot 2024-11-11 at 3 30 03 PM

Copy link
Member

Choose a reason for hiding this comment

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

you should see a stack trace in possibly the development.log or the evm.log

Copy link
Contributor Author

@liu-samuel liu-samuel Nov 11, 2024

Choose a reason for hiding this comment

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

Hmm i don't see anything that relates to the toolbar in the evm.log, just that it connects to the database

[----] I, [2024-11-11T15:55:34.662273#18468:5ec4]  INFO -- evm: MIQ(ManageIQ::Session.configure_session_store) Using session_store: ActionDispatch::Session::MemCacheStore
[----] I, [2024-11-11T15:55:34.820424#18468:5ec4]  INFO -- evm: MIQ(Vmdb::Initializer.init) Initializing Application: Program Name: bin/rails, PID: 18468, ENV['EVMSERVER']: 
[----] I, [2024-11-11T15:55:34.820474#18468:5ec4]  INFO -- evm: MIQ(Vmdb::Initializer.log_db_connectable) Successfully connected to the database.
[----] I, [2024-11-11T15:55:35.881939#18468:5ec4]  INFO -- evm: MIQ(Vmdb::Loggers.apply_config) Log level for azure has been changed to [WARN]
[----] I, [2024-11-11T15:55:35.882076#18468:5ec4]  INFO -- evm: MIQ(Vmdb::Loggers.apply_config) Log level for vim has been changed to [WARN]
[----] I, [2024-11-11T15:55:35.882171#18468:5ec4]  INFO -- evm: MIQ(Vmdb::Initializer.init) Initializing Application: Program Name: bin/rails, PID: 18468, ENV['EVMSERVER']: 
[----] I, [2024-11-11T15:55:35.882213#18468:5ec4]  INFO -- evm: MIQ(Vmdb::Initializer.log_db_connectable) Successfully connected to the database.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, then let's leave it for now

N_('Copy'),
:url => "resolve",
:url_parms => "?button=copy",
:klass => ApplicationHelper::Button::AeCopySimulate),
Copy link
Member

Choose a reason for hiding this comment

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

Do you see any other usages of this class? If not, can we delete it?

Copy link
Member

@Fryguy Fryguy Nov 11, 2024

Choose a reason for hiding this comment

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

I searched and I think you can delete all references to AeCopySimulate: https://github.com/search?q=org%3AManageIQ%20AeCopySimulate&type=code

- copied_target_class = session[:resolve_object][:new][:target_class]
- current_target_class = @edit[:new][:target_class]
- if copied_target_class == current_target_class
= link_to({:action => "resolve", :button => "paste"},
Copy link
Member

Choose a reason for hiding this comment

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

There's probably also a "resolve" action in the simulation controller that can be removed as well. Might involve some more searching though.

Copy link
Contributor Author

@liu-samuel liu-samuel Nov 12, 2024

Choose a reason for hiding this comment

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

def resolve
    custom_button_redirect = params[:button] == 'simulate' || params[:simulate] == 'simulate'
    assert_privileges(custom_button_redirect ? 'ab_button_simulate' : 'miq_ae_class_simulation')
    @explorer = true
    @breadcrumbs = []
    drop_breadcrumb(:name => _("Resolve"), :url => "/miq_ae_tools/resolve")
    @lastaction = "resolve"
    @right_cell_text = _("Simulation")
    case params[:button]
    when "throw", "retry" then resolve_button_throw
    when "copy"     then resolve_button_copy
    when "paste"    then resolve_button_paste
    when "simulate" then resolve_button_simulate
    else                 resolve_button_reset_or_none
    end
  end

I think this is what the resolve is referring to, so I should be able to remove the copy and paste cases but not the others since they deal with the actual simulation

@liu-samuel liu-samuel force-pushed the remove-automate-copy-paste branch from 079d068 to 11e0659 Compare November 12, 2024 16:49
@Fryguy
Copy link
Member

Fryguy commented Nov 12, 2024

LGTM - @GilbertCherrie Please review.

@GilbertCherrie GilbertCherrie merged commit f2c46c5 into ManageIQ:master Nov 13, 2024
14 of 15 checks passed
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.

3 participants