Support from Acquia helps fund testing for Drupal Acquia logo

Comments

TimelessDomain’s picture

Category: bug » task
Priority: Normal » Major

applied patch - no errors are appearing. the voting details tab appears correctly on content types with vote up/down turned on.

I do not have any voting widgets to test the actual voting though. (note: i did not test with normal node templates)
- Views integration is not working yet
Added: "Vote Up/Down: Node Widget" to a view , but it changes to "Broken/missing handler" after adding
- display suite integration is not working yet
no widgets appear

Is there any other ways to add the widget for further testing?

TimelessDomain’s picture

Status: Needs review » Reviewed & tested by the community

actually it works! THANKS! not ajax yet. but after clicking the page reloads & it shows the vote count (voted on teaser). upon clicking into node & vote results, it shows my vote. thanks! Now we need to get ajax, views integration, and display suite integration going.

univate’s picture

Ajax is working for me... not sure it you need to clear cache or anything?

I'm not using views so haven't look into the views integration.

rogical’s picture

+1

chipcleary’s picture

+1

patoshi’s picture

+10 damage

Kiorrik’s picture

+10 circle of healing

sub

Hydra’s picture

sub

DamienMcKenna’s picture

Subscribe.

Juan C’s picture

subscribe

Donaldd’s picture

subscribing

Sylense’s picture

+1 eager to see views and display suite integration

jwilson3’s picture

RTBC++

This really should get into the code base by now, so we can keep moving forward, with other patches. I recommend we break off Display Suite, and (eg Panels integration) into separate tickets.

franzkewd’s picture

Subscribe

js’s picture

Subscribe

drnikki’s picture

rtbc++. i'd like to get this in so we can add more patches for the other problems.

marvil07’s picture

Status: Reviewed & tested by the community » Postponed

I guess I did not publish enough my plan for d7 #749438-39: Port Vote Up/Down to D7

Postponing for now instead of close, for feedback on #1295574: Remove vud_{node,comment,term} modules in favour of vud_field

mathankumarc’s picture

It Works fine for me. Except Ajax. I'm getting ajax is undefined error. Just now I noticed that ajax.js was not included. Any Solution?

dankobiaka’s picture

You need to include the Drupal AJAX library in vud.theme.inc, before ctools_add_js('ajax-responder');

function vud_widget_proxy


drupal_add_library('system', 'drupal.ajax');
  
ctools_add_js('ajax-responder');
ctools_include('ajax');

boran’s picture

Are those fixes in git?
I see ctools_add_js and ctools_add_js but not drupal_add_library().

I would very much like to see vud_node continue, since a D6 site I manage needs to be migrated to D7 and vote counts on nodes and comments preserved..

univate’s picture

I don't see why we can't git these fixes into vud_node so that migrations from Drupal 6 are less painful.

I completely agree that the future should be vud_field. But since there is working code here why not let it live a little longer?

mathankumarc’s picture

thanks dankobiaka. thats solved my problem.

boran’s picture

Status: Needs review » Active

The last git commit was June 16 by marvil07, so the above fixes are not flowing in..

mathankumarc: I was unable to apply the patch at the very top of this issue to git HEAD, what branch are you using?

UPDATE: use "git apply" as opposed to "patch < PATHCHNAME"

Changing the thread to active since its an issue that definitely interests some of us.

Anonymous’s picture

Status: Postponed » Needs review

vud_node.patch queued for re-testing....didn't really mean to re-test it, but based off the comment above me I wanted to make sure the patch worked.

I applied the patch and it cleaned up errors on my node page, but I still don't see the ability to add a vote up/down field on my node.

boran’s picture

Status: Active » Needs review

Its easy to test manually:

mkdir vud_hack
cd vud_hack
git clone --branch 7.x-1.x http://git.drupal.org/project/vote_up_down.git

cd vote_up_down
wget http://drupal.org/files/issues/vud_node_0.patch

UPDATE: Trying with 'git apply'

git apply --stat vud_node_0.patch
 .cvsignore               |    3 --
 vud.install              |    4 +-
 vud.module               |   25 +++++++++-----
 vud.theme.inc            |    2 +
 vud_node/vud_node.module |   80 +++++++++++++++++++++++++++++++++-------------

git apply vud_node_0.patch
vud_node_0.patch:41: trailing whitespace.
error: .cvsignore: No such file or directory

Is there a way to tell git apply to continue despite .cvsignore?
(Well for now, I just edited the patch and deleted the first few lines)

And, also, then added the line and noted above for vud.theme.inc:
drupal_add_library('system', 'drupal.ajax');

boran’s picture

Could we please get the cumulative fixes committed, as I'm getting confused. I had developed other fixes in parallel (see #749438) and find it hard to keep track.

Status: Needs review » Needs work

The last submitted patch, vud_node.patch, failed testing.

jacobf’s picture

Status: Needs work » Needs review

Is there anywhere where i can get a .zip file of this module anywhere i have looked everywhere and cant download huge files at work. is there any other alternative? at the moment im running a local version of this on my computer.

klonos’s picture

@jacobf: Hey Jacob, welcome to Drupal & drupal.org! The module is available in both .zip & .tar.gz format in the project's page here: http://drupal.org/project/vote_up_down (scroll down to the "Downloads" section). I frankly don't understand what you mean by "huge" files. Both downloads are at the most around ~50-60kb.

Now, I am "obliged" to "smack you once across your head" mate ;) ...Please don't ask questions non-related to topics because it is frowned upon and considered rude here (you might see people refer to such off-topic questions as "issue hijacking"). It only adds "noise" to an issue where people try to solve a single specific problem. It makes it a very tedious job to try to read through and grok things - especially in issues with a lot of comments. It is not only your question, but the fact that someone else needs to reply to your question with an extra comment (like I was "forced" to do now).

Next time you need help with something:

- first search through the documentation
- then search in each module's issue queue (the "Issues for [project_name_here]" section on the right sidebar of each project) to see if your question has been asked and answered before
- if the above two steps failed to bring results, then file a new issue with your question and remember to select "support request" at the issue's category drop-down menu.

Now, an admin should delete both our comments ;)

iRex’s picture

I am using the dev version "Last updated: November 25, 2011 - 12:42", and I am still getting these errors:

Notice: Use of undefined constant NODE_BUILD_PREVIEW - assumed 'NODE_BUILD_PREVIEW' in vud_node_node_view() (line 147 of \sites\all\modules\vote_up_down\vud_node\vud_node.module).
Notice: Use of undefined constant NODE_BUILD_SEARCH_INDEX - assumed 'NODE_BUILD_SEARCH_INDEX' in vud_node_node_view() (line 148 of \sites\all\modules\vote_up_down\vud_node\vud_node.module).
Notice: Use of undefined constant NODE_BUILD_SEARCH_RESULT - assumed 'NODE_BUILD_SEARCH_RESULT' in vud_node_node_view() (line 149 of \sites\all\modules\vote_up_down\vud_node\vud_node.module).
Notice: Use of undefined constant NODE_BUILD_RSS - assumed 'NODE_BUILD_RSS' in vud_node_node_view() (line 150 of \sites\all\modules\vote_up_down\vud_node\vud_node.module).
Notice: Undefined property: stdClass::$build_mode in vud_node_node_view() (line 152 of \sites\all\modules\vote_up_down\vud_node\vud_node.module).
Notice: Undefined variable: a3 in vud_node_node_view() (line 161 of \sites\all\modules\vote_up_down\vud_node\vud_node.module).
Notice: Undefined variable: a3 in vud_term_node_view() (line 82 of \sites\all\modules\vote_up_down\vud_term\vud_term.module).
Notice: Use of undefined constant NODE_BUILD_PREVIEW - assumed 'NODE_BUILD_PREVIEW' in vud_node_node_view() (line 147 of \sites\all\modules\vote_up_down\vud_node\vud_node.module).
Notice: Use of undefined constant NODE_BUILD_SEARCH_INDEX - assumed 'NODE_BUILD_SEARCH_INDEX' in vud_node_node_view() (line 148 of \sites\all\modules\vote_up_down\vud_node\vud_node.module).
Notice: Use of undefined constant NODE_BUILD_SEARCH_RESULT - assumed 'NODE_BUILD_SEARCH_RESULT' in vud_node_node_view() (line 149 of \sites\all\modules\vote_up_down\vud_node\vud_node.module).
Notice: Use of undefined constant NODE_BUILD_RSS - assumed 'NODE_BUILD_RSS' in vud_node_node_view() (line 150 of \sites\all\modules\vote_up_down\vud_node\vud_node.module).
Notice: Undefined property: stdClass::$build_mode in vud_node_node_view() (line 152 of \sites\all\modules\vote_up_down\vud_node\vud_node.module).
Notice: Undefined variable: a3 in vud_node_node_view() (line 161 of \sites\all\modules\vote_up_down\vud_node\vud_node.module).
Notice: Undefined variable: a3 in vud_term_node_view() (line 82 of \sites\all\modules\vote_up_down\vud_term\vud_term.module).

Are the changes mentioned above supposed to fix these; and have the changes not been put in the dev version?

thanks

klucid’s picture

Where did you download that dev version from? I only see:

vote_up_down 7.x-1.x-dev
Posted by marvil07 on March 11, 2011 at 6:57am

Edit: My mistake, that same version was modified on the date you mentioned.

Luna Vulpo’s picture

same problem as reported iRex.
Error occurred when I try edit a node.
I use a just downloaded version.

Luna Vulpo’s picture

vud_node.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, vud_node.patch, failed testing.

Luna Vulpo’s picture

I patched by patch which is attached to the first post on this thread. it help for error submitted by iRex.
could you put this patch in repository?

drewish’s picture

I posted a patch on #1402758: Remove user votes page in favour of views that overlaps some with this. I think it's got a cleaner approach to vud_user_votes() though.

drewish’s picture

Luna Vulpo, I don't think you should be changing vud_update_6200(). It's designed to be run in D6… it should probably be removed and replaced by a hook_update_last_removed() that documents it's been removed.

marvil07’s picture

Status: Needs work » Postponed

As mentioned on comment 17, the real reason I am not applying this changes is because I want to move to "the field way", which is basically use vud_field as the base for all.

Postponing again. This issue probably will become "Port vud_node on top of vud_field", when #1363928: Upgrade paths from vud_{node,comment,term} to vud_field is ready.

rogical’s picture

very nice, very need, very D7 way

univate’s picture

FileSize
9.04 KB

For anyone using this, here is an updated patch for to latest dev.

klucid’s picture

Hi all,

How can I sponsor development of this module for D7? I really need this to launch one of my sites. I'm not really sure how "sponsoring" works.

Thank you,
J.R. Merlan
Klucid.com

DamienMcKenna’s picture

@klucid: Find someone who's willing to work on it and agree upon a fee.

klucid’s picture

Is this the proper thread to discuss? I posted to Drupal Paid Services last week: http://drupal.org/node/1402818)

rogical’s picture

guys are running to get this module on D7, may think of flag first, or rate, they can provide similar features.

drewish’s picture

@klucid, you'll really want to get buy in from the maintainer. i'd done a little work on this and they weren't really receptive to changes at this point. so the risk is that you'll pay someone to work on it, getting working on your site and then have the module go in a different direction and then find yourself with a custom fork of it.

klonos’s picture

I'm proposing to create a 2.x branch for the new field-based approach and have the 1.x branch be more of a straight port of the module from D6.

klucid’s picture

Thanks all, for your input. I think I'm going with the Rate module. it seems to do exactly what I need.

http://drupal.org/project/rate

sensei-master’s picture

Subscribe.

soulfroys’s picture

Welcome @modua! Please read this: Stop subscribing, start following

therobyouknow’s picture

Still see this problem myself the last entry was in March 2012 and the latest development release of the module is March 2013, so I would assume that any work from this issue would be in the module by now - correct?):

Notice: Use of undefined constant NODE_BUILD_PREVIEW - assumed 'NODE_BUILD_PREVIEW' in vud_node_node_view() (line 147 of ../sites/all/modules/vote_up_down/vud_node/vud_node.module).
Notice: Use of undefined constant NODE_BUILD_SEARCH_INDEX - assumed 'NODE_BUILD_SEARCH_INDEX' in vud_node_node_view() (line 148 of ../sites/all/modules/vote_up_down/vud_node/vud_node.module).
Notice: Use of undefined constant NODE_BUILD_SEARCH_RESULT - assumed 'NODE_BUILD_SEARCH_RESULT' in vud_node_node_view() (line 149 of ../sites/all/modules/vote_up_down/vud_node/vud_node.module).
Notice: Use of undefined constant NODE_BUILD_RSS - assumed 'NODE_BUILD_RSS' in vud_node_node_view() (line 150 of ../sites/all/modules/vote_up_down/vud_node/vud_node.module).
Notice: Undefined property: stdClass::$build_mode in vud_node_node_view() (line 152 of ../sites/all/modules/vote_up_down/vud_node/vud_node.module).
Notice: Undefined variable: a3 in vud_node_node_view() (line 161 of ../sites/all/modules/vote_up_down/vud_node/vud_node.module).

Diogenes’s picture

Status: Postponed » Active

I hereby volunteer to co-maintain this module.

The first time I tried Drupal (Version 6) I was blown away by the sophisticated user registration, comments, voting and privileges (roles). Everything seemed to work in D6.

Drupal 7 was released and, like a damn fool, I jumped on that wagon. It has been a long wait for updates.

I have a patch for vud_node.module that gets things working in the D6 fashion -- that is by adding widgets via the vud admin page (instead of adding field widgets to a content type). Both methods seem to work, even together. So it is possible to have two independent vote widgets if both are enabled. This is also a stupid idea -- but it's possible.

I am preparing another patch for the votingapi module - The "Vote results: value" did not work for me using Views. The vote results were always zero. The patch is simple and surprising.

This has all been in pursuit of a Drupal StackOverflow project. It's almost there. Still testing.

So if anyone is interested, say so. Even better, think about sponsoring a really worthwhile project.

marvil07’s picture

It has been a while without news here.

The main reason behind unifying into less modules was to be able to maintain this project better.

So, I will agree patches for vud_node if someone wants to co-maintain all vud_{node,comment,term} modules(and then work on other modules for 7.x-1.x), and if they go well, I will be happy to:

Diogenes’s picture

I agree that it would be good to hive off into different modules but there is the widgets folder -- needed by every breakout module. Should we make it a library? It has some great work in it.

I had the vud_node.module working before I even knew about the vud field option. The vud_comment seems to work though I have not tested it much. I have not tried the vud_term module yet. I'm not sure how that would be used.

I chopped out code for adding a voting widget to a teaser because it was causing problems that I did not care about. I don't think voting on a teaser is a good idea. Voting based on just a bit of information? Not the whole story? Bad practice IMO. It also complicates code.

I need the vud_ node and vud_comment modules working for my own project so I'll commit to getting those working properly first.

I'll post a patch for the vud_node.module next. If anyone has a compelling use case for having a vote widget on a teaser, please say so, otherwise it goes. I like pruning code more than writing it.

-Dio

Diogenes’s picture

@therobyouknow and others

I have attached a patch for the vud_node.module (7.x-1.x ) that should fix the error messages you reported and the WSOD problems found elsewhere. I have also attached the complete vud_node.module which you can test simply by replacing the old with the new (rename as necessary). The new one is smaller.

As mentioned previously, I chopped the code to add a vote widget to a teaser (saved time).

On testing, I discovered a problem with the node options screen. The options screen magically adds a new node type to its selection list after a new content type has been added (screenshot attached). I thought that a really cool feature.

A minor drawback to this approach is that you can select only one kind of widget for all content types. A MAJOR drawback is the the vote widget you select appears on every content type page, as long as just one is selected.

I did give the Vote Up/Down field a try at that point. It has a few issues but my overall impression is that it's like a 27 function Swiss army knife that is too big to carry around for a day hike. There are too many options that are not needed in most contexts and that serve to confuse. It seems contrary to D7 extensible design.

I liked the idea of being able to select a different widget for different content types (like comments). So I decided to try to implement a vud_node configuration in a table form for the vud_node module. Attached is a screenshot of HTML prototype to give you an idea. Implementing this as a Drupal form has been a challenge. The Form API does not handle tables well. I got some very strange results.

I am making progress but I am not quite there yet. It would help to get some encouragement and feedback here. There are 51 following this thread but I not sure how much interest there is any more.

lunk rat’s picture

@Diogenes: Thanks for your effort! I would love to see this as a mature D7 module. There is no other module that offers this functionality, right?

Lunk Rat

therobyouknow’s picture

Definitely some interested here @Diogenes - thank you for your efforts with fixing this issue! I would be happy to test and feedback.

nikkwong’s picture

Definitely interested but have no idea what you're doing. Keep up the good work

marvil07’s picture

Status: Active » Needs work

@Diogenes: Thanks for the new patch, I tried to make a quick review. In general, please make sure new patches are re-using code from latest patches other contributors added before.

+++ b/vud_node/vud_node.module
@@ -128,80 +102,31 @@ function vud_node_admin_settings() {
-  // avoid showing the widget in some node builds
-  $exclude_modes = array(
-    NODE_BUILD_PREVIEW,
-    NODE_BUILD_SEARCH_INDEX,
-    NODE_BUILD_SEARCH_RESULT,
-    NODE_BUILD_RSS,
-  );
-  if (in_array($node->build_mode, $exclude_modes)) {
-    break;

I do not see the point in removing this code. Please notice that previous patches changes this already, you should be using that code.

+++ b/vud_node/vud_node.module
@@ -229,20 +154,24 @@ function vud_node_tracker() {
+      $voter = '<span class="username" xml:lang="" typeof="sioc:UserAccount" property="foaf:name" datatype="">' .$account['name'] . '</span>';
       $rows[] = array(
-        theme('username', $account),

Manual html there is not needed, please use username theme function as the old code has been doing.

+++ b/vud_node/vud_node.module
@@ -254,44 +183,23 @@ function vud_node_tracker() {
 /**
  * Implementation of hook_link().
  */
-function vud_node_link($type, $object, $teaser = FALSE) {
+function vud_node_link($type, $object) {
   $links = array();

AFAIK there is not hook_link() in D7.

A minor drawback to this approach is that you can select only one kind of widget for all content types. A MAJOR drawback is the the vote widget you select appears on every content type page, as long as just one is selected.

That sounds like a bug on the code showing the widget, and if there is, should be fixed in this patch.

I did give the Vote Up/Down field a try at that point. It has a few issues but my overall impression is that it's like a 27 function Swiss army knife that is too big to carry around for a day hike. There are too many options that are not needed in most contexts and that serve to confuse. It seems contrary to D7 extensible design.

That's fine if you decide to not use that module, but as I mentioned before, the next major version will be using vud_field to replace all other vud_* modules. And BTW extending the module to be a field was about embracing D7 field ability.

I liked the idea of being able to select a different widget for different content types (like comments). So I decided to try to implement a vud_node configuration in a table form for the vud_node module. Attached is a screenshot of HTML prototype to give you an idea. Implementing this as a Drupal form has been a challenge. The Form API does not handle tables well. I got some very strange results.

That sounds good, but it sounds like a bad idea to be doing that with this issue, it should be done later to avoid confusion.

In the future, please mark the issue as "needs review" when adding a new patch. Details on issue queue documentation.

Also, as mentioned before, I will accept this patch if the author decides to co-maintain all vud_{node,comment,term} modules. But on this issuem please just focus on vud_node code.

Diogenes’s picture

Status: Needs work » Needs review
FileSize
37.81 KB
761.18 KB
711.15 KB

@lunk_rat, therobyouknow & nikkwong and any others who might be interested...

Attached is an updated vote_up_down module (7.x-1.x-dio). It can be installed in the normal manner or unzipped over an existing 7.x-1.x-dev vote_up_down directory without impacting any vud_fields that may have been added with 7.x-1.x-dev release.

Only the code in the vud_node directory has changed (with one small exception). So unzipping the 7.x-1.x-dev archive over the 7.x-1.x-dio install will restore your 7.x-1.x-dev code.

This version has a new vud_node configuration page (see attached screen shot) which allows any one of 5 vote widgets to be selected for each content type. The configuration page is a table form that automatically adjusts to the addition and deletion of new content types (see /admin/config/search/voteupdown/options).

The vud_node module should now work and be easy to setup and test. A vote tracking tab page will appear on each node page to help you track voting. I think everything is working now.

Please take a few minutes to test it out. If it works and you like it and if you want it to be supported, change the status below to RTBC and offer a few words of support.

If something does not work, let me know and I'll get on it.

Diogenes’s picture

@marvil07

The NODE_BUILD_PREVIEW problem goes back at least two years. It's leftover cruft from D6.

The code base I was originally working from was the 7.x-1.x-dev release (2013-Mar-04) downloaded from the project page. I got the same errors that have been previously reported. I then went at the code with a machete and got things working in an hour or two.

I then download the 7.x-1.x branch from drupal.org vote_up_down project page, created a new branch and applied the changes to the branch. The patch was made as the difference between the 2 branches.

The only file that changed was vud_node.module. I hacked out the code for the teaser because I did not need it and it was causing problems. The file went from 333 lines to 241 lines. The error messages and assorted WSOD all disappeared.

I acknowledged that the operation of the new vud_node.modue was not perfect, but at least it worked. The latest release corrects those problems.

You are correct about the the vud_node_links() not being a proper hook. I missed that on the first round. I didn't write that code either. It wasn't called anywhere so it wasn't being called at all. It's gone now.

The same was true for vud_node_content_extra_fields() -- not quite a real hook and not called anywhere else. GONZO baby.

Chopping vud_node_links() and vud_node_content_extra_fields() shaved another 58 lines of code from vud_node.module. Since then I have also hacked out the vud_node/views subdirectory and the 2 files inside it.

I've added about 58 lines of new code throughout the vud_node.module. An include file for the new form and a test template for new widgets have also been added (an aid for future development).

I could provide a patch file for all this, but it would quite large by now. Doing a code review of the patch would be pointless. The only way to really test this release is to TRY IT OUT.

As far as I can tell, this release above will not have any side effects on the vud_field, vud_comment or vud_term modules.

I can easily merge my local branch with the current 7.x-1.x from the drupal project repository and push the results up for a community wide review.

I await your decision.

Diogenes’s picture

marvil07 wrote:

+++ b/vud_node/vud_node.module
@@ -229,20 +154,24 @@ function vud_node_tracker() {
+      $voter = '<span class="username" xml:lang="" typeof="sioc:UserAccount" property="foaf:name" datatype="">' .$account['name'] . '</span>';
       $rows[] = array(
-        theme('username', $account),

Manual html there is not needed, please use username theme function as the old code has been doing.

Here is a patch that will fix that:

diff --git a/vud_node/vud_node.module b/vud_node/vud_node.module
index 5490151..6d0937d 100644
--- a/vud_node/vud_node.module
+++ b/vud_node/vud_node.module
@@ -142,19 +142,20 @@ function vud_node_tracker() {
     $rows = array();
 
     foreach ($votes as $vote) {
+      // OK - some wierd decontruction/reconstruction needed here for get the
+      // correct voter format for theme('username', $voter). A little ugly
+      // but it seems to work.
       $voter_uid = $vote['uid'];
-      $account = (array)user_load($voter_uid);
-
-      $voter = '<span class="username" xml:lang="" typeof="sioc:UserAccount" property="foaf:name" datatype="">' .$account['name'] . '</span>';
+      $voter = array('account' => user_load($voter_uid));
       $rows[] = array(
-        $voter,
+        theme('username', $voter),
         $vote['value'],
-        array('data' => format_date($vote['timestamp'], 'small'), 'class' => 'nowrap')
+        array('data' => format_date($vote['timestamp'], 'small'), 'class' => 'nowrap'),
       );
     }
     drupal_set_title(check_plain($node->title));
     $output = theme('table', array('header' => $header, 'rows' => $rows));
 
     return $output;
   }

therobyouknow’s picture

just a quick note of further support - thank you very much - I will test this out and get back to you. (I am finishing the final touches to my Vagrant VM based development environment for Drupal development, with Komodo for debugging - see my write up here: https://drupal.org/node/2055947 - may be of interest and use, feedback welcome.) Will also get back to you about vud though. Cheers.

Diogenes’s picture

I am posting an updated Vote Up/Down module along with the patch to apply to the 7.x-1.x branch of

https://drupal.org/project/vote_up_down.

A previous version has been marked RTBC by oneshot. This is encouraging.

https://drupal.org/node/749438#comment-7716913

This version should only effect the behavior of the vud_node module. The other sibling modules -- vud_field, vud_comment and vud_term should behave as before.

There is a new README file in the vud_node directory that summarizes the changes to the module.

The patch file is rather large. It might be easier to apply the patch and try out the new code rather than reviewing the code changes. A quick directory compare with meld or windiff will show that the other modules remain untouched.

My offer to co-maintain this module still stands. I hope I have demonstrated that I know what I'm doing and that I pose little to no risk to this project; that I can contribute in a positive way.

I'm not sure what more I can offer. Or what more I can do.

Diogenes’s picture

Issue tags: +voting
FileSize
32.93 KB
63.94 KB

I thought maybe I could edit and attach. I was wrong.

Here is what was supposed to be attached in the previous post.

MXT’s picture

Status: Needs review » Reviewed & tested by the community



I've just replaced vud module with vote_up_down_D7_AUG5.zip module provided by Diogenes in #64 and I can confirm that all works well for VUD on NODES.

All previous errors like:

have disappeared.

Thank you very much for your work!

+1 for Diogenes as co-maintainer for this module (may be this can give a sprint in its development...)

But I'm a bit confused now: I'm developing a very complex site with achievements module with custom logic on voting api: at the moment al works well checking for votes on comments and so on... BUT, due to https://drupal.org/node/1295574 do you think guys I have to definitively abandon vud_node and vud_comment in favour of vud_field?

Thank you very much

Diogenes’s picture

@MXT - thanks for the review and your support.

vud_node/vud_comment and vud_field are just different approaches to adding voting to a site. One can be used, or the other, or even both if the admin is careful about configuring. Which method you choose is a matter of preference.

There is no reason to abandon vud_node/vud_comment if they work. I prefer them myself because they are a little easier to undertand and configure.

One advantage of the vud_node/vud_comment modules is that they have been carried forward from D6, so it is easier to migrate from D6 to D7 with these modules. And there is no reason why these modules can't be migrated D8.

So vud_node/vud_comment won't be abandoned if they work and are being used. Not if I can help it.

MXT’s picture

So, @Marvil07, in reply to your #52:

So, I will agree patches for vud_node if someone wants to co-maintain all vud_{node,comment,term} modules (and then work on other modules for 7.x-1.x), and if they go well, I will be happy to:

  • Start a new 7.x-2.x branch for converting to fields (and therefore #1363928: Upgrade paths from vud_{node,comment,term} to vud_field and #1295574: Remove vud_{node,comment,term} modules in favour of vud_field will be moved there), as suggested before and as I originally wanted but couldn0t because of time.
  • Give co-maintenance access to someone contributing here and who want to take care of all vud_node, vud_comment and vud_term in 7.x-1.x, while I maintain 7.x-2.x and 7.x-1.x vud and vud_field.

that "someone" who can co-maintain is Diogenes, who has already proposed himself as co-maintainer and has already demonstrate that he can manage all stuff here.

Are you agree to give him co-maintenance access?

marvil07’s picture

Priority: Major » Normal
Status: Reviewed & tested by the community » Needs work

I agree that it would be good to hive off into different modules but there is the widgets folder -- needed by every breakout module. Should we make it a library? It has some great work in it.

The widgets folder is part of vud module, which is the one providing the "widgets"(ctool plugins for vud widgets), maybe that's not clear since it's in the main folder, but it can be noticed after reading vud's hook_ctools_plugin_directory().

You are correct about the the vud_node_links() not being a proper hook. I missed that on the first round. I didn't write that code either. It wasn't called anywhere so it wasn't being called at all. It's gone now.

hook_link() logic should just be moved, not removed, see the port note on at Move node, taxonomy, and comment links into $node->content; Deprecate hook_link()

The same was true for vud_node_content_extra_fields() -- not quite a real hook and not called anywhere else. GONZO baby.

That hook was a CCK hook, and in D7 in core was renamed to hook_field_extra_fields(), so it should not be removed either.

@MTX: The errors you mentioned in comment 65 were mostly solved in patch at comment 40. About abandoning vud_node in favor of vud-field, it's a choice. As mentioned by Diogenes, you could use either of them, but IMHO (and that's why I created that issue) we should move to core fields, but that's my suggestion.

I have made another review mainly looking at the code, not really on UI, and I do not really like some of the changes introduced by the last patch, and it is missing some of the suggestions I have already made.

Let me list them here:

  • It does not contain several fixes already provided in the last patch from #1194274-40: Port vud_node to D7. At least two: drupal_uninstall_module() fix and prevent showing the widget on some node builds fix.
  • Why removing the custom views field provided by vud_node? (Also it makes no sense to change the @hook_views_api()@ to use view 3 after that)
  • Why remove the hook_update_N @vud_node_update_6201()@?
  • I mentioned before that it is not a good idea to add features like changing the way the configuration is set on this issue, but it has been ignored. Also, the implementation of the new way of doing that IMHO is not really mature to be included upstream:
    • It is building a form manually and ignoring drupal forms API, that should be avoided. I understand that is not easy, but it is a really bad idea to do it manually. BTW the form submit is probably not fired because the form is not build using form api.
    • It is not usign ctools related functions to retrieve all widgets, but instead hardcoding them. Yes, it is mentioned on README, but that is just not using the internal vote up/down API correctly.
  • Why add a prototype file(widgets_table.php) to the main tree? It's direct php that should not be callable.
  • Why change the url fro vud_node options to admin/config/search/voteupdown/options, that sounds like global configuration, not node specific. The same for other metadata for this menu entry.
  • Why removing the pager from vud_node_tracker callback? FTR in the future(not at port) that should just be a view.
  • Several drupal coding standards bugs.

Other than that I can point several minor problems in the code too:

@@ -0,0 +1,81 @@
+This README addresses the *Port vud_node to D7* issue

In general I would recommend to change the way the README is written to stop assuming a patch.

@@ -0,0 +1,81 @@
+All of these sub-modules reference the code in the 'widgets' subdirectory.
+
+'widgets' has the code, stylesheets, and images to generate and trigger
+each of the different voting widgets. Currently there are 5 different vote
+widgets included with the module.

They do not really "reference" the widgets directory. They depend on vud module, which provides those default widgets, which again are ctools plugins.

@@ -77,136 +73,48 @@ function vud_node_tab_view_stats($node) {
-  if (in_array($node->type, variable_get('vud_node_types', array()), TRUE)) {
-    return TRUE;
-  }
+  return TRUE;

This logic needs to be re-writen/re-used, not by passed.

@@ -229,20 +137,26 @@ function vud_node_tracker() {
-    $criteria = array('entity_type' => 'node', 'entity_id' => $node->nid, 'tag' => $tag);
+    $criteria = array('entity_type' => $node->type, 'entity_id' => $node->nid, 'tag' => $tag);

It's fine to use node as entity_type, since that's what vud_node module cares about, so no need to change that.

@@ -229,20 +137,26 @@ function vud_node_tracker() {
-      $account = user_load($vote['uid']);
+      // OK - some weird decontruction/reconstruction needed to get the
+      // correct parameter format for the theme('username', $voter) call.
+      // Ugly and counter-intuitive but it seems to work.
+      $voter_uid = $vote['uid'];
+      $voter_info = array('account' => user_load($voter_uid));

This was already solved in a previous patch.

In summary, I think this code is not ready to be added yet.

Diogenes’s picture

Here is another summary:

  1. No D7 release of the vud_node module has ever worked.
    If one takes the time to go through the project commit history and the issue queue, this will become evident. Others have posted patches in the past, but they don't seem to make it into any release.
  2. The project page indicates that a module co-maintainer is needed.
  3. An offer to co-maintain is made. A series of patches are posted. Support is expressed for this effort by 5 others with two members giving it an RTBC (reviewed and tested by the community) seal of approval. Another has suggested that it's probably best if I was promoted to co-maintainer.
  4. The patch is "code reviewed" and a bunch of points are raised which are mostly trivial and sometimes ridiculous. The status is set back to "needs work". The project maintainer does not seem to care about points 1, 2 or 3.

The contributions of others are often ignored. In my case it is clear nothing I do will ever be deemed "good enough" for marvil07.

It's too bad that the drupal.org website does not have any way of putting matters like this to a vote. I'd even be willing to stand for election as a project maintainer based on the demonstrated value I could bring to the project.

And it's more than a little ironic that this is all about a broken and neglected module that is part of the Vote suite.

klonos’s picture

I would simply +1 #69 as a whole, but I really doubt it would help in any way other than perhaps upset marvil07 (even more). So, I'll refrain from making any comments other than:

@Diogenes:

It's too bad that the drupal.org website does not have any way of putting matters like this to a vote.

I honestly feel your frustration Jim. FYI: the "voting_up_down" or "votingupdown" (even "vud") namespaces are available and would be in tandem with "votingapi" that's used by the Voting API project. Also, you could kindly ask the Voting API maintainers to list your project to their "some examples of simple voting systems based on VotingAPI" section. Just saying mate ;)

Michael-IDA’s picture

@Diogenes:

Concur w/ klonos, skip the melodrama. It's been entirely too long, give the appropriate credits, but, please just fork this to a working project.

If you want to follow a precedented usage on namespace, use "vote_up_down_2"

Best,
Michael

jwilson3’s picture

This is classic:

And it's more than a little ironic that this is all about a broken and neglected module that is part of the Vote suite.

But, what if you just "bite the bullet" -- address the changes requested by the maintainer in #68, and see what happens next?

thermador’s picture

Well, #64 doesn't work for me. I get an AJAX error when trying to upvote a node.

Reading this thread makes me sick. What happened to "the community" eh?

marvil07 get off your ass, and down off you high horse, and fix this thing. Or just watch it die I guess.

I'm moving to the "Rate" module, and I suggest others to do the same.

An AJAX HTTP error occurred.
HTTP Result Code: 200
Debugging information follows.
Path: /vote/forum/2191/1/voteupdown/thumbs/XF3AU35sLvKulg7Nf46MPUBgogeo6FNaD0WlPdOB1tA/ajax
StatusText: parsererror
ResponseText: 
Fatal error:  Class name must be a valid object or a string in /path/to/drupal/includes/common.inc on line 7813
klonos’s picture

Issue summary: View changes
Status: Needs work » Closed (duplicate)
Parent issue: » #1360572: Roadmap for D7 of Vote up/down
Related issues: +#749438: Port Vote Up/Down to D7

@thermador: You honestly need to cool down mate. I am as frustrated about this issue as you, but you need to always keep in mind that the modules you use are created at the expense of someone else's free time and given to you (and all of us for that matter) for free. These people are not obliged to work for us on demand - especially when we talk to them like you do.

In your comment you never ask if Marco has free time and you assume that everything in his personal life is ok. To honest, we can never know if module developers are plagued by issues with their family, work, health etc. So, be a part of this "community" and be a little considerate about others.

If you take a look at the module's page you'll see that the project's maintenance status is set to "seeking co-maintainer(s)". There is no dedicated issue about that, but at least one use has expressed interest in the past (see my recent comment in #749438-137: Port Vote Up/Down to D7).

Now about the "under active development" development status, head over at #1134450: Automatically degrade maintenance and development status of projects over time and freely express your opinion (politely please) so we can do something about such matters globally in d.o.

Anyways, we already have too many issues about the D7 version of the module and splitting action between places is confusing and unproductive. Feel free to copy any comment worth it from here to #749438: Port Vote Up/Down to D7 if needed but lets focus on a single issue.

Diogenes’s picture

My apologies for leaving in a huff last time and then not replying. That was rude.

Klonos, Michael-Inet & jwilson3, your suggestions or words of support were appreciated. I deferred on all because, at the time, I was only permitted to create sandbox projects, which are almost invisible and certainly viewed as second class citizens.

I was also awaiting for approval on a contributed module. That experience was more frustrating than this one.

Thermador - I'm with you on the Rate module recommendation.

I'm with you on a few more things you posted about but I'll just leave it at that. Sorry that patch didn't work but you know how it is. I blame the NSA for everything now. ;-)

Diogenes’s picture

Attempting to tie two loose threads together...

https://drupal.org/comment/8354023#comment-8354023