GUI install multiple modules at once.

narendrar → changed the visibility of the branch 2.0.x to hidden.

narendrar → changed the visibility of the branch 3330887-gui-install-multiple to hidden.

This MR still needs work, but it would be helpful to get early feedback on code/functionality.

To test this feature, enable Allow multiple installs at once checkbox at admin/config/development/project_browser

thank you! a few initial thoughts testing the current state.

• on the first look it feels odd to have "Queue" as the only available option / button label. if i want to simply install a single module i have to still queue a module and then process the queue with just a single module in. i am unable to directly install that module with one click.
• Queue and Deque are quite abstract terms micro copy wise, same for the button label "Process queue". Plus "Process queue" sort of hides the fact that modules are being installed.
• when you queue a module the process queue button gets appended at the end of the result list. problem here is for one for screenreader users the appearence of that button isnt announced (same as the state change of the toggled button from queued to dequeued) but it is also sort of cumbersome for keyboard users to have the process queue button not in close proximity where they just queued another module. if you queue one of the modules at the top of your list and you have for example 48 modules shown per page it is quite the stretch to reach the process queue button at the end of the result list by just tabbing.

oh and one fun fact. i've tried to process a queue but that had no effect (the button label jumed back to process queue after after a while). i then had one module queued then switched the number of modules shown per page count. that made the list of results reload. but the spinner spun indefinitely before any new results showed. so after while i simply did a reload and now i am presented with the installer and i am forwarded to core/install.php :D that was completely unexpected. not sure if there is a way to provide any infos what might have caused that behavior? i dont want to just run a siteinstall and wipe the database (it seems to be it is still available and in place), wanted to wait on a feedback first. without the gui changes in front of me right now i am unable to ideate on potential other approaches ui wise for now.

ha it looks like it is reproducible. I have set up a new instance with the MR applied:

• i've queued token and pathauto
• then processed the queue.
• after a while processing the the spinner disappears again (by the way it would be good to have the spinner inline and in context of the process queue button, at the moment it is just an overlay across module cards, which has a high cognitive load)
• after that i simply changed from 12 to 48 modules shown -> the indefinite spinner starts again and on reload the drupal installer is shown again.

@rkoller, this is all good feedback but I think we might want to handle it in follow-up issues (as well as re-titling this issue for clarity). We're simply trying to get the plumbing in place to install multiple modules at once, which is a significant lift; the UI is less of a concern for the moment.

Tagging this for a follow-up to make sure that your feedback is captured in another issue.

all good, thanks for the clarification! the only detail perhaps relevant within the scope of this issue is the problem outlined in #10 ✨ GUI install multiple modules at once. Needs work , or is it only caused by the ui and isn't caused in the plumbing?

p.s. even if i run drush si another time i am still getting re directed to the install page

Still more work needs to be done, but here is current functionality:
1. In branch 2.0.x, After clicking Add and Install and then reloading page. (Does not worked, need to Unlock Install stage)
2. In branch 2.0.x, After clicking Add and Install and then increasing pagination size from 12 to 24 (It worked, but status on button does not changed to installed, will create issue for that)
3. Add multiple modules with multiple install MR and click Process Queue button and wait. (It works as expected)
4. Repeated step 1 for multiple install MR. (Same problem as step 1, need to unlock stage)
5. Repeated step 2 for multiple install MR. (Same problem as step 1, need to unlock stage, but if you wait, modules will get installed)

I discussed this issue with @bnjmnm today.

In reviewing the code with @narendrar, I felt a certain tension between what the code is trying to -- which, by the way, is a perfectly reasonable approach -- and the way Project Browser wants to work: install one thing at a time, creating and destroying a sandbox for each thing that gets installed. That has led to the current code needing to do some very tricky state management.

Then Ben and I remembered something -- Package Manager can require multiple things at once into a single sandbox! That, I realized, is game-changing.

Instead of trying to create a queue of projects to require into the sandbox, I think we should actually keep the current approach, wherein you create the sandbox, require everything, and then apply the sandbox, in one go. The only difference should be that, rather than requiring one module at a time into the sandbox, we require several!

To put it another way: if the user selects Token, Metatag, and Pathauto in Project Browser, then the following single call should be made to Package Manager:

$installer->require(['drupal/token', 'drupal/metatag', 'drupal/pathauto']);

This has the major advantage that it will immediately fail if any of the chosen projects have dependency conflicts amongst themselves. It also means that we get to keep the front-end changes relatively simple, and don't need to implement a queue.

That said, the main blocker to being able to do this is the fact that Project Browser has a route for requiring a project into the sandbox, and it assumes you're only requiring a single project, whose ID is passed in the URL. We need to change that route so that it can accept a list of project IDs, not just a single one.

So I propose the following: let's open an issue that blocks this one, and which changes the project_browser.stage.require route such that:

• Its URL becomes /admin/modules/project_browser/install-require. (No more {source} or {id} parameters).
• It is a POST route, not a GET route.
• The request body needs to be a JSON-encoded array of arrays, where each array is a source ID and project ID. Like this:
  

  [["some_source","some_project_id"],["some_source","another_project_id"]]
  

In that same issue, let's also change the project_browser.activate route to work the same way, so we can activate multiple projects at once.

Once we have that committed, we can come back to this issue, and then implement a much simpler set of changes that simply adjusts the current front-end code to pass a list of chosen projects to the backend. (The "Queue" and "Dequeue" stuff, and the "Install N projects" button, will function more or less the way they already do.)

I think this will get us where we need to be much more cleanly and quite a bit more simply. It's a win.

Blocker created: ✨ Change the project_browser.stage.require and project_browser.activate routes to accept a list of project IDs Active

And, unblocked!

narendrar → changed the visibility of the branch multiple_modules to hidden.

narendrar → changed the visibility of the branch 3330887-Testing-do-not-use to hidden.

Tests failure seems unrelated to this MR changes. See 💬 Nightwatch tests failing due to lack of "capabilities" - need Selenium driver Needs review and 📌 [IGNORE] Test issue Active

phenaproxima → changed the visibility of the branch gui_branch to hidden.

phenaproxima → changed the visibility of the branch gui_branch to active.

I solved the failing tests!

Turns out the problems is that the GitLab CI template now runs Chromedriver in W3C mode (at least when the next major version of core is in use), which doesn't allow you to "click" an option element. So I don't think our changes here introduced this problem - it was a latent thing waiting to be discovered. Having said that, the fix was easy; just handle option elements properly in clickWithWait(). We're good now.

Beyond that, I think something in the JS could be a little more cleanly encapsulated but I don't think I have too much to complain about, overall. Coding standards checks are failing, too, but should be easy to resolve.

I gave this another review and I'm a little concerned about certain scoping issues and a few other things. I don't want to hold up commit, but this feels like it could be a little tighter in some places...

I think in going "all in" on the multiple-at-a-time, it's good to remove / change the functionality of the "add and install" button. That button becomes more of a checkbox of what you're putting in your "Cart" (for lack of a better word), and we move the "Install >" action ("checkout") somewhere else (my current random thought is a sticky ribbon at the bottom of the page?)

I'm running low on things to complain about. You're probably sick of me kicking this back by now. But this really does make sense and I like the UX improvement we're doing here, but there are a few things I can't sign off on, at least not without more explanation:

• Changing that test assertion to account for a different number of items is a problem - that number shouldn't have changed. Nothing in this code should result in that. Changing it just to make the tests pass isn't good enough unless we can also explain why that change makes the test pass.
• I'm not clear on why the list of queued projects is segmented by tab. That really shouldn't be the case - projects are specific to a particular source. The queue should be everything queued across all sources, regardless of which tab they came from. The whole segmenting-things-by-tab is a pattern I would like to see removed, ultimately, from Project Browser.
• There are some random failures waiting to happen in the JS test coverage. Also, there's some coverage that was removed which I'm not sure should be.

Re #30,

1. This is failing in 2.0.x also https://git.drupalcode.org/project/project_browser/-/merge_requests/567#note_384275
2. Can we do it in a follow-up?
3. Done

This is failing in 2.0.x also

Oh, are these random failures?? If they are, I'm not clear on how changing the assertion is going to fix them.

Can we do it in a follow-up?

Sure, but I'm curious why. Is it harder to flatten the queue than to segment it by tab? If so, why is that?

Oh, are these random failures?? If they are, I'm not clear on how changing the assertion is going to fix them.

These are failure in 2.0.x and nothing to do with changes we have done.

Sure, but I'm curious why. Is it harder to flatten the queue than to segment it by tab? If so, why is that?

It's not hard to do, recommending it so that what you suggested can be done in a single issue. Currently we are showing 'Install selected projects' based on queued projects under specific plugin which otherwise needs to check each queued project and then needs to be updated.

I ran a git bisect and confirmed that 📌 Improve the categories filter type in context to the rest of the filter component ui Fixed broke the results count.

There are a few assertions which could be made less flaky, and one follow-up to file.

Otherwise, I'd call this RTBC from a code perspective. There are things we could fine-tune and streamline and clean-up but this appears to accomplish what it sets out to do.

@narendrar, can you add screenshots?

Marking for manual testing as well.

I have addressed all feedback and added screenshots.

I've tested the latest changes. one detail to note, the install selected projects button is visible and reachable with the toolbar module installed

while it is not visible nor reachable with the navigation module installed on drupal 11.x

Re #37, Thanks for reporting this @rkoller. Actually the install selected projects button was hidden under admin toolbar which is fixed now.

No complaints at the code level.

This has been manually tested now by @rkoller, and he didn't find too much, which is a good sign because he usually finds a lot! It's also been reviewed, although I'm not sure how deeply, by @bnjmnm - his feedback has not yet been addressed. Since that will help make the tests less fragile, I'm kicking this back to just do what he has asked, and once that's done, this is ready in my opinion.

I tried to address Ben's feedback in previous commit.

OK, well, as far as I can tell we're good to go here. At long last, let's ship it.

I've manually tested this and while I don't LOVE the UI, my feedback is very likely minor, most likely considered feature requests, and can readily be dealt with as a follow-up. This is a big step for 2.0.x-dev as we move forward. To get the plumbing in there's no reason to wait on this.

Huge!

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