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

Fix for skipping 'open.php' replacement in non standard drupal install #58

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kainuk
Copy link

@kainuk kainuk commented Jun 8, 2022

Before

Urls with extern/open.php are not replaced when civicrm is not installed in the sites/all/modules directory. The effects is that open clicks are not sent not to the proxy server but but direct to civicrm and lost.

After.

The urls are replaced.

Technical

  • Use $config->userFrameworkResourceURL instead of the literal path.
  • Maybe the code is nearly ready for WordPress or Joomla

Background

What this bug makes a bit complicated to find is that a tester often has direct access to the civicrm installation, so his click registers.

…he civicrm installed in the 'sites/all/modules/civicrm' directory. Sometimes this location is different, for example 'sites/all/modules/contrib/civicrm'. The $config->userFrameworkResourceURL points to this place. This commit removes the assumption.
@bjendres
Copy link
Member

bjendres commented Jun 8, 2022

Thanks @kainuk! @jensschuppe does this look good to you?

@bjendres bjendres added this to the 0.6 milestone Jun 8, 2022
@jensschuppe
Copy link
Contributor

I'm unfamiliar with what this code actually does, but using CMS-independent URLs seems very reasonable. There's also a sites/default/files thing in there. Should this also be replaced, @kainuk?

@kainuk
Copy link
Author

kainuk commented Jun 8, 2022

The code replaces an URL that leads to CiviCRM with one that leads to the Proxy

Replacing sites/default/files makes sense because that works only for drupal. Not sure it that's enough to make in CMS independent. This PR solves a problem for a Drupal customer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants