CRM-17633: Changes to support WP in it's own directory. #11031
Conversation
|
jenkins test this please |
|
Jenkins: [GitScan\Exception\ProcessErrorException] |
…w for civicrn.settings.extra.php for WP. Create template for civicrm.settings.extra.php. Update install/civicrm.php for all needed params for different common install scenarios. Fix linting issues in civicrm.php. CRM 16421 CRM 17633 - update CRM_Utils_System_WordPress to allow for common install configurations CRM-16421 - Convert constants to `$civicrm_paths` Following up on the discussion from [civicrm#10513](civicrm#10513), this converts the proposed constants `CIVICRM_UF_WP_BASEURL` and `CIVICRM_UF_ADMINURL` to variables in the `Paths` system. A few benefits: * Reduces code duplication between `civicrm.php` and `WordPress.php`. * Can construct sub-paths with prettier notation (`Civi::paths()->getUrl('[wp.frontend]/foo.txt')`) * Has options to output relative or absolute URLs * Can expand on `Paths` to provide more inspection/validation Notes: * `CIVICRM_UF_WP_BASEURL` => `wp.frontend.base` * `CIVICRM_UF_ADMINURL` => `wp.backend.base` ---------------------------------------- * CRM-16421: Work to get CiviCRM for WordPress in WordPress' official Repository https://issues.civicrm.org/jira/browse/CRM-16421 CRM-16421 - Assimilate `civicrm.settings.extra.php` into `civicrm.settings.php` ---------------------------------------- * CRM-16421: Work to get CiviCRM for WordPress in WordPress' official Repository https://issues.civicrm.org/jira/browse/CRM-16421 CRM-17633 merge current master changes to civicrm.settings.php.template
|
@civicrm-builder retest this please |
1 similar comment
|
@civicrm-builder retest this please |
|
I tested this pr and civicrm/civicrm-wordpress#105 on a wordpress install with wordpress in the root site directory and with wordpress in its own directory and did not get any issues regarding directories or resource urls. |
|
test this please |
|
@civicrm-builder test this please |
|
@eileenmcnaughton I am getting:
Do you know if there are issues with Jenkins? I can't imagine how these changes would trigger a failure there. These are all specific to the installer and only in the WP context. I was hoping to finally get these in for 4.7.26-rc testing as the best testing for the installer is via tarballs. |
|
@kcristiano It's most likely the issue discussed yesterday here: https://chat.civicrm.org/civicrm/pl/51tgxt8zajy3mjrcub69d5nnze |
|
retest this please |
|
@kcristiano tests are passing now & @alifrumin has tested. Code looks good. I feel comfortable that there is no impact outside WP dirs - giving this merge-ready in the hope @christianwach will take a look & confirm he is good with this but not sure it needs to be blocked on that given the input already given |
|
Thanks @eileenmcnaughton Can we also mark civicrm/civicrm-wordpress#105 this one as well? Thye are related and my hope is to get these in the next RC. As they affect the installer, the RC is a perfect way to validate my testing. |
|
hmm - we don't have those labels on that repo |
| ); | ||
| }) | ||
| ->register('wp.backend.base', function () { | ||
| return array('url' => CIVICRM_UF_BASEURL . 'wp-admin/'); |
There was a problem hiding this comment.
@kcristiano Are you sure that wp-admin/ should be hard-coded here? The admin_url filter allows this to be overridden, so if this path has to be correct without bootstrapping WordPress, then perhaps there needs to be a setting for it somewhere? OTOH, perhaps I'm raising an issue that's outside the scope of this PR.
There was a problem hiding this comment.
@christianwach I see the point and had originally done that. In one of the iterations (this PR in various forms dates back to Ft Collins Sprint in June 2016), @totten made the followimng PRs that have been added: kcristiano@48c4fe6 and kcristiano@8b0c250 This section kcristiano@8b0c250#diff-46f9229e858ccffc107ef60a91809c43L286 removed the call to admin_url(). I attempted to add back, but had errors depending on the WP configuration.
As I would much rather have this in and then improve than hold it back even longer, I consider switching to admin_url() as a TODO. We can create an issue for that and fix it after these changes are tested through an RC. As this is an installer patch and it affects both civicrm-core and civicrm-wordpress we'll get more testing once it hits RC.
There was a problem hiding this comment.
@kcristiano Gotcha. Seems like a good idea.
|
Ok - I think gotcha translates to 'yes this is ready to merge IMHO' - merging this & the packages one to hit the rc. |
Overview
Allow for civicrn.settings.extra.php for WP. Create template for civicrm.settings.extra.php. Update install/civicrm.php for all needed params for different common install scenarios. Fix linting issues in civicrm.php.
CRM 16421 CRM 17633 - update CRM_Utils_System_WordPress to allow for common install configurations
Replaces #10214
Before
WP install will fail if not installed in tyhe webroot. This supports https://wiki.civicrm.org/confluence/display/CRM/WordPress+installed+in+its+own+directory+issues
https://issues.civicrm.org/jira/browse/CRM-17633
After
Create civicrm.setting.phpo file based on WP directory structure in use
Comments
This is a rebase of the prior PR. Mertges 2 weeks ago had broken the original
Depends on civicrm/civicrm-wordpress#105