A recent improvement in this module was the sharing of the same imagecache preset defined for the facebook_status module. This results in some wacky behavior if that module is not used, however.

I think a good solution would be a selector for imagecache preset for user photos on this module's settings page.

CommentFileSizeAuthor
#7 activity-log-imagecache-1613522-7.patch765 bytesicecreamyou
#6 activity_log-imagecache-1613522-6.patch2.33 KBjessehs
#3 activity_log-imagecache-1613522-3.patch2.88 KBjessehs
#2 activity_log-imagecache-1613522-2.patch2.88 KBjessehs
#1 activity_log-imagecache-1613522-1.patch2.54 KBjessehs

Comments

jessehs’s picture

jessehs
he/him
Primary language English
Location Taos, New Mexico
commented
Status: Active » Needs review
StatusFileSize
new activity_log-imagecache-1613522-1.patch2.54 KB

I'm not too familiar with this module. Perhaps there are other places this variable could be used?

jessehs’s picture

jessehs
he/him
Primary language English
Location Taos, New Mexico
commented
StatusFileSize
new activity_log-imagecache-1613522-2.patch2.88 KB

I was thinking about this, and it seems like querying imagecache (and all modules who hook in at that point) could potentially be a performance hit. Here's a reworked patch that stores the default imagecache preset in a static variable, so that once it's calculated, it stays calculated for that particular bootstrap.

jessehs’s picture

jessehs
he/him
Primary language English
Location Taos, New Mexico
commented
StatusFileSize
new activity_log-imagecache-1613522-3.patch2.88 KB

Empty, not empty, it doesn't really matter does it?
;-)

icecreamyou’s picture

icecreamyou commented
Status: Needs review » Needs work

Thanks for the patches.

I was thinking about this, and it seems like querying imagecache (and all modules who hook in at that point) could potentially be a performance hit.

"Premature optimization is the root of all evil." Donald Knuth :-)

+++ b/activity_log.module
@@ -500,8 +500,25 @@ function activity_log_token_values($type, $object = NULL, $options = array()) {
     if (module_exists('imagecache_profiles') && module_exists('facebook_status')) {
       $account->imagecache_preset = variable_get('facebook_status_imagecache_preset', variable_get('user_picture_imagecache_profiles_default', ''));
     }

This code is basically what you have a problem with, right? My preferred solution here would be to just use variable_get('user_picture_imagecache_profiles_default', '') as the imagecache preset instead of adding yet another setting for it.

jessehs’s picture

jessehs
he/him
Primary language English
Location Taos, New Mexico
commented

Hehe, that's fine by me. I just wasn't sure if there was any problem with passing in an empty imagecache preset up until someone picks a default.

jessehs’s picture

jessehs
he/him
Primary language English
Location Taos, New Mexico
commented
Status: Needs work » Needs review
StatusFileSize
new activity_log-imagecache-1613522-6.patch2.33 KB

Here's what you suggested in #4.

Edit: er... I guess I misread your post. This is not what you suggested, although it does get rid of the static caching.
I see your point about using the profile picture default. That seems like an ideal solution.

icecreamyou’s picture

icecreamyou commented
StatusFileSize
new activity-log-imagecache-1613522-7.patch765 bytes

Heh. So here is the patch that does what I suggested. However I think that this doesn't actually do anything because technically if Imagecache Profiles is installed it should take over theme('user_picture') and render the correctly sized user picture anyway, right?