looks like this is the same as below but for 6.3.x
#275293: Uploading to files/subfolder leads to files/ instead files/subfolder
I updated the $dest check to try to create the directory if it isn't present.
function field_file_save_file($filepath, $validators = array(), $dest = FALSE) {
. . .
// Verify the directory and create if necessary. If the destination is not provided, or is not writable, then use the
// temporary directory.
if (empty($dest) || file_check_directory($dest, FILE_CREATE_DIRECTORY) === FALSE) {
$dest = file_directory_temp();
}
. . .
| Comment | File | Size | Author |
|---|---|---|---|
| filefied_add_destiantion.patch | 836 bytes | frankcarey |
Comments
Comment #1
frankcarey commentedmaking title show version
Comment #2
quicksketchIt's not a bug since it's not actually causing any problems (you're doing a custom import script, right?). I'd suggest doing what FileField does and just call field_file_check_directory() first, which is just like file_check_directory(), only it doesn't post messages when new directories are created. See the example usage in filefield_save_upload().
Comment #3
frankcarey commentedI put it as a bug because it seemed like this was the expected behavior, so I'll change it to a feature request.
yes field_file_check_directory() would be the preferred function here, avoiding the message. Any reason not to add it here instead of file_check_directory? Seems like it would be proper to try and create the directory instead of going straight for the backup.
I certainly call field_file_check_directory() first in my script, but this seems better moving forward, no? I can't think of a reason why you wouldn't want the destination created. This function doesn't throw an error if the location doesn't exist, it just ends up putting a file where you probably didn't want it.
Comment #4
quicksketchWell, taking a look at existing API functions on which this function is based:
http://api.drupal.org/api/function/file_save_upload/6
http://api.drupal.org/api/function/file_save_upload/7
Plus FileField's field_file_save_upload(), none of these functions create the directory. It must be created prior to the function call. So while there might be good reason to change it, it's also not the expected behavior of any existing similar API function. For the sake of consistency (and the fact that we should keep API changes minimum now that we're final release), I don't think this change should be made.
Comment #5
frankcarey commentedLike I said drupalcon, I'm really excited about FileAPI, and I do understand not wanting to change things once things are solid, but I still can't think of how this change would negatively effect anyone, or how the current functionality could be considered a feature. Those using the additional field_file_check_directory() would see no difference. Just because I can't think of any reasons, doesn't mean there aren't any though.
This is non-critical, though it did frustrate the hell out of me earlier today ;) I'm more than happy to let this sit in the queue for a while to see if a change is the right thing to do, and when.
Cheers for great work on FileAPI. Much appreciated
Comment #6
frankcarey commentedComment #7
quicksketchThis can be fixed in Drupal 7, but not here. We're already solidified in our APIs for the FileField 3.x version.
Comment #8
quicksketchWe've fixed this in Drupal 7: #515280: file_check_directory() should create recursively