Remove legacy settings code #8965
Conversation
GilbertCherrie
force-pushed
the
remove_legacy_settings_code
branch
from
d74459b to
26d720f
Compare
|
Checked commit GilbertCherrie@26d720f with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint |
| @edit[:new][:display][:locale] = params[:display_locale] if params[:display_locale] | ||
| @edit[:new][:display][:compare] = params[:display][:compare] if !params[:display].nil? && !params[:display][:compare].nil? | ||
| @edit[:new][:display][:drift] = params[:display][:drift] if !params[:display].nil? && !params[:display][:drift].nil? | ||
| when "ui_3" # Visual Settings tab |
There was a problem hiding this comment.
I believe ui_3 is still present, right? I think the comment here is actually wrong. Same with the comment for ui_4 below.
There was a problem hiding this comment.
Yeah ui_3 is still present. The only place I saw this function be called is on this line:
get_form_vars if @tabform != "ui_3"
Not sure if I understood this right or not but will get_form_vars only be called if @tabform is not equal to ui_3? This would mean that the ui_3 block in this function is never entered. Also, the code in these blocks doesn't seem related to the forms on the ui_3 and ui_4 tabs. The code seems to be referring to a [:display][:compare] value which comes from the ui_1 tab. Not sure if this was just a copy paste error or not.
There was a problem hiding this comment.
oh ok - I thought get_form_vars would have been called elsewhere, but if not, then this is fine.
|
Can you verify the My Settings pages still work with different users? This is the exact page I was on with my recent changes to fix non-admin users. |
There was a problem hiding this comment.
@Fryguy I think I'm running into an issue regarding user permissions however this is happening in master as well so it's unrelated to this pr. Basically, I'm getting an auth error when clicking on time profiles even though it looks like I have the permission (Main My Settings Page still works).
Also @GilbertCherrie, @Fryguy do either of you know what the Default Views permission is for?
|
@DavidResende0 Fixed that issue in this pr: #8969. Also I believe Default Views is for a deprecated feature and removed in this pr: ManageIQ/manageiq#22797 |
Remove legacy settings code.
Code related to theme was removed as this is related to a feature that no longer exists.
Code related to time profiles was removed as this is code is no longer needed after the time profile form was converted to React in this PR: #7761.
Code related to view_selected and configuration_view helper files was removed as this code is no longer needed after the visual settings form was converted to react in this PR: #7495.