From ae4b7247b81d5197ae3d63f5f695458a603cc00e Mon Sep 17 00:00:00 2001 From: Lauri Eskola Date: Tue, 20 Jun 2023 19:48:39 +0300 Subject: [PATCH] Issue #3059955 by oknate, Hardik_Patel_12, paulocs, phenaproxima, seanB, NikolaAt, smustgrave, bnjmnm, yogeshmpawar, rensingh99, xjm, chr.fritsch, lauriii, raman.b, Wim Leers, nikunj.shah, AndySipple, harshitthakore, saurabh.tripathi.cs, AaronMcHale, irene_dobbs, alexpott, andrewmacpherson, dorficus, effulgentsia, himanshu_sindhwani, codersukanta, pankaj.singh, tripurari, snehalgaikwad, priyanka.sahni: It is possible to overflow the number of items allowed in Media Library --- .../media_library/js/media_library.ui.js | 25 +-- .../media_library/src/Form/AddFormBase.php | 43 +++- .../media_library/src/Form/FileUploadForm.php | 7 +- .../views/field/MediaLibrarySelectForm.php | 23 +++ .../MediaLibraryTestBase.php | 20 +- .../WidgetOverflowTest.php | 183 ++++++++++++++++++ .../FunctionalJavascript/WidgetUploadTest.php | 14 +- 7 files changed, 292 insertions(+), 23 deletions(-) create mode 100644 core/modules/media_library/tests/src/FunctionalJavascript/WidgetOverflowTest.php diff --git a/core/modules/media_library/js/media_library.ui.js b/core/modules/media_library/js/media_library.ui.js index 4e5529dde85..843edeb6202 100644 --- a/core/modules/media_library/js/media_library.ui.js +++ b/core/modules/media_library/js/media_library.ui.js @@ -309,6 +309,17 @@ $('.js-media-library-selected-count').html(selectItemsText); } + function checkEnabled() { + updateSelectionCount(settings.media_library.selection_remaining); + if ( + currentSelection.length === settings.media_library.selection_remaining + ) { + disableItems($mediaItems.not(':checked')); + enableItems($mediaItems.filter(':checked')); + } else { + enableItems($mediaItems); + } + } // Update the selection array and the hidden form field when a media item // is selected. $(once('media-item-change', $mediaItems)).on('change', (e) => { @@ -345,7 +356,7 @@ item.value = currentSelection.join(); }); }); - + checkEnabled(); // The hidden selection form field changes when the selection is updated. $( once( @@ -353,17 +364,7 @@ $form.find('#media-library-modal-selection'), ), ).on('change', (e) => { - updateSelectionCount(settings.media_library.selection_remaining); - - // Prevent users from selecting more items than allowed. - if ( - currentSelection.length === settings.media_library.selection_remaining - ) { - disableItems($mediaItems.not(':checked')); - enableItems($mediaItems.filter(':checked')); - } else { - enableItems($mediaItems); - } + checkEnabled(); }); // Apply the current selection to the media library view. Changing the diff --git a/core/modules/media_library/src/Form/AddFormBase.php b/core/modules/media_library/src/Form/AddFormBase.php index 83742a3a172..ffa20acfbfe 100644 --- a/core/modules/media_library/src/Form/AddFormBase.php +++ b/core/modules/media_library/src/Form/AddFormBase.php @@ -6,6 +6,7 @@ use Drupal\Core\Ajax\AjaxResponse; use Drupal\Core\Ajax\CloseDialogCommand; use Drupal\Core\Ajax\FocusFirstCommand; use Drupal\Core\Ajax\InvokeCommand; +use Drupal\Core\Ajax\MessageCommand; use Drupal\Core\Ajax\ReplaceCommand; use Drupal\Core\Entity\Entity\EntityFormDisplay; use Drupal\Core\Entity\EntityStorageInterface; @@ -700,11 +701,22 @@ abstract class AddFormBase extends FormBase implements BaseFormIdInterface, Trus return $media->id(); }, $this->getAddedMediaItems($form_state)); + $selected_count = $this->getSelectedMediaItemCount($media_ids, $form_state); + $response = new AjaxResponse(); $response->addCommand(new UpdateSelectionCommand($media_ids)); $media_id_to_focus = array_pop($media_ids); $response->addCommand(new ReplaceCommand('#media-library-add-form-wrapper', $this->buildMediaLibraryUi($form_state))); $response->addCommand(new InvokeCommand("#media-library-content [value=$media_id_to_focus]", 'focus')); + $available_slots = $this->getMediaLibraryState($form_state)->getAvailableSlots(); + if ($available_slots > 0 && $selected_count > $available_slots) { + $warning = $this->formatPlural($selected_count - $available_slots, 'There are currently @total items selected. The maximum number of items for the field is @max. Please remove @count item from the selection.', 'There are currently @total items selected. The maximum number of items for the field is @max. Please remove @count items from the selection.', [ + '@total' => $selected_count, + '@max' => $available_slots, + ]); + $response->addCommand(new MessageCommand($warning, '#media-library-messages', ['type' => 'warning'])); + } + return $response; } @@ -750,17 +762,44 @@ abstract class AddFormBase extends FormBase implements BaseFormIdInterface, Trus // The added media items get an ID when they are saved in ::submitForm(). // For that reason the added media items are keyed by delta in the form // state and we have to do an array map to get each media ID. - $current_media_ids = array_map(function (MediaInterface $media) { + $media_ids = array_map(function (MediaInterface $media) { return $media->id(); }, $this->getCurrentMediaItems($form_state)); // Allow the opener service to respond to the selection. $state = $this->getMediaLibraryState($form_state); + $selected_count = $this->getSelectedMediaItemCount($media_ids, $form_state); + + $available_slots = $this->getMediaLibraryState($form_state)->getAvailableSlots(); + if ($available_slots > 0 && $selected_count > $available_slots) { + // Return to library where we display a warning about the overage. + return $this->updateLibrary($form, $form_state); + } + return $this->openerResolver->get($state) - ->getSelectionResponse($state, $current_media_ids) + ->getSelectionResponse($state, $media_ids) ->addCommand(new CloseDialogCommand()); } + /** + * Get the number of selected media. + * + * @param array $media_ids + * Array with the media IDs. + * @param \Drupal\Core\Form\FormStateInterface $form_state + * The current form state. + * + * @return int + * The number of media currently selected. + */ + private function getSelectedMediaItemCount(array $media_ids, FormStateInterface $form_state): int { + $selected_count = count($media_ids); + if ($current_selection = $form_state->getValue('current_selection')) { + $selected_count += count(explode(',', $current_selection)); + } + return $selected_count; + } + /** * Get the media library state from the form state. * diff --git a/core/modules/media_library/src/Form/FileUploadForm.php b/core/modules/media_library/src/Form/FileUploadForm.php index b7cbe7c6a6b..2afbb0ae571 100644 --- a/core/modules/media_library/src/Form/FileUploadForm.php +++ b/core/modules/media_library/src/Form/FileUploadForm.php @@ -165,8 +165,11 @@ class FileUploadForm extends AddFormBase { // @todo Move validation in https://www.drupal.org/node/2988215 '#process' => array_merge(['::validateUploadElement'], $process, ['::processUploadElement']), '#upload_validators' => $item->getUploadValidators(), - '#multiple' => $slots > 1 || $slots === FieldStorageDefinitionInterface::CARDINALITY_UNLIMITED, - '#cardinality' => $slots, + '#multiple' => TRUE, + // Do not limit the number uploaded. There is validation based on the + // number selected in the media library that prevents overages. + // @see Drupal\media_library\Form\AddFormBase::updateLibrary() + '#cardinality' => FieldStorageDefinitionInterface::CARDINALITY_UNLIMITED, '#remaining_slots' => $slots, ]; diff --git a/core/modules/media_library/src/Plugin/views/field/MediaLibrarySelectForm.php b/core/modules/media_library/src/Plugin/views/field/MediaLibrarySelectForm.php index fe3e6b6cf60..3d8d796487b 100644 --- a/core/modules/media_library/src/Plugin/views/field/MediaLibrarySelectForm.php +++ b/core/modules/media_library/src/Plugin/views/field/MediaLibrarySelectForm.php @@ -2,7 +2,9 @@ namespace Drupal\media_library\Plugin\views\field; +use Drupal\Core\Ajax\AjaxResponse; use Drupal\Core\Ajax\CloseDialogCommand; +use Drupal\Core\Ajax\MessageCommand; use Drupal\Core\Form\FormBuilderInterface; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Url; @@ -46,6 +48,14 @@ class MediaLibrarySelectForm extends FieldPluginBase { */ public function viewsForm(array &$form, FormStateInterface $form_state) { $form['#attributes']['class'] = ['js-media-library-views-form']; + // Add target for AJAX messages. + $form['media_library_messages'] = [ + '#type' => 'container', + '#attributes' => [ + 'id' => 'media-library-messages', + ], + '#weight' => -10, + ]; // Add an attribute that identifies the media type displayed in the form. if (isset($this->view->args[0])) { @@ -125,6 +135,19 @@ class MediaLibrarySelectForm extends FieldPluginBase { // Allow the opener service to handle the selection. $state = MediaLibraryState::fromRequest($request); + $current_selection = $form_state->getValue('media_library_select_form_selection'); + $available_slots = $state->getAvailableSlots(); + $selected_count = count(explode(',', $current_selection)); + if ($available_slots > 0 && $selected_count > $available_slots) { + $response = new AjaxResponse(); + $error = \Drupal::translation()->formatPlural($selected_count - $available_slots, 'There are currently @total items selected. The maximum number of items for the field is @max. Please remove @count item from the selection.', 'There are currently @total items selected. The maximum number of items for the field is @max. Please remove @count items from the selection.', [ + '@total' => $selected_count, + '@max' => $available_slots, + ]); + $response->addCommand(new MessageCommand($error, '#media-library-messages', ['type' => 'error'])); + return $response; + } + return \Drupal::service('media_library.opener_resolver') ->get($state) ->getSelectionResponse($state, $selected_ids) diff --git a/core/modules/media_library/tests/src/FunctionalJavascript/MediaLibraryTestBase.php b/core/modules/media_library/tests/src/FunctionalJavascript/MediaLibraryTestBase.php index 3211d49b5dd..dabb30683c0 100644 --- a/core/modules/media_library/tests/src/FunctionalJavascript/MediaLibraryTestBase.php +++ b/core/modules/media_library/tests/src/FunctionalJavascript/MediaLibraryTestBase.php @@ -354,17 +354,21 @@ abstract class MediaLibraryTestBase extends WebDriverTestBase { * @param string $expected_announcement * (optional) The expected screen reader announcement once the modal is * closed. + * @param bool $should_close + * (optional) TRUE if we expect the modal to be successfully closed. * * @todo Consider requiring screen reader assertion every time "Insert * selected" is pressed in * https://www.drupal.org/project/drupal/issues/3087227. */ - protected function pressInsertSelected($expected_announcement = NULL) { + protected function pressInsertSelected($expected_announcement = NULL, bool $should_close = TRUE) { $this->assertSession() ->elementExists('css', '.ui-dialog-buttonpane') ->pressButton('Insert selected'); - $this->waitForNoText('Add or select media'); + if ($should_close) { + $this->waitForNoText('Add or select media'); + } if ($expected_announcement) { $this->waitForText($expected_announcement); } @@ -400,6 +404,18 @@ abstract class MediaLibraryTestBase extends WebDriverTestBase { } } + /** + * De-selects an item in the media library modal. + * + * @param int $index + * The zero-based index of the item to unselect. + */ + protected function deselectMediaItem(int $index): void { + $checkboxes = $this->getCheckboxes(); + $this->assertGreaterThan($index, count($checkboxes)); + $checkboxes[$index]->uncheck(); + } + /** * Switches to the grid display of the widget view. */ diff --git a/core/modules/media_library/tests/src/FunctionalJavascript/WidgetOverflowTest.php b/core/modules/media_library/tests/src/FunctionalJavascript/WidgetOverflowTest.php new file mode 100644 index 00000000000..b1601760da1 --- /dev/null +++ b/core/modules/media_library/tests/src/FunctionalJavascript/WidgetOverflowTest.php @@ -0,0 +1,183 @@ +getTestFiles('image') as $image) { + $extension = pathinfo($image->filename, PATHINFO_EXTENSION); + if ($extension === 'png') { + $this->image = $image; + } + } + + if (!isset($this->image)) { + $this->fail('Expected test files not present.'); + } + + // Create a user that can only add media of type four. + $user = $this->drupalCreateUser([ + 'access administration pages', + 'access content', + 'create basic_page content', + 'create type_one media', + 'create type_three media', + 'view media', + ]); + $this->drupalLogin($user); + } + + /** + * Uploads multiple test images into the media library. + * + * @param int $number + * The number of images to upload. + */ + private function uploadFiles(int $number): void { + /** @var \Drupal\Core\File\FileSystemInterface $file_system */ + $file_system = $this->container->get('file_system'); + + // Create a list of new files to upload. + $filenames = $remote_paths = []; + for ($i = 0; $i < $number; $i++) { + $path = $file_system->copy($this->image->uri, 'public://'); + $path = $file_system->realpath($path); + $this->assertNotEmpty($path); + $this->assertFileExists($path); + + $filenames[] = $file_system->basename($path); + $remote_paths[] = $this->getSession() + ->getDriver() + ->uploadFileAndGetRemoteFilePath($path); + } + $page = $this->getSession()->getPage(); + $page->fillField('Add files', implode("\n", $remote_paths)); + $this->assertMediaAdded(); + $assert_session = $this->assertSession(); + foreach ($filenames as $i => $filename) { + $assert_session->fieldValueEquals("media[$i][fields][name][0][value]", $filename); + $page->fillField("media[$i][fields][field_media_test_image][0][alt]", $filename); + } + } + + /** + * Tests that the Media Library constrains the number of selected items. + * + * @param string|null $selected_operation + * The operation of the button to click. For example, if this is "insert", + * the "Save and insert" button will be pressed. If NULL, the "Save" button + * will be pressed. + * + * @dataProvider providerWidgetOverflow + */ + public function testWidgetOverflow(?string $selected_operation): void { + // If we want to press the "Save and insert" or "Save and select" buttons, + // we need to enable the advanced UI. + if ($selected_operation) { + $this->config('media_library.settings')->set('advanced_ui', TRUE)->save(); + } + + $assert_session = $this->assertSession(); + $page = $this->getSession()->getPage(); + $this->drupalGet('node/add/basic_page'); + // Upload 5 files into a media field that only allows 2. + $this->openMediaLibraryForField('field_twin_media'); + $this->uploadFiles(5); + // Save the media items and ensure that the user is warned that they have + // selected too many items. + if ($selected_operation) { + $this->saveAnd($selected_operation); + } + else { + $this->pressSaveButton(); + } + $this->waitForElementTextContains('.messages--warning', 'There are currently 5 items selected. The maximum number of items for the field is 2. Please remove 3 items from the selection.'); + // If the user tries to insert the selected items anyway, they should get + // an error. + $this->pressInsertSelected(NULL, FALSE); + $this->waitForElementTextContains('.messages--error', 'There are currently 5 items selected. The maximum number of items for the field is 2. Please remove 3 items from the selection.'); + $assert_session->elementNotExists('css', '.messages--warning'); + // Once the extra items are deselected, all should be well. + $this->deselectMediaItem(2); + $this->deselectMediaItem(3); + $this->deselectMediaItem(4); + $this->pressInsertSelected('Added 2 media items.'); + } + + /** + * Tests that unlimited fields' selection count is not constrained. + * + * @param string|null $selected_operation + * The operation of the button to click. For example, if this is "insert", + * the "Save and insert" button will be pressed. If NULL, the "Save" button + * will be pressed. + * + * @dataProvider providerWidgetOverflow + */ + public function testUnlimitedCardinality(?string $selected_operation): void { + if ($selected_operation) { + $this->config('media_library.settings')->set('advanced_ui', TRUE)->save(); + } + + $assert_session = $this->assertSession(); + // Visit a node create page and open the media library. + $this->drupalGet('node/add/basic_page'); + $this->openMediaLibraryForField('field_unlimited_media'); + $this->switchToMediaType('Three'); + $this->uploadFiles(5); + if ($selected_operation) { + $this->saveAnd($selected_operation); + } + else { + $this->pressSaveButton(); + } + + if ($selected_operation !== 'insert') { + $this->pressInsertSelected(); + } + // There should not be any warnings or errors. + $assert_session->elementNotExists('css', '.messages--error'); + $assert_session->elementNotExists('css', '.messages--warning'); + $this->waitForText('Added 5 media items.'); + } + + /** + * Data provider for ::testWidgetOverflow() and ::testUnlimitedCardinality(). + * + * @return array[] + * Sets of arguments to pass to the test method. + */ + public function providerWidgetOverflow(): array { + return [ + 'Save' => [NULL], + 'Save and insert' => ['insert'], + 'Save and select' => ['select'], + ]; + } + +} diff --git a/core/modules/media_library/tests/src/FunctionalJavascript/WidgetUploadTest.php b/core/modules/media_library/tests/src/FunctionalJavascript/WidgetUploadTest.php index 37be455fc3d..223c98b0a21 100644 --- a/core/modules/media_library/tests/src/FunctionalJavascript/WidgetUploadTest.php +++ b/core/modules/media_library/tests/src/FunctionalJavascript/WidgetUploadTest.php @@ -6,7 +6,7 @@ use Drupal\media\Entity\Media; use Drupal\Tests\TestFileCreationTrait; /** - * Tests that uploads in the Media library's widget works as expected. + * Tests that uploads in the 'media_library_widget' works as expected. * * @group media_library * @@ -23,7 +23,7 @@ class WidgetUploadTest extends MediaLibraryTestBase { protected $defaultTheme = 'stark'; /** - * Tests that uploads in the Media library's widget works as expected. + * Tests that uploads in the 'media_library_widget' works as expected. */ public function testWidgetUpload() { $assert_session = $this->assertSession(); @@ -204,7 +204,8 @@ class WidgetUploadTest extends MediaLibraryTestBase { // Assert we can now only upload one more media item. $this->openMediaLibraryForField('field_twin_media'); $this->switchToMediaType('Four'); - $this->assertFalse($assert_session->fieldExists('Add file')->hasAttribute('multiple')); + // Despite the 'One file only' text, we don't limit the number of uploads. + $this->assertTrue($assert_session->fieldExists('Add file')->hasAttribute('multiple')); $assert_session->pageTextContains('One file only.'); // Assert media type four should only allow jpg files by trying a png file @@ -547,7 +548,8 @@ class WidgetUploadTest extends MediaLibraryTestBase { // Assert we can now only upload one more media item. $this->openMediaLibraryForField('field_twin_media'); $this->switchToMediaType('Four'); - $this->assertFalse($assert_session->fieldExists('Add file')->hasAttribute('multiple')); + // Despite the 'One file only' text, we don't limit the number of uploads. + $this->assertTrue($assert_session->fieldExists('Add file')->hasAttribute('multiple')); $assert_session->pageTextContains('One file only.'); // Assert media type four should only allow jpg files by trying a png file @@ -618,7 +620,9 @@ class WidgetUploadTest extends MediaLibraryTestBase { $selection_area = $this->getSelectionArea(); $assert_session->checkboxChecked("Select $existing_media_name", $selection_area); $selection_area->uncheckField("Select $existing_media_name"); - $assert_session->hiddenFieldValueEquals('current_selection', ''); + $page->waitFor(10, function () use ($page) { + return $page->find('hidden_field_selector', ['hidden_field', 'current_selection'])->getValue() === ''; + }); // Close the details element so that clicking the Save and select works. // @todo Fix dialog or test so this is not necessary to prevent random // fails. https://www.drupal.org/project/drupal/issues/3055648