Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for must use state of AspireUpdate #79

Open
asirota opened this issue Oct 31, 2024 · 14 comments · May be fixed by #136
Open

Add support for must use state of AspireUpdate #79

asirota opened this issue Oct 31, 2024 · 14 comments · May be fixed by #136
Assignees
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Milestone

Comments

@asirota
Copy link
Member

asirota commented Oct 31, 2024

Introduce a flag to enable must use loader file. @costdev and @afragen made a generic mu-loader plugin that loads plugins from wp-content/plugins as mu-plugins yet still allows them to be updated.

https://gist.github.com/afragen/9117fd930d9be16be8a5f450b809dfa8

Flag is a Boolean constant AP_MUST_USE to enable this functionally.

@asirota asirota added the enhancement New feature or request label Oct 31, 2024
@asirota asirota added this to the Phase 1 milestone Oct 31, 2024
@asirota
Copy link
Member Author

asirota commented Oct 31, 2024

I noticed the license of this code is MIT. Do we want to mix licenses in one project?

@namithj
Copy link
Contributor

namithj commented Oct 31, 2024

We won't use that code

@asirota
Copy link
Member Author

asirota commented Oct 31, 2024

@afragen why is the code MIT licensed? Why license at all and not just submit to the project as gpl?

@afragen
Copy link
Contributor

afragen commented Oct 31, 2024

MIT license is the least restrictive license.

@afragen
Copy link
Contributor

afragen commented Oct 31, 2024

@costdev
Copy link
Contributor

costdev commented Oct 31, 2024

FYI: This code was written months ago, and licensing it with the least restrictive license made sense to ensure that it could be used without concerns.

@namithj Can you expand on "We won't use that code"?

@namithj
Copy link
Contributor

namithj commented Oct 31, 2024

We need to build a class and our scenario just focuses on this plugin to follow the coding convention used elsewhere in the plugin.

Our use case is way simpler

@afragen
Copy link
Contributor

afragen commented Oct 31, 2024

@costdev
Copy link
Contributor

costdev commented Nov 4, 2024

I'm having second thoughts on how this should be implemented.

  1. A Must-Use plugin is a serious decision by a site owner.
    • They should only be making a plugin Must-Use if they have the basic filesystem knowledge to be able to rectify it.
  2. Adding checks in AspireUpdate to copy a Must-Use loader into the wp-content/mu-plugins directory, or remove it, depending on the status of the AP_MUST_USE constant isn't clean:
    • An extra page refresh may be required when viewing the Installed Plugins list. A site owner may not realise to do this and may think they've done something wrong or that the feature isn't working.
    • Write permissions won't necessarily be available for the mu-plugins directory.
    • AspireUpdate can't cover all bases, such as if the plugin is deleted via FTP/SSH or by a future bug in WordPress Core.

As an alternative, I think we should include the MU Loader file in AspireUpdate/mu/aspire-update-mu-loader.php, with the necessary changes to load AspireUpdate as a Must-Use plugin, and provide instructions in the documentation. It's much safer to do it this way.

@namithj
Copy link
Contributor

namithj commented Nov 5, 2024

This was my original position on this matter, MU should only be for people who know what they are getting into and know how to get out of it if things go wrong. Let's just provide the loader and instructions on how to set this up as an MU plugin.

@asirota
Copy link
Member Author

asirota commented Nov 5, 2024

Sounds good. I assume this is the loader code @afragen provided.

https://gist.github.com/afragen/9117fd930d9be16be8a5f450b809dfa8

I believe I can add the path to the plugin loader file into the array as per comments.

Other than placing it in the appropriate directory, how should it be called from the plugin?

@afragen
Copy link
Contributor

afragen commented Nov 5, 2024

Yes it's the same loader code, though technically @costdev and I both wrote it. 😉

@costdev
Copy link
Contributor

costdev commented Nov 5, 2024

@asirota Other than placing it in the appropriate directory, how should it be called from the plugin?

No need for it to be called from the plugin. Once the site owner places the file in the wp-content/mu-plugins directory, Core will load the Must-Use loader file on the next page load.

@asirota asirota added the documentation Improvements or additions to documentation label Nov 5, 2024
@asirota asirota self-assigned this Nov 5, 2024
@asirota asirota removed this from Project Phase 1 Nov 5, 2024
@costdev costdev linked a pull request Nov 5, 2024 that will close this issue
@costdev
Copy link
Contributor

costdev commented Nov 5, 2024

@asirota I've added a PR as there were some more minor changes needed (such as changing the plugin name and description headers, the namespace to avoid clashes, the PHPCS exclusion, etc).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants