Hello, I've been testing this module and looks good.

Have you any plans for Drupal 7 version, or would you be okay for me to port to version 7?

Thanks!

Comments

scuba_fly’s picture

Since there is no reaction...
Did you start on the drupal 7 version?

I'm a bit new to drupal development but I wanna help porting this to D7.
I need something like this for my drupal7 site.
How do i start creating the D7 version? I'm experienced in PHP / mySQL and javascript. also using drupal6 and 7 for some months now. but it's time to really get involved ;)

thnx!

Tim Jones Toronto’s picture

StatusFileSize
new14.52 KB

Hi there! Cool.

I made an early start from latest 6.x dev. Notes as follows:

1. Run thru Coder and made recommended changes.
2. Created the basic 7.x structure so that it can be worked on (attached).
3. Next job is hook_menu() - change menu path to Version 7 http://drupal.org/node/719612
4. Then to change 'node_get_types(' line for admin form.
http://api.drupal.org/api/drupal/modules--node--node.module/function/nod...

I'm sorry I didn't get time to do more. I can do some testing if you want to carry on coding this?

Cheers,
Tim

hydra’s picture

subscribe

nkschaefer’s picture

StatusFileSize
new13.38 KB

Hi,

I picked up on this and have completed a working Drupal 7 version. I did make a couple of changes:

  • The pop_links_stats database table is now Views-integrated.
  • The administrative reports screens (at admin/reports/pop/links) are now provided by default views added by this module (this cut down on the amount of code needed in the module).
  • The Google Chart API module (chart module) is now longer required; the administrative screen that uses that module is now only accessible if you have the module installed, but there should be no errors if you don't. Also, that module looks like it's falling out of maintenance (like this one), and apparently there's a new, interactive Google Chart API - so maybe we can eventually come up with a more flexible chart, or provide charts through Views, or leave it up to individual site admins to set up their own charts using other, Views-integrated modules.
  • The number of clicks on a node is exposed as a pseudo-field via hook_field_extra_fields(), so it can be moved around by the administrator in the Field UI.
  • The text that shows the number of clicks on a node is now generated within a theme function, so it can easily be overridden.
  • The Javascript is now only added when nodes are being viewed, rather than in hook_init(). Since it only works with nodes (at least for now), it seemed overkill to add the Javascript to every page.
  • It no longer sends the base path to its own Javascript setting, since Drupal already keeps track of this in Drupal.settings.basePath.
  • The Javascript file now uses Drupal.behaviors, as Javascript files are expected to do.

Anyway, try it out and let me know if there are problems. I think it'll be handy to get this working.

Also, is anyone still maintaining this module? Should some of us apply for co-maintainership? And in the meantime, should I start a sandbox project with this D7 version?

Edit: I've also considered letting Views take over the block provided by this module. Would that be overkill?

Thanks,
Nathan

nkschaefer’s picture

Status: Active » Needs review
StatusFileSize
new13.44 KB

And of course, I forgot something.

I just added back in an access check I had commented out, and I also made sure the module cleans up after itself -- when a node is deleted, all associated data from this module, in the pop_links_stats table as well as the votingapi tables, is deleted as well.

Please use this version when you check it out (ignore the other file I uploaded).

Thanks,
Nathan

nkschaefer’s picture

StatusFileSize
new15.51 KB

After thinking about this some more, I realized that this is going to need some more flexibility. In the interest of making this more extensible (so we could include entity types other than nodes in the future), this now works off Javascript settings instead of automatically looking for node divs.

I also wanted to handle the case where a node's links are shown in Views rather than in the node body. To do this, I wrote a new Views handler that extends the one for Field API fields -- it adds the needed Javascript file and settings when the view is rendered so that users who click on a link in a views field belonging to a node will have their votes counted as clicks on the parent node.

Additionally, in the interest of future-proofing and increasing flexibility, I removed the "Popular links" block code and replaced it with another default view that builds that block. This way, if Voting API's code ever changes, this code won't break, as long as they keep their Views integration working.

I've added another updated (zipped) version of the module; I've also created a sandbox page at http://drupal.org/sandbox/nkschaefer/1238548 where you can all pull the project with git and create issues for suggestions/questions/patches/bugs/whatever.

I hope to get this version up on the main project page, if possible.

Tim Jones Toronto’s picture

Great Nathan! I have only just caught up on this, so will do some testing...

Thanks for your work.

Tim

Tim Jones Toronto’s picture

So far, I have the following errors with Drupal (7.7):

1. After enabling the module, it gives:

DatabaseSchemaObjectExistsException: Table pop_links_stats already exists. in DatabaseSchema->createTable() (line 629 of C:\xampp\htdocs\includes\database\schema.inc).

2. On adding a link (after node submit), it gives:

Warning: Parameter 1 to pop_links_node_view() expected to be a reference, value given in module_invoke_all() (line 819 of C:\xampp\htdocs\includes\module.inc).

Cannot get much further showing any Views data with above errors so far.

Cheers!

nkschaefer’s picture

StatusFileSize
new54.14 KB

Hey -

Thanks for testing this out. It's nice to get some feedback (after posting many patches and hearing nothing from anyone).

I can't figure out why the hook_install() is being called whenever the module is enabled - I was getting the same "table exists" error. As a quick fix, I added a check for !db_table_exists() before attempting to add the table.

As for the other error - it's weird that I wasn't getting it. Maybe we have different versions of PHP, and mine is more laid back about passing variables by reference or something. Anyway, I split the hook_node_view() into a hook_node_view() and hook_node_view_alter(), the latter of which can definitely handle receiving variables by reference. This seems to work just as well for me and should hopefully stop the errors on your end. Let me know if this works better.

If anyone cares, I also pushed the changes to the sandbox project, if you're using Git or whatever.

Thanks,
Nathan

nkschaefer’s picture

I should add that I also changed the code a little, so when counting the number of clicks, it selects the sum, not number, of all votes. This shouldn't matter for many people, but I'm currently importing stuff from another website and rather than creating a bunch of individual votes, I just imported all clicks-to-date as single, high-value votes. That change allows this to work well and shouldn't hurt anything for those using this module normally.

Tim Jones Toronto’s picture

Cool. I will test tomorrow I hope :) Bye for now.

nkschaefer’s picture

Status: Needs review » Fixed

Okay - I'm a maintainer now, and there's a 7.x dev release up. Please open new issues for any problems you encounter with the new release. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.