Improved handling for optionwidgets

I just committed changes to the way option values are stored that seems to make more sense, but it won't do what you describe here.

The old way was that you provided a list of numbers for a number field or text values for a text field and the module stored the index number of your choice, which of course would break any time you changed the list. I changed it so the module stores the actual value you put in the option list (either a number or a text value).

The only way to control both an integer value and a label would be to set up a way to collect both in both number and text fields, then change the optionwidget module to get the option list from the field which would require a fair bit of rework. I'm supposed to focus on bug fixes at this time and this is more like a feature request, so I'm going to re-categorize it as a feature request and leave it on the queue.

Yes, the forms api supports doing this, no question about that. Your original question looked like you wanted to create something like:
10 => 'First'
20 => 'Second'
30 => 'Third'
where the index was the value stored in the database and the label is what is displayed to the user. In other words, where you can arbitrarily control both the index and the label. When I said you can't do that now, that is what I was referring to.

Here's how optionwidgets interacts with the number and text fields:

1) You create either a text or number field. In that field you provide a list of allowed values ('First', 'Second', 'Third') for a text field or (10, 20, 30) for a number field.
2) Optionwidgets creates an array out of your provided values. In the original module, your optionwidget text field would create an array like:
1 => 'First'
2 => 'Second'
3 => 'Third'
Then your users would see those labels, but the program would save a value of 1, 2 or 3 to the database (even if it was a text field). That works OK if you don't care what is in the database and if you don't change the list, but if you later come back and change the list to insert another value between 'First' and 'Second', the index values stored in the database would be all wrong. So I added drupal_map_assoc() to change the option array to:
'First' => 'First'
'Second' => 'Second'
'Third' => 'Third'
That will now store 'First', 'Second', or 'Third' to the database.

However, if what you really wanted was for 'First' to always be stored as 10 and 'Second' to always be stored as '20', you cannot do that with cck as it is currently coded. To do that, we would need to change the place where you create your allowed values list in the number and text fields so you can indicate both the index that you want in the database and the label you want displayed to the user for each value, then alter optionwidgets to use that new information to create the array.

Does that make sense now? I know this is confusing.

And even more confusingly, in the original code, if you create a list like:
15
20
25
in the number module, the optionwidget was storing those values as
1
2
3
Using drupal_map_assoc they now get stored as 15, 20, and 25.

But you still can't display something different to the user.

Will this feature get commited? Seems like something anyone who creates realistic forms may need this...

I need it in multiple places right now...

I hope this would be available in usable form soon... Need to roll out some custom CCK types ASAP.

I tried the last patch (on 10/20 CCK, v 1.56.2.22) and 8 out of 8 hunks failed...

Could you suggest what might be wrong? I have never done patching before.

That code is not going to be the right way to do this, since it makes changes to the content module, which isn't necessary or desirable for something specific to a particular widget. Instead, I have done a re-write of the text, number, and optionwidgets modules to fix a number of problems and improve the way that optionwidgets works. The attached zip file (for version 4.7) has replacements for those three files that will do the following:

1) Handling for the allowed values array is moved into the fields instead of the widget, which makes those values available to views and should pretty much get views handling of option values working correctly.

2) The handling for allowed values is expanded. In addition to the current method (type in a list of allowed values where the value will be used as the label) you can now also type in a list of key|label pairs where the key will be stored in the database and the label will be displayed to the user - OR - you can insert a php script that will return a keyed array of allowed values. You can use the last option to call a function that will return the array you want to use, or to pull an option array out of a database. This should provide ways to create almost any array you want to use for the allowed values, which is a big improvement on the current method.

I am attaching these as replacement files rather than a patch because there are lots of people who can't apply a patch, and I want to increase the potential for getting this tested before I commit it.

Let me know how this works. If it works well, I'll go ahead and commit it.

And for those who like to see patches so they can tell what changed, here it is as a patch.

The extra filter is a good idea. I should also maybe add another formatter that will allow you to view the raw data instead of the label.

When you say the selector didn't work, I suspect it's because the existing data has gotten stored in different ways. Is your database storing 1, 2, 3 values for your allowed values? (i.e. you have a list of allowed values like 15, 30, 45, 60 and those values are stored in your database as 1, 2, 3, and 4). The plain allowed values list is problematic because it's hard to tell what value is actually stored in the database. One way around this would be require that the allowed values list indicates the key as well as the label. Then for anyone migrating existing data from an older version I can have the module insert the 1, 2, 3 keys into the existing allowed values list so that it works right.

What were you using for allowed values? I can't replicate the problem.

Thanks fortytwo, that was indeed part of the problem, but I found one other place that needed a fix. I have made these changes and also included an optionwidgets.install file that will update the an existing database to use these new options. It does not update your actual data, instead it updates your current allowed values lists to use the key|label format to remove any ambiguity about what value is actually stored in the database.

To use this, upload the files to your cck folder, run the update program and review the messages to see if it seems to be making the right decisions about what the key|label options should be for each of your optionwidget fields. Then go to the field settings where you can give each option a user-friendly name.

Oops, wait a minute. I meant to add in the additional views filters, too. I'll get a better file posted shortly.

This is very good idea, but if we driving at separate user from real value store in database, maybe it will be good if we introduce this feature to text fields too. For example, if user type in text field "Super", in dabase will store "10" etc.

Moreover, I am not sure that if in database we have "10", browser show to user "Super" in node view, in node edit form and in Views, but this is what we want, right?

I have this problem in hook_field_formatter() (in developing Boolean field) when I tried to implement above behavior, but the problem is where user type in text field "Yes/No", this value should be validate as 0/1, but isn't and we have illegal value. So there should be possible to transale label-value (Yes/No) to key-value (0/1) and correct validate this label-value.

I've got the updated files now and I think it is working right.

@kjw83, I think this will do most of what you are describing. There is still an issue about storing a null value when the field is not required which needs to be addressed, but I don't want to stuff too much into this at one time since it will become unweildy.

The purpose of these changes are:

1) Extend the way the allowed values can be defined so you can control both a key and a label and so you can construct an allowed values array from php code.

2) Make sure that when you are using an allowed values list that your label gets substituted back in in when viewing the node or creating views instead of showing you the raw database value (the current method)

3) Make sure that views filters incorporate the labels to make the selection work more naturally.

This is not the end of everything that might be done to optionwidgets and the text and number modules, just trying to get the above job done.

I'd really like to get some reviews of how well this is working without any requests for additional features. Once it's working and committed, we can start up separate issues for other features that are needed.

There is an optionwidgets.install module here, too, so run the update.php program. And, obviously, we are still testing this so don't use this anywhere important until we verify it is working correctly :-)

Karen, I'm full supporter your guidelines. Now I don't have any data for update.php, so I created some simple data with optionwidgets and your update script done its job correctly.

I only wonder why your update script add labels if it is optional. For example: 1, 2, 3 changed to 1|1, 2|2, 3|3. This is redundant and I think the better solution is that after update user will add labels only where it is needful.

I am trying to implement this patch and there is a problem with loading of saved values into the form.

I created a checkbox field with value "1|Enabled". Widget showed up fine and I saved the value with checked box. The data shows up in database properly as 1. However when I open edit form the checkbox is NOT checked.

One more suggestion (feature request). Allow labels to be generated by running value as argument to a function.

This could make implementation of this request (http://drupal.org/node/97966) trivial through using format_interval()

Function-based definition could be valuable beyond dates, as functions could be used for theming labels is others ways.

Yet another problem!

I created a field and set the optionwidget to a set of intervals. Then I created two instances in two different content types. Unfortunately they appear to share the same allowed values!

I think allowed values should be selected separately per instance / widget to allow greater flexibility.

In no particular order, the answers to these questions:

1) Allowed values are set up in the field not the widget. That means that if the same field is used in more than one content type, both content types will have the same list of allowed values. The allowed values list cannot be made specific to an field instance without creating all kinds of other problems, the most important of which is that Views won't work right. If you need a different list of allowed values, you will need to create a different field. I can see that there would be times it would be nice to be able to do this but it is definitely not something we can do at this time.

2) I set up the keys and labels in the conversion script even though both are not required as a tool to make sure the conversion goes smoothly. The program is actually scanning the database to try to make sure that the values marked as the keys are the values in the database, and I want users who have existing data to have a chance to see what the program has come up with and confirm that it looks right. Afterward, you can change them if you want.

3) You can use php code to create the array, both the keys and the labels, so you can incorporate intervals or whatever else you want. No change is needed for that. I want to keep this very simple and a block where you can input php code should be sufficient.

4) I need to investigate the issue about the checkbox not being checked. There is some FAPI quirkiness about checkboxes that might be causing that. I also need to check into the issue about not being able to set an empty value. I'll take a look at those issues shortly.

Karen, thanks for following up. Further comments:

1) OK, I do not know enough to question your reasoning :)

2) Hmmm... I am not sure which part of my comments you are referring to. All of my tests above were on content types / fields created from scratch, so conversion should not be the problem.

3) I just tried this option and it is broken - shows either manually entered list of values or empty select if none are provided. Plus this option is not prominently mentioned in the help text for the "Allowed Values".

4) Thanks. The problem is definitely here, interestingly enough only for checkboxes but not selects.

========== Code that breaks #3 ==============

Entered in PHP Code textarea

_optionwidget_interval('feed_refresh');

Defined in a module (yes it is enabled and tested in devel execute block):

function _optionwidget_interval($type) {
	
	switch ($type) {
		case 'feed_refresh':
			$keys = array(
				3600, 7200, 10800, 21600, 32400, 43200, 64800,	// hours
				86400, 172800, 259200, 							// days
				604800, 1209600, 2419200);						// weeks
			break;
	}

	foreach ($keys as $key) {
		$label	= format_interval($key);
		$result.= "$key|$label\n";
	}
	return $result;
}

I was answering questions asked by several people. #2 was in answer to mki's question. I'm sure it would have been helpful for me to address each one to the one who asked the question, but I was in a hurry :-)

I'm sure the documentation is not as good as it could be, this is a new feature and I'm trying to get it tested. The reason your script does not work is that you are returning a string and you must return an array. See my original instructions in #13. I'll fix the instructions once I'm sure it's working correctly.

I'm going to try to nail down the checkbox issue now...

Well, I replaced result generation part of my function:

//		$result.= "$key|$label\n";
		$result[$key] = $label;

But the widget still does not work. Only shows the manually entered value or none if none available.

Maybe you need to include the file where the function is? Try just doing this as php code in the php block, rather than as a function and see if it works. If it does, it looks like it is not finding your function to execute it. BTW, don't put the function into the php box, just the code inside it.

The function runs fine in devel module execute block...

Moreover, the code fails even when I take the modified function body and put it into PHP Code box:

$keys = array(3600, 7200, 10800, 21600, 32400, 43200, 64800, 86400, 172800, 259200, 604800, 1209600, 2419200);						
foreach ($keys as $key) {
	$label	= format_interval($key);
//	$result.= "$key|$label\n";
	$result[$key] = $label;
}

Out of ideas what could cause this...

@dkruglyak, I finally had time to get back and finish this. I tried putting your code (from #37) in the php box and it works just fine for me, once you add at the end:

return $result;

Is that all you are missing? If that still doesn't work for you, I need more info about your system and the field you are using. What version of php are you using? What type of field are you creating? What widget are you using?

I updated the patch and have tried all kinds of combinations and everything works fine for me -- php code, selects, checkboxes, radios. So I'm going to post the re-written code for review again because I would like to be able to get this committed.

If anyone has problems, please let me know what php version and type of field and widget you are using and what you are using for allowed values (either the list or the code).

This patch is against 4.7. Once I know it is working, I'll get it working for 5.x, too.

Forgot to change the status again...

I made some slight improvements to this, mainly I made the allowed values array static so it does not get recalculated so often and I added in a theme for an empty value so you can make it blank or display 'N/A' or whatever else you want it to look like by overriding the theme.

I'm pretty confidant this is working right now in both 4.7 and 5.x. I found the source of the checkbox problem and fixed it and php code seems to be working fine, so I have committed this.