Version 2.0.10 breaks existing installations

There should be an update hook if a new module needs to be installed.

A change like this should not be done in a patch-level release.

An update hook would be too late in this case, since processes like a cache refresh will have failed by then already. That's why this one is really tricky.

Hi, until the bug is not solved please add a warning on the description in the module page. Thank you.

Facing the same issue

Followed with 📌 Option to merge terms (Term Merge integration) Needs review
As - term_merge:term_merge is in dependencies
Quick fix drush en term_merge is needed in old sites

I suggest a quick fix follow up release with a hook update will help a lot

/**
 * Issue #3444071: Enable the Term Merge module
 */
function taxonomy_manager_update_8201() {
  if (!\Drupal::moduleHandler()->moduleExists('term_merge')) {
    \Drupal::service('module_installer')->install(['term_merge'], FALSE);
  }
}

In some cases, this may work. But if the latest composer update brought some new services, then you would have to refresh cache before you can run the update, but that cache refresh fails.

Yes, just worked with the same fix.
Tested and looks good for me with taxonomy_manager 2.0.10 and Drupal 10.2.5

Solution depends on the pipeline. hook update works for me with such deploy:
1. composer install
2. drush updb -y
3. drush cim -y
4. drush cr
Note, that on local I run drush updb with this patch and exported configs with enabled term_merge module.

I re-tested #7-9 with fresh install and had error for not enabled term_reference_change module dependency:

Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException: The service "term_merge.term_merger" has a dependency on a non-existent service "term_reference_change.migrator".

Fixed, updated MR and created this patch.

Got it.
I agrees with Volodymyr .. Keep the default code for install.
In my testing round I did not face issues.

Switching to your MR changes and patch.
Better logic.

Thanks Vselivanov,
Path #12 works on Drupal 10.2.4

If updates need to be run before a cache clear, that should be clearly documented on the release notes.

Patch #12 works for me.

This was my experience coming from 2.0.9 in Drupal 10.2:

1. Updated 2.0.9 to 2.0.10 via `composer update`.
2. term_merge and term_reference_change modules got installed with 2.0.10.
3. Ran `drush updb` and got the error "Error: Class "Drupal\term_merge\Form\MergeTerms" not found in include() (line 21 of /var/www/html/web/modules/contrib/taxonomy_manager/src/Form/MergeTermsForm.php) #0 /var/www/html/vendor/composer/ClassLoader.php(576): include()"
4. Applied patch #12 in Composer with cweagans/composer-patches => `composer update`.
5. `drush updb` => 'yes' at the prompt.
6. `drush cex` => 'yes' at the prompt.
7. `drush cr`

All seems OK.

On a 2nd site, I added the patch before updating and it went smoothly.

As Jürgen already said, simply enabling the module in an update hook won't work. Using automatic deployments you cannot simply run drush updb multiple times, they will break on errors.
If some parts of your code rely on external modules, it's advisable to move this code into a submodule. In this case, all code related to MergeTermForm should live in a submodule of Taxonomy Manager.
Otherwise you'll break the majority of the sites having Taxonomy Manager.

Same issue. I enabled term_merge on my local development containers and the error is now gone - I have not yet tested our automated deployment to other servers - but I agree that there needs to be a smoother upgrade path in place.

• Smoother update process
• Smoother deployable process

I agree with Stefan on

In this case, all code related to MergeTermForm should live in a submodule of Taxonomy Manager.

Well, based on D7 version and https://www.drupal.org/project/term_merge/issues/3160872 ✨ Integration with Taxonomy Manager Needs review it seems taxonomy_manager had built-in term merge integration, so I don't agree to move it to the separate module. Besides, we should add it to the taxonomy_manager module composer.json dependency.
However, it is an issue and the upgrade path should be tested.

A change this big should not be done in a patch-level release. This is a breaking change. Adding it as a submodule would be fine.

I've reverted the commits integrating Term Merge and added a submodule to provide the functionality.

> A change this big should not be done in a patch-level release. This is a breaking change. Adding it as a submodule would be fine.
Well, a new major version sounds like a good approach. As it was said and if we check the official module info - https://www.drupal.org/project/taxonomy_manager → and
D7 version had this functionality without submodules (correct me if I'm wrong). And I think it makes sense to provide this support out of the box.
To me, it sounds like a missed feature request.

This is a critical issue, I've discussed it with one more maintainer (Klausi) and we agreed to use a submodule approach.
I'll merge this PR and prepare a new release.

A new release is here - https://www.drupal.org/project/taxonomy_manager/releases/2.0.11 →
I'm moving it to fixed. For term merge submodule issues let's open new issues.

Andriy Khomych → closed merge request !31

Great!

> For term merge submodule issues let's open new issues.

Uhm, the merge request already includes the new submodule: Taxonomy Manager Merge. All code related to the integration of Term Merge went into this module.

> Uhm, the merge request already includes the new submodule: Taxonomy Manager Merge. All code related to the integration of Term Merge went into this module.

Yup, I worked fast on this issue, so in case we have some submodule issues, let's open new issues :)

So, removing that dependency and publishing yet another minor, just broke some sites again, since an enabled module is no longer available, as it's removed from the requirement list. :-( Just saying, as people who fixed the problem manually, they just got hit a second time.

One suggestion: When making a release, make an -rc release and wait for it to be two weeks old without any release-blocker issues arising before making a full release. Most problems like this would be caught in the release candidate stage.

Follow-up issue for 'missing module' problems after updating to 2.0.11: https://www.drupal.org/project/taxonomy_manager/issues/3445744 🐛 Missing modules after updating from 2.0.10 to 2.0.11 RTBC

> just broke some sites again, since an enabled module is no longer available, as it's removed from the requirement list

Uhm, I hate to say this, but you should always export your configuration if you change anything on your site (including updating modules). If you did, the list of enabled modules is saved to {config-export-directory}/core.extension.yml and no module is uninstalled accidentally because it isn't required by other config anymore.

At least the release notes should have made some notes about the changes and possible outcomes.

@stBorchert, that's not the issue. Of course, we have saved config. But the code of the enabled module is no longer there, because the 2.0.11 update removed that dependency and now the modules is gone, but still enabled.

> One suggestion: When making a release, make an -rc release and wait for it to be two weeks old without any release-blocker issues arising before making a full release. Most problems like this would be caught in the release candidate stage.

Thanks, I will take it into account.

> At least the release notes should have made some notes about the changes and possible outcomes.

Yeah, I've updated the release notes.

> So, removing that dependency and publishing yet another minor, just broke some sites again, since an enabled module is no longer available, as it's removed from the requirement list. :-( Just saying, as people who fixed the problem manually, they just got hit a second time.

I see :( Well, in such a case I suggest adding them manually back and uninstalling them. Sorry for the inconvenience.

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