8.2.16

- D8CORE-3967 Edited embeddable retains title (#97) (8f701e0)

.circleci/config.yml

@@ -4,10 +4,13 @@ version: 2
 ## Defines images and working directory.
 defaults: &defaults
   docker:
-    - image: pookmish/drupal8ci:pcov
+    - image: pookmish/drupal8ci:latest
     - image: selenium/standalone-chrome:latest
-    - image: mariadb:10.3
+    - image: circleci/mysql:5.7
       environment:
+        MYSQL_DATABASE: drupal
+        MYSQL_USER: drupal
+        MYSQL_PASSWORD: drupal
         MYSQL_ALLOW_EMPTY_PASSWORD: 1
   working_directory: /var/www/html
 

.github/pull_request_template.md

@@ -1,29 +1,66 @@
-# READY FOR REVIEW/NOT READY
+# NOT READY FOR REVIEW
 - (Edit the above to reflect status)
 
 # Summary
 - TL;DR - what's this PR for?
 
-# Needed By (Date)
-- When does this need to be merged by?
+# Review By (Date)
+- When does this need to be reviewed by?
+
+# Criticality
+- How critical is this PR on a 1-10 scale? Also see [Severity Assessment](https://stanfordits.atlassian.net/browse/D8CORE-1705).
+- E.g., it affects one site, or every site and product?
 
 # Urgency
-- How critical is this PR?
+- How urgent is this? (Normal, High)
+
+# Review Tasks
+
+## Setup tasks and/or behavior to test
+
+1. Check out this branch
+2. Rebuild Cache and import config `drush cr ; drush ci`
+3. Navigate to...
+4. Verify...
+
+### Site Configuration Sync
+
+- Is there a config:export in this PR that changes the config sync directory?
+
+## Front End Validation
+- [ ] Design is approved by @ user?
+- [ ] HTML validation: Is the markup using the appropriate semantic tags and [passes validation](https://validator.w3.org/nu/)? Or, [QA request ticket created](https://github.com/SU-SWS/template_warehouse/blob/master/jira_templates/QA_request_template.txt)?
+- [ ] Cross-browser testing: Has been performed? Or, [QA request ticket created](https://github.com/SU-SWS/template_warehouse/blob/master/jira_templates/QA_request_template.txt)?
+- [ ] Automated accessibility: Scans performed? Or, [QA request ticket created](https://github.com/SU-SWS/template_warehouse/blob/master/jira_templates/QA_request_template.txt)?
+- [ ] Manual accessibility: Manually tested? Or, [QA request ticket created](https://github.com/SU-SWS/template_warehouse/blob/master/jira_templates/QA_request_template.txt)?
+
+## Backend / Functional Validation
+### Code
+- [ ] Are the naming conventions following our standards?
+- [ ] Does the code have sufficient inline comments?
+- [ ] Is there anything in this code that would be hidden or hard to discover through the UI?
+- [ ] Are there any [code smells](https://blog.codinghorror.com/code-smells/)?
+- [ ] Are tests provided? eg (unit, behat, or codeception)
 
-# Steps to Test
+### Code security
+- [ ] Are all [forms properly sanitized](https://www.drupal.org/docs/8/security/drupal-8-sanitizing-output)?
+- [ ] Any obvious [security flaws or new areas for attack](https://www.drupal.org/docs/8/security)?
 
-1. Do this
-1. Then this
-2. Then this
+## General
+- [ ] Is there anything included in this PR that is not related to the problem it is trying to solve?
+- [ ] Is the approach to the problem appropriate?
 
 # Affected Projects or Products
 - Does this PR impact any particular projects, products, or modules?
 
 # Associated Issues and/or People
-- JIRA ticket
+- JIRA ticket(s)
 - Other PRs
 - Any other contextual information that might be helpful (e.g., description of a bug that this PR fixes, new functionality that it adds, etc.)
 - Anyone who should be notified? (`@mention` them here)
 
-# See Also
-- [PR Checklist](https://gist.github.com/sherakama/0ba17601381e3adbe0cad566ad4d80a5)
+# Resources
+- [AMP Tool](https://stanford.levelaccess.net/index.php)
+- [Accessibility Manual Test Script](https://docs.google.com/document/d/1ZXJ9RIUNXsS674ow9j3qJ2g1OAkCjmqMXl0Gs8XHEPQ/edit?usp=sharing)
+- [HTML Validator](https://validator.w3.org/)
+- [Browserstack](https://live.browserstack.com/dashboard) and link to [Browserstack Credentials](https://asconfluence.stanford.edu/confluence/display/SWS/External+Account+Credentials)

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Stanford Media
 
+8.x-2.16
+--------------------------------------------------------------------------------
+_Release Date: 2021-06-11_
+
+- D8CORE-3967 Edited embeddable retains title, tests (#97) (8f701e0)
+
 8.x-2.15
 --------------------------------------------------------------------------------
 _Release Date: 2021-04-09_

modules/media_duplicate_validation/media_duplicate_validation.info.yml

@@ -2,7 +2,7 @@ name: 'Media Duplicate Validation'
 type: module
 description: 'Media Validation plugins to help prevent duplication of media items'
 core_version_requirement: ^8.8 || ^9
-version: 8.x-2.15
+version: 8.x-2.16
 package: media
 dependencies:
   - drupal:media

src/Plugin/Validation/Constraint/EmbeddableConstraintValidator.php

@@ -2,7 +2,7 @@
 
 namespace Drupal\stanford_media\Plugin\Validation\Constraint;
 
-use Drupal\stanford_media\Plugin\media\Source\Embeddable;
+use Drupal\stanford_media\Plugin\media\Source\EmbeddableInterface;
 use Symfony\Component\Validator\Constraint;
 use Drupal\media\Plugin\Validation\Constraint\OEmbedResourceConstraintValidator;
 
@@ -19,23 +19,15 @@ public function validate($value, Constraint $constraint) {
     $media = $value->getEntity();
     $source = $media->getSource();
 
-    if (!($source instanceof Embeddable)) {
-      throw new \LogicException('Media source must implement ' . Embeddable::class);
+    if (!($source instanceof EmbeddableInterface)) {
+      throw new \LogicException('Media source must implement ' . EmbeddableInterface::class);
     }
 
     // If this is an unstructured embed, do our validation here.
     // Otherwise, pass it along to the oEmbed validation.
-    if ($source->hasUnstructured($media)) {
-      // Do not allow oEmbed values on unstructured embeds.
-      if ($source->hasOEmbed($media)) {
-        $this->context->addViolation($constraint->oEmbedNotAllowed);
-        return;
-      }
-      return;
+    if (!$source->hasUnstructured($media)) {
+      parent::validate($value, $constraint);
     }
-
-    parent::validate($value, $constraint);
-
   }
 
 }

src/Plugin/media/Source/Embeddable.php

@@ -29,7 +29,7 @@
  *   allowed_field_types = {"string", "string_long"},
  * )
  */
-class Embeddable extends OEmbed {
+class Embeddable extends OEmbed implements EmbeddableInterface {
 
   /**
    * The name of the oEmbed field.
@@ -83,15 +83,7 @@ public function buildConfigurationForm(array $form, FormStateInterface $form_sta
   }
 
   /**
-   * Gets the value for a metadata attribute for a given media item.
-   *
-   * @param \Drupal\media\MediaInterface $media
-   *   A media item.
-   * @param string $name
-   *   Name of the attribute to fetch.
-   *
-   * @return mixed|null
-   *   Metadata attribute value or NULL if unavailable.
+   * {@inheritDoc}
    */
   public function getMetadata(MediaInterface $media, $name) {
     if ($this->hasUnstructured($media)) {
@@ -113,60 +105,25 @@ public function getMetadata(MediaInterface $media, $name) {
    * @return mixed|null
    *   Metadata attribute value or NULL if unavailable.
    */
-  public function getUnstructuredMetadata(MediaInterface $media, $name) {
-    // We short-circuit requests for URL here, to ensure kernel tests pass.
-    if (($name = 'url') && ($this->hasOEmbed($media))) {
-      return NULL;
-    }
-
+  protected function getUnstructuredMetadata(MediaInterface $media, $name) {
     switch ($name) {
-      case 'default_name':
-        if ($title = $this->getMetadata($media, 'title')) {
-          return $title;
-        }
-        return parent::getMetadata($media, 'default_name');
-
-      case 'thumbnail_uri':
-        return parent::getMetadata($media, 'thumbnail_uri');
-
-      case 'html':
-        return $media->get($this->unstructuredField)->getValue();
+      case 'title':
+        return $media->label();
     }
   }
 
   /**
-   * Is there a value for the oEmbed URL?
-   *
-   * @param \Drupal\media\MediaInterface $media
-   *   A media item.
-   *
-   * @return bool
-   *   TRUE means it has an Unstructured embed, FALSE means that field is empty
-   */
-  public function hasOEmbed(MediaInterface $media) {
-    return !$media->get($this->oEmbedField)->isEmpty();
-  }
-
-  /**
-   * Is there a value for the Unstructured Embed?
-   *
-   * @param \Drupal\media\MediaInterface $media
-   *   A media item.
-   *
-   * @return bool
-   *   TRUE means it has an Unstructured embed, FALSE means that field is empty
+   * {@inheritDoc}
    */
   public function hasUnstructured(MediaInterface $media) {
-    return !$media->get($this->unstructuredField)->isEmpty();
+    return !$media->get($this->unstructuredField)->isEmpty() && $media->get($this->oEmbedField)->isEmpty();
   }
 
   /**
    * {@inheritDoc}
    */
   public function getSourceFieldConstraints() {
-    return [
-      'embeddable' => [],
-    ];
+    return ['embeddable' => []];
   }
 
   /**

src/Plugin/media/Source/EmbeddableInterface.php

@@ -0,0 +1,26 @@
+<?php
+
+namespace Drupal\stanford_media\Plugin\media\Source;
+
+use Drupal\media\MediaInterface;
+use Drupal\media\Plugin\media\Source\OEmbedInterface;
+
+/**
+ * Interface EmbeddableInterface.
+ *
+ * @package Drupal\stanford_media\Plugin\media\Source
+ */
+interface EmbeddableInterface extends OEmbedInterface {
+
+  /**
+   * Is there a value for the Unstructured Embed?
+   *
+   * @param \Drupal\media\MediaInterface $media
+   *   A media item.
+   *
+   * @return bool
+   *   TRUE means it has an Unstructured embed, FALSE means that field is empty
+   */
+  public function hasUnstructured(MediaInterface $media);
+
+}

stanford_media.info.yml

@@ -3,7 +3,7 @@ description: Provides media module configuration and plugins.
 core_version_requirement: ^8.8 || ^9
 package: media
 type: module
-version: 8.x-2.15
+version: 8.x-2.16
 dependencies:
   - dropzonejs:dropzonejs
   - drupal:breakpoint

tests/codeception/acceptance/StanfordMediaCest.php

@@ -14,4 +14,29 @@ public function testArcGis(AcceptanceTester $I){
     $I->canSee('Embeddable ArcGis has been created.');
   }
 
+  /**
+   * After editing an embeddable, the name shouldn't change.
+   */
+  public function testMediaNameSave(AcceptanceTester $I) {
+    $media = $I->createEntity([
+      'bundle' => 'embeddable',
+      'name' => 'Foo',
+      'field_media_embeddable_code' => '<iframe width="560" height="315" src="https://www.youtube.com/embed/-DYSucV1_9w" title="YouTube video player"></iframe>',
+    ], 'media');
+    $I->logInWithRole('administrator');
+    $I->amOnPage('/admin/content/media');
+    $I->canSeeLink('Foo');
+
+    $I->amOnPage($media->toUrl('edit-form')->toString());
+    $I->canSeeInField('Embed Code', '<iframe width="560" height="315" src="https://www.youtube.com/embed/-DYSucV1_9w" title="YouTube video player"></iframe>');
+    $I->canSeeInField('oEmbed URL', '');
+    $I->fillField('Embed Code', '<iframe width="565" height="310" src="https://www.youtube.com/embed/-DYSucV1_9w" title="YouTube video player"></iframe>');
+    $I->click('Save');
+    $I->canSee('has been updated.');
+
+    $I->amOnPage('/admin/content/media');
+    $I->cantSee('has been updated');
+    $I->canSeeLink('Foo');
+  }
+
 }

tests/src/Kernel/Plugin/media/Source/EmbeddableTest.php

@@ -159,15 +159,12 @@ public function testEmbeddableSource() {
     $this->assertCount(15, $oembed_media_source->getMetadataAttributes());
     $this->assertArrayHasKey('embeddable', $oembed_media_source->getSourceFieldConstraints());
 
-
     $unstructured_media_source = $this->unstructured_media->getSource();
     $this->assertEquals($this->iframe_code, $unstructured_media_source->getSourceFieldValue($this->unstructured_media));
     $this->assertTrue($unstructured_media_source->hasUnstructured($this->unstructured_media));
     $this->assertCount(15, $unstructured_media_source->getMetadataAttributes());
-    $this->assertArrayHasKey('embeddable', $unstructured_media_source->getSourceFieldConstraints());
-    $this->assertNull($unstructured_media_source->getUnstructuredMetadata($this->unstructured_media, 'title'));
+    $this->assertNotNull($unstructured_media_source->getMetadata($this->unstructured_media, 'title'));
     $this->assertStringContainsString('iframe src', $unstructured_media_source->getSourceFieldValue($this->unstructured_media));
-
   }