When the diff-module is disabled (but was enabled at some point diff_update_7307() fails with:

SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'module' cannot be null

. user_role_grant_permissions() Only works for permissions of enabled modules.

Comments

torotil created an issue. See original summary.

alan d.’s picture

alan d. commented
Category: Bug report » Support request
Status: Active » Postponed (maintainer needs more info)

Sadly, that is Drupal, but why is that an issue?

If disabled, this update shouldn't be run and if it becomes enabled, the update should be outstanding still!? What did you use to run the updates when it was disabled?

FYI, the reason there is no disabled state in D8...

#1199946: Disabled modules are broken beyond repair so the "disable" functionality needs to be removed

torotil’s picture

torotil commented

drush updb as well as update.php always update all installed modules. Not only the enabled ones.

torotil’s picture

torotil commented
Status: Postponed (maintainer needs more info) » Active

  • Alan D. committed ebbb81c on 7.x-3.x 
    Issue #2845895 by torotil, Alan D.: diff_update_7307 assumes the module...
alan d.’s picture

alan d. commented
Category: Support request » Bug report
Priority: Normal » Major
Status: Active » Fixed

You are right, wonder what brain surgeon thought doing this was a good thing.

Give it a shot (latest tarball) or the quick hack for diff_update_7307() and I can roll a new release if you have no issues.

http://cgit.drupalcode.org/diff/diff/diff.install?id=ebbb81c

  if (module_exists('diff')) {
    user_role_grant_permissions(DRUPAL_ANONYMOUS_RID, array('diff view changes'));
    user_role_grant_permissions(DRUPAL_AUTHENTICATED_RID, array('diff view changes'));
  }
torotil’s picture

torotil commented

I don't think it's a good idea to fix it this way. If the module is re-enabled later the update is already run and therefore has no effect.

  • Disable module.
  • drush updb -> schema_version=7307
  • Enable module
  • drush updb -> nothing

I think it would be better to fail (ie with a DrupalUpdateException()) and tell users to either uninstall or enable the module. Although consequences in this case aren't as severe, since the database is still in a consistent state.

alan d.’s picture

alan d. commented

A warning message is shown during the updates, which I personally think is enough if it doesn't break anything.

Status: Fixed » Closed (fixed)

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