Optional installation?

[afragen]

From @mgibbs189

Some users might not want plugins to be installed automatically. In each item's JSON settings, what are your thoughts on supporting an (optional) confirm setting? If it exists and is set to TRUE, the user will need to confirm installation (by clicking an "Install" link within an admin notice).

{
  "github-updater": {
    "name": "GitHub Updater",
    "slug": "github-updater/github-updater.php",
    "git": "github",
    "uri": "https://github.com/afragen/github-updater",
    "branch": "master",
    "confirm": true,
    "token":null
  }
}

[afragen]

This may be a bit more difficult than we think. Besides, if a plugin is truly a dependency, shouldn't it always be installed and activated?

[mgibbs189]

Not necessarily. A plugin could be an optional dependency. E.g. it's not required for the original plugin to run, but would enable extra functionality of the original plugin when active.

Regarding the code above, I probably should've changed the param from confirm to optional.

[afragen]

If the dependency is optional should we ask to install, install and not activate, or something different? Right now I'm not sure. I haven't really thought about how to make it optional.

[mgibbs189]

Here's an idea for optional dependencies:

When a user clicks Install Now, the plugin will be installed and activated.

[afragen]

I can certainly code that up, but we would have to figure something about the text as it would need to be more generic.

So optional dependencies would be listed under the plugin wanting them installed.

[afragen]

I think is going to require the json file, and likely a bit of code, to be rewritten. No other clean way to ensure that the dependencies are correctly listed with their plugins.

[afragen]

Matt, I have to say I'm failing in trying to get that Install Now link to work.

Somehow I need to use the link to run the install() method but it's not working for me.

[mgibbs189]

I'll try to take a look early next week. We may want to step back from the code and try to nail down all the requirements first.

[afragen]

I just merged much of what I was working on today. It will install all listed plugins but will not activate optional dependencies. Additionally, the dependent plugin and all dependencies are labeled. FYI, if testing the json format changed.

[afragen]

So I tried merging what you've done with this to allow for a json config file and multiple plugins. It's in the js-install branch. I can't seem to get the javascript to work in the after_plugin_row_. Heck, I'm totally lost with this at the moment. 😝

[mgibbs189]

Haha, I'll check it out shortly.

[afragen]

Thanks Matt

[afragen]

I've got the additional plugin rows for each optional install listed but the don't work. 😝

It would be great and future proof concept to be able to have them install similarly to how shiny updates work.

[mgibbs189]

How were you thinking it would look when a plugin has multiple dependencies?

• Should each dependency be on its own line, with an "Install" button beside it?
• Should a single "This plugin has dependencies" link appear, and clicking it would show a Modal box with all dependencies?
• Should there be an "Install All Dependencies" link appear somewhere?

You're right... optional dependencies make things a lot more complicated 😉

[afragen]

It would likely look and feel most like core if each dependency were on it's own line with an 'Install' button. If the install then looks/works like shiny updates then it's a bonus.

Once installed the line would then disappear, on a page reload. If the dependency is installed and not activated, we should probably include an 'Activate' button.

[afragen]

@mgibbs189 I just had a thought as another dev was interested in this for his themes, the current method of adding to after_plugin_row_ won't work consistently for themes. Rather than reinvent a new method for themes I think returning to a functionality within an admin notices is the way to go.

I'm going to make a new branch for this and we should probably work along that path. Let me know your thoughts.

[mgibbs189]

@afragen Are we just referring to plugins able to register themes as dependencies? Or can a theme itself require another theme as a dependency?

Sorry for the delay, been swamped with FacetWP work this week

[afragen]

No, a theme registering plugins as dependencies. As we consciously try to get theme developers to separate form and function, they will require certain plugins for function.

Don't worry about being swamped with paid work. 👍

[afragen]

@mgibbs189 I made a number of simplifications to js-install-admin-notice and fixed it to work with themes.

Of course, we ( you 😈 ) still need to get the js working.

[senlin]

I understand that this (optional dependency) never saw the light of day?

[afragen]

@senlin it's there. It will show up in an admin notice. The admin notice is dismissable for a week at a time.

What didn't work was adding the dependencies to the after_plugin_row or after_theme_row.

Just add "optional: true" to the JSON.

[senlin]

The thing is that the notice actually cannot be dismissed; after a refresh it pops right up again.

we posted at the same time, so in the json file I'm doing it correct.

Yeah, the after_plugin_row would have been very nice :)

[senlin]

You can try this out using my lean-wp plugin. I have just added there admin menu manager plugin as "optional". If you don't activate that plugin and dismiss the notice, it comes right back at you after a refresh.

Also a nice addition would be to only show the notice on the Plugins page, not on the other backend pages as that might come across as pretty irritating ;)

[afragen]

I just tried it out using lean-wp. The required plugins installed and activated. The optional plugin admin notice did not reappear on any page reload after being dismissed.

Do you have something interfering with transients perhaps?

[senlin]

huh?
That is strange, the only 2 other plugins that are active on my dev site are Yoast SEO and Hide SEO Bloat and then there is a deactivated Jetpack.

I will reset the lot and report back.

[afragen]

I hope the Lean-WP plugin is also active. 😉

[senlin]

hahaha, yup it is.

Did a reset with a default theme (twentyseventeen) and still the dismissed notice returns right after the page refresh.

[senlin]

Yes, it disappears when I click the dismiss icon. And then I visit the Dashboard or any other page in the backend and then it shows again.

[afragen]

Can you test locally? It did work for me on my test site.

[senlin]

Also locally doesn't work. Dismissing works, but after refresh the whole text is back again.

Hopefully we are talking about the same thing?

[afragen]

We are talking about the same thing.

[afragen]

I only tested dismissing the install not the activation. Let me test.

[senlin]

dismissing the install gives the same result for me: also comes back :(

[afragen]

Unfortunately I'm away for the weekend and don't have my laptop. It's still dismissed for me after reinstalling and re-activating. I'll have to figure out how to remove the transient before I can test more.

[senlin]

No worries Andy, please enjoy your weekend!

This is not something urgent and I am not even entirely sure whether I need to include that plugin as there seem to be a few hiccups, which basically is also the reason I want to make it an optional dependency in the first place...

I won't be releasing Lean WP this weekend, so that's all right and I can always add the dependency later on!

Have a great weekend!

[afragen]

Out of curiousity are there any JS errors after you click the dismiss?

[senlin]

As far as I can see, not

[senlin]

and right after clicking the dismiss icon I can see this in the console

[senlin]

The Post tab of that console shows this

[senlin]

Response is empty

[afragen]

I'll have to test more next week.

[senlin]

Sure, no worries, I need to run now too.

Talk later.

[afragen]

There should be a slug param present.

[senlin]

not there I'm afraid :(

[afragen]

https://github.com/afragen/wp-dependency-installer/blob/develop/wp-dependency-installer.php#L227

[afragen]

@senlin I've been playing around and if you've got an object cache running then the transient storing the timeout gets stored there. If that cache clears sooner, and it's likely to, then the notice will come back sooner.

[senlin]

The problem that I have been experiencing is that the notice comes back on page refresh. And the backend usually doesn't use caching or does it?

[senlin]

Hi Andy, how are you doing?
As you can see from the referenced thread, this issue has popped up its head again.

Is there anything that can be done to resolve this?

[afragen]

The admin notice should be dismissed for a week.

[senlin]

Yes, it should, but it seems that sometimes it works and sometimes it doesn't.

I have tried to find the period of one week in the code and cannot find it. If I can, then I can simply change it to forever. Is there a way to do that?

[afragen]

The line is here, https://github.com/afragen/wp-dependency-installer/blob/master/wp-dependency-installer.php#L380

The -7 should be changed to -forever, I think. That's in the other library and I'll have to check.

I'll try to test again in a clean install this weekend.

[senlin]

Thanks Andy, before I change the number I will await your tests then.

[afragen]

@senlin I think the issue is I needed to ensure that the admin notice dismissal dependency is initialized. I have it initialized in another plugin so I missed it here.

Replace your current wp-dependency-installer.php with the one from develop.

Let me know if the dismissal works. If it works, I'll push to master and a release so a composer update will bring in the changes.

[senlin]

Hi @afragen it looks like you nailed it!

I can confirm that using the wp-dependency-installer.php of the develop branch indeed enables dismissing the notice and keeping it dismissed after a refresh.

Regarding the composer update should I run that in my own repo? I can't just replace the file?

P.S. I already checked the time of dismissal in the other library and it seems to be 'forever', not '-forever'.

[afragen]

You can just replace the file.

[senlin]

figured it out with composer update :)
many thanks that this issue is finally solved and wish you a great weekend!