Specify default /images folder

I second this.

In fact, integration of imagefield functionality with the image galleries would be best as image management is a big issue when you have to hand this part of the cms over to clients for them to use.

Heres a patch (my first one - so please let me know if anythings incorrect) that allows you to specify a directory for 'imagefield' to use. Its set relative to the site 'files' folder, and can be different per imagefield within a content-type.

The patch was created for the current cvs version of imagefield (version 1.9 2006/08/02).

An 'Image Path' field is added to the Imagefield settings page & the folder should be created upon submit (see my todo/problems below). The path is prepended to the image file name in the 'files' table, so it should function as normal.

A new function 'imagefield_check_directory' has been added to handle the creation of the directory.

ToDo/Problems:

- Recursive subdirectory problem - file_check_directory seems unable to create more than 1 level of folders at a time (e.g. it can create 'product-images' but not 'product-images/sub-dir') - I found this under the 'imagecache' issue tracker http://drupal.org/node/70782 - but that uses its own recursive create code - I didn't want to just copy/duplicate it - should this perhaps be added to the main drupal api?

- The imagefield_check_directory is called using the forms api '#after_build' attribute - as used in the system.module, I'm not sure if this is the correct way to do it here - but the validate operation caught in 'imagefield_field_settings' doesn't give access to the form object to handle errors (I may be wrong).

All comments, corrections and suggestions welcome.

Updated the status - as I forgot above.

This looks cool...
#after_build is a good spot to do the validation in... Go ahead and test if everything exists... if it doesn't do something like form_set_error($element['#parents'][0], 'Yo, I can't create you directory, do it yourself. I won't let you submit this until I can write to it... Muahahahhaaha');

you might work it better than that, and wrap it in t(); see filesystem_validate_path() in filesystem.module...

.darrel.

@dopry: thanks for the tips - will look into this tomorrow when I have a abit of spare time & will update this issue with my progress.

@kiteatlas: no probs ;) I only recently took the jump with cck/views - mainly because I needed image support - & dopry's doing a great job in this area (as well as other areas)! I really like the interface & way the module works, so just wanted to contribute a few minor features that I thought could benefit the project.
Enjoy your 'ten days off the screen' - a holiday I hope :)

Heres an updated patch with the following changes:

• Removed the use of the 'file_check_directory' form error display - handled manually instead
• Added checks if the dir exists & is writable (based on filesystem 'filesystem_validate_path()')
• All error messages are wrapped in t()

This at least means users are informed if the directory isn't created - e.g. the sub-dir problem mentioned previously.

This hasn't been tested with the preview option, as that seems to need a little work anyway - but according to some of your other comments dopry, this will be handled better by your filesystem module? So have left it for now.

Comments & feedback welcome :)

Hi kiteatlas,

No probs - glad its working for you ok :)

Great patch. Should definetly be included in future releases. Does it do max files in folders?

I'm planning on adding this feature. Do you want a default images folder that can be over ridden per field instance and do you want to to add some automagic hashing to keep folders from getting too full. Personally I wouldn't mind just using field label or another static value that the user doesn't have to enter. Any opinions?

Hi Dopry,

As you suggested - you could use the image field label as the folder name by default (may have to check if it already exists?) but also have an optional 'folder name' field that allowed users to override the default if they'd like.

This would be very usefull for myself, not sure how you/others feel?

As for the delete/max file functionailty - would it make sense to have an optional 'max size' (or similar') textfield to allow users to set the max size in MB to allow, rather than max number of files??

Thanks for your great work on this :)

I was thinking of a something to keep the folder from getting too full. I'm interested in max number of files as I'm anticipating several thousand images for one content type and lookups are bound to slow down if all files are in the same folder.

There needs to be an upgrade path for existing imagefields for this to really be rolled in. It also needs to handle database updates when the file path is changed. I think it is reasonable to leave moving the files up to the administrator for now.

I'm working on taking this patch and fixing it up a bit, including an update for the 5 version (HEAD currently). Dopry, maybe imagefield shouldn't update the database locations when updating the field. The Drupal site files directory doesn't update the database or file locations when you change it, so I don't think there is an expectation that imagefield would perform this operation for you either. For all existing imagefields, the files path would simply be empty.

Here's a patch implementing the custom directory behavior, with other minor changes. Here's how image directories work:

- All current imagefields start with an empty 'image directory' option (the root of the files folder)
- Changing an existing imagefield image directory affect all NEW images, existing images using that field retain the same location.
- Image directories may be any depth within the files directory. For example 'images/my_custom/path' is totally acceptable, even if directories 'my_custom' and 'path' don't yet exist (they will be created)

Other changes include:
- Theme functions now accept the URL of the image rather than the entire file object. The reasoning being to minimize logic in theme functions and because the 'filepath' contained in the file object is not accurate for temporary files and we have no way of finding which directory the 'filename' parameter references.

- Change in the SESSION variable storage. Now is of the format

$_SESSION['imagefield'][$fieldname][$content_type];

which adds the name of the content type into the array. This allows the 'image_path' variable to be matched to the proper field/widget combination in hook_menu(). It also slightly decreases the chances of one session mistakenly inserting an image in a different node if two nodes of separate types are simultaneously edited by the same person (yeah right! ;-P).

- Minor updates to the UI dealing with uploading images

And the 5.0 (HEAD for imagefield) version of the patch. Complete functional usage with Drupal 5.0. (info file still needs updating using patch from this issue.)

the patch does not work, to fix it add "$file = array();" on line 49

nescius, could you be more clear about what isn't working with the previous patch? Adding $file = array(); causes imagefields with multiple values to only accept one image at a time. If you upload an image, update, then upload another it replaces the previous one.

@quicksketch,
could you try to split the patch into its parts, The case changes, style updates, and interface updates should be their own patches. They make it harder for me to see the functional changes.

The changes also need to be documented.

So things that catch me immediately on this patch... It adds a new layer in the session tree. this creates a set of inner loops I'd rather avoid if possible. Is there a particular reason for this? Couldn't the data be simple prepended to the final filepath or attached to the file construct we're already storing in memory?
It also leads to alot of api changes which I'd prefer not to make if possible...

whats with the static $imagefield_upload_processed = FALSE;

I'd like to get this in before the week is out. I can help you with it, but I'm time tight until thursday.

dopry, thanks for the extensive review. Here's my list:

could you try to split the patch into its parts, The case changes, style updates, and interface updates should be their own patches. They make it harder for me to see the functional changes.

The changes also need to be documented.

Sure, I'll do as best I can.

So things that catch me immediately on this patch... It adds a new layer in the session tree. this creates a set of inner loops I'd rather avoid if possible. Is there a particular reason for this? Couldn't the data be simple prepended to the final filepath or attached to the file construct we're already storing in memory?

Sure we could try to simplify the session data structure. In the end, we really just need the image path for the preset saved in the session, possibly simply appended as you recommend.

whats with the static $imagefield_upload_processed = FALSE;

In Drupal 5 'prepare form values' is called twice, for reasons I'm not sure. The static variable prevents the SESSION variable from being updated twice.

1) seperate non-function / aesthetic changes from patch.

I know its hard not to make them... roll a patch just for them on one of the cool days.. in this case the little changes consume a significant number of lines in the patch.

2) Session....

try appending the data you need on the file contruct already in _SESSION. If possible when the file is uploaded and prepped prepend it to the dst filepath so we don't even have to worry about it. I'm sure there is a more fluid way to get it in.

3) In Drupal 5 'prepare form values' is called twice

Oh that was fixed with... http://drupal.org/node/99096 and in fact cck was misbehaving.....

You're a D-Gangsta for werkin' on this stuff. I really appreciate it.

Sweet, I've got a super-shortened patch for you now. Much cleaner and works better too!. Rather than trying to figure out a path for the preview in hook_menu, I just moved the logic to where the file is initially uploaded (duh!). The value is saved in $_SESSION['imagefield'][$fieldname]['preview'], like before, only now it is more available for usage.

After that change, the theme functions worked much better as-is and didn't need the API change I thought necessary. There are still a few very minor changes to theme functions, but only dealing with file locations or uploading.

Nice work on fixing that CCK 5 bug! I'll post the 5 version of this patch shortly.

And 5.0 version... static variable removed of course, works great :)

awesome applied to DRUPAL-4-7 and HEAD branches... much cleaner solution. Moving the path generation stuff out of the menu was an awesome move. I'm almost ready to release a 4.7.2 and 5.0.1 woot!

.darrel.