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

Clarifying migration path for D7 Entity API #179

Open
argiepiano opened this issue Feb 7, 2022 · 18 comments
Open

Clarifying migration path for D7 Entity API #179

argiepiano opened this issue Feb 7, 2022 · 18 comments

Comments

@argiepiano
Copy link

A common issue for people migrating from D7 is the fact that some similar contrib modules are named differently in Backdrop CMS. The most egregious example is Entity API, which had a entity machine name in D7. Backdrop's core entity is not the same as D7 contrib entity. In most cases people will need Entity Plus (e.g. for metadata wrapper).

We have seen a case recently on Zulip of someone who did not realize this; it took them a while to be able to get the error log from the database to discover that they needed Entity Plus.

Given that D7 Entity API is used by almost 500,000 D7 sites, I'd like to suggest that we include a mention of this (i.e. if your site uses Entity API you will most likely need Entity Plus in Backdrop). Probably the best place for this is:

https://docs.backdropcms.org/documentation/step-1-planning (under "To Replace")
or
https://docs.backdropcms.org/documentation/step-2-preparation (under step 6).

As more people start migrating, I anticipate this will be a common occurrence that may discourage some from proceeding.

@argiepiano
Copy link
Author

argiepiano commented Feb 7, 2022

Perhaps the clarification could go even further: people should disable ALL contrib modules that depend on D7 Entity API before running update.php. This is especially true for contrib modules that are called the same in both D7 and Backdrop (e.g. ubercart, rules, entity_token, etc)

@bugfolder
Copy link
Contributor

bugfolder commented Feb 7, 2022

I agree with the need (and that Entity->Entity Plus is an important case: common, and easy to get tied up in knots over).

Some Backdrop modules (e.g., Rules) add E+ as a dependency; but that's easy to miss, and any custom module being ported won't know that it needs to add E+ as a dependency.

And for modules that are part of a long chain of dependencies (e.g., E+ <-- Rules <-- Ubercart), missing one of the early dependencies will result in the disabling and non-migration of all the dependent downstream modules. So the penalty for not getting this right can be high for the user.

So, as they say in the improv world, "Yes, AND...". In addition to having a page complete list of contributed modules that are no longer necessary, I'd like to suggest we create a page "complete list of contributed modules that need (or are recommended for) replacement," which gets mentioned in the "Categorize Drupal contributed modules" section. That new page could have two sections:

  • Modules that (almost certainly) need replacements, like replacing Entity with E+, adding Entity Token, etc.
  • Modules that we recommend replacement (like the Colorbox -> Featherlight mentioned on the Step 1 page).

@bugfolder
Copy link
Contributor

people should disable ALL contrib modules that depend on D7 Entity API before running update.php

Not sure we should recommend this, because update.php will perform upgrade steps, and for complex sets of modules, like Ubercart and its dependencies, that's a LOT of modules to have to re-enable and re-run update.php (possibly many times, given nested dependencies).

Also, if a module is missing that a downstream module depends on, the downstream module will get disabled automatically as part of running upgrade.php. (That's why Ubercart has stub modules.)

@bugfolder
Copy link
Contributor

I think that on the overview page we might emphasize the value of setting up a local copy of a Drupal site and rehearsing the migration on it before attempting the upgrade on their live site. Maybe that's already obvious to do, but it wouldn't hurt to mention it explicitly.

@argiepiano
Copy link
Author

Not sure we should recommend this, because update.php will perform upgrade steps, and for complex sets of modules, like Ubercart and its dependencies, that's a LOT of modules to have to re-enable and re-run update.php (possibly many times, given nested dependencies).

Also, if a module is missing that a downstream module depends on, the downstream module will get disabled automatically as part of running upgrade.php. (That's why Ubercart has stub modules.)

I see.

I think I don't understand the migration process correctly (I haven't migrated sites - my sites are entity- and process-heavy, with very little content, and I have actually done most of the migration by rewriting my custom modules).

I thought that someone needed to:

  • In D7, disable modules that did not exist in Backdrop before exporting the DB
  • Leave common modules enabled

The issue with the latter is that people will leave Rules or Ubercart enabled before exporting/importing the DB, only to get a lot of errors (undefined function provided by Entity Plus) when running update.php, no? And they may think that B's core entity is the equivalent to D7 contrib entity.

@bugfolder
Copy link
Contributor

So, the recommended procedure for Ubercart is that you not only leave all the UC (and Rules) modules enabled in D7, you install the stub modules in D7 and enable them in D7. That way, when you run update.php, the UC and Rules modules stay enabled AND they run their update hooks that do all the required data transformation.

@bugfolder
Copy link
Contributor

Same applies for upgrading Rules from D7.

@argiepiano
Copy link
Author

Yes, the stub modules! I knew that, just forgot 😊 .

So, what happens after update.php is completed? Are any hook calls invoked at this point? I'm asking because the Zulip poster posted errors that seemed to stem from hook_token_info_alter() being invoked. Perhaps this happened after he was done with update.php

@bugfolder
Copy link
Contributor

So, what happens after update.php is completed? Are any hook calls invoked at this point?

In principle, some hook calls could be, I guess. I couldn't tell from the Zulip stream what was causing the reported problem.

If an enabled module required Entity Plus, but didn't explicitly include it in its dependencies, that could cause problems like what was reported.

@argiepiano
Copy link
Author

The report included the bd error log that indicates Call to undefined function entity_plus_get_all_property_info() which was invoked from a hook: entity_token_token_info_alter() which is an implementation of hook_token_info_alter in the module entity_token.module.

entity_token.module does include Entity Plus as a dependency, so this is not the case:

If an enabled module required Entity Plus, but didn't explicitly include it in its dependencies, that could cause problems like what was reported.

So, the issue was that the hook was invoked before the real entity plus was enabled (not the stub). So, if these hooks are invoked as part of update.php, we have a problem. I'll try to do some testing.

@VasasA
Copy link

VasasA commented Feb 20, 2022

There are some modules, what have got special upgrade process from D7. (Rules, Organics Group, etc.)
We should alert users to this on the "Upgrade the Drupal site" page. We should improve this paragraph:
Before:

3. Download all contributed modules, themes, and layouts for your Backdrop site, and place them
in the appropriate directories.

After:

3. Download all contributed modules, themes, and layouts for your Backdrop site, and place them
in the appropriate directories. Some modules have a special upgrade process. (For example: [Rules](https://backdropcms.org/project/rules))
Check the README of the modules.

@argiepiano
Copy link
Author

@VasasA, good suggestion. I wish we could provide PRs, but we can't as this is in the docs DB. Someone with access (e.g. @bugfolder) needs to fix this.

I suggest expanding what you proposed a bit to

3. Download all contributed modules, themes, and layouts for your Backdrop site, and place them
in the appropriate directories. D7 modules that depend on Entity API (such as [Rules](https://backdropcms.org/project/rules), [Ubercart](https://backdropcms.org/project/ubercart) and others) have a special upgrade process. Check the README of the modules, and the README for Entity Plus.

I will be adding information to Entity Plus about the stub modules as well.

@VasasA
Copy link

VasasA commented Feb 20, 2022

@argiepiano I agree with your expanded version 👍 Thank You!

@bugfolder
Copy link
Contributor

These changes are good, but they're appearing in step 3 of the upgrade process, and success actually requires attention and action in steps 1 and 2. So, I've incorporated this information, and then some:

The page https://docs.backdropcms.org/documentation/upgrading-drupal-7-modules-that-have-backdrop-replacements is pretty bare-bones, so I solicit further additions to both sections. There's probably other modules that we should recommend replacements for; the Colorbox->Featherlight one was already part of the documentation.

@argiepiano
Copy link
Author

Thanks, @bugfolder. The changes look good to me. One suggestion: you mention (in several spots) Drupal 7 "Entity module" - the actual human name of this module is Entity API. I suggest we change those mentions in steps 1, 2 (subpage) and 3 to that.

@bugfolder
Copy link
Contributor

Thanks, fixed!

I was looking at the machine name and inferring the human name, which is, of course, a hazardous practice.

The plural/singular discrepancy in Entity Tokens (entity_token) always gets me, too.

@bugfolder
Copy link
Contributor

If we get backdrop/backdrop-issues#5499 in, we'll have to update all this again, but that will be a nice situation to be in.

@argiepiano
Copy link
Author

Great! Thanks.

The plural/singular discrepancy in Entity Tokens (entity_token) always gets me, too.

Yeah, that's a tricky one. I think they avoided the plural _tokens in the name to stay away from the hook_tokens()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants