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

Installables: add uninstaller factory for points type settings #733

Open
JDGrimes opened this issue Oct 16, 2017 · 5 comments
Open

Installables: add uninstaller factory for points type settings #733

JDGrimes opened this issue Oct 16, 2017 · 5 comments
Assignees

Comments

@JDGrimes
Copy link
Member

Would be particularly useful because points type settings don't need to be removed when WordPoints itself is going to be uninstalled, because in that case the points types will be deleted entirely. That is a bit of an edge-case, however.

Note that currently there isn't even a function to delete a particular points type setting, but see #314.

@JDGrimes
Copy link
Member Author

Also worth noting that the points functions aren't loaded by default during uninstall.

@JDGrimes
Copy link
Member Author

The biggest issue here is that these functions will end up calling is_wordpoints_network_active() when retrieving the option. Which gives a doing-it-wrong error during uninstall. To avoid that, we'd have to retrieve the option directly. So that's why this would need to be a part of core, because in case we change how points type settings are stored in the future.

@JDGrimes JDGrimes self-assigned this Oct 17, 2017
@JDGrimes JDGrimes added this to the 2.4.0 milestone Oct 17, 2017
@JDGrimes
Copy link
Member Author

One problem with extensions that would like to use this is that the extensions are uninstalled before the components are. So the autoloading for the points component, where this class ought to be stored, won't be set up yet. This would mean that it would need to be included manually. That seems kind of silly though, since when WordPoints as a whole is being uninstalled, we don't need to run this routine anyway. (And when only the extension is being installed, the points component will probably be loaded already. This isn't guaranteed, however, since the points component can be deactivated.)

These are general issues with uninstallers being provided by components, that we haven't really thought about yet, I guess.

Just setting up autoloading for the classes directories for the components, or just including the file specifically, won't necessarily be sufficient in every case either, since the uninstallers may also use other component functions. Extensions will either need to always check if these uninstallers exist before using them, or we will need to load the components entirely whenever an extension is being uninstalled.

I guess this really relates to dependency management. In cases like this, an extension's dependencies need to be loaded when it is uninstalled. Which is something I guess we haven't really thought much about.

@JDGrimes
Copy link
Member Author

That is a question that I really don't want to tackle this close to release. But even if we punt this ticket, I guess we've already raised the question with the points hooks uninstaller factory. At present we do use that factory internally though, so it isn't just intended for extensions. But we did intend that extensions could use it also.

Perhaps we just shouldn't over-complicate things right now. Neither of these uninstallers are going to be dependent on the component's functions (at least not right now), and so at present it is safe to include/autoload them. Of course it is possible for that to change in the future, which is why we'd prefer to resolve the larger issue, but perhaps we just shouldn't tackle it.

@JDGrimes
Copy link
Member Author

Let's punt this for now, and wait to introduce this until we've resolved this problem.

@JDGrimes JDGrimes removed this from the 2.4.0 milestone Oct 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant