E2E_Core_PathUrlTest::testGetUrl_WpAdmin() fails because CiviCRM routing is confusing
The gist of the test: it calls cv url civicrm/contribute?reset=1
and asserts that the URL will open in the WordPress backend UI (aka /wp-admin/
).
The test is failing. There are currently two patches:
- 25476: Automatically choose frontend/backend based on the route metadata
- 26772: Change the test to specifically request backend
Both have issues. So here's a deep-dive into the context.
Background
- CiviCRM runs on Drupal/Backdrop and WordPress/Joomla.
- On WordPress/Joomla, the "frontend" and "backend" are different sub-applications (e.g.
/
vs/wp-admin/
). The applications have very different URL structures. To make a hyperlink, you first decide which sub-application to target -- and then pick a page within it. - On Drupal/Backdrop, it's one application, and all URLs have similar structure. To make a hyperlink, you simply identify the page.
- (Drupal/Backdrop UX can sometimes distinguish frontend/backend -- but it's a visual choice based on local configuration. It's not a structural part of the URL.)
- On WordPress/Joomla, the "frontend" and "backend" are different sub-applications (e.g.
- CiviCRM's routing system integrates into every UF/CMS, but (internally) it is closer to Drupal's. There is one
civicrm_menu
with all frontend+backend pages.- On Drupal/Backdrop, CiviCRM's routes pass directly to CMS routes.
- On WordPress/Joomla, CiviCRM's routing integrates with both subapplications (ie "frontend" and "backend").
- CiviCRM stores a flag
bool is_public
for each route. - CiviCRM includes routes which can be classified as:
- Purely backend (ex:
civicrm/dashboard
) - Purely frontend (ex:
civicrm/event/register
) - Purely web-service (ex:
civicrm/payment/ipn
) - Multi-homed
- Ex:
civicrm/profile/view
has use-cases for frontend and backend - Ex:
civicrm/ajax/api4/%
has use-cases for frontend, backend, and web-service
- Ex:
- Purely backend (ex:
- CiviCRM has a function
CRM_Utils_System::url()
- At first glance, it resembles Drupal's
\url()
. You typically just give the page (e.g.url('civicrm/foo/bar')
). - Over time, several additional parameters were added -- notably, the 6th parameter
bool $frontend
and the 7th parameterbool $forceBackend
.
- At first glance, it resembles Drupal's
Problems
- The status-quo invites bugs between CMS's.
- A developer working on Drupal will have trouble recognizing the importance of the 6th and 7th parameters. (Those params do nothing on Drupal - and they're buried at the end of method.)
- The status-quo invites bugs between interactive and automatic processes.
- A developer defines a custom token and tests it interactively; then at runtime, the token is sent by an automatic process. But the interactive/automatic split is orthogonal to frontend/backend split. Sometimes, interactive/automatic agree with each other (both frontend or both backend), and sometimes they disagree (one frontend, one backend).
- Overall, what tends to happen is:
- Developer writes a call like
CRM_Utils_System::url('civicrm/foo/bar')
, and it looks pretty. - They test, and it works beautifully on their system.
- They publish, and it fails on other systems.
- Someone writes a patch to add 5 more parameters (
url('civicrm/foo/bar', '', FALSE, NULL, TRUE, TRUE)
). And then it's OK.
- Developer writes a call like
- The DX is awkward and invites bugs (in core and contrib) -- but it becomes more annoying for
cv
UX.- If you need a 5th/6th param in PHP code, then you'll eventually figure it out and commit the update to your codebase. Then you forget about it.
-
cv
has some commands to facilitate manual testing and E2E testing (iecv url
,cv http
,cv open
). These are things that you improvise. Mismatched URLs can be annoying anytime you use these subcommands.
What to do
There are two PRs to fix the test, and honestly - I don't really like either.
- 26772 makes the red mark go away, but it leaves the underlying issue (poor DX for PHP and poor UX for cv).
- 25476 aims to fix the underlying issue, but it's probably too facile. The current metadata can distinguish "purely frontend" pages from "purely backend" pages, but it cannot recognize "purely web-service" or "multi-homed". It probably messes-up some scenarios for those.
- Fixing this probably requires some more aggressive transition in the contract.
- Ex: Improve the routing metadata so that we can distinguish web-service routes and multi-home routes.
- Ex: Define a different class or function for generating URLs (with a more usable signature).
- Another option is to leave
civicrm-core
as-is -- and only updatecv
.- Ex: Give
cv
the mechanism to resolve a frontend/backend based on metadata. - Ex: Change
cv
to complain if you don't a specify frontend/backend flag. - Either way, it feels like a bit of a wasted opportunity to only patch
cv
when we know that other users ofCRM_Utils_System::url()
get confused about the frontend/backend flags.
- Ex: Give