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

Install: save all site IDs at once when network-wide on multisite #704

Closed
JDGrimes opened this issue Jul 11, 2017 · 3 comments
Closed

Install: save all site IDs at once when network-wide on multisite #704

JDGrimes opened this issue Jul 11, 2017 · 3 comments

Comments

@JDGrimes
Copy link
Member

Currently we save each one separately, which is sub-optimal.

See #404 (comment)
Related: #703

@JDGrimes JDGrimes added this to the 2.4.0 milestone Jul 11, 2017
@JDGrimes
Copy link
Member Author

JDGrimes commented Jul 29, 2017

On further looking into the code, I don't think that we currently save site IDs when installed network-wide at all. add_installed_site_id() is only called in install() when $network is false.

However, this behavior assumes that we will always be installed on every single site on multisite when network installed, which is not true. If we have been inactive a while before we are uninstalled, then we won't have been installed on every site, if new ones were created during that time. See #404 (comment).

I suppose that this also leads to another issue, which is what happens if we were then reactivated rather than uninstalled. I suppose that we wouldn't get installed on those sites that had been newly added, although we should be. So that is another bug that ought to be addressed.

@JDGrimes
Copy link
Member Author

Our install routine does run when the plugin is reactivated though. So perhaps installing on newly added sites would happen automatically anyway?

@JDGrimes
Copy link
Member Author

We decided in #714 (comment) that there really is not a compelling reason to change the behavior to track the site IDs when network-active; as such, this ticket is invalid.

Note that if we decide to revisit this in the future, we'd have to decide what to do there when skipping install on large networks, since in that case we technically wouldn't have installed on all of the sites.

@JDGrimes JDGrimes removed this from the 2.4.0 milestone Aug 14, 2017
JDGrimes added a commit that referenced this issue Aug 21, 2017
This matches the behavior of the old un/installer class.

We don't need to track the site IDs we're installed on if we're network
installed, because then we just uninstall on all sites. We had
contemplated changing this, but decided that there wasn't sufficient
reason.

See #404, #704, #714
JDGrimes added a commit that referenced this issue Aug 21, 2017
To ensure that it is added to a list, if the installable object is added
in a code update after the extension is initially released.

We also tweak the installable class to ensure that site IDs are only
added once.

Fixes #715
See #404, #714, #713, #704
@JDGrimes JDGrimes added this to Completed in Un/installer rewrite Sep 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

1 participant