Created attachment 1526895 [details] settings_number_issues.patch Description of problem: The Settings -> Workers screen is completely broken for a number of reasons. A common theme here is that if the "real" value can't be selected, then the first value in the dropdown is selected instead, which in all cases is 200 MB. 1. Many dropdowns do not show the correct values on initial load. 1a. Some dropdowns are not big enough. For example, MiqVimBrokerWorker default memory threshold is 1 GB, but the drop down doesn't have 1 GB, so 200 MB is selected instead. 1b. Some of the code is not accessing the values from the correct location. For example, the event_catcher has a :defaults section in the config, but the code is not honoring that and simply putting the value in the base :event_catcher section 1c. get_worker_setting occasionally returns the string form of a setting instead of resolving it to an integer. Then the option_for_select tries to select that, but since dropdown only has integer values, nothing is selected and the default of 200 MB is selected instead. 2. Values are stored into the settings incorrectly. 2a. The values are supposed to be written into the settings as a rails method style (e.g. "400.megabytes"), but instead integers are stored. 2b. Because integers are stored, some of the values when refreshing the page end up causing problem 1 above. For example, if you choose "1.1 GB" in the UI, this stores the value of 1181116006 into the database. However, the value from calling 1.1.gigabytes in Rails is 1181116006.4. Notice these are two different numbers. This happens for two reasons. 2b-1. The way the dropdown is built is with code like the following (1.gigabytes..1.5.gigabytes).step(100.megabytes) do This is not valid, because 100.megabytes != 0.1.gigabytes with the way Rails methods work. 100.megabytes # => 104857600 0.1.gigabytes # => 107374182.4 The more accurate way to build this drop down is something like the following. (1..1.5).step(0.1).map(&:gigabytes).each do 2b-2. Notice that 1.1.gigabytes is a float. Yes, I admit it's weird to have partial bytes, but that's the way it works. However, the code calls .to_i_with_method to resolve the "1.1.gigabytes" string instead of .to_f_with_method. This causes the UI to store a value that can't be mapped back to a rails method string. 3. If a customer sets a custom value through Advanced Settings, it's highly likely they will choose a value not in the dropdown, and subsequently the dropdown will show 200 MB instead. Version-Release number of selected component (if applicable): master, but this looks like it goes back some versions How reproducible: Always Additional info: Some of the above is my observations based on my trying to fix it. I've attached a patch where I started fixing this (apologies for all of the debug code), but I've been on this for a number of days now, and I can't get it to work, so I think this might be better solved by someone in the UI team. Alternately, we may choose to just completely remove this screen. I'm not sure customers use it, and I'm pretty sure CEE, when telling customer to change a value, go directly to Advanced Settings anyway.
Dennis, what are your thoughts on my last comment in the Additional Info part above?
Note for the developer...there is a method on worker models to get the config settings path, instead of building them up manually. For example, [1] pry(main)> MiqGenericWorker.config_settings_path => [:workers, :worker_base, :queue_worker_base, :generic_worker] Unfortunately there is no method to know when a `:defaults` key should be tacked on the end, but I don't see why we couldn't add another method to core...something like `MiqGenericWorker.config_settings_path_defaults`
Possible duplicate of 1656873
I think #1656873 is just part 1a of this BZ. That is, this is issue a compendium of numerous issues of which #1656873 is only a part. Dennis had asked me to describe the expectations for this form, so that it may be easier to develop and test against... - Dropdowns will show the value as configured. - If a value is not in the drop down it should be shown in the Dropdown anyway and selected, perhaps stating that it is a custom value (e.g. "Custom (1022030583)") (Note 1) - A user can make any number of changes to this page and hit Save. - When saving, only the changes should be stored in the database. (Note 2) - When saving, the values should be stored to the database in the Rails method format (e.g. "400.megabytes") if possible. If for some reason it is not possible, then the integer should be stored. (Note 3) - When the page reloads after pressing save, then all values should show the newly saved configuration. (Note 1) - This is technically "new" functionality because none of that presently happens but is a source of a lot of the issues. - This should thus handle custom modifications a user does to Advanced Settings directly. - This should thus also handle values that are greater than the values in the dropdown. - This should thus also prevent a dropdown with no selection from erroneously showing the 1st value in the list (Note 2) - Currently, everything gets stored to the database, even values that are not changed - this may be due to the fact that it "undoes" the Rails method string values and stored them as integers even if not actually changed (Note 3) - It should always be possible to store the Rails method format, because the dropdowns are created *from* Rails method format, but I added the "if possible" just in case. - Note that this has backend complications that need to be handled. For example "1.1.gigabytes" is a float value, and if the code chooses to convert it cannot lose that, otherwise it may not be able to reverse the operation to recreate the Rails method.
Mike, https://bugzilla.redhat.com/show_bug.cgi?id=1656873 is related however this is a more comprehensive request.
Hello, 1) It is possible to fix it in Ruby, but it will take some time to figure everything out. 2) or we can get data for this screen from API (which exists) and converted this form into data-driven-form format which should be more easy and more maintainable way. However, this solution can't be backported. So, we could remove this screen from older version (see 'Alternately, we may choose to just completely remove this screen (...)') and add it back in a next release. So, how should I continue?
https://github.com/ManageIQ/manageiq-ui-classic/pull/5528
New commit detected on ManageIQ/manageiq-ui-classic/master: https://github.com/ManageIQ/manageiq-ui-classic/commit/a75f649f54a0cd65a540ad7c425f8b68e77cf5a0 commit a75f649f54a0cd65a540ad7c425f8b68e77cf5a0 Author: Richard Vsiansky <r.vsia> AuthorDate: Tue May 7 07:32:36 2019 -0400 Commit: Richard Vsiansky <r.vsia> CommitDate: Tue May 7 07:32:36 2019 -0400 Convert Workers form to DDF https://bugzilla.redhat.com/show_bug.cgi?id=1672437 app/javascript/components/workers-form/helpers.jsx | 237 + app/javascript/components/workers-form/workers-form.jsx | 202 + app/javascript/components/workers-form/workers.schema.js | 232 + app/javascript/forms/mappers/formsFieldsMapper.jsx | 2 + app/javascript/packs/component-definitions-common.js | 2 + app/javascript/spec/workers-form/numeral-helpers.test.js | 93 + 6 files changed, 768 insertions(+)
Verified on 5.11.0.18.
Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory, and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://access.redhat.com/errata/RHBA-2019:4199