media_vimeo 2.1 was recently released:

https://www.drupal.org/node/2496657

Looks like mostly bug fixes.

The Video widget and adding embeding videos in the WYSIWYG editor will need manual testing!

CommentFileSizeAuthor
#12 update_media_vimeo_to-2498437-12.patch2.64 KBjfrederick
#5 panopoly_widgets-update_media_vimeo_to-2498437-5.patch1.91 KBmglaman
#2 panopoly_widgets-media-vimeo-2.1-2498437-2.patch551 bytesdsnopek

Comments

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
Issue summary: View changes
Issue tags: +Novice
Related issues: +#2504829: Update Vimeo configuration export

This should be easy enough to do, but let's try and solve #2504829: Update Vimeo configuration export before commiting this.

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
Status: Active » Needs review
StatusFileSize
new panopoly_widgets-media-vimeo-2.1-2498437-2.patch551 bytes

Here's a patch! I'll run it on Travis-CI in a moment.

EDIT: Here's the build on Travis-CI: https://travis-ci.org/panopoly/panopoly/builds/69170302

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
Status: Needs review » Needs work

The tests failed! At the very least, this might need updates to the tests if they aren't correctly detecting the videos any more.

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
Parent issue: » #2530998: [meta] Get Panopoly 1.x into maintenance mode (so we can focus on Drupal 8)
mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Status: Needs work » Needs review
StatusFileSize
new panopoly_widgets-update_media_vimeo_to-2498437-5.patch1.91 KB

The issue is because the missing Vimeo Image display format. Patch includes that, and does some weight altering to ensure the video formatter comes before image.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

Running test on my fork: https://travis-ci.org/mglaman/panopoly/builds/70541730

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
Status: Needs review » Needs work

When running this patch on Travis, it sees the Feature as overridden:

Component type: file_display
array(
< 'video__default__media_vimeo_image' => array(
< 'api_version' => 1,
< 'name' => 'video__default__media_vimeo_image',
< 'settings' => array(
< 'image_style' => 'panopoly_image_video',
< ),
< 'status' => TRUE,
< 'weight' => 1,
< ),
'video__default__media_youtube_image' => array(
'api_version' => 1,

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented

The code for this looks right! We'll probably need an update hook to clear the right caches so the new 'video__default__media_vimeo_image' display is recognized on existing sites.

Trying the tests again: https://travis-ci.org/panopoly/panopoly/builds/71442396

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented

Tests still failing. :-/

jfrederick’s picture

jfrederick commented

I don't know anything about Travis CI builds, but I only see [error] messages referring to the now-fixed menu_block bug #2497297: Errors on non-interactive install with drush site-install (in line 699 of https://travis-ci.org/panopoly/panopoly/jobs/71442397 and line 764 of https://travis-ci.org/panopoly/panopoly/jobs/71442398), so I wonder if the tests are worth running again?

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented

Here's a new build:

https://travis-ci.org/panopoly/panopoly/builds/71929797

However, looking at the last build, it still looks like it's the Feature being overridden (see #7) that's causing the failure. I think something in the patch needs to be changed so the new bit of config in the Feature will take effect.

jfrederick’s picture

jfrederick commented
Status: Needs work » Needs review
StatusFileSize
new update_media_vimeo_to-2498437-12.patch2.64 KB

The Feature was overridden because the new file display in the Feature was not recorded in the Feature's .info file. Here is an updated patch.

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented

Ah, ok! Here's a new Travis build:

https://travis-ci.org/panopoly/panopoly/builds/71997737

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented

The feature is no longer overridden in the tests! But the tests are still failing for reasons that aren't related to this patch (because even tests without this patch are failing) so I've opened a new issue for that: #2537902: Chrome tests failing on Travis-CI

Once that issue is fixed and tests are passing in general, I expect this issue will pass tests and we can commit!

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented

Now that #2537902: Chrome tests failing on Travis-CI is fixed, kicking off a new Travis build:

https://travis-ci.org/panopoly/panopoly/builds/72509587

I'm expecting this to pass. And if it does, then it's time to commit! :-)

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented

Hiding old patches.

  • dsnopek committed d80e1ba on 7.x-1.x 
    Update Panopoly Widgets for Issue #2498437 by dsnopek, mglaman,...
dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
Status: Needs review » Fixed

Tests passed (finally)! Committed. :-)

Status: Fixed » Closed (fixed)

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