Remove the UI for installing/updating modules from update module if it is not fixed in time for release

Added related issue.

Yes this features could be drop but I think it's a nice value to have it.
As you said few developers or builders will be using this but this feature offers a larger perspective for Drupal 8.

As we can see with Wordpress, a lot of people is using the module installation from the admin because a lot of hosting companies is offering to auto install and deploy the CMS trough interfaces like plesk and I can see more and more companies offering to deploy Drupal the same way.
So I really think this feature need to be done to allow all of this potential users the ability to use and extend the drupal without using an FTP (yes a lot of those people even don't know what FTP is) like they can actually do with some other CMS.
For me cutting this functionality is like loosing some "market part" especially with all the efforts made to build-in a lot of new features without any code to produce to run them.

About the issues, we already have two critical issues on this system and I think we need quickly to postpone this or to debug and offer this tool to the users.

So we will try to fix #2042447: Install a module user interface does not install modules (or themes) first since this feature is clearly used by some people, despite the usability problems with it. However, if that issue (which I've marked major again) does not land before we get closer to release, then we do this instead, and can re-add the feature in a minor version if desired.

I object to removing this functionality. I know I should use drush, but I don't, so this is absolutely critical to me. I spend a lot of time playing with contrib modules and seeing what works and what fails and report bugs, so if you don't make it work you lose me.

I use this feature extensively on D7 and have never seen any problems with it.

To summarize the scope of the problem, #2042447: Install a module user interface does not install modules (or themes) has been sort of hijacked/rescoped for a critical regression in the feature that allows you to download a module to the file system from Drupal's UI. That issue was previously about the severe usability issues with it (the UI tells you that you are "installing" a module but it only downloads it without turning it on, and the user has no indication then that another step of enabling the module is required). There's also the security implications. But right now the feature hardly works at all in D8.

To be clear, we are talking about the "Install a module" button+UI that gets added with Update Manager, correct? The one that lets you install a module using a url or upload through the browser.

Adding #2344017: All modules loaded in authorize.php. I assume that authorize.php was intended to sandbox dangerous operations.

Oops. Apparently I forgot to update the issue the other day to note that I pinged the security team about this, since this feature originally was pushed by them.

The motivation for this was UX, adoption, admin delight, etc. The GSOC project for it was mentored by security team members b/c it's so easy to get the security elements wrong.

I don't think removing this requires a security team review.

I don't think this feature is release blocking, so I agree with demoting the issues to fix it and revisiting them before release. I'd also be comfortable with removing if it is not ready by RC1. It's a good feature from a security point of view so worthwhile keeping in core if we can but not release blocking.

Related: #2477789: Use composer to build sites.

This is now captured as part of #2485119: [meta] The Drupal 8.0.0-rc1 Release Checklist. Downgrading.

We spoke about this on our branch maintainer call today. While there's no way to know when we're going to hit the "~5 critical issue" threshold, the DA is in the process of planning a D8 Accelerate sprint from July 2 - 8. So July 1 is a logical date to revisit this and any other "fix it or nix it" issues. (If we still have a ridiculous number of criticals at that point, we'll move the evaluation date.)

FYI the issue on which this is postponed (#2042447: Install a module user interface does not install modules (or themes)) is RTBC

Just to provide an update here, we're now down to 9 release blockers (holy smokes!), making this an opportune time to revisit issues like this.

In speaking with David_Rothstein, who's done the most work on Update Manager lately, there are two critical issues remaining with its functionality:

1. #2042447: Install a module user interface does not install modules (or themes), which was RTBC on July 1.
2. #2426969: Dynamic redirects are no longer possible in the batch API (breaks updating existing modules or themes with the Update Manager), which is actually a Batch API issue. It had a working patch in February, but is currently blocked on tests.

David advocated strongly for not removing the functionality, since the feature doesn't make much sense outside of core. Worst-case, if it's not ready in time for RC1, there's a killswitch in update_manager_access() which we could have return FALSE and then hide the UI. This would be less disruptive than trying to gut the module and piece it back together again in 8.1.x+.

Given #23, here's my suggestion: upload a patch to this issue that makes update_manager_access() return FALSE, and get that patch to pass both automated tests and manual confirmation that it adequately resolves the title of this issue without any unexpected side effects. But hold off on committing that patch for another week or two to give time for #23.1 and #23.2 to land, and thereby make this patch unnecessary.

I read through this issue more carefully, and I think it made sense as a hypothetical when it was originally opened but doesn't serve a purpose anymore and should just be closed. We've known for a while that the Update Manager doesn't require any kind of large refactoring, so there's no reason to expect it to meaningfully hold up Drupal 8's release.

Keeping this open despite that is just a distraction, and also sets a bad precedent. There are many features in core that aren't required for Drupal to work. Most of them are less popular with Drupal's user base than this one, and pretty much all of them could be more reasonably implemented as a contrib module than this can. If I discover a critical bug in the Forum module, or the Quick Edit module, and if it isn't fixed right away, should I demote the issue to major and file a separate issue to remove the module from core? Of course not - not without a very specific reason. But that hasn't ever even been identified here. So:

1. We added this feature in Drupal 7 but it has fallen into disrepair.

   There are two critical issues but the disrepair isn't very deep:

   a. #2042447: Install a module user interface does not install modules (or themes) took some sleuthing by a few people to track down, but the fix isn't that complicated in the end. It's basically just that some API changes were made to request processing in Drupal 8 and the Update Manager was not changed to reflect them. The fix in that patch is relatively small and has not changed since February; pretty much all work since then has been related to automated tests (see below).

   b. #2426969: Dynamic redirects are no longer possible in the batch API (breaks updating existing modules or themes with the Update Manager) isn't even a direct bug in the Update Manager; it's redirect functionality that went missing from the batch API and that needs to be restored either way (just happens to have a devastating effect on the Update Manager). The fix is straightforward and already written.

   Obviously the big issue here is lack of automated tests - neither of the above problems would have occurred in the first place if these parts of the Update Manager were tested. Previously, the nature of the Update Manager meant that those things were impossible to test, but in #2042447: Install a module user interface does not install modules (or themes) we've now introduced a way to do it; anything that sites with a web-writable docroot can do via the Update Manager user interface can now have automated tests written for it (still not possible to test the FTP/SSH case, though). That issue already has tests written for the install case, and once it's committed it will be possible to write tests for the update case in #2426969: Dynamic redirects are no longer possible in the batch API (breaks updating existing modules or themes with the Update Manager) as well.

   Beyond the above I do not think there are any release blockers related to the Update Manager. The only thing that comes to mind is that after #2042447: Install a module user interface does not install modules (or themes), the links you see at the end of the install process are still a mess: double-escaped HTML, and the URLs point to e.g. /core/authorize.php/admin/modules too. This might not be a release blocker but it's certainly embarrassing if not fixed. The double-escaping is fixed by #1885564: theme.maintenance.inc (authorize.php) - Convert theme_ functions to Twig (part of an existing meta issue) and as far as I know it just needs a simple reroll after #2042447: Install a module user interface does not install modules (or themes) is committed. The URL issue is referenced in #2526392: authorize.php redirects to authorize.php/<none> at the end of the batch run but may need its own issue instead. I don't know if it's an easy fix, but in Drupal 7 you can just call url('admin') from authorize.php or any other external script and it just works, so if it turns out there's no easy way to do that in Drupal 8 that would be a problem on its own.

2. Core developers (and contrib developers too mostly) never use it.

   I don't know if they use it, but there is no shortage of core developers working on it. I see 10-15 experienced core developers who have contributed to fixing #2042447: Install a module user interface does not install modules (or themes), most of them in the last few months. And a lot of end users have been helping test that issue also. This is despite the fact that:

   a. For the last 8 months the issue has been demoted to "major" even though it's actually critical, which has the perverse effect of making it less likely that people will work on it.

b. The Update Manager is not a feature that extremely early adopters of Drupal 8 are likely to care about, so I doubt anyone is getting directly paid to work on it.

3. #2042447: Install a module user interface does not install modules (or themes) has been sort of hijacked/rescoped for a critical regression in the feature that allows you to download a module to the file system from Drupal's UI. That issue was previously about the severe usability issues with it (the UI tells you that you are "installing" a module but it only downloads it without turning it on, and the user has no indication then that another step of enabling the module is required).

   I don't see where that issue was ever about a usability problem. It looks to me like it it was always about the critical regression.

   I also don't think a severe usability problem exists. It doesn't leave you hanging at the end with no indication - there's a "next steps" link on the screen that says "Enable newly added modules" which takes you back to the modules page so you can enable them there. There is definitely room for improvement in that workflow, but I haven't ever heard of it causing major problems.

Given #23, here's my suggestion: upload a patch to this issue that makes update_manager_access() return FALSE

There is already an issue (and older patch) for that at #2055185: If allow_authorize_operations is FALSE, print a better error about it on Update Manager routes

the UI tells you that you are "installing" a module but it only downloads it without turning it on

I was surprised there doesn't seem to be an issue specifically about the "install" wording, so I created one and wrote an initial patch: #2543066: "Install new module" and "Install new theme" links are confusing

See also: #992190: Link to enable a new module after adding it via the Update Manager is confusing - allow users to enable it directly from that screen instead

+1 to all in #25, this is a critical regressions

#2042447: Install a module user interface does not install modules (or themes) has been committed, so this is no longer an issue.

Yes, but that's not the whole story, let's get a bit more discussion here first.

Okay. Went through Update Manager with a fine-toothed comb earlier tonight while reviewing/committing #2042447: Install a module user interface does not install modules (or themes). (See #2042447-166: Install a module user interface does not install modules (or themes) and down for the nitty-gritty details.)

So I believe we can officially declare Update Manager "functional" now. It still needs more work to be "shippable." The issue referenced in #2526392: authorize.php redirects to authorize.php/<none> at the end of the batch run got rolled into the above, so that one's no longer one of the "update manager critical" issues. However, #1885564: theme.maintenance.inc (authorize.php) - Convert theme_ functions to Twig still is one of them, because without that, the final page ends up looking like this:

Also (I don't know of an issue for this; if there isn't one, I can create it tomorrow), the links are going to the wrong place. If you try and navigate to /core/authorize.php/admin/modules you get a raw Apache 403 error.

However, both of those should be pretty straight-forward to sort out. The Twig issue already has tons of work done on it, and it's not like we don't have other ugly double-escaping bugs in core. And the other is a matter of fixing some link destinations.

Most of the other stuff in here are either "major" bugs or UI improvements that could happen in any minor release.

Therefore, I lean towards won't fix on this, myself. If anything, I think we should double-down on this feature in the Composer World™ #2538090: Allow the Update Manager to automatically resolve Composer dependencies. It's also very strategic, given competitors like e.g. WordPress ships with a much more fully-featured module browser/installer than we currently have, and inability to find good projects is one of the top concerns of site builders.

Update:

1. The double-escaped links and the broken link destinations are now both being handled via the patch at #1885564: theme.maintenance.inc (authorize.php) - Convert theme_ functions to Twig.
2. #2426969: Dynamic redirects are no longer possible in the batch API (breaks updating existing modules or themes with the Update Manager) was committed before #2042447: Install a module user interface does not install modules (or themes) so it actually went in without update tests. I created a new issue to add those tests now at #2548511: Write tests for the Update Manager downloading and applying updates for a module. Getting that in is pretty important, I think.

#1885564: theme.maintenance.inc (authorize.php) - Convert theme_ functions to Twig is now in. Just RTBCed #2548511: Write tests for the Update Manager downloading and applying updates for a module.

Discussed this with most of the other core committers today (@catch, @alexpott, @effulgentsia, @xjm).

While there were still some concerns raised about the long-term maintenance of this module (David_Rothstein, joelpittet and others have been total champs, but have not put forward an offer for MAINTAINERS.txt so this module still is effectively unmaintained), the goal of fixing it up prior to release has definitely been met. Between fixing the upstream batch API issues exposed by the module (#2426969: Dynamic redirects are no longer possible in the batch API (breaks updating existing modules or themes with the Update Manager)), the fundamental problems with the installation process itself caused by decay over time (#2042447: Install a module user interface does not install modules (or themes)), and test coverage to make sure that never happens again (#2548511: Write tests for the Update Manager downloading and applying updates for a module - not committed yet, but close), and finally cleaning up the markup which is what started unraveling this sweater to begin with :) (#1885564: theme.maintenance.inc (authorize.php) - Convert theme_ functions to Twig).

There were also concerns raised regarding how this module will interact in the Composer world (#2477789: Use composer to build sites and its ilk), but the answer is that it doesn't really. All this UI does is download the module files and get them in place, no different from a site builder right-clicking on a .tar.gz file on a project page and putting it in /modules themselves. It's down to either contrib authors to implement hook_requirements() themselves, core to provide this information itself (#2536576: Add composer_dependency module, have it help users figure out how to install Composer dependencies and/or #2494073: Prevent modules which have unmet Composer dependencies from being installed), or the UI to be extended to embrace Composer directly #2538090: Allow the Update Manager to automatically resolve Composer dependencies. (My personal hope is for the latter approach, since that would bring the benefits of Composer to developers without screwing site builders in the process.)

Therefore, officially marking this won't fix. Thank you SO much to everyone who worked hard to keep this feature. Many, many non-developers thank you. :)

Thanks for the nice summary.

While there were still some concerns raised about the long-term maintenance of this module (David_Rothstein, joelpittet and others have been total champs, but have not put forward an offer for MAINTAINERS.txt so this module still is effectively unmaintained)

For what it's worth, I think I'd be willing to sign up to maintain this at the "intermediate" level currently being discussed at #1854480: Remove inactive maintainers from MAINTAINERS.txt (i.e. commit to helping with security issues and that kind of thing) but I don't want to commit to maintaining it beyond that.

In general, maintainers tend to stick around for a shorter time than the Drupal release cycle anyway, so I think whether or not there is a current maintainer is not too relevant to the question of whether something should be in core.

I won't reopen this because I don't use the module, but one other thing to think about here is that the module is still completely non-functioning for a module that requires a composer library. The specific solution to that problem is being discussed in other issues, of course, but right now, downloading a module like that could blow up your site. I'm not sure if we want to protect against that in core, but it's something to consider, at least.