Right now, when you create a form File field, the values returned back from the file, arn't available in your _submit() function through the Form API.

This is Drupal 5 code....

function rc_importer_menu($may_cache){
  $items = array();
    $items[] = array(
      'path' => 'mymodule',
      'title' => t('My Module'),
      'callback' => 'drupal_get_form',
      'callback arguments' => 'mymodule_admin',
    );
  return $items;
}

function mymodule_admin(){
  $form['file'] = array(
    '#type' => 'file',
    '#title' => t('Some File'),
    //'#required' => TRUE, // Always returns an error, even though a file is successfully uploaded
  );
  $form['rc_importer_submit'] = array(
    '#value' => t('Submit'),
    '#type' => 'submit',
    '#title' => t('Submit'),
  );
  return $form;
}

function rc_importer_admin_2_submit($form_id, $values) {
  print_r($values['file']); // Returns nothing
}

Usually with the file form field, you have to manually handle the file with file_check_upload and file_save_upload, but some of this stuff could be moved into the Form API so that we could manage it with $form['file'] within the _submit code.

At a minimum, we could add #required to the file handling code. It's even documented in here, but doesn't actually handle it.

Files: 
CommentFileSizeAuthor
#20 upload_field.patch17.35 KBAlan D.
Failed: Failed to apply patch. View
#15 220944.patch5.68 KBdopry
Failed: Failed to apply patch. View
#4 formapi_file-220944-4.patch816 bytesRobLoach
Failed: 7168 passes, 19 fails, 0 exceptions View

Comments

RobLoach’s picture

For more clarification, some of this code should be included in the Form API itself when handling the file upload, so that you can interface with the file through $form_values['upload'].

drewish’s picture

+1, i'm all over this. this is the goal that got me started on all the file issues (#115267, #142995, etc)

also, marked http://drupal.org/node/219710 as a duplicate.

drewish’s picture

You should be able to specify the validator functions that get passed to file_save_upload().

RobLoach’s picture

Status: Active » Needs review
FileSize
816 bytes
Failed: 7168 passes, 19 fails, 0 exceptions View

The attached patch adds handling to the file field via the Form API. It might be missing a few things, but the basics are there. When you upload a file, information about the file is passed back through the Form API. Here is a module that tests it.....

fileformtest.info

; $Id$
name = File Form Test
description = A test for the file form.
core = 6.x

fileformtest.module

// $Id$

function fileformtest_menu() {
  return array(
    'fileformtest' => array(
      'title' => 'File Form Test',
      'page callback' => 'drupal_get_form',
      'page arguments' => array('fileformtest_admin'),
      'access arguments' => 'administer users',
    )
  );
}
function fileformtest_admin() {
  $form = array();
  $form['#attributes'] = array('enctype' => 'multipart/form-data');
   $form['upload'] = array(
    '#title' => 'Upload',
    '#type' => 'file',
    '#required' => TRUE,
  );
   $form['submit'] = array(
    '#type' => 'submit',
    '#value' => t('Upload'),
  );
  return $form;
}

function fileformtest_admin_submit($form_id, $form) {
  $file = $form['values']['upload'];
  drupal_set_message("You uploaded $file->filename, it is $file->filesize bytes, and has a mimetype of $file->filemime.");
}

In this example, you're given a page with just a file upload field and a Upload button. When you hit the Upload button without a file selected, the Form API tells you that the file is required (#required is set to true). When you upload a file, it tells you information about the file (see fileformtest_admin_submit).

Note how little code there is in fileformtest_admin_submit. We don't have to manually manage the file, as the Form API does it for us. All information about the file is passed to $form['values']['upload'].

dopry’s picture

I think it would be much cooler to get a file object as the value. Another step forward would be a new fileupload element for formapi that allows you to specify valid extensions, max filesize and additional file validation callbacks and argument. The asynchronous nature of the D6 file.inc should make this possible.

.darrel.

drewish’s picture

dopry, in #3 i mentioned adding the validators via the form element and if you look at the code that Rob Loach posted shows pulling the $file object from the form values. i get the feeling you posted without reading everyone else's comments very closely ;)

RobLoach’s picture

I tried passing the #element_validate array of methods through to file_save_upload as the following:

...
    
    // Process the list of element validators through file_save_upload.
    $validators = array();
    foreach($form['#element_validate'] as $validator) {
      $validators[$validator] = array($edit, $form);
    }
    
    // Save the file and return false when it fails (for validation).
    $file = file_save_upload($name, $validators);

...

... This doesn't really help, as the #element_validator is already called on the object. I'm not sure having the validators passed through to file_save_upload, unless we're going give the "file" field a #max_size, #valid_extensions, etc. These all could easily be managed through the #element_validator property, or $form['#validate']....

drewish’s picture

Here's the code I'd like to write to use this feature:

function fileformtest_admin() {
  $form = array();
  $form['upload'] = array(
    '#title' => 'Upload',
    '#type' => 'file',
    '#required' => TRUE,
    '#validators' => array(
      'file_validate_extensions' => array('mp3', 'wav'),
      'file_validate_size' => array(2 * 1048576),
    ).
  );
   $form['submit'] = array(
    '#type' => 'submit',
    '#value' => t('Upload'),
  );
  return $form;
}

I don't think it would be unreasonable to add a new property that's specific to the file element. And the FormsAPI should take care of adding the 'multipart/form-data' attribute to the form.

cburschka’s picture

And the FormsAPI should take care of adding the 'multipart/form-data' attribute to the form.

Amen, amen. (#137932: Automatic enctype on adding a file field)

Edit: The referenced issue does not actually contain any development towards this; it was left open.

cburschka’s picture

Re

    '#validators' => array(
      'file_validate_extensions' => array('mp3', 'wav'),
      'file_validate_size' => array(2 * 1048576),
    ).

I'm not sure if this pattern ideally matches the new Form API. D6 already got rid of custom arguments to submit handlers and demanded that these use cases switch to $form_state instead. The custom arguments here make a lot of sense, however.

Wim Leers’s picture

Subscribing.

"#validators" is too generic IMHO, i.e. somebody who's new to FAPI might think it's a generic FAPI property. I think something like "#file_validators" would be better.

dopry’s picture

@drewish, actually I did look at the patch posted quite closely... There are no validators being passed to file_save_upload, but I did somehow forget that file_save_upload returns and object these days, so that is my bad :)

what happened to custom arguments in FAPI? I totally like those things.
I think #file_validators is a better name than #validators which is too close to #validate.

chx’s picture

why not #element_validate? what we got against the generic fapi property?

drewish’s picture

#file_validators sounds great to me.

chx, #element_validate is used by the fapi as callbacks right? for this we just want to hand off the validator function names and params to file_save_upload(). i'm not sure that it would mesh correctly.

dopry’s picture

FileSize
5.68 KB
Failed: Failed to apply patch. View

So I'd rather see something like the attached... there is one issue with this... js uploads with upload.module are broken... because upload_js() is broken... It does $form_state['values'] = $_POST which just isn't proper.

However normal uploads via preview/save seem to be working with this patch...

dopry’s picture

in retrospect it may be a good idea to call this new file element 'drupal_file'... since there may still be need for the basic file upload... unless we want to convert the theme files and user pictures to use the new file handling as well. but that is a little scope creepy to me...

cburschka’s picture

Why scope creep? If by theme files you mean the ones in the theme folder, those would not be touched by the file upload changes anyway - and if you mean custom uploaded icons and logos, there is no reason not to use the new file handling. No need for a drupal_file or an advanced_{anything} field - the file field should remain in such a form that it is applicable for all purposes.

dopry’s picture

@Arancaytar: there are two layers to the api provided by file.inc. There are the file_* functions which operate on Drupal file objects, and the _file_* functions which operate on file paths. If we change the way the file element operates then we exclude every developer who may want to handle files exclusive of drupal's higher level file functions that also provide database synchronization. Which I think is a pretty strong reason. I was pointing at custom logos, favicons, and user pictures as examples of places where Drupal's higher level file api is not currently used.

I think anything beyond the task of building a better file element for handing more complex file structures is golden. I mean who says #type => 'file' can't return a path to work with the lower level of the file.inc api, and that #type => 'drupal_file' can't return a drupal file object to work with the higher level file.inc api. And who says that developers should only have one way of working with files conveniently available to them in drupal?

One file element cannot be applicable for all purposes and that my point in suggesting making the new one drupal_file.

.darrel.

coupet’s picture

subscribing

Alan D.’s picture

FileSize
17.35 KB
Failed: Failed to apply patch. View

Hi guys

Any progress on such a widget? It'd be damn useful for those that are by-passing CCK, although I must say that the old imagefield rocks for smaller projects, ;]

I drove into the FAPI for the first real time and made a very rough FAPI upload field that mimics the more complex widgets. In case anyones interested, I've attached it as a patch. I'd love feedback. It is ALPHA status.

It provides a file and an image field wrapped in a fieldset. My primary interest is the image field, but providing a file field was fairly simple.

<?php

/**
 * Implementation of hook_elements
 * 
 * Defines an file form element that is linked into the native Drupal file handling system.
 */
function upload_field_elements() {
  $type['upload_field'] = array(
    '#input' => TRUE,
    '#collapsible' => TRUE,
    '#collapsed' => FALSE,
    '#default_value' => '',
    '#process' => array('_upload_field_expand'),
    '#element_validate' => array('upload_field_element_validate'),
    '#file_formatter' => 'upload_field_preview',
    '#file_validators' => array(),
  );
  $type['image_upload_field'] = array(
    '#input' => TRUE,
    '#collapsible' => TRUE,
    '#collapsed' => FALSE,
    '#default_value' => '',
    '#process' => array('_upload_field_expand'),
    '#element_validate' => array('upload_field_element_validate'),
    '#file_formatter' => 'upload_field_image_preview',
    '#file_validators' => array('file_validate_is_image' => array()),
  );
  return $type;
}
?>

The element holds files marked for deletion using a flag, thus the "#element_validate" is used to check that a required field is not deleted.

Basic overflow of usage: Enable it and generate a form:

<?php

  ## Create the form, $node->image is a standard SELECT * FROM files object
  // it's a file upload ...
  $form['#attributes'] = array("enctype" => "multipart/form-data");
  // the widget
  $form['image'] = array(
    '#type' => 'image_upload_field',
    '#title' => t('Image'),
    '#required' => FALSE,
    '#default_value' => $node->image,
  );
  // some values to embed into the widgets frameset
  $form['image']['image_title'] = array(
    '#type' => 'textfield', '#title' => t('Title attribute'), '#size' => 60,
    '#maxlength' => 255, '#default_value' => $node->image_title);
  $form['image']['alt'] = array(
    '#type' => 'textfield', '#title' => t('Alt text'),  '#size' => 60,
    '#maxlength' => 255, '#default_value' => $node->alt);

  ## And on submit, node_save, or whatever, ...
  // $node->image is the file upload_field value form form_state
  // Sets the status of the replaced / deleted files to temp for cron to clean up
  $image_id = 0;
  if($node->image) {
    // Currently only does replace and 'max_image_size' is a imagefield preset
    $image_id = upload_field_save($node->image, 'dest_directory', FILE_EXISTS_RENAME, 'max_image_size');
  }    
  // save the file id somewhere, status is set during save to perminant
  // and it should be copied into the destination directory

?>

Follows the same process of using form_type_hook_value to save the temporary file, but it also stores the upload in the session to keep state between form loads. It minimises weight by coupling very closely with the native Drupal file functions. It feels a bit raw using the session and passing around element keys to keep the form data separated, but it seems to be working. I couldn't figure out how to use the $form_values['storage'] without interfering with the standard form workflow.

Once I run out of things to do, ... May, 2010 maybe, I'm thinking about cleaning it up, adding some js upload support, maybe multiple file support, and contributing. I'm just starting to try and tie it in more closely with imagecache for the image processing. It's too nice to ignore. I may even look at error handling soon...

Patch failed to apply. More information can be found at http://testing.drupal.org/node/13989. If you need help with creating patches please look at http://drupal.org/patch/create

Alan D.’s picture

The attachment is not a patch, just a combined .info and .module file. So lacks the default "no image" gif, and css/pngs for theming the upload_field.

RobLoach’s picture

Haha, DrupalTestBed isn't as smart as we all thought!

Anonymous’s picture

Status: Needs review » Needs work

The last submitted patch failed testing.

RobLoach’s picture

lol

Alan D.’s picture

Hi all

Two things

1) I have a fairly complete proof of concept working here http://drupal.org/project/upload_element. It effectively handles the element value as a Drupal {files} object.

2) Does anyone have permissions to remove that patch above, or a link to report it as bad?

Cheers

jhedstrom’s picture

Subscribing.

grendzy’s picture

subscribing

quicksketch’s picture

Status: Needs work » Closed (duplicate)

This is handled in #391330: File Field for Core, which includes the #type => 'manged_file' element. It accepts a default value of any $fid, and returns an $fid in the $form_state. It takes all the functionality of FileField and puts it into a reusable FAPI element, with AJAX upload and progress bar support.