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

Set "cms.root" URL in addition to Path #188

Merged
merged 2 commits into from
Mar 24, 2020

Conversation

christianwach
Copy link
Member

Overview

The code in Civi::Paths::getVariable() does not ensure all attributes of a named variable are set, so partially setting the attributes of cms.root sometimes causes "Cannot resolve path using 'cms.root.url'" exceptions to be thrown. This is relatively obscure because it requires a particular WordPress Multisite setup to occur, but the effect is repeatable.

Before

"Cannot resolve path using 'cms.root.url'" exceptions are thrown. "Settings - Resource URLs" screen renders incorrectly in WordPress Multisite on a Subsite:

Screen Shot 2020-03-24 at 11 01 40

After

"Cannot resolve path using 'cms.root.url'" exceptions are not thrown. "Settings - Resource URLs" screen renders correctly in WordPress Multisite on a Subsite.

Technical Details

This PR is necessary because of an incomplete check for the existence of all attributes in Paths.php. Having said that, it's still better to assign both attributes early than have Civi::Paths::getVariable() recalculate them later.

@kcristiano
Copy link
Member

code is sensible. Should have been there from the start.

I did an r-run on master and works as expected.

@kcristiano kcristiano merged commit 7c1ee3f into civicrm:master Mar 24, 2020
@christianwach christianwach deleted the cms-root branch April 2, 2020 19:10
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.

2 participants