Posted by cmjns on June 12, 2009 at 3:46pm
| Project: | Viewfield |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | task |
| Priority: | normal |
| Assigned: | keithm |
| Status: | closed (fixed) |
Issue Summary
When I create a viewfield, and I want to force default, there is no way for me to set the view argument per instance-- I have to set it "globally", for all instances. This means that I cannot change the view via arguments for each node type to which the field is added. It's a minor annoyance, but I've run into it a lot.
Comments
#1
#2
What makes you think this is a CCK issue ?
#3
This is something we can look at further once we're able to take a pass at how defaults are handled.
Jer
#4
First stab at this. Patch is getting larger than the entire module though. ;)
The module update path for this patch may delay the next release.
#5
Subscribing...
#6
+1 especially for http://drupal.org/node/704014
Cheers
#7
I rerolled #4 for the current 6.x-1.x-dev. The rerolled patch applies cleanly and works pretty well, passing a argument per content type and/or per instance. Legacy tokens (e.g., %nid) are working fine but token module tokens are not working.
#8
Updated #7 to fix Token module replacement and remove the 'token_enabled' concept. Legacy token and Token module token arguments should both work now, per content type or instance.
I'd appreciate some feedback on this patch.
#9
+++ viewfield.module@@ -37,35 +41,18 @@ function viewfield_field_settings($op, $field) {
case 'validate':
- if ($field['force_default'] && $field['multiple']) {
+ if ($field['multiple'] && $field['widget']['force_default']) {
This looks odd -- are the widget settings really available in field settings...?
+++ viewfield.module@@ -191,74 +159,62 @@ function viewfield_elements() {
+ '#default_value' => $element['#default_value']['vname'],
...
+ // If there is only one option, only show arguments.
+ if (count($options) == 1 && !$is_field_settings_form) {
+ list($key, $label) = each($options);
+ $element['vname']['#access'] = FALSE;
+ $element['vname']['#default_value'] = $key;
}
Why is the #default_value changed to the first value of #options here? That doesn't look correct to me.
+++ viewfield.module@@ -191,74 +159,62 @@ function viewfield_elements() {
+ if (module_exists('token')) {
+ $element['vargs']['#description'] .=
+ '<br />' . t('Available tokens: %nid for the id of the current node, %author for the node author, %viewer for the viewing user, or any token from the placeholder token list.')
...
+ else {
+ $element['vargs']['#description'] .= '<br />' . t('Available tokens: %nid for the id of the current node, %author for the node author, and %viewer for viewing user.');
+ }
Let's not duplicate the code for this #description
Powered by Dreditor.
#10
- if ($field['force_default'] && $field['multiple']) {+ if ($field['multiple'] && $field['widget']['force_default']) {
Yes, this is wrong. Reverted to original (pre-patch) code.
The code in the patch:
$element['vname'] = array('#type' => 'select',
'#title' => $element['#title'],
'#options' => $options,
'#default_value' => $element['#default_value']['vname'],
'#required' => $element['#required'],
'#access' => $is_field_settings_form || !$field['widget']['force_default'],
'#description' => $element['#description'],
);
// If there is only one option, only show arguments.
if (count($options) == 1 && !$is_field_settings_form) {
list($key, $label) = each($options);
$element['vname']['#access'] = FALSE;
$element['vname']['#default_value'] = $key;
}
is derived from this section in viewfield-6.x-1.x-dev :
if (count($options) > 1) {$element['vname'] = array(
'#type' => 'select',
'#options' => $options,
'#default_value' => $element['#default_value']['vname'],
'#title' => $element['#title'],
'#required' => $element['#required'],
'#description' => $element['#description'],
);
}
else {
// There's only the one view, so only show the arguments.
list($key, $label) = each($options);
$element['vname'] = array(
'#type' => 'value',
'#value' => $key,
);
}
The logic in the 1-option case remains essentially the same.
Removed duplicated legacy token #description in viewfield_elements().
#11
Patch is working for me
#12
Patch now includes code to migrate global default value settings to instance level.
#13
Patch rerolled for latest dev. Please review and test. Once this patch is committed I'll move to the D7 port.
#14
Oh, we're going to commit this into a new 2.x branch?
Might make sense, yeah.
+++ b/viewfield.install@@ -112,3 +112,39 @@ function viewfield_update_6001() {
+ $result = db_query(
+ "SELECT * FROM {". content_instance_tablename() ."} nfi ".
+ "LEFT JOIN {". content_field_tablename() ."} nf " .
+ "ON nf.field_name = nfi.field_name " .
+ "WHERE nf.type = 'viewfield' "
+ );
It would make sense to define the results of those functions as variables, to subsequently be able to use a single string for the query (on one line) having the $variables embedded. They also seem to be used in the other query of this update.
+++ b/viewfield.install@@ -112,3 +112,39 @@ function viewfield_update_6001() {
+ $query_string = "UPDATE {" . content_instance_tablename() . "} SET widget_settings='%s' WHERE field_name='%s' AND type_name='%s'";
Can we rename $query_string to $sql?
Would be nice to use spaces between = assignments in the SQL.
+++ b/viewfield.install@@ -112,3 +112,39 @@ function viewfield_update_6001() {
+ $update = db_query($query_string, $settings_update['widget_settings'], $settings_update['field_name'], $settings_update['type_name']);
+ $ret[] = array('success' => $update !== FALSE, 'query' => sprintf($query_string, $settings_update['widget_settings'], $settings_update['field_name'], $settings_update['type_name']));
Why don't we use update_sql() here?
Otherwise, the code looks good and ready to be committed. I guess we can deal with any problems in follow-up patches as they arise.
Powered by Dreditor.
#15
Rerolled with recommendations.
#16
http://drupalcode.org/project/viewfield.git/commit/f5dec34