Can you create the AttachmentThumbnail preset using hook_imagecache_default_presets ?

CommentFileSizeAuthor
#9 itweak_upload_dataPersistance.patch720 bytesrbrandon
itweak_upload_imagecache_default_presets.patch4.78 KBferdi

Comments

iva2k’s picture

iva2k commented
Assigned: Unassigned » iva2k
iva2k’s picture

iva2k commented
Status: Active » Postponed (maintainer needs more info)

@ferdi
Please enlighten me for the benefits of this... Besides smaller code, what is the value added?

iva2k’s picture

iva2k commented

OK, had few minutes to try it out, here are the cons (-) and pros (+):

(+) code is smaller
(-) patch needed to use fuzz (but it is normal for a -dev code)
(-) user looses ability to delete the preset
(+) preset can be reverted to the default
(-) the patch removes install and update_6000... need to also patch README.txt
(+) preset works
(-) user has to enable view permissions for AttachmentThumbnail by hand
(-) Nobody chimed in on this in 5 weeks... probably no one cares

I have exhausted my few minutes, gotta go.

@ferdi
Can you also submit a patch for the README.txt file, so the instructions would change appropriately? I will then commit, and upon release will push the changes to the project page. I will add a CHANGELOG line.

iva2k’s picture

iva2k commented
Status: Postponed (maintainer needs more info) » Needs work

Found one more (-) for the proposed use of hook - now user has to enable view permissions for AttachmentThumbnail by hand. That puts me back on the fence from "almost decided for". Definitely needs some more work. Will edit my previous post.

ferdi’s picture

ferdi commented

I think the main advantage is that by using the public API of imagecache module you dont have to do too much maintenance work in the feature 8).
The main reason I wrote this patch is because it was not really working correctly (not creating the presets) on my "install profile"
Also , I'm not sure that enabling certain permissions for certain roles would be a feature that many user would want to have. That's something for site
admins to decide. You never know what role 1 means .
If you are ok with this I can clean up and resubmit the patch (readme.txt etc).

thanks again for your great work!
Ferdi

iva2k’s picture

iva2k commented

Thanks for your kind offer! Readme.txt does need some updating - I will appreciate that. I already have a sandbox image with the first patch applied - that's where I tested it, and I didn't see there much code cleanup is needed.

I think that three roles - unauthenticated, admin, and authenticated user are automatic and hard-coded, thus they will never change, unless someone heavily hacks drupal core for their site. That's why I did the permissions, so it will work "out of the box" - allowing all users to view the thumbnails by default. Maybe you can add two roles preset to the patch, thus maintaining original functionality - this will be enough for me to roll it into the -dev.

iva2k’s picture

iva2k commented
Status: Needs work » Fixed

I correct myself - there are 2 hard-coded roles - 1 and 2 (anonymous and authenticated).

What I found about imagecache preset permissions is that they prevent view until permission is granted. For a thumbnail it is too restrictive. So instead of completely removing, I changed the existing update code to only grant permissions, and use your hook_imagecache_default_presets. That way the preset will show thumbnails for all users until admin changes the permission. It's more intuitive this way.

I've checked it in into -dev CVS. Release will soon follow.

Status: Fixed » Closed (fixed)

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

rbrandon’s picture

rbrandon commented
StatusFileSize
new itweak_upload_dataPersistance.patch720 bytes

Thanks this patch was just what we wanted.