diff --git a/core/modules/media/src/Element/MediaManagedFile.php b/core/modules/media/src/Element/MediaManagedFile.php index e3fbbe8..07b5148 100644 --- a/core/modules/media/src/Element/MediaManagedFile.php +++ b/core/modules/media/src/Element/MediaManagedFile.php @@ -11,14 +11,20 @@ */ class MediaManagedFile extends ManagedFile { + /** + * {@inheritdoc} + */ public static function processManagedFile(&$element, FormStateInterface $form_state, &$complete_form) { if (!empty($element['#value']['target_id'])) { // We're editing an entity with a previously-saved Media entity // reference(s). In order not to confuse ManagedFile, load those entities' // source fids and store them where it expects them. $media_entity = Media::load($element['#value']['target_id']); + // @TODO: Could $media_entity be empty here in any scenario? $media_source_field = $media_entity->getSource()->getConfiguration()['source_field']; $file_value = $media_entity->get($media_source_field)->getValue(); + // @TODO: Should / could we assume here that this will always have only + // 1 value? foreach ($file_value as $value) { $element['#value']['fids'][] = $value['target_id']; } diff --git a/core/modules/media/src/Form/MediaInlineForm.php b/core/modules/media/src/Form/MediaInlineForm.php index 988de58..8ac4d8c 100644 --- a/core/modules/media/src/Form/MediaInlineForm.php +++ b/core/modules/media/src/Form/MediaInlineForm.php @@ -79,23 +79,18 @@ public static function createInstance(ContainerInterface $container, EntityTypeI /** * {@inheritdoc} */ - public function getEntityType() { - return $this->entityType; - } - - /** - * {@inheritdoc} - */ public function entityForm(array $entity_form, FormStateInterface $form_state) { // Build the media entity form. + // @TODO Why shouldn't we assume this is a MediaInterface instead? /** @var \Drupal\Core\Entity\ContentEntityInterface $entity */ $entity = $entity_form['#entity']; - $form_display = $this->getFormDisplay($entity, $entity_form['#form_mode']); + $form_display = EntityFormDisplay::collectRenderDisplay($entity, $entity_form['#form_mode']); $form_display->buildForm($entity, $entity_form, $form_state); $entity_form['#weight'] = 100; // In the service of keeping this form as user-friendly as possible in the // context of a parent entity form, only show required fields. + // @TODO Inject the service. /** @var \Drupal\Core\Entity\EntityFieldManager $entity_field_manager */ $entity_field_manager = \Drupal::service('entity_field.manager'); $field_definitions = $entity_field_manager->getFieldDefinitions('media', 'file'); @@ -106,6 +101,7 @@ public function entityForm(array $entity_form, FormStateInterface $form_state) { $entity_form[$field_name]['#access'] = FALSE; } else { + // @TODO: Why is this here? $entity_form[$field_name]['#element_validate'] = []; } @@ -118,7 +114,8 @@ public function entityForm(array $entity_form, FormStateInterface $form_state) { ->getName(); $entity_form[$source_field]['#access'] = FALSE; - // Inline entities inherit the parent language. + // Inline entities inherit the parent language, so hide translation-related + // fields as well. $langcode_key = $this->entityType->getKey('langcode'); if ($langcode_key && isset($entity_form[$langcode_key])) { $entity_form[$langcode_key]['#access'] = FALSE; @@ -136,19 +133,4 @@ public function entityForm(array $entity_form, FormStateInterface $form_state) { return $entity_form; } - /** - * Gets the form display for the given entity. - * - * @param \Drupal\Core\Entity\ContentEntityInterface $entity - * The entity. - * @param string $form_mode - * The form mode. - * - * @return \Drupal\Core\Entity\Display\EntityFormDisplayInterface - * The form display. - */ - protected function getFormDisplay(ContentEntityInterface $entity, $form_mode) { - return EntityFormDisplay::collectRenderDisplay($entity, $form_mode); - } - } diff --git a/core/modules/media/src/MediaInlineFormInterface.php b/core/modules/media/src/MediaInlineFormInterface.php index fc6ea3a..d624e66 100644 --- a/core/modules/media/src/MediaInlineFormInterface.php +++ b/core/modules/media/src/MediaInlineFormInterface.php @@ -6,19 +6,11 @@ use Drupal\Core\Form\FormStateInterface; /** - * Defines the interface for inline form handlers. + * Defines the interface for Media inline form handlers. */ interface MediaInlineFormInterface extends EntityHandlerInterface { /** - * Gets the entity type managed by this handler. - * - * @return \Drupal\Core\Entity\EntityTypeInterface - * The entity type. - */ - public function getEntityType(); - - /** * Builds the entity form. * * @param array $entity_form diff --git a/core/modules/media/src/Plugin/Field/FieldWidget/MediaFileWidget.php b/core/modules/media/src/Plugin/Field/FieldWidget/MediaFileWidget.php index 7ac3eb3..e103467 100644 --- a/core/modules/media/src/Plugin/Field/FieldWidget/MediaFileWidget.php +++ b/core/modules/media/src/Plugin/Field/FieldWidget/MediaFileWidget.php @@ -2,6 +2,7 @@ namespace Drupal\media\Plugin\Field\FieldWidget; +use Drupal\Core\Field\FieldDefinitionInterface; use Drupal\Core\Field\FieldItemListInterface; use Drupal\Core\Form\FormStateInterface; use Drupal\file\Entity\File; @@ -53,14 +54,14 @@ public function formElement(FieldItemListInterface $items, $delta, array $elemen 'description' => '', ]; - /** @var \Drupal\media\Entity\MediaType $media_type */ $target_bundle = array_shift($this->fieldDefinition->getSetting('handler_settings')['target_bundles']); + /** @var \Drupal\media\Entity\MediaType $media_type */ $media_type = MediaType::load($target_bundle); $source = $media_type->getSource(); $source_data_definition = FieldItemDataDefinition::create($source->getSourceFieldDefinition($media_type)); $file_item = new FileItem($source_data_definition); - $element_info = $this->elementInfo->getInfo('media_managed_file'); + $element_info = $this->elementInfo->getInfo('media_managed_file'); $cardinality = $this->fieldDefinition->getFieldStorageDefinition()->getCardinality(); $element += [ @@ -74,9 +75,9 @@ public function formElement(FieldItemListInterface $items, $delta, array $elemen '#extended' => TRUE, // Add properties needed by value() and process() methods. '#field_name' => $this->fieldDefinition->getName(), - // This is actually talking about the bundle ON WHICH this field is - // placed, not the bundle(s) TO WHICH it can refer. In fact, it seems only - // to be used, unnecessarily, on FileWidget::validateMultipleCount(). + // This is actually talking about the entity type ON WHICH this field is + // placed, not the entity type TO WHICH it can refer. In fact, it seems + // only to be used, unnecessarily, on FileWidget::validateMultipleCount(). '#entity_type' => $items->getEntity()->getEntityTypeId(), '#display_field' => (bool) $field_settings['display_field'], '#display_default' => $field_settings['display_default'], @@ -100,6 +101,7 @@ public function formElement(FieldItemListInterface $items, $delta, array $elemen '#upload_validators' => $element['#upload_validators'], '#cardinality' => $cardinality, ]; + // @TODO: Inject the service. $element['#description'] = \Drupal::service('renderer')->renderPlain($file_upload_help); $element['#multiple'] = $cardinality != 1 ? TRUE : FALSE; if ($cardinality != 1 && $cardinality != -1) { @@ -130,7 +132,7 @@ public function massageFormValues(array $values, array $form, FormStateInterface } /** - * {@inheritdoc} + * // @TODO update the docblock, the inherited one doesn't make sense anymore. */ public static function value($element, $input, FormStateInterface $form_state) { $return = parent::value($element, $input, $form_state); @@ -145,8 +147,10 @@ public static function value($element, $input, FormStateInterface $form_state) { $file = File::load($fid); /** @var \Drupal\media\MediaInterface $media_entity */ $media_entity = Media::create([ + // @TODO: Shouldn't media entities automatically deal with empty names? 'name' => $file->getFilename(), 'bundle' => 'file', // TODO: generalize (note that we have to apply this to MediaInlineFileWidgetTest::setUp() too, in createdMediaType() call) + // @TODO: Inject the service. 'uid' => \Drupal::currentUser()->id(), // TODO: figure out langcode in this context. // 'langcode' => !empty($element['#langcode']) ? $element['#langcode'] : LanguageInterface::LANGCODE_DEFAULT, @@ -156,17 +160,20 @@ public static function value($element, $input, FormStateInterface $form_state) { ->getSourceFieldDefinition($media_entity->bundle->entity) ->getName(); $media_entity->set($source_field, $fid); + // @TODO: Figure out how to handle (if we want to handle it) what to do + // with media entities created by an "abandoned" upload (e.g. when the + // user uploads the file, then does not save the host form). $media_entity->save(); $return['mids'][] = $media_entity->id(); } } - else if (!empty($input['mids'])) { + elseif (!empty($input['mids'])) { $mid = $input['mids']; - $media_fields = 'media_' . $mid; + $media_fields_key = 'media_' . $mid; - if (!empty($input[$media_fields])) { + if (!empty($input[$media_fields_key])) { $media_entity = Media::load($mid); - foreach ($input[$media_fields] as $field_name => $field_value) { + foreach ($input[$media_fields_key] as $field_name => $field_value) { $media_entity->set($field_name, $field_value); } $media_entity->save(); @@ -175,14 +182,14 @@ public static function value($element, $input, FormStateInterface $form_state) { // Warn process() that this is a preexisting value. $return['media_already_saved'] = TRUE; } - $return['mids'] = [$input['mids']]; + $return['mids'] = [$mid]; } return $return; } /** - * {@inheritdoc} + * // @TODO update the docblock, the inherited one doesn't make sense anymore. */ public static function process($element, FormStateInterface $form_state, $form) { $element = parent::process($element, $form_state, $form); @@ -194,7 +201,7 @@ public static function process($element, FormStateInterface $form_state, $form) // first time. Just show the icon and name. $mid = $element['#value']['target_id']; } - else if (!empty($element['#value']['mids']) && count($element['#value']['mids']) === 1) { + elseif (!empty($element['#value']['mids']) && count($element['#value']['mids']) === 1) { // If $element['mids']['#value'] has multiple values, that means this is // the first pass after someone uploaded several files; ignore. If it has // only ONE, then this media entity was just created and we want to force @@ -228,8 +235,8 @@ public static function process($element, FormStateInterface $form_state, $form) '#uri' => $media_entity->getSource()->getMetadata($media_entity, 'thumbnail_uri'), '#weight' => -20, ]; - $link_ra = Link::createFromRoute($media_entity->getName(), 'entity.media.canonical', ['media' => $mid])->toRenderable(); - $element[$form_element_name]['media_name'] = $link_ra + ['weight' => -10]; + $link_build = Link::createFromRoute($media_entity->getName(), 'entity.media.canonical', ['media' => $mid])->toRenderable(); + $element[$form_element_name]['media_name'] = $link_build + ['#weight' => -10]; if ($show_form) { // Create and add the media entity form. @@ -239,10 +246,13 @@ public static function process($element, FormStateInterface $form_state, $form) '#form_mode' => $form_mode, '#parents' => $form_element_parents, ]; + // @TODO: Inject the service. $inline_form_handler = \Drupal::entityTypeManager() ->getHandler('media', $form_mode); $media_form = $inline_form_handler->entityForm($media_form, $form_state); + // @TODO Investigate the possibility of wrapping the media form elements + // in a container, to avoid inadvertently overrides during this merge. $element[$form_element_name] += $media_form; } } @@ -307,4 +317,11 @@ public static function submit($form, FormStateInterface $form_state) { static::setWidgetState($parents, $field_name, $form_state, $field_state); } + /** + * {@inheritdoc} + */ + public static function isApplicable(FieldDefinitionInterface $field_definition) { + return parent::isApplicable($field_definition); + // @TODO Restrict this to only fields targeting a single media type. + } } diff --git a/core/modules/media/tests/src/FunctionalJavascript/MediaInlineFileWidgetTest.php b/core/modules/media/tests/src/FunctionalJavascript/MediaInlineFileWidgetTest.php index f35addb..22fdc2e 100644 --- a/core/modules/media/tests/src/FunctionalJavascript/MediaInlineFileWidgetTest.php +++ b/core/modules/media/tests/src/FunctionalJavascript/MediaInlineFileWidgetTest.php @@ -4,7 +4,7 @@ use Drupal\field\Entity\FieldConfig; use Drupal\field\Entity\FieldStorageConfig; -use Drupal\Tests\field\Unit\FieldStorageConfigAccessControlHandlerTest; +use Drupal\media\MediaInterface; /** * @group media @@ -18,7 +18,26 @@ class MediaInlineFileWidgetTest extends MediaJavascriptTestBase { protected $strictConfigSchema = FALSE; - protected $node_type; + /** + * The bundle name of the test node. + * + * @var string + */ + protected $nodeTypeId; + + /** + * The bundle name of the test media. + * + * @var string + */ + protected $mediaTypeId; + + /** + * An indexed array of valid paths for test files. + * + * @var array + */ + protected $testFilePaths; /** * {@inheritdoc} @@ -26,9 +45,10 @@ class MediaInlineFileWidgetTest extends MediaJavascriptTestBase { protected function setUp() { parent::setUp(); $media_type = $this->createMediaType(['bundle' => 'file'], 'file'); + $this->mediaTypeId = $media_type->id(); + // Create two test fields on this media type, one of them required. $media_required_field_storage = FieldStorageConfig::create([ 'type' => 'string', - 'cardinality' => -1, 'entity_type' => 'media', 'field_name' => 'field_media_required', 'settings' => [ @@ -36,20 +56,56 @@ protected function setUp() { ], ]); $media_required_field_storage->save(); - $media_required_field = FieldConfig::create([ + FieldConfig::create([ 'field_storage' => $media_required_field_storage, - 'bundle' => $media_type->id(), + 'bundle' => $this->mediaTypeId, 'required' => TRUE, + ])->save(); + entity_get_form_display('media', $media_type->id(), 'add_inline') + ->setComponent('field_media_required', ['type' => 'string_textfield']) + ->save(); + $media_nonrequired_field_storage = FieldStorageConfig::create([ + 'type' => 'string', + 'entity_type' => 'media', + 'field_name' => 'field_media_nonrequired', + 'settings' => [ + 'max_length' => 255, + ], ]); - $media_required_field->save(); -// $media_form_display = entity_get_form_display('media', $media_type->id(), 'add_inline'); -// $media_form_display->setComponent('field_media_required', ['type' => 'string_textfield']); -// $media_form_display->save(); + $media_nonrequired_field_storage->save(); + FieldConfig::create([ + 'field_storage' => $media_nonrequired_field_storage, + 'bundle' => $this->mediaTypeId, + 'required' => FALSE, + ])->save(); + // Deliberately add it to the form, to test that even then we don't show + // this field when embedding the form inside the widget. + entity_get_form_display('media', $this->mediaTypeId, 'add_inline') + ->setComponent('field_media_nonrequired', ['type' => 'string_textfield']) + ->save(); + entity_get_display('media', $this->mediaTypeId, 'default') + ->setComponent('field_media_file', [ + 'type' => 'file_default', + 'label' => 'hidden', + 'settings' => [], + ]) + ->setComponent('field_media_required', [ + 'type' => 'string', + 'label' => 'hidden', + 'settings' => [], + ]) + ->setComponent('field_media_nonrequired', [ + 'type' => 'string', + 'label' => 'hidden', + 'settings' => [], + ]) + ->save(); $node_type = $this->drupalCreateContentType(); - $this->node_type = $node_type->id(); + $this->nodeTypeId = $node_type->id(); $field_storage = FieldStorageConfig::create([ 'type' => 'entity_reference', + 'cardinality' => -1, 'entity_type' => 'node', 'field_name' => 'field_media_reference', 'settings' => [ @@ -57,43 +113,90 @@ protected function setUp() { ], ]); $field_storage->save(); - $field = FieldConfig::create([ + FieldConfig::create([ 'field_storage' => $field_storage, - 'bundle' => $node_type->id(), + 'bundle' => $this->nodeTypeId, 'settings' => [ - 'file_extensions' => 'pdf', 'handler_settings' => [ 'target_bundles' => [ - $media_type->id(), + $this->mediaTypeId => $this->mediaTypeId, ], ], ], - ]); - $field->save(); - $form_display = entity_get_form_display('node', $node_type->id(), 'default'); - $form_display->setComponent('field_media_reference', ['type' => 'media_file']); - $form_display->save(); + ])->save(); + entity_get_form_display('node', $this->nodeTypeId, 'default') + ->setComponent('field_media_reference', ['type' => 'media_file']) + ->save(); + entity_get_display('node', $this->nodeTypeId, 'default') + ->setComponent('field_media_reference', [ + 'type' => 'entity_reference_entity_view', + 'label' => 'hidden', + 'settings' => [ + 'view_mode' => 'full', + ], + ])->save(); + + // Create a couple of files for testing. + for ($i = 0; $i < 2; $i++) { + $test_filename = $this->randomMachineName() . '.txt'; + $test_filepath = 'public://' . $test_filename; + file_put_contents($test_filepath, $this->randomMachineName()); + $this->testFilePaths[$i] = \Drupal::service('file_system')->realpath($test_filepath); + } + } /** - * {@inheritdoc} + * Tests the file widget behavior.. */ public function testInlineFileWidget() { - $this->drupalGet('/node/add/' . $this->node_type); - $assert = $this->assertSession(); - $element = $assert->elementExists('css', 'input[type="file"]'); - $filepath = \Drupal::root() . '/' . drupal_get_path('module', 'media') . '/tests/fixtures/example_1.pdf'; - $this->getSession()->getPage()->attachFileToField($element->getAttribute('id'), $filepath); - // Does the new media item's "name" field exist? - $this->waitUntilVisible('input[value="example_1.pdf"]', 10000); - // Does the other required field for this media type exist? - $assert->elementExists('css', 'input[name*="field_media_required"]'); // fails - // Can I upload a second file and get a second media item's "name" field? - $element = $assert->elementExists('css', 'input[type="file"]'); // fails - $filepath = \Drupal::root() . '/' . drupal_get_path('module', 'media') . '/tests/fixtures/example_2.pdf'; - $this->getSession()->getPage()->attachFileToField($element->getAttribute('id'), $filepath); - $this->waitUntilVisible('input[value="example_2.pdf"]', 10000); - // Next: save the node and then follow a link to a working media entity. + $assert_session = $this->assertSession(); + $page = $this->getSession()->getPage(); + + $this->drupalGet('/node/add/' . $this->nodeTypeId); + + // Ensure the media field is present, but no required fields exist in there. + $media_field = $assert_session->elementExists('css', 'details[data-drupal-selector="edit-field-media-reference"]'); + $assert_session->elementNotExists('css', '.required', $media_field); + + // Upload a file and see that new required fields appeared. + $first_upload_element_id = 'edit-field-media-reference-0-upload'; + $page->attachFileToField($first_upload_element_id, $this->testFilePaths[0]); + $result = $assert_session->waitForButton('Remove'); + $this->assertNotEmpty($result); + $media_field = $assert_session->elementExists('css', 'details[data-drupal-selector="edit-field-media-reference"]'); + // The name field should be present and required. + $assert_session->elementExists('css', '.field--name-name input.required', $media_field); + // The additional field sould be present and required. + $assert_session->elementExists('css', '.field--name-field-media-required input.required', $media_field); + // The non-required field should not be there. + $assert_session->elementNotExists('css', '.field--name-field-media-nonrequired input', $media_field); + + // By now the media entity should already exist, check that. + $media_id = $this->container->get('entity.query')->get('media') + ->sort('mid', 'DESC') + ->execute(); + $media_id = reset($media_id); + /** @var MediaInterface $media */ + $media = $this->container->get('entity_type.manager')->getStorage('media')->loadUnchanged($media_id); + $this->assertTrue($media->isPublished()); + + // Save the node and check page elements correspond to what is expected. + $node_title = 'Host Node 1'; + $page->fillField('Title', $node_title); + $required_text = $this->randomMachineName(); + $page->fillField('field_media_required', $required_text); + $page->pressButton('Save'); + // The node has been correctly saved. + $assert_session->pageTextContains("{$this->nodeTypeId} $node_title has been created"); + // We have a reference media container. + $media_field = $assert_session->elementExists('css', '.field--name-field-media-reference'); + // We have a file link inside that container. + $assert_session->elementExists('css', '.field--name-field-media-file a', $media_field); + // The required field is present. + $required_element = $assert_session->elementExists('css', '.field--name-field-media-required', $media_field); + // The required field text is what we expect. + $this->assertEquals($required_text, $required_element->getText()); } }