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

Make compatible with WP and Joomla #1

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Conversation

agh1
Copy link

@agh1 agh1 commented Apr 28, 2016

The path to webhook.php was hard-coded with a Drupal location for civicrm.config.php, and you can't look up the CMS without bootstrapping Civi, and you can't do that without the civicrm.config.php.

Instead, I made the webhook be a regular CiviCRM page so CiviCRM will already be bootstrapped by the time you get there. It does mean the basic authentication method for protecting the file won't work, but I added an option for a secret code that can add some protection against random posts.

@fulltrucker
Copy link
Contributor

fulltrucker commented May 20, 2016

Hi Andrew, questions:

  1. So these changes would truly make this extension CMS-agnostic?
  2. And it would run fine in Drupal with these changes?
  3. And does that include D6?

@agh1
Copy link
Author

agh1 commented May 20, 2016

We only have these changes running in production on WordPress (we had used the original extension as-is for Drupal sites prior to running into problems with WP), but there's nothing WP-specific here.

What originally prompted all this work was line 47 of webhook.php which has a Drupal-specific path to civicrm.config.php. However, fixing this caused a chicken-and-egg problem: you can't know what path without knowing the CMS, and you can't know the CMS without bootstrapping CiviCRM, and you can't bootstrap Civi without that path.

Instead, I implemented a CiviCRM page callback. That gives it a standard URL such as:

By the time the page code runs, you're just dealing with standard CiviCRM. The bulk of CRM/Sendgrid/Page/Webhook.php is just the old webhook.php

So the short answers to the questions are:

  1. Yes, it should be totally CMS-agnostic.
  2. That means it should run fine with D7, though it ought to be tested. If you're coming from the old version of the extension, however, you'll need to update the webhook over at SendGrid.
  3. Similarly, it should be fine with D6, but again, it needs to be tested.

@mediasunrise
Copy link

Andrew -- Thank you for this. I did not originally see that the SendGrid extension is not compatible with WordPress. Hence we've been using it for a few weeks. Found two problems and eager to know whether your changes address both: First not able to access the SendGrid Event Notification Configuration, per the extension directions. But the second problem, which is much more significant, is that the Sendgrid extension requires authentication every time anyone tries to access Civicrm, including to access public directory listings and forms. Does your fix resolve this? Can you provide instructions for a newbie to understand your commits and integrate the same. Thanks for your help.

@agh1
Copy link
Author

agh1 commented Aug 12, 2016

@mediasunrise sorry I missed your message earlier. I haven't experienced either of the problems you describe. I have no idea why you're experiencing the first problem, and consequently, I'm not sure if my changes would fix them.

On the other hand, the authentication is part of how the original extension protects the webhook script from getting called by just anyone. My changes get rid of that and instead rely on a random code that gets set in your configuration.

If you're looking for how to install the extension, I'd suggest simply getting the entire extension from https://github.com/agh1/com.imba.sendgrid as-is.

@mediasunrise
Copy link

Thanks, since I left this message I uninstalled the sendgrid extension and I'm using Sparkpost now. But may go back to sendgrid. Appreciate your response.

@mwileczka
Copy link

What's the status of this in combination with the 4.7 branch that is not stable? I've been trying to get the 4.7 beta working on WP CivicRM 4.7 and it doesn't seem to be working at all.

@konadave
Copy link
Collaborator

konadave commented Sep 7, 2016

@mwileczka I haven't had a chance to review/test/merge this pull request. Since IMBA is actually on the 4.6 branch, it is not a priority at the moment. For now, I would suggest getting the extension from https://github.com/agh1/com.imba.sendgrid

@dsnopek
Copy link

dsnopek commented Oct 30, 2017

FYI, I think this PR is also necessary for CiviCRM and Drupal 8 too since the code layout is a little different than with Drupal 7. I haven't fully tested it yet, though!

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 this pull request may close these issues.

6 participants