The table {node_counter} is only used and modified by the statistics.module, but it is installed by the node.module. Let's switch the table to statistics.install so it can be uninstalled along with the statistics.module.

CommentFileSizeAuthor
#6 440768-followup-D7.patch644 bytesdave reid
#3 440768-statistics-node-counter-table-D7.patch6.62 KBdave reid
#1 440768-statistics-node-counter-table-D7.patch7.04 KBdave reid

Comments

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Title: Transfer ownership of the {node_counter} table from node.module » Transfer the {node_counter} table from node.module
Assigned: Unassigned » dave reid
Status: Active » Needs review
StatusFileSize
new 440768-statistics-node-counter-table-D7.patch7.04 KB

Having this table owned by statistics.module will also allow us to potentially do some cool things with adding a counter API to allow others things to be 'counted' and not just nodes. Anyway, patch ready for review with update functions included.

j.somers’s picture

j.somers commented
Status: Needs review » Needs work

Calling update.php fails with the following error message:

An error occurred.
http://localhost/drupal/update.php?id=35&op=do

Fatal error: Call to undefined function drupal_get_install_schema_version() in /var/www/drupal/modules/node/node.install on line 389

I am unable to find this function on the d.o API site, nor in any of the files on the CVS repository and no results on Google.

I have no idea why the patch might have worked on the test server and requested a retest, is it possible update.php is not called when applying a patch?

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Needs work » Needs review
StatusFileSize
new 440768-statistics-node-counter-table-D7.patch6.62 KB

Whoops. Yeah, the testing bot doesn't run update functions. *wish it did*

Revised, fixed patch.

j.somers’s picture

j.somers commented
Status: Needs review » Reviewed & tested by the community

This one applied fine, update.php ran fine and everything still works OK.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Nice one. Committed to CVS HEAD. Thanks!

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Fixed » Needs review
StatusFileSize
new 440768-followup-D7.patch644 bytes

Just noticed one thing...currently if the user has the statisitics.module disabled, but not uninstalled, the current node_update function will remove the node_counter table since drupal_get_installed_schema_version('statistics') will return 0. Revised followup patch checks if the schema equals SCHEMA_UNINSTALLED (-1).

dries’s picture

dries commented
Status: Needs review » Fixed

Committed to CVS HEAD. Thanks for the correction.

Jaza’s picture

Jaza commented
Status: Fixed » Needs work

If this table is now owned by statistics.module, then shouldn't it be renamed to have a 'statistics_' prefix? I.e. rename it to 'statistics_node_counter'.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Needs work » Fixed

Jaza, that's the next step I was going to do (but in a separate issue). I've actually been thinking about renaming/abstracting the table so it can be used for more than just nodes. See #103866: Add a general counter API.

Status: Fixed » Closed (fixed)

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