Convert fivestar/node_type pairings into fivestar field widgets

This looks good, There's a couple of mistakes such as ..

+++ b/fivestar.moduleundefined
@@ -196,15 +195,10 @@ function fivestar_theme() {
+  db_delete('table')

Should be fivestar

But all it all it looks really good. I'm wondering if we want to have all those fields. Some of the settings are really minor, I'm not sure if its justified to have a position and a position teaser, we can just stash that in a settings field.

Also if we're going to switch to using a database, we might as well work with the return data as an object instead of an associative array, I find it easier. Any comments?

Powered by Dreditor.

@james.elliott no need. The fivestar field already has an tag support and there is an exposed star formatter that lets you pretty much replace the regular fivestar nodetype settings with a field that a user can vote against.

Which is why I would rather remove the entire fivestar/node_type settings and concentrate on the field as its way more robust.

At first I thought about using hook_field_extra_fields but then I decided that making the fivestar_field better would just be a better solution to all the current fivestar problems.

@james, nice!, I'll be testing this today.

I'm applying this patch right now.

I know there's some incosistency with the field which is why I was thinking of moving everything to either the formatter or the instance settings. I agree that the instance settings does make more sense.

Also there is a missing target now, but not essentially self, but none. Which essentially would treat the field as just a cck widget and not so much a votingapi target.

I'll have more feedback once I apply this patch. Thanks for the great work.

Patches doesn't apply cleanly :(

@james, I'm completely on board with almost all the changes you're requesting but some of these changes can be separated out.

+++ b/fivestar.moduleundefined
@@ -467,21 +437,17 @@ function fivestar_validate_target($entity_type, $id, $tag, $uid = NULL) {
+  $fields = field_read_fields(array('module' => 'fivestar'));
+  ¶
+  foreach($fields as $field) {
+    if ($field['settings']['axis'] == $tag) {
+      $params = array(
+        'entity_type' => $entity_type,
+        'field_name' => $field['field_name'],
+      );
+      $instance = field_read_instances($params);
+      if(!empty($instance)) {
+        return TRUE;
       }
     }

This is it's own patch and can be its own issue.

+++ b/includes/fivestar.field.incundefined
@@ -56,25 +53,30 @@ function fivestar_field_settings_form($field, $instance) {
-  if (empty($dynamic_options)) {
-    drupal_set_message(t('No potential target fields are available for the %type bundle. Create a node reference field in this bundle to make it easier to assign a vote to a node.', array('%type' => $instance['bundle'])), 'warning');
-  }
-
-  $dynamic_options = array('' => '<'. t('none') . '>') + $dynamic_options;
   $form['dynamic_target'] = array(
     '#title' => t('Voting target'),
     '#type' => 'select',
     '#default_value' => isset($field['settings']['dynamic_target']) ? $field['settings']['dynamic_target'] : '',
     '#options' => $dynamic_options,
-    '#description' => t('The voting target will make the value of this field cast a vote on another node. Use node reference fields module to create advanced reviews. Use the Parent Node Target when using fivestar with comments. More information available on the <a href="http://drupal.org/handbook/modules/fivestar">Fivestar handbook page</a>.'),
+    '#attributes' => array(
+      'class' => array('fivestar-target'),
+    ),
+    '#description' => t('The item that this widget will case a vote on. Use node reference fields module to create advanced reviews. Use the Parent Node Target when using fivestar with comments. More information available on the <a href="http://drupal.org/handbook/modules/fivestar">Fivestar handbook page</a>.'),
   );
 
   if (user_access('use PHP for fivestar target')) {
+    $form['dynamic_target']['#options']['custom_php'] = t('Custom PHP');
+    ¶
     $form['php_target'] = array(
       '#type' => 'fieldset',
       '#title' => t('Voting target PHP code'),
       '#collapsible' => TRUE,
       '#collapsed' => empty($field['php_target']),
+      '#states' => array(
+        'visible' => array(
+          '.fivestar-target' => array('value' => 'custom_php'),
+        ),
+      ),

This is another separate issue one witch is a duplicate of another already open issue.

+++ b/includes/fivestar.field.incundefined
@@ -83,18 +85,21 @@ function fivestar_field_settings_form($field, $instance) {
-  else {
-    $form['php_target']['php_target'] = array(
-      '#type' => 'value',
-      '#value' => isset($field['settings']['php_target']) ? $field['settings']['php_target'] : '',
-    );
+  else if ($field['settings']['dynamic_target'] == 'custom_php') {
+    $form['dynamic_target']['#options'] = array('custom_php' => t('Custom PHP'));
+    $form['dynamic_target']['#disabled'] = TRUE;
+    $form['dynamic_target']['#description'] = t('This field has been configured using advanced settings that you do not have access to change.  Contact the site administrator if you believe this to be in error');

Same as above.

+++ b/includes/fivestar.field.incundefined
@@ -160,7 +165,8 @@ function _fivestar_field_helper($entity_type, $entity, $field, $instance, $langc
-  if (isset($field['settings']['php_target']) && !empty($field['settings']['php_target']['php_target'])) {
+
+  if ($field['settings']['dynamic_target'] == 'custom_php' && !empty($field['settings']['php_target']['php_target'])) {
     // Use eval rather than drupal_eval to allow access to local variables.
     $target = eval($field['settings']['php_target']['php_target']);
   }
@@ -176,8 +182,8 @@ function _fivestar_field_target($entity, $field, $item, $langcode) {

@@ -176,8 +182,8 @@ function _fivestar_field_target($entity, $field, $item, $langcode) {
       $target = $entity->$field['dynamic_target'];
     }
   }
-  elseif (isset($item['target'][0]['nid'])) {
-    $target = $item['target'][0]['nid'];
+  else {
+    $target = $entity->nid;

Also an unrelated change.

Powered by Dreditor.

Another issue that can be resolve before this one, making this patch a lot smaller and easier to grok. #1203428: Better Fivestar access check for the exposed field

i applied all the patches according to #26
i run drush updb after every patch, but 0005 throws an error

7004 install throws an error
http://screensnapr.com/v/9bKoWs.png

you remove it at line 656
-function fivestar_get_suffix($node_type, $tag) {
and you call it later like at line 121.

got some warnings too:

Notice: Undefined index: node in fivestar_fivestar_target_info() (Zeile 1491 von /var/www/WORKSPACE_DRUPAL/drupal_test/sites/all/modules/contrib/fivestar/fivestar.module).
Warning: in_array() [function.in-array]: Wrong datatype for second argument in fivestar_fivestar_target_info() (Zeile 1491 von /var/www/WORKSPACE_DRUPAL/drupal_test/sites/all/modules/contrib/fivestar/fivestar.module).
Notice: Undefined index: node in fivestar_fivestar_target_info() (Zeile 1491 von /var/www/WORKSPACE_DRUPAL/drupal_test/sites/all/modules/contrib/fivestar/fivestar.module).
Warning: in_array() [function.in-array]: Wrong datatype for second argument in fivestar_fivestar_target_info() (Zeile 1491 von /var/www/WORKSPACE_DRUPAL/drupal_test/sites/all/modules/contrib/fivestar/fivestar.module).

subscribing

i did what has be done here #5
http://drupal.org/node/1173814

added a comment, votet, hit preview, verything ok, hit save, error

errors

http://screensnapr.com/v/LBU0Wv.png

Hmm, trying to decide which one to tackle 1st, this one or #1222330: Add an exposed stars widget and remove the exposed stars formatter, this does remove all the complexities of having to support two different ways of using fivestar.

.....

Decided to go with this one.

-- Here's a re-rolled.

This is now fixed. On to the next one.

--
http://drupalcode.org/project/fivestar.git/commit/89ab6b67b54d63efb19416...