Ensure VideoEmbedThumbnailFormatter has the correct dependencies on the selected image style when being exported. [#2676938]

Problem/Motivation

When some formatter configuration for the thumbnail formatter is exported and the associated image style isn't there, it blows up.

Proposed resolution

To help prevent this, we should make sure the image style is added as a dependency to the formatter configuration.

Remaining tasks

Create a patch that overrides ::calculateDependencies for VideoEmbedThumbnailFormatter.

User interface changes

None.

API changes

None.

Data model changes

None.

Comment	File	Size	Author
#32	2676938-thumbnail-dependencies-32.patch	10.37 KB	Sam152
#32	8.x-1.x: PHP 5.5 & MySQL 5.5, D8.1 29 pass PHP 5.5 & PostgreSQL 9.1, D8.1 27 pass, 2 fail PHP 5.5 & SQLite 3.8, D8.1 29 pass PHP 5.6 & MySQL 5.5, D8.1 29 pass PHP 7 & MySQL 5.5, D8.1 29 pass PHP 7 & PostgreSQL 9.1, D8.1 29 pass PHP 7 & SQLite 3.14, D8.1 29 pass
#32	interdiff.txt	5.09 KB	Sam152
#23	2676938-thumbnail-dependencies-23.patch	8.93 KB	Sam152
#23	8.x-1.x: PHP 5.5 & MySQL 5.5, D8.1 28 pass, 3 fail PHP 5.5 & PostgreSQL 9.1, D8.1 28 pass, 3 fail PHP 5.5 & SQLite 3.8, D8.1 28 pass, 3 fail PHP 5.6 & MySQL 5.5, D8.1 28 pass, 3 fail PHP 7 & MySQL 5.5, D8.1 28 pass, 3 fail PHP 7 & PostgreSQL 9.1, D8.1 27 pass, 2 fail PHP 7 & SQLite 3.14, D8.1 28 pass, 3 fail
#23	interdiff.txt	1.65 KB	Sam152
#20	2676938-thumbnail-dependencies-20.patch	8.93 KB	Sam152
#20	8.x-1.x: PHP 5.5 & MySQL 5.5, D8.1 30 pass PHP 5.5 & PostgreSQL 9.1, D8.1 30 pass PHP 5.5 & SQLite 3.8, D8.1 30 pass PHP 5.6 & MySQL 5.5, D8.1 30 pass PHP 7 & MySQL 5.5, D8.1 30 pass PHP 7 & PostgreSQL 9.1, D8.1 30 pass PHP 7 & SQLite 3.14, D8.1 30 pass
#20	interdiff.txt	695 bytes	Sam152
#12	interdiff.txt	6.56 KB	Sam152
#12	2676938-thumbnail-dependencies-12.patch	8.84 KB	Sam152
#12	8.x-1.x: PHP 5.5 & MySQL 5.5, D8.1 29 pass, 2 fail PHP 5.5 & PostgreSQL 9.1, D8.1 27 pass, 4 fail PHP 5.5 & SQLite 3.8, D8.1 29 pass, 2 fail PHP 5.6 & MySQL 5.5, D8.1 29 pass, 2 fail PHP 7 & MySQL 5.5, D8.1 29 pass, 2 fail PHP 7 & PostgreSQL 9.1, D8.1 29 pass, 2 fail PHP 7 & SQLite 3.14, D8.1 29 pass, 2 fail
#4	2676938-thumbnail-dependencies-4.patch	6.51 KB	Sam152
#4	8.x-1.x: PHP 5.5 & MySQL 5.5, D8.1 29 pass, 1 fail PHP 5.5 & PostgreSQL 9.1, D8.1 25 pass, 4 fail PHP 5.5 & SQLite 3.8, D8.1 29 pass, 1 fail PHP 5.6 & MySQL 5.5, D8.1 29 pass, 1 fail PHP 7 & MySQL 5.5, D8.1 29 pass, 1 fail PHP 7 & PostgreSQL 9.1, D8.1 29 pass, 1 fail PHP 7 & SQLite 3.14, D8.1 29 pass, 1 fail
#4	interdiff.txt	3.02 KB	Sam152
#2	2676938-thumbnail-dependencies-2.patch	3.49 KB	Sam152
#2	8.x-1.x: PHP 5.5 & MySQL 5.5, D8.1 27 pass PHP 5.5 & PostgreSQL 9.1, D8.1 25 pass, 2 fail PHP 5.5 & SQLite 3.8, D8.1 27 pass PHP 5.6 & MySQL 5.5, D8.1 27 pass PHP 7 & MySQL 5.5, D8.1 27 pass PHP 7 & PostgreSQL 9.1, D8.1 27 pass PHP 7 & SQLite 3.14, D8.1 27 pass

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

27 February 2016 at 09:24

Sam152 created an issue. See original summary.

Comment #2

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented 1 March 2016 at 09:30

Status:

Active

» Needs review

File	Size
2676938-thumbnail-dependencies-2.patch	3.49 KB
8.x-1.x: PHP 5.5 & MySQL 5.5, D8.1 27 pass PHP 5.5 & PostgreSQL 9.1, D8.1 25 pass, 2 fail PHP 5.5 & SQLite 3.8, D8.1 27 pass PHP 5.6 & MySQL 5.5, D8.1 27 pass PHP 7 & MySQL 5.5, D8.1 27 pass PHP 7 & PostgreSQL 9.1, D8.1 27 pass PHP 7 & SQLite 3.14, D8.1 27 pass

Initial patch based entirely on the core ImageFormatter. Need to look into how this can be tested.

Also need to deal with the colorbox formatter which also has an image style.

Comment #3

1 March 2016 at 09:29

Status:

Needs review

» Needs work

The last submitted patch, 2: 2676938-thumbnail-dependencies-2.patch, failed testing.

Comment #4

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented 1 March 2016 at 10:02

Status:

Needs work

» Needs review

File	Size
interdiff.txt	3.02 KB
2676938-thumbnail-dependencies-4.patch	6.51 KB
8.x-1.x: PHP 5.5 & MySQL 5.5, D8.1 29 pass, 1 fail PHP 5.5 & PostgreSQL 9.1, D8.1 25 pass, 4 fail PHP 5.5 & SQLite 3.8, D8.1 29 pass, 1 fail PHP 5.6 & MySQL 5.5, D8.1 29 pass, 1 fail PHP 7 & MySQL 5.5, D8.1 29 pass, 1 fail PHP 7 & PostgreSQL 9.1, D8.1 29 pass, 1 fail PHP 7 & SQLite 3.14, D8.1 29 pass, 1 fail

Added a test. Tests both the thumbnail and colorbox formatter, so this should fail until the colorbox formatter has been accounted for.

Comment #5

1 March 2016 at 10:05

Status:

Needs review

» Needs work

The last submitted patch, 4: 2676938-thumbnail-dependencies-4.patch, failed testing.

Comment #6

1 March 2016 at 10:05

The last submitted patch, 4: 2676938-thumbnail-dependencies-4.patch, failed testing.

Comment #7

1 March 2016 at 10:05

The last submitted patch, 4: 2676938-thumbnail-dependencies-4.patch, failed testing.

Comment #8

1 March 2016 at 10:05

The last submitted patch, 4: 2676938-thumbnail-dependencies-4.patch, failed testing.

Comment #9

1 March 2016 at 10:06

The last submitted patch, 4: 2676938-thumbnail-dependencies-4.patch, failed testing.

Comment #10

1 March 2016 at 10:06

The last submitted patch, 4: 2676938-thumbnail-dependencies-4.patch, failed testing.

Comment #11

1 March 2016 at 10:06

The last submitted patch, 4: 2676938-thumbnail-dependencies-4.patch, failed testing.

Comment #12

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented 1 March 2016 at 10:48

Status:

Needs work

» Needs review

File	Size
2676938-thumbnail-dependencies-12.patch	8.84 KB
8.x-1.x: PHP 5.5 & MySQL 5.5, D8.1 29 pass, 2 fail PHP 5.5 & PostgreSQL 9.1, D8.1 27 pass, 4 fail PHP 5.5 & SQLite 3.8, D8.1 29 pass, 2 fail PHP 5.6 & MySQL 5.5, D8.1 29 pass, 2 fail PHP 7 & MySQL 5.5, D8.1 29 pass, 2 fail PHP 7 & PostgreSQL 9.1, D8.1 29 pass, 2 fail PHP 7 & SQLite 3.14, D8.1 29 pass, 2 fail
interdiff.txt	6.56 KB

Added test assertions for when the image style is deleted and added support in the colorbox formatter for onDependencyRemoval and calculateDependencies.

Still 1 failing colorbox test.

Comment #13

1 March 2016 at 10:50

Status:

Needs review

» Needs work

The last submitted patch, 12: 2676938-thumbnail-dependencies-12.patch, failed testing.

Comment #14

1 March 2016 at 10:51

The last submitted patch, 12: 2676938-thumbnail-dependencies-12.patch, failed testing.

Comment #15

1 March 2016 at 10:52

The last submitted patch, 12: 2676938-thumbnail-dependencies-12.patch, failed testing.

Comment #16

1 March 2016 at 10:53

The last submitted patch, 12: 2676938-thumbnail-dependencies-12.patch, failed testing.

Comment #17

1 March 2016 at 10:54

The last submitted patch, 12: 2676938-thumbnail-dependencies-12.patch, failed testing.

Comment #18

1 March 2016 at 10:54

The last submitted patch, 12: 2676938-thumbnail-dependencies-12.patch, failed testing.

Comment #19

1 March 2016 at 10:54

The last submitted patch, 12: 2676938-thumbnail-dependencies-12.patch, failed testing.

Comment #20

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented 1 March 2016 at 10:54

Status:

Needs work

» Needs review

File	Size
interdiff.txt	695 bytes
2676938-thumbnail-dependencies-20.patch	8.93 KB
8.x-1.x: PHP 5.5 & MySQL 5.5, D8.1 30 pass PHP 5.5 & PostgreSQL 9.1, D8.1 30 pass PHP 5.5 & SQLite 3.8, D8.1 30 pass PHP 5.6 & MySQL 5.5, D8.1 30 pass PHP 7 & MySQL 5.5, D8.1 30 pass PHP 7 & PostgreSQL 9.1, D8.1 30 pass PHP 7 & SQLite 3.14, D8.1 30 pass

Annoying, but this should green everything up.

Comment #21

benjy CreditAttribution: benjy at PreviousNext commented 1 March 2016 at 10:55

Status:

Needs review

» Needs work

+++ b/src/Plugin/Field/FieldFormatter/Thumbnail.php
@@ -36,6 +38,13 @@ class Thumbnail extends FormatterBase implements ContainerFactoryPluginInterface
+  protected $image_style_storage;

@@ -126,9 +135,10 @@ class Thumbnail extends FormatterBase implements ContainerFactoryPluginInterface
+    $this->imageStyleStorage = $image_style_storage;

Assigned to the wrong variable name.

+++ b/src/Tests/FormatterDependenciesTest.php
@@ -0,0 +1,109 @@
+class FormatterDependenciesTest extends WebTestBase {

I'd have thought you'd have gotten away with KTB just for testing the dependencies?

Comment #22

benjy CreditAttribution: benjy at PreviousNext commented 1 March 2016 at 10:55

Status:

Needs work

» Needs review

Cross posted with the bot.

Comment #23

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented 1 March 2016 at 11:08

File	Size
interdiff.txt	1.65 KB
2676938-thumbnail-dependencies-23.patch	8.93 KB
8.x-1.x: PHP 5.5 & MySQL 5.5, D8.1 28 pass, 3 fail PHP 5.5 & PostgreSQL 9.1, D8.1 28 pass, 3 fail PHP 5.5 & SQLite 3.8, D8.1 28 pass, 3 fail PHP 5.6 & MySQL 5.5, D8.1 28 pass, 3 fail PHP 7 & MySQL 5.5, D8.1 28 pass, 3 fail PHP 7 & PostgreSQL 9.1, D8.1 27 pass, 2 fail PHP 7 & SQLite 3.14, D8.1 28 pass, 3 fail

Thanks good catch.
KTB may have worked, but knew I could get this going fast with the web test base already in the module.

Comment #24

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented 1 March 2016 at 11:10

I might try a KTB version if I get time and see if it's easy enough to convert. This really isn't using a browser, but it is using some methods off WebTestBase.

Comment #25

1 March 2016 at 11:11

Status:

Needs review

» Needs work

The last submitted patch, 23: 2676938-thumbnail-dependencies-23.patch, failed testing.

Comment #26

1 March 2016 at 11:12

The last submitted patch, 23: 2676938-thumbnail-dependencies-23.patch, failed testing.

Comment #27

1 March 2016 at 11:12

The last submitted patch, 23: 2676938-thumbnail-dependencies-23.patch, failed testing.

Comment #28

1 March 2016 at 11:13

The last submitted patch, 23: 2676938-thumbnail-dependencies-23.patch, failed testing.

Comment #29

1 March 2016 at 11:13

The last submitted patch, 23: 2676938-thumbnail-dependencies-23.patch, failed testing.

Comment #30

1 March 2016 at 11:14

The last submitted patch, 23: 2676938-thumbnail-dependencies-23.patch, failed testing.

Comment #31

1 March 2016 at 11:14

The last submitted patch, 23: 2676938-thumbnail-dependencies-23.patch, failed testing.

Comment #32

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented 1 March 2016 at 14:06

Status:

Needs work

» Needs review

File	Size
interdiff.txt	5.09 KB
2676938-thumbnail-dependencies-32.patch	10.37 KB
8.x-1.x: PHP 5.5 & MySQL 5.5, D8.1 29 pass PHP 5.5 & PostgreSQL 9.1, D8.1 27 pass, 2 fail PHP 5.5 & SQLite 3.8, D8.1 29 pass PHP 5.6 & MySQL 5.5, D8.1 29 pass PHP 7 & MySQL 5.5, D8.1 29 pass PHP 7 & PostgreSQL 9.1, D8.1 29 pass PHP 7 & SQLite 3.14, D8.1 29 pass

KTBified.

Comment #33

1 March 2016 at 14:10

Status:

Needs review

» Needs work

The last submitted patch, 32: 2676938-thumbnail-dependencies-32.patch, failed testing.

Comment #34

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented 1 March 2016 at 14:12

Status:

Needs work

» Needs review

Comment #35

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented 1 March 2016 at 14:12

Comment #36

benjy CreditAttribution: benjy at PreviousNext commented 2 March 2016 at 08:02

Status:

Needs review

» Reviewed & tested by the community

Looks good apart from there is no upgrade path. Should be as simple as re-saving the form that selects the image and then re-exporting the config.

Comment #37

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented 2 March 2016 at 08:05

Yeah, I think it's fine for this to be an additional benefit for new installations and installations that decide to resave their formatters.

Thanks for the review.

Comment #38

2 March 2016 at 08:06

Sam152 committed aa95d3a on 8.x-1.x

Issue #2676938 by Sam152, benjy: Ensure VideoEmbedThumbnailFormatter has...

Comment #39

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented 2 March 2016 at 08:07

Status:

Reviewed & tested by the community

» Fixed

Comment #40

16 March 2016 at 08:14

Status:

Fixed

» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.