The Drupal Core "Add new module" feature is a menu option on the "Extend" menu. It offers the option to add a module to the site from a URL or through an upload. It does not use Composer, so seems at odds with the Project browser approach.
This menu could be confusing for novice Drupal site builders, as it is not obvious that it is unrelated to the Project browser, and that dependencies will not be managed for modules added in this way.
Open the Extend menu on any D8+ site. Click on "Add new module".
Ultimately this option should be removed from Drupal core, or at least not available on the default menu system. Issue #3285191: [meta] Only support Drupal core installs managed by composer is open for this.
As an interim measure, consider adding a new module that simply disables the "Add new module". The module could go further, and prevent access to the Add new module page, but this seems excessive since the goal is simply to prevent confusion.
After discussion, the preferred option seems to be adding a checkbox to the settings page, and disabling the "Add new module" page if it is checked.
I'm unsure whether the module should be a sub-module of Project browser, or an independent project.
It would be useful for the module to be enabled when the DrupalPod link opens so that new users trying out Project Browser for the first time aren't confused by the menu option.
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
lostcarpark commentedComment #3
lostcarpark commentedComment #5
lostcarpark commentedI have created a simple module called "Disable add new moldule" as a sub-module.
I used a RouteSubscriber to deny access to the "/admin/modules/install" route. I had hoped this would hid the menu, since normally the menu system will hide menus you don't have access to, but I think the admin user has access to all menus.
Instead I used "hook_menu_links_discovered_alter" to remove any menu items with the route.
I also added a functional test to verify access is denied to the Add new module page, and that uninstalling the module restores access.
I considered adding a test that the menu was removed, but I don't think Add new module appears on any menu unless you have Admin Toolbar enabled, and I didn't want to add a test that depends on a contrib module.
Comment #6
lostcarpark commentedComment #7
lostcarpark commentedOne question to consider is should this be a sub-module, or just a setting in the main project browser modules? Or perhaps even just decide that .tar modules are incompatible with PB and disable the page when PB is enabled.
Advantages of merging: Keeps everything self contained.
Advantages of separate module: Module only needs to do one thing. When .tar functionality gets removed from core, easier to remove from PB.
Thoughts?
Comment #8
lostcarpark commentedAfter discussion with @chrisfromredfin and @timplunkett on Slack, I have moved the functionality into the Project Browser module. I've added an option to the Project browser settings, enabled by default.
TODO:
At present, after changing "Disable add new menu" setting, it is necessary to rebuild cache for the changes to take effect. The menu cache should invalidate when the setting changes.
Add additional tests to test for changing setting.
Comment #9
lostcarpark commentedI tried various methods of clearing partial cache to make "Add new module" page show after changing the setting. I could get the menu item to appear. However, I was unable to find a cache to clear to make the page accessible. At present, I am using
drupal_flush_all_caches();, which is a bit heavy handed, but it does avoid adding new dependency injection, so will make it easier to remove this feature when the "Add new module" page gets removed in Drupal core.Still need to add some tests for the settings page.
Comment #10
lostcarpark commentedUpdated issue title and description to reflect that this is now a setting in the PB module, not a separate module.
Comment #11
lostcarpark commentedAdded test to verify that unchecking the "Disable add new module" enables the Add new module page, and that re-checking it disables the page again.
I think this is ready for review.
Comment #12
fjgarlin commentedI tested via drupalpod and the functionality is the expected one.
I made a few comments in the code, mostly about which caches to clear (if any). As they are mostly questions rather than asking for changes (I guess it depends on the answers) I'm leaving the issue status as it is now.
Comment #13
fjgarlin commentedI added some suggestions (most of them really minor).
I also re-tested things via drupalpod, and found a weird behavior:
* When checking the disable checkbox, things work as expected. The link to install new modules shows before the change, and does not show after the change.
* When unchecking the disable checkbox, things don't change. The links to install new modules does not show before the change, and still does not show after the change.
Obviously, this is due to not clearing all caches now, but I still think it's the way to go, so it seems like we might need to add something more so that both scenarios work as expected.
Comment #14
lostcarpark commentedComment #15
fjgarlin commentedWe still need to address the case where the link won't show back when unticking the checkbox.
We also need to clear the render cache. I've been testing and adding this line fixes the issue, we just need to add the service via dependency injection.
\Drupal::service('cache.render')->invalidateAll();Comment #16
lostcarpark commentedThanks! I've added the render cache invalidation.
Comment #17
fjgarlin commentedGreat. I've tested multiple times and reviewed the code a few times as well. I'm going to mark it RTBC.
Thanks so much for this feature!
Comment #18
chrisfromredfinDouble-checked, reviewed, and tested in DrupalPod -- and love it!
Comment #20
chrisfromredfin