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

Plugin conflict with CiviCRM #24

Closed
MegaphoneJon opened this issue Feb 27, 2018 · 2 comments · Fixed by civicrm/civicrm-wordpress#125
Closed

Plugin conflict with CiviCRM #24

MegaphoneJon opened this issue Feb 27, 2018 · 2 comments · Fixed by civicrm/civicrm-wordpress#125

Comments

@MegaphoneJon
Copy link

Thanks for this great package!

I've found that it fails when CiviCRM is installed on a site. I tracked it down to this line, which checks that the $_GET supervariable is empty. CiviCRM sets an element in that array on every page load.

What is the use case that this line is trying to prevent? Is it necessary, or can I submit a PR to remove it?

@aaemnnosttv
Copy link
Owner

Thanks @MegaphoneJon ! This is the first report of a conflict with another plugin, although I would say though that CiviCRM is conflicting with the login command.

The purpose of the conditional that you're referencing is to minimize the footprint of the login server on page load time, since it is running on every page load. Since magic login URLs don't have any query string, checking that $_GET is empty is an easy way to qualify the request early on if it is one that should be looked at more closely as a potential login request.

I would say that CiviCRM is being a bit careless in adding a parameter to every request; I find it hard to see why that would really be necessary.

I see you have an issue+PR open with them as well which looks like the right solution to me, so let's see how things go there. I will likely not change this conditional until there is an actual use case for it by the login server itself. Thanks for your help and understanding 👍

@aaemnnosttv
Copy link
Owner

Closing this one for now.

kcristiano added a commit to tadpolecc/civicrm that referenced this issue Jul 19, 2018
props @MegaphoneJon
The wp-cli-login-server plugin checks for the presence of elements in the $_GET array, and fails if any are found. This conflicts with CiviCRM, which places:

$_GET['civicrm_install_type'] = 'wordpress';

into every page load.

I confirmed that this value is only used for install, and placed a conditional around it to only insert pre-install. I successfully installed CiviCRM and engaged in normal use with the conditional applied, and confirmed that this fixed the WP plugin.

This is intended to close aaemnnosttv/wp-cli-login-command#24.

Signed-off-by: Kevin Cristiano <kcristiano@tadpole.cc>
kcristiano added a commit to tadpolecc/civicrm that referenced this issue Jul 19, 2018
props @MegaphoneJon
The wp-cli-login-server plugin checks for the presence of elements in the $_GET array, and fails if any are found. This conflicts with CiviCRM, which places:

$_GET['civicrm_install_type'] = 'wordpress';

into every page load.

I confirmed that this value is only used for install, and placed a conditional around it to only insert pre-install. I successfully installed CiviCRM and engaged in normal use with the conditional applied, and confirmed that this fixed the WP plugin.

This is intended to close aaemnnosttv/wp-cli-login-command#24.

Signed-off-by: Kevin Cristiano <kcristiano@tadpole.cc>
andyburnsco pushed a commit to andyburnsco/civicrm that referenced this issue Mar 16, 2021
props @MegaphoneJon
The wp-cli-login-server plugin checks for the presence of elements in the $_GET array, and fails if any are found. This conflicts with CiviCRM, which places:

$_GET['civicrm_install_type'] = 'wordpress';

into every page load.

I confirmed that this value is only used for install, and placed a conditional around it to only insert pre-install. I successfully installed CiviCRM and engaged in normal use with the conditional applied, and confirmed that this fixed the WP plugin.

This is intended to close aaemnnosttv/wp-cli-login-command#24.

Signed-off-by: Kevin Cristiano <kcristiano@tadpole.cc>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants