Inconsistent autogenerated values for user fields with options. about paid-memberships-pro HOT 2 OPEN

Tricky.

There are likely other edge cases when labels are only sometimes give for the options. I wonder what happens if you e.g. have options like:

:Select
Yes
No:No

We have code here that checks if an array is non-associative (i.e. the keys for the array elements are 0, 1, 2, 3...). If so, we assume that the keys aren't important indexes and so we can update the array so all keys match the values... which is better for searching or later updating the list of options.

Because the Select value is set to "" here, the array of options is non-associative. So the code expects the keys to be set for each element in the array.

Here is the code that processes the list of options: https://github.com/strangerstudios/paid-memberships-pro/blob/dev/includes/fields.php#L1580-L1595

The work around here is to be explicit and use Yes:Yes, No:No for the option lines.

Adding an empty Select to the start of an array is a common feature. Do we want to support this format? Or address this in another way (some fields tools have a checkbox to include a "Select One" option... I like letting folks define the label themselves in the list.

Supporting this in general is pretty difficult.

First we have to recognize ":Select" for what it is, an extra empty option at the top of a list we otherwise want our keys and values to be equal. But folks say "Select" or "Choose One" or a different language. Do we just recognize when the first item in the string is set to ""? There are many reasons this may be the case. I don't think we can assume an empty-keyed first item is a special option that can be ignored.

Maybe we adjust the code that processes the list of options and if any of the options have keys set, then let's assume the intention was for the other options to use the values as keys. Is this a good assumption? I think possibly. This allows folks to use a shorthand where they only set a different key/label when needed, otherwise, we assume it's the same. If so, we should update the code in includes/fields.php to first loop through the items and look for a ":" separator. If found, then instead of the else condition there saying $options[] = $settings_option;, it should say $options[$settings_option] = $settings_option;

Side note, we should probably be trimming in the else condition there always.

Should we instead throw an error RE requiring all options have keys when only some do? Only if we think assuming "Yes" is "Yes:Yes" here is a bad assumption or if there is other trouble folks can get into by mixing and matching the formatting of their options.

from paid-memberships-pro.

To me, it does not feel like the flexibility that we are trying to give admins by making : optional is worth the amount of time we spend trying to eliminate every edge case. If we made a bold decision to require that all field options be key:label, we could enforce that requirement on user field save and "fix" user input that doesn't fit that pattern. Users can then see the correct way for the data to be formatted and understand what their user fields will look at on the frontend.

ex. User tries to save option Yes, after save if they edit the field again, they will see Yes:Yes.

Or, maybe we even enforce key:label in the UI by having two separate text fields for each option: one for key and a separate text field for label. In this case, we could automatically "guess" at option keys based on the label that the admin creates, similar to how we automatically try to guess field names from field labels that are entered.

from paid-memberships-pro.