Warn user if other modules were added or updated before install

Created on 15 November 2021, about 3 years ago
Updated 13 October 2023, about 1 year ago

Problem/Motivation

Follow up to #3245770: Create a service to composer install via package_manager from Automatic Updates β†’

When doing a Composer require of a new module it is possible that an existing module will be update or another module that is a dependency will be added.

Proposed resolution

We need to write a validator that listens to StatusCheck (we fire that). We can borrow from \Drupal\automatic_updates\Validator\StagedProjectsValidator - this is AU's validator that checks that nothing besides core has changed.

Several things could happen:
=====
- asking for Pathauto, ex.g., could additionally add Token. This is a warning, where we could allow the user to proceed.
- we might update a dependent module, like asking for Pathauto requires Token 1.3 or greater, and 1.1.0 is installed. In this case, we can proceed, but we should also tell the user that database updates MAY be required in this case, and to run updates AFTER the operation completes. They should have the opportunity to cancel.
- we might update a dependent module, which then removes a dependency of it. Ex.g. pathauto requires token 2.0, and token 1.5 is installed. Token 1x depends on ctools, but token 2x does not, and so we remove ctools. BUT, if ctools is enabled, we should ERROR and not allow the user to proceed until they uninstall ctools.

We should additionally check for error conditions by subscribing in PreApply, because we shouldn't assume that the UI button was removed or disabled or whatever by the StatusCheck, so this is extra safe.

Remaining Tasks

- βœ… Implement PreApplyEventSubscriber
- [ ] write test for PreApplyEventSubscriber
- [ ] implement StatusCheck
- [ ] write test(s) for StatusChecks (at least three cases)

πŸ“Œ Task
Status

Needs work

Version

1.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States tedbow Ithaca, NY, USA

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • First commit to issue fork.
  • πŸ‡ΊπŸ‡ΈUnited States chrisfromredfin Portland, Maine

    According to the docs in package_manager.api.php

     * - \Drupal\package_manager\Event\PostRequireEvent
     *   Dispatched after one or more Composer packages have been added to the
     *   stage directory. This event may be dispatched multiple times during the
     *   stage life cycle, and receives a list of the packages which were required
     *   into the stage directory. (Note that this is a list of packages which
     *   were specifically *asked for*, not the full list of packages and
     *   dependencies that was actually installed.)

    Doesn't this mean PostRequireEvent can't tell us everything that will be changed?

  • πŸ‡ΊπŸ‡ΈUnited States chrisfromredfin Portland, Maine

    In talking to Ted, we can check (1) what was asked for by us vs. (2) what was actually installed.
    - We need our own validator--will deal with presenting the confirmation page, probably which subscribes to StatusCheck.

    We would write a validator like \Drupal\automatic_updates\Validator\StagedProjectsValidator (this shows how we can see what changed)

    In the rare case that it might remove a dependency, we might want it to be an actual ERROR, because we'll want them to uninstall the module first.

    Should we check if database updates are needed after install? Worth a look in a separate issue. (They already have a StageDbUpdateValidator)

    Allow proceed only on _warnings_ coming back.

    Should ALSO subscribe to PreApply, so we can see if if it's about to remove a module that is currently enabled. Force user to uninstall.

    We fire StatusCheck. PreApply is a PM one.

  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    Custom Commands Failed
  • @chrisfromredfin opened merge request.
  • Status changed to Needs work over 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States chrisfromredfin Portland, Maine

    WIP MR in progress. It implements the PreApplyEventSubscriber but nothing with StatusCheck yet.

  • πŸ‡ΊπŸ‡ΈUnited States chrisfromredfin Portland, Maine
Production build 0.71.5 2024