Adding files should be a multi-step process

Yeah I like the idea of making this a multi-step process that maybe skips itself if there are no fields available. If we store the file entity with $file->status = 0, this means the file is temporary, and we can change $file->status = FILE_PERMANENT when the user completes step 2. System module automatically cleans up and deletes temporary files.

related discussion #1426730: Automatically open the file edit modal after uploading a file in the media module issue queue

Tagging

The patch adds a multi-step wizard for adding files. This wizard now always consists of three steps:

1. Upload file
2. Choose filetype
3. Fill in fields from Field API

There is only one filetype to choose from until #1292382: Make it possible to create any number of custom File Types has been fixed. Note that the file is saved directly after uploading, without knowing the correct filetype yet. This patch overwrites $file->type before saving. This change is not yet saved in the file object, which will be fixed by #1637382: type of file is overwritten in file_entity_file_presave()..

Todo's:

1. Skip second step is there is only one candidate filetype (based on mimetype)
2. Skip thirth step if filetype has no fields
3. Make wizard work for multiple file uploads
4. Integrate with the File Access API (requires #1227706: Add a file entity access API)

Patch attached skips steps 2 and 3 if not applicable and integrates with the File Access API.

Required patches before applying this patch:
- http://drupal.org/node/1292382#comment-6122536
- http://drupal.org/node/1227706#comment-6121346

FYI, we had an interesting brainstorming about improving the UX/usability of adding one or multiple files to a Drupal site in Barcelona. Basically we came up with a way of just having one modal screen and a dynamic AJAX driven interface.

It's one of things I got on my now even longer todo-list to type up. Expect it within the next week or so.

Ah, you recovered from the weekend :)

The patch already needs to be changed since the multiple file types thing is also changing. Let me know when you wrote it all down so we can continue the discussion.

Well, sort of recovering. Its easy since meeting so many great people at events like this is really energizing.

I'll post here when I have got to writing it up. Thanks for your great help.

@tsvenson, #7:Hey Thomas! Did you manage to make some time to perhaps file an issue I can follow on that modal screen + dynamic AJAX driven interface you talked about in Barcelona?

@klonos: Not yet. I'm currently working on a bigger document that covers much more. Hope to have a first draft ready in a week or two.

Cross referencing #1678106: Mobile Friendly Media Browser

Rerolled patch in #6 against HEAD. Also, note to testers - run drush updb

I tested the patch in #13 after applying the previous patches. It seemed like there was an issue where if a file that was uploaded had multiple applicable file types, you could select the filetype fine, but if it had any associated fields, the form would skip the entry of those fields. I've attached a modified version of #13's patch to correct this behavior.

The wizard seems to work fine, but renamings in file_entity causes troubles in media module which makes testing a bit harder.

One thing that doesn't work fine is the ctools model popup. The popup doesn't close after finishing the form. I tried removing the redirect and dived into the ctools code, but couldn't get rid of the pesky popup...

Anyway, I've renamed file_access() to file_entity_access() to make this compatible with the latest patches from #1227706: Add a file entity access API. I also changed the label for filetype. There was a "@todo" there to replace it show the label instead of machine name. It's replaced by a call to entity_label().

Note that the access api patch is still a dependency. I used the following patch for testing:
http://drupal.org/node/1227706#comment-6623878

Forgot to mention, the problem with the popup is on the popup used for adding files from the media selector field (on a node edit page), not on the content -> files tab.

Tested patch from #15 using latest media 7.x-2.x-dev. #1227706 is comitted so did not have to apply patch. Remember to update db and clear caches if upgrading Media or File Entity as there are many function changes.

• Fixed modal not closing by passing $form_state['file].
• Added a few safety checks.
• Reinstated some code in file_entity_add_upload_submit() such as Save messages and Form destination.
• Bit of code clarity formatting.

Working great for me. Please note if you are a user who can edit the uploaded file you will be redirected to the edit modal after file upload. This is set by the Media module so if that is an issue it should be opened there.

entity_label() was returning $file->filename, which was a bit confusing. I've hooked a label callback into the entity to help clarify Step 2 of the upload wizard.

Ahh, no on 19 - there is an existing implementation of entity_label() that seems to make sense (see file_entity_edit_submit()). Instead, I moved this back over to type, but this time with file_entity_type_get_name().

Ok, setting status to start test.

crossposting..

#21: file_entity-multi_part_form_upload-1054616-21.patch queued for re-testing.

haha mauritsl, I'll stop pressing buttons :) We're in a race condition.

Since this module has a dream of core, I'm thinking we shouldn't extend this wizard to support plupload. Currently, there is no multiple file upload support in this wizard patch. Should we invest in this direction as well? Or... commit this patch for single uploads, and open a new issue for multiples?

There is already a ticket for that: #1599892: Plupload integration broken

I suggest committing this single file upload thing, after we have done a little more testing.

In the interest of testing - I added two test cases with a handful of steps each for this feature.

You're a hero!

I tested adding files using both the file tab and the media browser (wysiwyg plugin). Everything works well.

Only one thing in the media browser.. It redirects to the edit page after a file has been added. That redirect should now be removed, but this is in the media module, not file_entity. I suggest to open an issue there after this has been committed. Tinker already said this in comment 17 :)

Marking this patch as RTBC.

Rerolled against recent commit: http://drupalcode.org/project/file_entity.git/commit/11894c1

Also, tossed in a couple minor Coder recommendations and fixed a spelling error.

#32: file_entity-multi_part_form_upload-1054616-32.patch queued for re-testing.

There we go!!

Just a quick look

+++ b/file_entity.pages.incundefined
@@ -116,45 +190,130 @@ function file_entity_usage_page($file) {
+  // Check fields from Field API.
+  if (db_select('field_config_instance', 'fci')
+          ->fields('fci', array('id'))
+          ->condition('fci.entity_type', 'file')
+          ->condition('fci.bundle', $type)
+          ->condition('fci.deleted', 0)
+          ->range(0, 1)
+          ->execute()
+          ->fetchField()) {
+    return TRUE;
   }
+  return FALSE;

any reason why we are not using http://api.drupal.org/api/drupal/modules%21field%21field.info.inc/functi... here?

+++ b/file_entity.pages.incundefined
@@ -200,8 +358,8 @@ function file_entity_add_upload_multiple($form, &$form_state, $params = array())
-    $form['upload']['#upload_location'] . '/' :
-    variable_get('file_default_scheme', 'public') . '://';
+      $form['upload']['#upload_location'] . '/' :
+      variable_get('file_default_scheme', 'public') . '://';

why adding indentation?

+++ b/file_entity.pages.incundefined
@@ -411,13 +569,14 @@ function file_entity_delete_form($form, &$form_state, $file) {
-      '%title' => entity_label('file', $file),
-    )),
-    'file/' . $file->fid,
-    $description,
-    t('Delete')
+      '%title' => entity_label('file', $file)
+      )),
+      'file/' . $file->fid,
+      $description,
+      t('Delete')

same..also comma is removed from '%title'

Thanks for the review. I've fixed these coding style errors and use field_info_instances instead. There is no reason for the custom function.

Can confirm that the flow still works good using field_info_instances().

I leave it to someone else to give the RTBC now ;)

#37 applied and passed :)

Non functional review.

+++ b/tests/file_entity.testundefined
@@ -291,7 +291,114 @@ class FileEntityTypeTestCase extends FileEntityTestHelper {
+   * Make sure candidates are presented in the case of multiple ¶
+   * file types.
+   */  ¶

+++ b/tests/file_entity.testundefined
@@ -291,7 +291,114 @@ class FileEntityTypeTestCase extends FileEntityTestHelper {
+    $user = $this->drupalCreateUser(array('create files'));
+    $this->drupalLogin($user);
+    ¶
+    // Step 1: Upload file
+    $file = reset($this->files['image']);
+    $edit = array();

+++ b/tests/file_entity.testundefined
@@ -291,7 +291,114 @@ class FileEntityTypeTestCase extends FileEntityTestHelper {
+    $edit['files[upload]'] = drupal_realpath($file->uri);
+    $this->drupalPost('file/add', $edit, t('Next'));
+    ¶
+    // Step 2: Select file type candidate
+    $this->assertText('Image 1', 'File candidate list item found.');

+++ b/tests/file_entity.testundefined
@@ -291,7 +291,114 @@ class FileEntityTypeTestCase extends FileEntityTestHelper {
+   * NOTE: Depends on file_entity.module default 'image' type.
+   */  ¶
+  function testTypeWithoutCandidates() {
+    // Attach a text field to the default image file type.

+++ b/tests/file_entity.testundefined
@@ -291,7 +291,114 @@ class FileEntityTypeTestCase extends FileEntityTestHelper {
+    $user = $this->drupalCreateUser(array('create files'));
+    $this->drupalLogin($user);
+    ¶
+    // Step 1: Upload file
+    $file = reset($this->files['image']);
+    $edit = array();
+    $edit['files[upload]'] = drupal_realpath($file->uri);
+    $this->drupalPost('file/add', $edit, t('Next'));
+    ¶
+    // Step 2: Complete field widgets
+    $langcode = LANGUAGE_NONE;

Remove trailing spaces where ¶ symbol appear. Not critical but it should match coding requirements.

Rerolled #37 against #32, passes Coder minor, improved docs, removed trailing commas.

No functional changes, but did a sanity check to see if everything still works.

I can confirm that it does not add minor issues in coder.

This looks good:)

+++ b/file_entity.pages.incundefined
@@ -200,8 +342,8 @@ function file_entity_add_upload_multiple($form, &$form_state, $params = array())
-    $form['upload']['#upload_location'] . '/' :
-    variable_get('file_default_scheme', 'public') . '://';
+      $form['upload']['#upload_location'] . '/' :
+      variable_get('file_default_scheme', 'public') . '://';

Adding indentation again.

+++ b/file_entity.pages.incundefined
@@ -403,7 +545,7 @@ function file_entity_delete_form($form, &$form_state, $file) {
-    '#value' => $file->fid,
+    '#value' => $file->fid

Removing comma

+++ b/file_entity.pages.incundefined
@@ -432,7 +573,7 @@ function file_entity_delete_form_submit($form, &$form_state) {
-      '%title' => entity_label('file', $file),
+      '%title' => entity_label('file', $file)

Removing comma

+++ b/file_entity.pages.incundefined
@@ -451,7 +592,7 @@ function file_entity_multiple_delete_form($form, &$form_state, array $files) {
-    '#tree' => TRUE,
+    '#tree' => TRUE

same

+++ b/file_entity.pages.incundefined
@@ -469,13 +610,13 @@ function file_entity_multiple_delete_form($form, &$form_state, array $files) {
-      '#suffix' => $title . "</li>\n",
+      '#suffix' => $title . "</li>\n"

same

+++ b/file_entity.pages.incundefined
@@ -469,13 +610,13 @@ function file_entity_multiple_delete_form($form, &$form_state, array $files) {
-    '#value' => 'delete',
+    '#value' => 'delete'

same

Fixed issues from #43. No functional changes.

Tested patch. Patch is clean and functional

Ok, looks good, has tests, i love it, what more is needed?
I commited this. Could someone open an issue to media queue to remove the edit screen after uploading a file? i guess it is not needed anymore;)

Thanks all!!

#1848034: Remove ctools edit form after uploading a new file in media browser

This change doesn't allow the File Name of the new file to be edited in the upload flow; only the "normal" fields are available for editing. Is this an oversight? Editing of the file name was previously supported at the time of upload when this functionality was handled by Media.

Is it possible to disable this multi-step feature if we want the file uploading process to be one step? I imagine that some situations won't call for multiple steps like this.

If not, I can try to work up a patch. I'm just checking if it's already built.

The idea is that steps will be skipped when not applicable. That is, if you upload an image, you should not be asked to select the file type when there is only one file type for images. And when it doesn't have additional fields, another page will be skipped as well. So when a situation "does not call for multi-step", it shouldn't be, automatically.

Works well when you upload a new file, but unfortunatly doesn't seem to be the case if you add a file from Web URL... or am I missing a configuration somewhere ?

Per my complaint in #49, here's a new bug #2074625: Allow filename (and possibly other attributes) to be edited during upload process

Is it possible to skip the next step of adding fields attached to the files and simply be done with creating the (file) entities?

With Pulpload and bulk upload, the editing area can become real long and unnecessary for some usecase.