Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dev/drupal#172 - module status used incorrectly in drupal 8 to determine which modules to care about for managed entities #22350

Merged
merged 1 commit into from
Jan 2, 2022

Conversation

demeritcowboy
Copy link
Contributor

Overview

https://lab.civicrm.org/dev/drupal/-/issues/172

See also conversation at #22337 (comment)

Before

Includes ALL non-core modules, and then indicates disabled status by incorrectly using the installation status. (In drupal 8 there is no such thing as disabled - "Do or do not - there is no try".)

After

Only includes installed modules, and then indicate enabled, since installed and enabled are the same in drupal 8.

Technical Details

This makes it more like the drupal 7 version

$q = db_query('SELECT name, status FROM {system} WHERE type = \'module\' AND schema_version <> -1');
foreach ($q as $row) {
$result[] = new CRM_Core_Module('drupal.' . $row->name, $row->status == 1);
, i.e.

Drupal 7: schema_version <> -1 means installed, status == 1 means enabled
Drupal 8: status == 1 means installed (and implicitly then enabled), status == 0 means uninstalled. Schema version not relevant.

Comments

You'll notice line 557 is unused. That's what #22337 is about.

@civibot
Copy link

civibot bot commented Dec 31, 2021

(Standard links)

@civibot civibot bot added the master label Dec 31, 2021
@demeritcowboy demeritcowboy force-pushed the managed-entities-drupal branch from 9ec0796 to a0e8006 Compare December 31, 2021 15:32
@seamuslee001
Copy link
Contributor

@demeritcowboy needs a rebase

@demeritcowboy demeritcowboy force-pushed the managed-entities-drupal branch from a0e8006 to 591e26f Compare January 1, 2022 00:20
@seamuslee001
Copy link
Contributor

Jenkins re test this please

@colemanw colemanw merged commit d49f3bc into civicrm:master Jan 2, 2022
@colemanw
Copy link
Member

colemanw commented Jan 2, 2022

Makes sense, thanks @demeritcowboy for figuring that out.

@demeritcowboy demeritcowboy deleted the managed-entities-drupal branch January 3, 2022 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants