Create a PreApply validator that prevents Drupal projects from being removed if they are enabled

Need help with:
In EnabledProjectsValidator.php $module_list = $this->moduleExtensionList->getList(); does not contain the test_module2, which I am enabling in EnabledProjectsValidatorTest.php
So can I use $this->configFactory->get('core.extension')->get('module'); to do that.

There's not enough detail in #5. Where exactly lies the problem? You're reporting the symptom of what's not working, but what did your research/debugging show about where the problem definitely cannot be and where it possibly could be?

https://git.drupalcode.org/project/automatic_updates/-/merge_requests/82... and the first 2 hunks in https://git.drupalcode.org/project/automatic_updates/-/merge_requests/82... were my work after pairing with @yash.rode this morning for 1.5 hours.

@yash.rode, I asked you to write a comment on the issue about why those changes were necessary. I don't see that comment yet?

I got ahead with the previous problem, I have added package type in FixtureManipulator but the problem in below debug output is, the path of our test site(/private/tmp/package_manager_testing_roottest85927549/active) is not there in the $searchdirs where ExtensionDiscovery::scan() is going to scan for the extensions. So it won't be able to find /modules/contrib/test_module2/

Hi @wim.leers, I am trying to find the reason why we have to remove the node_modules/ingnore.txt and I can't remember the actual reason of removing it but .../active/core/composer.json doesn't exist(which causes the test to fail) when we have node_modules/ingnore.txt otherwise composer.json exists?

In a call with @tedbow and @phenaproxima came to a conclusion that, we can't do it this way because we won't be able to see the module we are adding in the test directory in ExtensionDiscovery::scan() as it uses the local directory to find the extensions and not the test directory.
We are going with the use of ModuleHandler by getting a list of enabled modules and then adding an custom created extension to that list and then setting the newly created extensions List.

\Drupal\automatic_updates\Validator\StagedProjectsValidator will also throw exception if an enabled module is removed so what should we do with that? for now I have disabled that validator.

Looks good overall, some questions need answering and some further simplification is possible.

addressed feedback.

Everything looks good, Nice work ✌️.

I like how straightforward this validator is. It leverages our API nicely and the test covers all cases, as far as I can tell. I think it still needs some clean-up, but no major refactoring.

Thanks to @phenaproxima's review, I spotted something else — most of my feedback needs input/confirmation from him before you should act on it, @yash.rode, so assigning to @phenaproxima!

This is really great work. I did a touch of clean-up in the test, but there really wasn't a whole lot to do!

I think all this needs is two follow-ups and a @todo, then it's good to go. For reference, the follow-ups are:

• To rename InstalledPackage::getProjectName() and InstalledPackagesList::getPackageByDrupalProjectName() (no need for a todo here)
• To make ComposerInstallersTrait add the default configuration, for DX purposes. (This one does need a todo)

@phenaproxima mentioned the 2 follow-ups that we need:

1. Done: 📌 Document the relations between packages, projects, extensions, modules, themes, and profiles Needs work — thanks @yash.rode!
2. Done: 📌 Set the default installer paths for Drupal in installComposerInstallers() Fixed — thanks @yash.rode!

Removed tag.

The end result looks amazing! 🤩👍

Just one nitpicky question left for @phenaproxima!

I think that, if we want to make all injected dependencies readonly, it's something we should do in a follow-up consistently across the module, rather than piecemeal. So that's why I reverted that change for now.

But otherwise, glad to get this safeguard in!

👍

Done: 📌 All injected services should be marked readonly Fixed .

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