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

Use copy_params_if_present #6105

Closed
21 tasks done
hstastna opened this issue Aug 27, 2019 · 5 comments
Closed
21 tasks done

Use copy_params_if_present #6105

hstastna opened this issue Aug 27, 2019 · 5 comments

Comments

@hstastna
Copy link

hstastna commented Aug 27, 2019

We can use new copy_params_if_present method in many places in the code and to improve functionality ("" vs nil issues and bad response of Add/Save buttons), to simplify the code.

I used grep -rI '= params\[:.*\] if params' to find the appropriate places.

hstastna pushed a commit to hstastna/manageiq-ui-classic that referenced this issue Sep 2, 2019
hstastna pushed a commit to hstastna/manageiq-ui-classic that referenced this issue Sep 2, 2019
hstastna pushed a commit to hstastna/manageiq-ui-classic that referenced this issue Sep 4, 2019
Issue: ManageIQ#6105

Use the new method also in miq_request_methods.rb
hstastna pushed a commit to hstastna/manageiq-ui-classic that referenced this issue Sep 5, 2019
hstastna pushed a commit to hstastna/manageiq-ui-classic that referenced this issue Sep 5, 2019
hstastna pushed a commit to hstastna/manageiq-ui-classic that referenced this issue Sep 5, 2019
@hstastna
Copy link
Author

hstastna commented Sep 6, 2019

I'm closing this issue as all the related PRs got merged.

@hstastna hstastna closed this as completed Sep 6, 2019
@romanblanco
Copy link
Member

@hstastna

app/controllers/configuration_controller.rb:      @edit[:new][:display][:compare] = params[:display][:compare] if !params[:display].nil? && !params[:display][:compare].nil?
app/controllers/configuration_controller.rb:      @edit[:new][:display][:drift] = params[:display][:drift] if !params[:display].nil? && !params[:display][:drift].nil?
app/controllers/configuration_controller.rb:      @edit[:new][:display][:compare] = params[:display][:compare] if !params[:display].nil? && !params[:display][:compare].nil?
app/controllers/configuration_controller.rb:      @edit[:new][:display][:drift] = params[:display][:drift] if !params[:display].nil? && !params[:display][:drift].nil?
app/controllers/configuration_controller.rb:      @edit[:new][:display][:compare] = params[:display][:compare] if !params[:display].nil? && !params[:display][:compare].nil?
app/controllers/configuration_controller.rb:      @edit[:new][:display][:drift] = params[:display][:drift] if !params[:display].nil? && !params[:display][:drift].nil?
app/controllers/miq_ae_class_controller.rb:    @edit[:new][:namespace] = params[:namespace] if params[:namespace]
app/controllers/miq_ae_class_controller.rb:      session[:field_data][:name] = @edit[:new_field][:name] = params[:field_name] if params[:field_name]
app/controllers/miq_ae_class_controller.rb:      session[:field_data][:datatype] = @edit[:new_field][:datatype] = params[:field_datatype] if params[:field_datatype]
app/controllers/miq_ae_class_controller.rb:      session[:field_data][:default_value] = @edit[:new_field][:default_value] = params[:field_default_value] if params[:field_default_value]
app/controllers/miq_ae_class_controller.rb:      session[:field_data][:default_value] = @edit[:new_field][:default_value] = params[:field_password_value] if params[:field_password_value]
app/controllers/miq_ae_class_controller.rb:      session[:field_data][:name] = @edit[:new_field][:name] = params[:cls_field_name] if params[:cls_field_name]
app/controllers/miq_ae_class_controller.rb:      session[:field_data][:datatype] = @edit[:new_field][:datatype] = params[:cls_field_datatype] if params[:cls_field_datatype]
app/controllers/miq_ae_class_controller.rb:      session[:field_data][:default_value] = @edit[:new_field][:default_value] = params[:cls_field_default_value] if params[:cls_field_default_valu»
app/controllers/miq_ae_class_controller.rb:      session[:field_data][:default_value] = @edit[:new_field][:default_value] = params[:cls_field_password_value] if params[:cls_field_password_va»
app/controllers/catalog_controller.rb:        @edit[:new][:st_prov_type] = params[:st_prov_type] if params[:st_prov_type]
app/controllers/catalog_controller.rb:    @edit[:new][:manager_id] = params[:manager_id] if params[:manager_id]
app/controllers/catalog_controller.rb:    @edit[:new][:template_id] = params[:template_id] if params[:template_id]
app/controllers/chargeback_controller.rb:    @edit[:new][:description] = params[:description] if params[:description]
app/controllers/chargeback_controller.rb:    @edit[:new][:cbshow_typ] = params[:cbshow_typ] if params[:cbshow_typ]
app/controllers/miq_request_controller.rb:    @edit[:reason] = params[:reason] if params[:reason]
app/controllers/pxe_controller/iso_datastores.rb:    @edit[:new][:ems_id] = params[:ems_id] if params[:ems_id]
app/controllers/mixins/playbook_options.rb:      @edit[:new][:inventory_type] = params[:inventory_type] if params[:inventory_type]
app/controllers/mixins/playbook_options.rb:      @edit[:new][:hosts] = params[:hosts] if params[:hosts]
app/controllers/ops_controller/db.rb:    @sb[:zone_name] = params[:zone_name] if params[:zone_name]
app/controllers/ops_controller/settings.rb:    @ldap_info[:ldaphost] = params[:user_proxies][:ldaphost] if params[:user_proxies] && params[:user_proxies][:ldaphost]
app/controllers/ops_controller/settings.rb:    @ldap_info[:ldapport] = params[:user_proxies][:ldapport] if params[:user_proxies] && params[:user_proxies][:ldapport]
app/controllers/ops_controller/settings.rb:    @ldap_info[:basedn] = params[:user_proxies][:basedn] if params[:user_proxies] && params[:user_proxies][:basedn]
app/controllers/ops_controller/settings.rb:    @ldap_info[:bind_dn] = params[:user_proxies][:bind_dn] if params[:user_proxies] && params[:user_proxies][:bind_dn]
app/controllers/ops_controller/settings.rb:    @ldap_info[:bind_pwd] = params[:user_proxies][:bind_pwd] if params[:user_proxies] && params[:user_proxies][:bind_pwd]
app/controllers/ops_controller/ops_rbac.rb:    @edit[:new][:user]     = params[:user]     if params[:user]
app/controllers/ops_controller/ops_rbac.rb:    @edit[:new][:user_id]  = params[:user_id]  if params[:user_id]
app/controllers/ops_controller/ops_rbac.rb:    @edit[:new][:name] = params[:name] if params[:name]
app/controllers/report_controller/schedules.rb:    @edit[:new][:email][:from] = params[:from] if params.key?(:from)
app/controllers/report_controller/schedules.rb:    @edit[:email] = params[:email] if params.key?(:email)
app/controllers/miq_policy_controller/alerts.rb:    @edit[:new][:email][:from] = params[:from] if params.key?(:from)
app/controllers/miq_policy_controller/alerts.rb:    @edit[:email] = params[:email] if params.key?(:email)
app/controllers/mixins/ems_common/angular.rb:        @edit[:new][:tenant_mapping_enabled] = params[:tenant_mapping_enabled] if ems.class.supports_cloud_tenant_mapping?
app/controllers/ops_controller/settings/analysis_profiles.rb:    @edit[:new][:name]        = params[:name]        if params[:name]
app/controllers/ops_controller/settings/analysis_profiles.rb:    @edit[:new][:description] = params[:description] if params[:description]
app/controllers/ops_controller/settings/common.rb:      @sb[:form_vars][:session_timeout_mins] = params[:session_timeout_mins] if params[:session_timeout_mins]
app/controllers/ops_controller/settings/common.rb:      @sb[:form_vars][:session_timeout_hours] = params[:session_timeout_hours] if params[:session_timeout_hours]
app/controllers/ops_controller/settings/common.rb:      @sb[:form_vars][:agent_heartbeat_frequency_mins] = params[:agent_heartbeat_frequency_mins] if params[:agent_heartbeat_frequency_mins]
app/controllers/ops_controller/settings/common.rb:      @sb[:form_vars][:agent_heartbeat_frequency_secs] = params[:agent_heartbeat_frequency_secs] if params[:agent_heartbeat_frequency_secs]
app/controllers/ops_controller/settings/common.rb:      @sb[:form_vars][:agent_log_wraptime_days] = params[:agent_log_wraptime_days] if params[:agent_log_wraptime_days]
app/controllers/ops_controller/settings/common.rb:      @sb[:form_vars][:agent_log_wraptime_hours] = params[:agent_log_wraptime_hours] if params[:agent_log_wraptime_hours]
app/controllers/mixins/actions/vm_actions/rename.rb:          @edit[:new][:name] = params[:name] if params[:name]

some of these look like they could be converted to use copy_param(s)_if_present too.

@hstastna
Copy link
Author

hstastna commented Sep 6, 2019

@romanblanco I was not making changes in the places where we would need to copy just one param with copy_param_if_present and where "" vs nil values did not seem to be crucial. Also many of those conditions are little more complicated and we could break something if we changed them. It already happened to me with one of those.

@hstastna
Copy link
Author

hstastna commented Sep 6, 2019

Anyway, if you think I should reopen the issue and investigate more each of the cases above, just tell me ;)

@romanblanco
Copy link
Member

@hstastna which one of them you had the problems with? I feel like we can still get rid of the cases in app/controllers/ops_controller/settings/common.rb for example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants