diff --git a/core/modules/file/file.module b/core/modules/file/file.module index 18326d8..81aa42f 100644 --- a/core/modules/file/file.module +++ b/core/modules/file/file.module @@ -699,8 +699,17 @@ function file_cron() { * An array of file entities or a single file entity if $delta != NULL. Each * array element contains the file entity if the upload succeeded or FALSE if * there was an error. Function returns NULL if no file was uploaded. + * + * @deprecated in Drupal 8.4.x, will be removed before Drupal 9.0.0. + * For backwards compatibility use core file upload widgets in forms. + * + * @internal + * This function wraps file_save_upload() to allow correct error handling in + * forms. + * + * @todo Revisit after https://www.drupal.org/node/2244513. */ -function file_save_upload_from_form($element, FormStateInterface $form_state, $delta = NULL, $replace = FILE_EXISTS_RENAME) { +function _file_save_upload_from_form(array $element, FormStateInterface $form_state, $delta = NULL, $replace = FILE_EXISTS_RENAME) { // Get all errors set before calling this method. This will also clear them // from $_SESSION. $errors_before = drupal_get_messages('error'); @@ -719,6 +728,7 @@ function file_save_upload_from_form($element, FormStateInterface $form_state, $d if (count($errors_new) > 1) { // Render multiple errors into a single message. + // This is needed because only one error per element is supported. $render_array = [ 'error' => [ '#markup' => t('One or more files could not be uploaded.'), @@ -755,6 +765,10 @@ function file_save_upload_from_form($element, FormStateInterface $form_state, $d * Temporary files are periodically cleaned. Use the 'file.usage' service to * register the usage of the file which will automatically mark it as permanent. * + * Note that this function does not support correct form error handling. The + * file upload widgets in core do support this. It is advised to use these in + * any custom form, instead of calling this function. + * * @param string $form_field_name * A string that is the associative array key of the upload form element in * the form array. @@ -786,11 +800,9 @@ function file_save_upload_from_form($element, FormStateInterface $form_state, $d * array element contains the file entity if the upload succeeded or FALSE if * there was an error. Function returns NULL if no file was uploaded. * - * @deprecated in Drupal 8.4.x, will be removed before Drupal 9.0.0. - * This function should not be used for form validation, use - * file_save_upload_from_form() instead. + * @see _file_save_upload_from_form() * - * @see file_save_upload_from_form() + * @todo: move this logic to a service in https://www.drupal.org/node/2244513. */ function file_save_upload($form_field_name, $validators = [], $destination = FALSE, $delta = NULL, $replace = FILE_EXISTS_RENAME) { $user = \Drupal::currentUser(); @@ -1281,7 +1293,7 @@ function file_managed_file_save_upload($element, FormStateInterface $form_state) $files_uploaded = $element['#multiple'] && count(array_filter($file_upload)) > 0; $files_uploaded |= !$element['#multiple'] && !empty($file_upload); if ($files_uploaded) { - if (!$files = file_save_upload_from_form($element, $form_state)) { + if (!$files = _file_save_upload_from_form($element, $form_state)) { \Drupal::logger('file')->notice('The file upload failed. %upload', ['%upload' => $upload_name]); return []; } diff --git a/core/modules/file/src/Tests/SaveUploadFormTest.php b/core/modules/file/src/Tests/SaveUploadFormTest.php index c795120..6d51198 100644 --- a/core/modules/file/src/Tests/SaveUploadFormTest.php +++ b/core/modules/file/src/Tests/SaveUploadFormTest.php @@ -5,11 +5,11 @@ use Drupal\file\Entity\File; /** - * Tests the file_save_upload_from_form() function. + * Tests the _file_save_upload_from_form() function. * * @group file * - * @see file_save_upload_from_form() + * @see _file_save_upload_from_form() */ class SaveUploadFormTest extends FileManagedTestBase { @@ -79,7 +79,7 @@ protected function setUp() { } /** - * Tests the file_save_upload_from_form() function. + * Tests the _file_save_upload_from_form() function. */ public function testNormal() { $max_fid_after = db_query('SELECT MAX(fid) AS fid FROM {file_managed}')->fetchField(); @@ -134,7 +134,7 @@ public function testHandleExtension() { // The file being tested is a .gif which is in the default safe list // of extensions to allow when the extension validator isn't used. This is // implicitly tested at the testNormal() test. Here we tell - // file_save_upload_from_form() to only allow ".foo". + // _file_save_upload_from_form() to only allow ".foo". $extensions = 'foo'; $edit = [ 'file_test_replace' => FILE_EXISTS_REPLACE, @@ -155,7 +155,7 @@ public function testHandleExtension() { file_test_reset(); $extensions = 'foo ' . $this->imageExtension; - // Now tell file_save_upload_from_form() to allow the extension of our test image. + // Now tell _file_save_upload_from_form() to allow the extension of our test image. $edit = [ 'file_test_replace' => FILE_EXISTS_REPLACE, 'files[file_test_upload][]' => drupal_realpath($this->image->getFileUri()), @@ -173,7 +173,7 @@ public function testHandleExtension() { // Reset the hook counters. file_test_reset(); - // Now tell file_save_upload_from_form() to allow any extension. + // Now tell _file_save_upload_from_form() to allow any extension. $edit = [ 'file_test_replace' => FILE_EXISTS_REPLACE, 'files[file_test_upload][]' => drupal_realpath($this->image->getFileUri()), @@ -369,7 +369,7 @@ public function testDrupalMovingUploadedFileError() { * Tests that form validation does not change error messages. */ public function testErrorMessagesAreNotChanged() { - $error = 'An error message set before file_save_upload_from_form()'; + $error = 'An error message set before _file_save_upload_from_form()'; $edit = [ 'files[file_test_upload][]' => drupal_realpath($this->image->getFileUri()), @@ -380,10 +380,10 @@ public function testErrorMessagesAreNotChanged() { $this->assertRaw(t('You WIN!'), 'Found the success message.'); // Ensure the expected error message is present and the counts before and - // after calling file_save_upload_from_form() are correct. + // after calling _file_save_upload_from_form() are correct. $this->assertText($error); - $this->assertRaw('Number of error messages before file_save_upload_from_form(): 1'); - $this->assertRaw('Number of error messages after file_save_upload_from_form(): 1'); + $this->assertRaw('Number of error messages before _file_save_upload_from_form(): 1'); + $this->assertRaw('Number of error messages after _file_save_upload_from_form(): 1'); // Test that error messages are preserved when an error occurs. $edit = [ @@ -396,10 +396,10 @@ public function testErrorMessagesAreNotChanged() { $this->assertRaw(t('Epic upload FAIL!'), 'Found the failure message.'); // Ensure the expected error message is present and the counts before and - // after calling file_save_upload_from_form() are correct. + // after calling _file_save_upload_from_form() are correct. $this->assertText($error); - $this->assertRaw('Number of error messages before file_save_upload_from_form(): 1'); - $this->assertRaw('Number of error messages after file_save_upload_from_form(): 1'); + $this->assertRaw('Number of error messages before _file_save_upload_from_form(): 1'); + $this->assertRaw('Number of error messages after _file_save_upload_from_form(): 1'); // Test a successful upload with no messages. $edit = [ @@ -410,10 +410,10 @@ public function testErrorMessagesAreNotChanged() { $this->assertRaw(t('You WIN!'), 'Found the success message.'); // Ensure the error message is not present and the counts before and after - // calling file_save_upload_from_form() are correct. + // calling _file_save_upload_from_form() are correct. $this->assertNoText($error); - $this->assertRaw('Number of error messages before file_save_upload_from_form(): 0'); - $this->assertRaw('Number of error messages after file_save_upload_from_form(): 0'); + $this->assertRaw('Number of error messages before _file_save_upload_from_form(): 0'); + $this->assertRaw('Number of error messages after _file_save_upload_from_form(): 0'); } /** diff --git a/core/modules/file/tests/file_test/src/Form/FileTestSaveUploadFromForm.php b/core/modules/file/tests/file_test/src/Form/FileTestSaveUploadFromForm.php index d06f53f..ef61fb3 100644 --- a/core/modules/file/tests/file_test/src/Form/FileTestSaveUploadFromForm.php +++ b/core/modules/file/tests/file_test/src/Form/FileTestSaveUploadFromForm.php @@ -135,7 +135,7 @@ public function validateForm(array &$form, FormStateInterface $form_state) { // The test for drupal_move_uploaded_file() triggering a warning is // unavoidable. We're interested in what happens afterwards in - // file_save_upload_from_form(). + // _file_save_upload_from_form(). if ($this->state->get('file_test.disable_error_collection')) { define('SIMPLETEST_COLLECT_ERRORS', FALSE); } @@ -143,9 +143,9 @@ public function validateForm(array &$form, FormStateInterface $form_state) { $form['file_test_upload']['#upload_validators'] = $validators; $form['file_test_upload']['#upload_location'] = $destination; - drupal_set_message($this->t('Number of error messages before file_save_upload_from_form(): @count.', ['@count' => count(drupal_get_messages('error', FALSE))])); - $file = file_save_upload_from_form($form['file_test_upload'], $form_state, 0, $form_state->getValue('file_test_replace')); - drupal_set_message($this->t('Number of error messages after file_save_upload_from_form(): @count.', ['@count' => count(drupal_get_messages('error', FALSE))])); + drupal_set_message($this->t('Number of error messages before _file_save_upload_from_form(): @count.', ['@count' => count(drupal_get_messages('error', FALSE))])); + $file = _file_save_upload_from_form($form['file_test_upload'], $form_state, 0, $form_state->getValue('file_test_replace')); + drupal_set_message($this->t('Number of error messages after _file_save_upload_from_form(): @count.', ['@count' => count(drupal_get_messages('error', FALSE))])); if ($file) { $form_state->setValue('file_test_upload', $file); diff --git a/core/modules/inline_form_errors/tests/src/Functional/FormErrorHandlerFileUploadTest.php b/core/modules/inline_form_errors/tests/src/Functional/FormErrorHandlerFileUploadTest.php new file mode 100644 index 0000000..9535c99 --- /dev/null +++ b/core/modules/inline_form_errors/tests/src/Functional/FormErrorHandlerFileUploadTest.php @@ -0,0 +1,94 @@ + 'page', 'name' => 'page'])->save(); + + // Add a file field. + FieldStorageConfig::create([ + 'entity_type' => 'node', + 'field_name' => 'field_ief_file', + 'type' => 'file', + 'cardinality' => 1, + ])->save(); + + FieldConfig::create([ + 'field_name' => 'field_ief_file', + 'label' => 'field_ief_file', + 'entity_type' => 'node', + 'bundle' => 'page', + 'required' => TRUE, + 'settings' => ['file_extensions' => 'png gif jpg jpeg'], + ])->save(); + + EntityFormDisplay::create([ + 'targetEntityType' => 'node', + 'bundle' => 'page', + 'mode' => 'default', + 'status' => TRUE, + ])->setComponent('field_ief_file', [ + 'type' => 'file_generic', + 'settings' => [], + ])->save(); + + EntityViewDisplay::create([ + 'targetEntityType' => 'node', + 'bundle' => 'page', + 'mode' => 'default', + 'status' => TRUE, + 'label' => 'hidden', + 'type' => 'file_default', + ])->save(); + + // Create and login a user. + $account = $this->drupalCreateUser([ + 'access content', + 'access administration pages', + 'administer nodes', + 'create page content', + ]); + $this->drupalLogin($account); + } + + /** + * Tests that the required field error is displayed as inline error message. + */ + public function testFileUploadErrors() { + $this->drupalGet('node/add/page'); + $edit = [ + 'edit-title-0-value' => $this->randomString(), + ]; + $this->submitForm($edit, t('Save')); + + $error_text = $this->getSession()->getPage()->find('css', '.field--name-field-ief-file .form-item--error-message')->getText(); + + $this->assertEquals('field_ief_file field is required.', $error_text); + } + +} diff --git a/core/modules/inline_form_errors/tests/src/FunctionalJavascript/FormErrorHandlerFileUpload.php b/core/modules/inline_form_errors/tests/src/FunctionalJavascript/FormErrorHandlerFileUpload.php deleted file mode 100644 index 202d56a..0000000 --- a/core/modules/inline_form_errors/tests/src/FunctionalJavascript/FormErrorHandlerFileUpload.php +++ /dev/null @@ -1,95 +0,0 @@ - 'page', 'name' => 'page'])->save(); - - // Add the file field under test. - FieldStorageConfig::create([ - 'entity_type' => 'node', - 'field_name' => 'field_ief_file', - 'type' => 'file', - 'cardinality' => 1, - ])->save(); - - FieldConfig::create([ - 'field_name' => 'field_ief_file', - 'label' => 'field_ief_file', - 'entity_type' => 'node', - 'bundle' => 'page', - 'required' => TRUE, - 'settings' => ['file_extensions' => 'png gif jpg jpeg'], - ])->save(); - - EntityFormDisplay::create([ - 'targetEntityType' => 'node', - 'bundle' => 'page', - 'mode' => 'default', - 'status' => TRUE, - ])->setComponent('field_ief_file', [ - 'type' => 'file_generic', - 'settings' => [], - ])->save(); - - EntityViewDisplay::create([ - 'targetEntityType' => 'node', - 'bundle' => 'page', - 'mode' => 'default', - 'status' => TRUE, - 'label' => 'hidden', - 'type' => 'file_default', - ])->save(); - - // Create and login a user. - $account = $this->drupalCreateUser([ - 'access content', - 'access administration pages', - 'administer nodes', - 'create page content' - ]); - $this->drupalLogin($account); - } - - /** - * Tests if the required field error is displayed as inline error message. - */ - public function testFileUploadErrors() { - $this->drupalGet('node/add/page'); - $edit = [ - 'edit-title-0-value' => $this->randomString(), - ]; - $this->submitForm($edit, t('Save')); - - $selector = '.field--name-field-ief-file .form-item--error-message'; - $actual = $this->getSession()->getPage()->find('css', $selector)->getText(); - - $this->assertEquals('field_ief_file field is required.', $actual); - } - -} diff --git a/core/modules/locale/src/Form/ImportForm.php b/core/modules/locale/src/Form/ImportForm.php index db4b6aa..5a9b1a3 100644 --- a/core/modules/locale/src/Form/ImportForm.php +++ b/core/modules/locale/src/Form/ImportForm.php @@ -155,7 +155,7 @@ public function buildForm(array $form, FormStateInterface $form_state) { * {@inheritdoc} */ public function validateForm(array &$form, FormStateInterface $form_state) { - $this->file = file_save_upload_from_form($form['file'], $form_state, 0); + $this->file = _file_save_upload_from_form($form['file'], $form_state, 0); // Ensure we have the file uploaded. if (!$this->file) { diff --git a/core/modules/system/src/Form/ThemeSettingsForm.php b/core/modules/system/src/Form/ThemeSettingsForm.php index 8fe01d2..9fdfea7 100644 --- a/core/modules/system/src/Form/ThemeSettingsForm.php +++ b/core/modules/system/src/Form/ThemeSettingsForm.php @@ -366,7 +366,7 @@ public function validateForm(array &$form, FormStateInterface $form_state) { $validators = ['file_validate_is_image' => []]; // Check for a new uploaded logo. - $file = file_save_upload_from_form($form['logo']['settings']['logo_upload'], $form_state, 0); + $file = _file_save_upload_from_form($form['logo']['settings']['logo_upload'], $form_state, 0); if ($file) { // Put the temporary file in form_values so we can save it on submit. $form_state->setValue('logo_upload', $file); @@ -375,7 +375,7 @@ public function validateForm(array &$form, FormStateInterface $form_state) { $validators = ['file_validate_extensions' => ['ico png gif jpg jpeg apng svg']]; // Check for a new uploaded favicon. - $file = file_save_upload_from_form($form['favicon']['settings']['favicon_upload'], $form_state, 0); + $file = _file_save_upload_from_form($form['favicon']['settings']['favicon_upload'], $form_state, 0); if ($file) { // Put the temporary file in form_values so we can save it on submit. $form_state->setValue('favicon_upload', $file);