Files download count and 'Top Downloaded Files' is currently GROUP BY f.filename, which is giving inaccurate stats, as filename can be repeated for different content types/content fields that are stored under different folders.
e.g.
files/images/a.jpg
files/a_field/a.jpg
files/a_field/2011/a.jpg
files/b_field/a.jpg

It should be GROUP BY f.fid instead, IMO.

CommentFileSizeAuthor
#1 download_count-group_by_fid-1251524-2.patch7.14 KBckng

Comments

ckng’s picture

ckng
Primary language English
commented
Status: Active » Needs review
StatusFileSize
new download_count-group_by_fid-1251524-2.patch7.14 KB

Another possible scenario where using f.filename could cause problem is case sensitive file system.

Attached patch
- changed to use GROUP BY dc.fid
- removed no longer needed GROUP BY f.filename
- corrected db_query to use parametrized queries (not user input, so not a sql injection risk, still..)
- simplify some db code replication

WorldFallz’s picture

WorldFallz commented
Status: Needs review » Postponed (maintainer needs more info)

my sloppy queries notwithstanding (i've learned A LOT since then, lol -- and love d7's db api), I'm not sure this is the right change. Obviously I have to clean up the query and account for mixed case filenames, but the same file attached to different nodes (and therefore having different fid's) would be counted separately.

I seem to remember this was done by filename to count the files regardless of to what they were attached. Seems more like it should be an option-- though in the d7 world, most likely this stuff will be provided by a view and can simply be customized.

let me know what you think-- and if anyone else has an opinion of counting by filename or fid please speak up!

WorldFallz’s picture

WorldFallz commented
Title: Inaccurate download count, should be GROUP BY fid » FEEDBACK PLS - Inaccurate download count, should be GROUP BY fid
ckng’s picture

ckng
Primary language English
commented
Version: 6.x-2.0 » 6.x-2.x-dev

In my case, the files although are of same filename are not the same file, and they could be attached to different nodes and content types as well.

Besides when displaying stats per file on a node, it should be by fid since only file downloaded through that node should be take into account?

The filename title case issue can be solved by converting all to lower case before saving on server side.
Imagine non-tech savvy users saving image by the default app filename, Image.jpg and all will ended up counted the same, not a very good idea, IMO.

WorldFallz’s picture

WorldFallz commented
Status: Postponed (maintainer needs more info) » Closed (works as designed)

I think you're correct about grouping on FID and in the future (probably the d8 version), I'll make the change. However, I'm not going to make this type of fundamental change to the existing versions at this point (particularly since no one felt strongly enough about to even comment on this issue, lol).

In the meantime, I've parametized all the sql queries regardless of whether or not they needed it. Much easier to keep track of that way. I should have a new point release of the d6 version and full release of d7 in the next day or so.

WorldFallz’s picture

WorldFallz commented

And, most importantly, thanks for contributing!