Closed (won't fix)
Project:
Acidfree Albums
Version:
5.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
15 Aug 2007 at 21:50 UTC
Updated:
1 Jan 2011 at 03:17 UTC
Moving the second part of http://drupal.org/node/167582 here.
Tintiifax asked, how can i count the number of images in one album to show the number of images in a block?
Here's a patch that creates a block to show the item counts in AF albums.
| Comment | File | Size | Author |
|---|---|---|---|
| af_count_block.txt | 6.05 KB | nancydru |
Comments
Comment #1
vhmauery commentedA couple of things...
I would rather you didn't try to rearrange the if statement into a case statement in the same patch that is adding this feature. I can do that later when I integrate it. I want something that introduces minimal change so it is easy to review.
The idea is fine with me, though with large sites, a call to taxonomy_get_tree() is lethal.
The block content needs some help. You coded up a big no-no. You directly printed something that a user inputted. This is a major XSS hole. A user could create an album with a malicious title or description and get admin access to the site or worse. When dealing with user-created data, always print it to the screen with check_plain(). I would also suggest wrapping the album names in a call to l() so they can be links to the album rather than just plain text. To do this, you will need to get the tid-> nid mapping, which would require a call to
$album = _album_from_tid($term->tid)for each term that taxonomy_get_tree() returns. Once again, this incurs a lot of cpu overhead and is lethal for sites with hundreds of albums.And finally, the tree does not render properly. I get a never ending list of
tags ever get closed. I would probably use a recursive rendering mechanism, but that is just me. Do it however you want, but be sure to test it on a tree that has several branches.
Comment #2
vhmauery commentedhehe. I should really learn to preview first and submit later. The above should read And finally, the tree does not render properly. I get a long nested list of
<ul>blocks where neither the<ul>tags nor the<li>tags ever get closed. I would probably use a recursive rendering mechanism, but that is just me. Do it however you want, but be sure to test it on a tree that has several branches.Comment #3
nancydru"rearrange" - I ordinarily wouldn't do this, but I had to do it in the one spot to sort it out for myself. In hindsight, it probably wasn't necessary. But if you ever get to reworking that code, you might consider going to a switch/case format for ease of maintenance and readability.
"big no-no" - you're right, a significant oversight on my part. On my sites that use AF, all albums are created by the admin, so it's not a worry.
"idea is fine with me" - frankly, I wouldn't put this on my site; I think it's a lot of clutter with little value, but someone wanted it and I didn't feel like doing some real work...
"does not render" - hmm, my test site does have multiple branches and it works fine.
So, before I go the effort to fix what you see, I'd like to know that someone actually wants this.
Comment #4
mwheinz commentedClosing. 3 years of no activity.