I edited one of the Lullabot podcasts to change the description, and after doing so the play/download counts were reset back to 0. :( That ain't good. :(

We're using:

// $Id: audio.module,v 1.105 2007/03/07 18:56:34 drewish Exp $

I see that there's one version newer of that file, but it didn't seem to touch any code about that.

CommentFileSizeAuthor
#14 audio_count.patch2.12 KBmfb
#8 count_2.patch9.8 KBwebchick
#7 count_1.patch9.79 KBwebchick
#4 count_0.patch8.08 KBwebchick
#3 count.patch7.42 KBwebchick

Comments

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Title: Re-saving audio node resets play and downloads counts. :( » Play and download counts should not be versioned?

Now using brand-spankin' new 5.x-2.x-dev, problem still persists.

The problem is actually tied to the use of revisions. When I check the audio table, all the counts are there, they're just associated with only the particular vid.

Possible solutions:
1) Don't version these fields. Have them always take the total.
2) Have the play/download counts display a the sum of all download/play counts for that nid.

mfb’s picture

mfb
they or he
commented

My suggestion would be a separate table for the counts, with nid as the key.
This could also improve db performance on very busy sites, since the frequent writes would only be happening to the audio_count table.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Active » Needs review
StatusFileSize
new count.patch7.42 KB

Good idea! Here's a patch. Because it drops the play/download count columns from audio, it is destructive and should be thoroughly tested on a test database ;)

Thanks very much for luke-jr in #mysql on irc.freenode.net for help on the tricky query to update the play/download counts. You rule! :D

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
StatusFileSize
new count_0.patch8.08 KB

Oops. Forgot about the insert case.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Title: Play and download counts should not be versioned? » Re-saving revisioned audio node resets play and downloads counts.

Fixing title.

mfb’s picture

mfb
they or he
commented

I think views_audio.inc will also need to be updated to reflect the schema change.
also i think everything could be $ret[] = update_sql, including inserts and create tables?

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Needs review » Needs work
StatusFileSize
new count_1.patch9.79 KB

Here's a patch which seems like it should work but really doesn't. ;)

a) the drop columns are failing... no idea why, since I didn't change them since the previous patch where they were working.
b) Views doesn't "get" the schema change, even after I cleared the cache. :(

Help? :(

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
StatusFileSize
new count_2.patch9.8 KB

Cool, this gets rid of the update errors (duh!). Views is still a problem.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

To reproduce, click "Add" next to the audio view. Make it a table view, and add the "play count" and "download count" fields. Navigate to ?q=audio and see:

user warning: Unknown column 'audio.play_count' in 'field list' query: pager_query SELECT node.nid, node.created AS node_created_created, node.title AS node_title, node.changed AS node_changed, audio.play_count AS audio_play_count, audio.download_count AS audio_download_count FROM node node LEFT JOIN audio audio ON node.vid = audio.vid WHERE (node.type IN ('audio')) AND (node.status = '1') ORDER BY node_created_created DESC LIMIT 0, 10 in /Applications/MAMP/htdocs/5x/includes/database.mysql.inc on line 172.
mfb’s picture

mfb
they or he
commented

I actually don't have any database errors.. perhaps it was cached from before you applied the patch?

mfb’s picture

mfb
they or he
commented

I'm not sure what's "normally" done in these cases, but it might also be a good idea for this patch to update the tablename in in the view_tablefield table, and clear out all the views caches.

rjleigh’s picture

rjleigh commented

I found this bug too - is there a reason why this patch (or another fix) never got into the codebase?

There doesn't seem to be audio revision support in general - there's no tab added to the node when revisions are available.

mfb’s picture

mfb
they or he
commented

the one question I had about this patch was, would the update script also need to update the tablename columns in the view_* tables? I'd assume so... (I wonder if there's been another similar case of new data structure for a contrib module, with views already having been defined using the old structure)

mfb’s picture

mfb
they or he
commented
Status: Needs work » Needs review
StatusFileSize
new audio_count.patch2.12 KB

Here's a lighter-weight patch that simply resolves the issue without changing table structure.

drewish’s picture

drewish commented
Status: Needs review » Fixed

thanks, committed to HEAD.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

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