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

Replace manual class creation with injection or extensions #16

Closed
robbieaverill opened this issue Dec 6, 2017 · 6 comments · Fixed by #18
Closed

Replace manual class creation with injection or extensions #16

robbieaverill opened this issue Dec 6, 2017 · 6 comments · Fixed by #18

Comments

@robbieaverill
Copy link
Contributor

The way that bind_manipulation_capture() creates a spy class for the database driver could potentially be improved.

FullTextSearch used the same method in the SS3 version, and has been replaced with Injector overloading, however this won't work for both this module in combination with that module too. See relevant conversation here: silverstripe/silverstripe-fulltextsearch#184

We discussed adding sufficient extension points to DB::manipulate so that we wouldn't need this at all, but that may not happen until SS 4.1 if at all.

@robbieaverill
Copy link
Contributor Author

This has become more of a priority now since CWP 2.0 contains the following modules that are all trying to proxy the database connector:

  • lekoala/silverstripe-debugbar
  • silverstripe/fulltextsearch
  • this module

We could create a new module e.g. silverstripe/databaseproxy which could expose event hooks in some cases, and each of these modules could require it. Perhaps a sort of event/subscriber type situation.

cc @tractorcow

@robbieaverill
Copy link
Contributor Author

Removed from 2.0.0 stable milestone for now

@tractorcow
Copy link

I have a concept in mind for a generic class proxy but we don't have time to do it before 2.0.0.

@robbieaverill
Copy link
Contributor Author

Nice! Added back to milestone

@robbieaverill
Copy link
Contributor Author

PR at #18

@robbieaverill robbieaverill removed their assignment Mar 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment