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();
  }
 . . .
CommentFileSizeAuthor
filefied_add_destiantion.patch836 bytesfrankcarey

Comments

frankcarey’s picture

Title: Uploading to files/subfolder leads to files/ instead files/subfolder » Uploading to files/subfolder leads to files/ instead files/subfolder [6.3.x]

making title show version

quicksketch’s picture

Title: Uploading to files/subfolder leads to files/ instead files/subfolder [6.3.x] » field_file_save_file() does not create sub-directories
Category: bug » support

It'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().

frankcarey’s picture

Title: field_file_save_file() does not create sub-directories » Get field_file_save_file() to create sub-directories
Category: support » feature

I 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.

quicksketch’s picture

Well, 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.

frankcarey’s picture

Like 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

frankcarey’s picture

Priority: Normal » Minor
quicksketch’s picture

Status: Active » Closed (won't fix)

This can be fixed in Drupal 7, but not here. We're already solidified in our APIs for the FileField 3.x version.

quicksketch’s picture