In working on views integration with my custom module recently I ran into an issue with a boolean field. The way my table was set up I had a field called 'privacy' that if left FALSE/0/NULL meant it was private and TRUE/1/NOT NULL meant it wasn't private. I attempted to set up a boolean field definition as found below
$data['my_table']['private'] = array(
'title' => t('Privacy'),
'help' => t('Whether or not the record is private.'),
'field' => array(
'handler' => 'views_handler_field_boolean',
'click sortable' => TRUE,
'output formats' => array(
'private-notprivate' => array(t('Private'), t('Not Private')),
),
'not' => TRUE,
),
'filter' => array(
'handler' => 'views_handler_filter_boolean_operator',
'label' => t('Privacy'),
'type' => 'yes-no',
'use equal' => TRUE, // Use status = 1 instead of status <> 0 in WHERE statement
),
'sort' => array(
'handler' => 'views_handler_sort',
),
);
I passed the 'not' attribute of the field as TRUE hoping this would automatically reverse the boolean value or at least automatically check the 'not' form element in the options form. Alas it did not. I jumped on IRC had a chat with merlinofchaos realized that what I was attempting was not possible and so proposed making it possible.
This could happen one of two ways and I have provided a patch for either instance. First off views_data for boolean fields would be defined the same with the exception of now being able to pass a 'not' key as illustrated above. The two options for how to handle a field defining this are as follows.
- We auto check the 'Reverse' checkbox.
This option is provided for in patch 1. I personally don't like this option because it still allows the end user to muck it up and the developer is not able to account for his boolean field being in reverse easily. - We auto reverse the field by default.
This allows the developer to more tightly control how his data is being used. The reverse checkbox is left unchecked. If the end user does check 'reverse' then we simply leave the value as is. This is in patch 2. This also provides for the instances where there is no definition of 'not' in the field definition
Those are the options. merlinofchaos said he was noncommittal about it either way. So I thought I'd put it up for some discussion. Either way i think it's a good feature to give views developers.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | Boolean-reverse-1198446-4.patch | 527 bytes | arcaneadam |
| #3 | Boolean-reverse-1198446-3.patch | 975 bytes | arcaneadam |
| Boolean-Reverse-2.patch | 601 bytes | arcaneadam | |
| Boolean-Reverse-1.patch | 664 bytes | arcaneadam |
Comments
Comment #1
dawehnerIn general this is code is quite confusing. Isn't it enough to check for !empty($options['not']) and set the default value of $options['not'] to the value in the definition, if set?
Comment #2
arcaneadam commentedThat is what the first suggestion does. It defaults the $options['not'] checkbox to checked.
As I said I still don't like that b/c it allows the intention of the developer to be misunderstood.
Making the code in patch 2 slightly clearer could be done, I just put it together in the most compact way. I could expand it into a few conditionals with commenting to make it more clear.
Comment #3
arcaneadam commentedI realized that initially I had implemented option 2 this in a much easier and simpler manner. If the handler defines the field as a reverse then we simply reverse the values not worrying about the checkbox. Then if the checkbox is checked it gets reversed back. It's much easier to see what happening in this patch.
Comment #4
arcaneadam commentedAww crud. I accidentally included a change I made for a separate issue in the last patch. Re-rolling patch.
Comment #5
dawehnerOne thing i don't get is this code:
You know this is not part of your change, but it could be directly be related. It seems to be a bug here, because
this string is used nowhere else.
Comment #6
tim.plunkettTriggering the testbot.
Comment #7
tim.plunkettThe patch in #4 doesn't seem to do anything.