Problem/Motivation

The autologout_update_9201 hook in autologout.install attempts to
install the js_cookie module as a dependency.

However, js_cookie is no longer listed as a dependency in
autologout.info.yml. Running drush updb on a fresh install or on
sites that do not have js_cookie available results in a fatal error:

  Unable to install modules js_cookie due to missing modules js_cookie.

Steps to reproduce

  1. Install Drupal with the autologout module version 8.x-1.4.
  2. Run drush updb to ensure the database is up to date.
  3. Update the autologout module to version 8.x-1.7.
  4. Run drush updb.
  5. Observe the error: Unable to install modules js_cookie due to missing modules
    js_cookie.

Proposed resolution

Remove the autologout_update_9201 hook from autologout.install
entirely, as the js_cookie dependency no longer exists in the module.

Remaining tasks

  • Push, Review and merge patch.

User interface changes

None.

API changes

None.

Data model changes

None.

Issue fork autologout-3580316

Command icon Show commands

Start 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:

About issue forks

Comments

joaopauloscho created an issue. See original summary.

joaopauloscho’s picture

joaopauloscho commented

I'm working on this.

joaopauloscho opened merge request !95

joaopauloscho’s picture

joaopauloscho commented
Status: Active » Needs review

Ready for review, thanks.

yannickoo’s picture

yannickoo
Location Berlin
commented
Status: Needs review » Needs work

Really nice, thanks for this! I think you would need to implement hook_update_last_removed to let Drupal be aware of that.

jonathandealmeida made their first commit to this issue’s fork.

jonathandealmeida’s picture

jonathandealmeida commented
Status: Needs work » Needs review

The hook_update_last_removed() was added.

yannickoo’s picture

yannickoo
Location Berlin
commented
Status: Needs review » Reviewed & tested by the community

Thanks for quick action, RTBC from my side 💪

the_g_bomb’s picture

the_g_bomb commented

As a thought, is it worth keeping the js_cookie dependency in the composer.json file for anyone else upgrading after this goes in, so that the module can be properly uninstalled?

We can drop the composer dependency in a future release.

We could also return a message saying the dependency has been dropped and the module can be uninstalled if it is no longer required elsewhere. If it is needed, users need to make sure they add it to their own composer json file.

the_g_bomb’s picture

the_g_bomb commented

This should target the 2.x branch
As per the comment here:
https://www.drupal.org/project/autologout/issues/3308456#comment-16516672
We should be be mindful of the doc block comments

the_g_bomb opened merge request !97

the_g_bomb’s picture

the_g_bomb commented

Adding an alternative as a Proof of concept solution

yannickoo’s picture

yannickoo
Location Berlin
commented

I can confirm that I had to re-add the Composer dependency to fix our pipeline when deploying this 😇 Having that dependency back will simplify the process.

the_g_bomb’s picture

the_g_bomb commented

Sorry, I've been really busy at work, and I hope to get back to this asap.

  • the_g_bomb committed 409b7df8 on 2.x 
    bugfix: #3580316 autologout_update_9201 fails after js_cookie dependency...
the_g_bomb’s picture

the_g_bomb commented
Version: 8.x-1.x-dev » 2.x-dev
Status: Reviewed & tested by the community » Fixed

Merged, thank you.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • the_g_bomb committed 409b7df8 on 8.x-1.x 
    bugfix: #3580316 autologout_update_9201 fails after js_cookie dependency...
ressa’s picture

ressa
he/him
commented

Thanks for working on this. I just updated to 2.0.3 and saw this message:

> [notice] The js_cookie library is no longer used by autologout and can be uninstalled if it is not needed by any other modules.

But when I tried I got this message (and I now see "require": { "drupal/js_cookie": "*" } in composer.json):

Removal failed, drupal/js_cookie is still present, it may be required by another package. See `composer why drupal/js_cookie`.
Composer [remove drupal/js_cookie] failed, composer command failed: exit status 2. stderr=

After reading the comments here, maybe the update feedback text could be updated to something like this, to be more precise?

> [notice] The js_cookie library is no longer used by autologout. For stability during upgrades, it will remain a requirement for now, and will be dropped as a Composer dependency in a future release.

the_g_bomb’s picture

the_g_bomb commented

Hmm, in retrospect, the wording could be clearer.
The intention was uninstalled means drush pmu js_cookie
Removed would be composer remove drupal/js_cookie

When I release a 3.0 version, the composer JSON file won't have drupal/js_cookie included in it, so the module will get removed if nothing else needs it.

ressa’s picture

ressa
he/him
commented

Thanks for a fast answer @the_g_bomb, and you're so right: To install/uninstall (Drush/GUI) or download/delete (Composer) a module are two different things. I conflated them, thinking that if a module is uninstalled, it ought to be deletable as well, which it is not in this case ... And a perfectly fine choice, since it prevents problems during the phase out of js_cookie, and I totally support that model.

So the wording is correct, but perhaps we could tag on a sentence, something along these lines?

> [notice] The js_cookie library is no longer used by autologout and can be uninstalled if it is not needed by any other modules. It will be removed as a Composer dependency in version 3.

Status: Fixed » Closed (fixed)

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