I've just updated to Panopoly from 7.x-1.28 to 7.x-1.30 and during the application of database updates, both via `drush updb` and via /update.php I am encountering the following error on Panopoly_widgets schema update #7016:

The following updates returned messages
panopoly_widgets module
Update #7016

    Failed: PDOException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'module' cannot be null: INSERT INTO {role_permission} (rid, permission, module) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2); Array ( [:db_insert_placeholder_0] => 2 [:db_insert_placeholder_1] => use media wysiwyg [:db_insert_placeholder_2] => ) in user_role_grant_permissions() (line 3163 of /data/www/modules/user/user.module).

Using `drush updb` this caused two other db updates (Field Group #7008 and File Entity #7216) to not proceed, although these two other updates seem to have applied correctly through /update.php, which generated the above error on Panopoly_widgets #7016.

Thoughts?

Comments

nerdcore created an issue. See original summary.

nerdcore’s picture

I see that there is no checking whether a role exists or not during the call to user_role_grant_permissions:

https://api.drupal.org/comment/52883#comment-52883

I do not have the permission 'use media wysiwyg' defined within my site, therefore granting this permission is pretty sure to fail.

dsnopek’s picture

Status: Active » Needs work
StatusFileSize
new762 bytes

Hm. The error is actually that the 'module' is set to NULL. Do you have the media_wysiwyg module enabled? If so, what version is the module?

Since media_wysiwyg is actually enabled by panoploy_wysiwyg, we probably should have put this update hook over there. :-/ At the very least, at this point we should wrap this in a check for media_wysiwyg.

nerdcore’s picture

@dsnopek, no the media_wysiwyg module appears to be in state "not installed" on this site.

FWIW it appears as version 7.x-2.0-beta1 located in profiles/panopoly/modules/contrib/media/modules/media_wysiwyg

Ether’s picture

Wrong place to post. Sorry.

aprohl5’s picture

Status: Needs work » Reviewed & tested by the community

This patch worked for me. It appears that panopoly_widgets has no issue queue so this seems like a reasonable place to have this issue posted to me.

dsnopek’s picture

Status: Reviewed & tested by the community » Needs work

@aprohl5: Thanks for the testing!

Unfortunately, I think this patch still isn't quite right. It fixes the update hook, but we still have something in the feature for a permission that won't exist unless you have media_wysiwyg installed.

I need to think about this more, but I'm feeling like we should:

  1. Duplicate this update hook in panopoly_wysiwyg (where it probably should have been in the first place)
  2. Apply the fix in the current patch or just empty out the update hook altogether. I'm torn on whether we should be correcting for changes to media that don't affect Panopoly features that are in use. Maybe we should keep it just for consistency so that people who updated at different times don't have different default configurations?
  3. Move the 'defaultconfig' for the 'use media wysiwyg' permission to panopoly_wysiwyg

I'm going to work on this more later today!

  • dsnopek committed c726946 on 7.x-1.x
    Update Panopoly Widgets and WYSIWYG for Issue #2646372 by dsnopek:...
dsnopek’s picture

Performed the changes mentioned in #7! Fixed.

  • dsnopek committed c726946 on 8.x-2.x
    Update Panopoly Widgets and WYSIWYG for Issue #2646372 by dsnopek:...

  • dsnopek committed 96982fd on 7.x-1.x
    Issue #2646372 by dsnopek: Panopoly_widgets Schema Update 7016 Integrity...
  • dsnopek committed 9c314e1 on 7.x-1.x
    Issue #2646372 by dsnopek: Panopoly_widgets Schema Update 7016 Integrity...

  • dsnopek committed 96982fd on 8.x-2.x
    Issue #2646372 by dsnopek: Panopoly_widgets Schema Update 7016 Integrity...