(dev/core#4433) WordPress::url() - By default, choose frontend or backend based on route

[totten]

Overview

(See also: https://lab.civicrm.org/dev/core/-/issues/4433)

The method CRM_Utils_System::url($path, ...) is used to construct the full URL for a route. On WordPress, it is important to determine whether the route is intended for frontend/public usage or backend/private usage. URLs may look like:

## Backend / Private
http://wpmaster.bknix:8001/wp-admin/admin.php?page=CiviCRM&q=civicrm%2Fcontribute&reset=1

## Frontend / Public - With Clean URLs
http://wpmaster.bknix:8001/civicrm/contribute/transact/?reset=1&id=1

## Frontend / Public - Without Clean URLs
http://wpmaster.bknix:8001/civicrm?civiwp=CiviCRM&q=civicrm%2Fcontribute%2Ftransact&reset=1

We recently started to see failures in E2E\Core\PathUrlTest::testGetUrl_WpAdmin(), which asserts that a particular URL is backend/private.

• Example failure 1
• Example failure 2

The proximate cause is that cv url changed to mirror the default from CRM_Utils_System::url(). IMHO, the root-cause is that the contract for CRM_Utils_System::url() is confusing. This patch tries to make the contract easier.

Of course, the fact that I am confused by the contract means that I may not fully understand. Hopefully @christianwach or @kcristiano understand WP URLs' better and can give some feedback.

Before

CRM_Utils_System::url() accepts two optional parameters, bool $frontend=FALSE and bool $forceBackend=FALSE.

Conceptually, the choice is binary - you either have a frontend URL or a backend URL. But the contract allows four choices (FALSE,FALSE and TRUE,FALSE and FALSE,TRUE and TRUE,TRUE). It seems clear enough when you have TRUE,FALSE or FALSE,TRUE -- but the meaning of FALSE,FALSE and TRUE,TRUE seems ambiguous.

Additionally, this seems related to -- but different from -- the property civicrm_menu.is_public. I can understand the cases where either:

• Route declares is_public=1 and url() is called with $frontend=TRUE
• Route declares is_public=0 and url() is called with $forceBackend=TRUE

But the other possibilities (is_public=1 with $forceBackend=TRUE) seem confusing to me.

After

CRM_Utils_System::url() still accepts two optional parameters, bool $frontend=FALSE and bool $forceBackend=FALSE. (You can't remove them without a hard break.)

In the default case (FALSE,FALSE) and in the other ambiguous case (TRUE,TRUE), it will lookup the value of is_public and use that. For example:

• CRM_Utils_System::url('civicrm/contribute/transact', 'reset'=1) ==> Use the frontend URL (because the route specifies is_public=1 and the caller hasn't indicated a clear preference).
• CRM_Utils_System::url('civicrm/contribute', 'reset'=1) ==> Use the backend URL (because the route specifies is_public=0 and the caller hasn't indicated a clear preference)

But if the caller does have a specific request (TRUE,FALSE or FALSE,TRUE), then that is respected.

On my local, this appears to fix testGetUrl_WpAdmin().

Comments

I don't understand everything in WP URL's, so hopefully folks with experience there.

I was concerned about possibility of performance impact -- e.g. you don't want SELECT is_public FROM civicrm_menu every time the system renders a url(). The approach in here should only require 1 moderate-sized cache-item (1-2 kb).

There are some routes like civicrm/ajax/api4/* which are sort of fundamentally ambiguous -- they're headless, so calling them "frontend" or "backend" is sort of meaningless. Maybe that needs some kind of r-run to ensure that API4 REST calls work in both UIs?

If this is all too aggressive, I could do a narrower fix in cv-only -- e.g. we retain the current contract for CRM_Utils_System::url(), and then update cv url to pick its default convention based on is_public.

[civibot]

(Standard links)

• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[totten]

Requested a supplemental run of phpunit-e2e for this PR on wp-demo: https://test.civicrm.org/job/CiviCRM-Manual-Test/37/

(UPDATE) After pushing 4b904de, fired another run: https://test.civicrm.org/job/CiviCRM-Manual-Test/38/

[christianwach]

Conceptually, the choice is binary

@totten I think that param exists because two different URLs can cause CiviCRM to render the same content in different contexts, e.g.

# Clean URL on front-end
https://example.com/civicrm/event/info/?reset=1&id=1

# Query var URL on back-end
https://example.com/wp-admin/admin.php?page=CiviCRM&q=civicrm%2Fevent%2Finfo&id=1&reset=1

FWIW I have never used the latter except by accidentally clicking an Event title on CiviCRM's "Manage Events" screen.

I guess the latter URL is the result of TRUE, TRUE but haven't tested to find out conclusively... either way, it's a URL that is "sort of front end because of the rendered content and sort of back-end because of the context".

Also FWIW these params existed before I started rewriting CRM_Utils_System_WordPress::url() to allow for Clean URLs and I basically left their logic (if one can call it that) intact.

[totten]

FWIW I have never used the latter except by accidentally clicking an Event title on CiviCRM's "Manage Events" screen. ... it's a URL that is "sort of front end because of the rendered content and sort of back-end because of the context".

Thanks for that example. I played around in the "Manage Events" area and (for comparison) also played around in "Manage Contribution Pages" and "Profiles".

I think we can make the case for the title link in "Manage Events" to change, e.g.

• The other links to civicrm/event/info all go to the frontend UI. (The other links appear in "Manage Events" and "Configure Event X".)
• If you view civicrm/event/info in backend UI, it has internal links ("Register Now") which go to the frontend UI.
• If you compare "Manage Events" to "Manage Contribution Pages" and "Profiles"... they all format the title differently. "Manage Events" does the hyperlink. "Profiles" does edit-in-place. "Manage Contribution Pages" does simple/static text.

But since I looked at those other examples... here's another oddball. civicrm/profile and civicrm/profile/edit. As a site-builder, you might define profiles for collecting information about constituents (frontend) and staff (backend); or you might provide directory listsing (with different info for frontend and backend users). This is probably OK for site-builders - the current UI only shows the public link, and (if you want a backend link) the onus is on the person who prepares the link. But maybe I need to search universe to see about extensions that compose profile links...

[christianwach]

@totten Yeah, it's definitely confusing that different components do different things. Good to have teased out the scope of the issue, though, but hard to see a consistent solution.

[totten]

The main obstacle to this PR was the existence multi-homed routes (eg civicrm/profile/* and civicrm/ajax/api4/* have use-cases for frontend and backend) -- where metadata doesn't always give a decisive routing. So there are many scenarios for requesting a URL ("use frontend" vs "use backend" vs "inherit current" vs "use default"), and squeezing those variations into the current method-signature is... a bit much.

So I'll close this. The near-term test-failure is mitigated by #26772, and the bigger DX issue has been split out as https://lab.civicrm.org/dev/core/-/issues/4433 with current draft #26861.