(dev/core#4433) - Implement Civi::url() with prefixes and OOP enhancements

[totten]

Overview

This is a concrete demonstration of some ideas from https://lab.civicrm.org/dev/core/-/issues/4433.

Before

This function exists:

CRM_Utils_System::url(NULL, NULL, FALSE, 1, '', TRUE, __CLASS__, NULL, TRUE);

And that does... something. Have fun figuring it out...

After

echo Civi::url('frontend://civicrm/event/info?id=123&reset=1');

echo Civi::url('//civicrm/profile/view?reset=1')
  ->addQuery(['gid' => $x, 'id' => $y])

echo Civi::url('assetBuilder://crm-l10n.js')
  ->addQuery(['locale' => $x])

(Docblock has more examples...)

Technical Details

In this rendition:

• The entry-point (frontend/backend/etc) is a URI scheme (frontend://).
• It respects URI convention for inheriting current scheme (//foo.txt means "get foo.txt with current scheme)
• The list of URI schemes is borrowed from other places in core -- e.g. the Guzzle-test system has frontend://ROUTE and backend://ROUTE. Some of the asset/resource management supports assetBuilder://NAME and ext://EXT/FILE.

[civibot]

Thank you for contributing to CiviCRM! ❤️ We will need to test and review the PR. 👷

Introduction for new contributors

• If this is your first PR, an admin will greenlight automated testing with the command ok to test or add to whitelist.
• A series of tests will automatically run. You can see the results at the bottom of this page (if there are any problems, it will include a link to see what went wrong).
• A demo site will be built where anyone can try out a version of CiviCRM that includes your changes.
• If this process needs to be repeated, an admin will issue the command test this please to rerun tests and build a new demo site.
• Before this PR can be merged, it needs to be reviewed. Please keep in mind that reviewers are volunteers, and their response time can vary from a few hours to a few weeks depending on their availability and their knowledge of this particular part of CiviCRM.
• A great way to speed up this process is to "trade reviews" with someone - find an open PR that you feel able to review, and leave a comment like "I'm reviewing this now, could you please review mine?" (include a link to yours). You don't have to wait for a response to get started (and you don't have to stop at one!) the more you review, the faster this process goes for everyone 😄
• To ensure that you are credited properly in the final release notes, please add yourself to contributor-key.yml
• For more information about contributing, see CONTRIBUTING.md.

Quick links for reviewers

• ➡️ Demo site for this PR
• 📖 Review standards
• 🗒️ Review template (brief or verbose)

[colemanw]

Ooh, an undocumented flag that overrules htmlize! Yet another blow to its fragile ego... perhaps leading it to the brink of a well-deserved existential crisis.

[colemanw]

@totten this flag is still undocumented. Should it be removed?

[totten]

The "undocumented" note had confused me, but I see now: it's documented on Civi::url(...$flags...) but not Url::useFlags($flags). That's easy to cleanup.

I should take a stab at the Smarty side before deciding whether to remove that flag. My suspicion is that it'll make it easier to tune Smarty. (Quick estimate: in core, there are ~530 places where Smarty wants HTML and ~130 places where it wants plain text.)

[totten]

@colemanw So I've done a bunch of updates

• Add test-coverage and many more docblocks.

• Add support for Angular-style URL data (backend://civicrm/a/#/mailing/[id]?angularDebug=1)

• Add better support for static asset URLs (eg asset://[civicrm.packages]/foobar.js)

• Add support for metadata-based routing (default://civicrm/admin)

• Define a consistent policy on escaping:

  • Whenever you pass in a string (setQuery('id=123&msg=hello+world')), then treat it as a literal URL fragment.
  • Whenever you pass an array (setQuery(['id' => 123, 'msg' => 'hello world'])), then we apply escaping.

• Define a Smarty utility {url}...{/url}:

  <a href="{url}backend://civicrm/admin?reset=1{/url}">Administration</a>
  
  {url assign=adminUrl}backend://civicrm/admin?reset=1{/url}
  <a href="{$adminUrl}">Administration</a>
  
  <a href="{url contact=100 profile=200}current://civicrm/profile/view?id=[contact]&gid=[profile]{/url}">
  
  <a href="{url 1=$contacts[$i].contact_id 2=$profile.id}current://civicrm/profile/view?id=[1]&gid=[2]{/url}">

So overall, from my POV, it has all the functionality I've expected.

There are a couple possible areas of further consideration:

• The Second Hard Problem of Computer Science - The scheme names (frontend://, backend://, asset://, assetBuilder://) have a sort of logic to them. But are they good names?... frontend:// and backend:// are kinda long. Maybe asset:// should be like resource:// or res://. Yaddayadda.
• Trial Conversions - Do a batch of conversions (CRM_Utils_System::url() to Civi::url(); and {crmURL} to {url}). See if they feel alright.
• URL Contexts/Default Options - So there's this recurring bug where you make a URL, but sometimes it doesn't look the way you expect, and so you have to go back and finesse options.
  • Basically, when running code, you're in some operative context. And I think we come with different intuitions about how URLs should be formulated in each context.
    • Displaying a web-page. Here, relative URLs are nice. In case of ambiguity, you should lean toward keeping links in the same UI (i.e. frontend-pages tend to link to frontend-pages).
    • Generating an email message. Here, absolute URLs are nice. In case of ambiguity, you should lean toward frontend UI (even though the email might be generated from backend UI, from automated job, etc)
    • Running admin/dev tools (cv url, cv http). Here, absolute URLs are nice. You're not in frontend:// or backend://, so you could lean on metadata instead (default://).
  • I added some stuff to support these intuitions. This seems to work for cv. But the email-generating scenario seems important, and I haven't tried it. The concept feels tentative until the email side has been done.

Those are all kind of debatable. Anyway, I'm not inclined to self-block any more... so I've removed "Draft/WIP" flag.

[christianwach]

(Sorry, still not clear how the review process works here)

str_starts_with() is PHP 8.0+ so we'll need a shim to continue support for PHP 7.x.

[christianwach]

Am i right in thinking that routing to $userSystem->url() here provides an opportunity (sometime in the future) to replace it with something less problematic?

[totten]

@christianwach I do like the idea of $userSystem having some more influence/visibility on the $scheme. For example, I don't think it's great for frontend:// and service:// to be conjoined like that. I'll add on some ideas for that...

[totten]

@christianwach So does 59af62f seem better/worse/same? It delegates to $userSystem->getRouteUrl() instead of $userSystem->url(). The main differences are:

• UF::getRouteUrl() receives the name (frontend, backend, service) instead of multiple bools. It can potentially receive other/custom schemes if the UF has other environments.
• UF::getRouteUrl() doesn't have to deal with the fragment or absolute/relative flag. (These seem amenable to generic handling.)

Another thing the UF might influence is the interpretation of current:// (ie detectScheme()). With the current PR, the UF-integration could set civicrm_url_defaults before invoking the page.

[christianwach]

So does 59af62f seem better/worse/same?

@totten Better in that the inscrutable boolean params are somewhat hidden. Nice one :)

[christianwach]

@totten Thanks for starting to address this issue. I like the direction of travel - it's certainly a more readable way of collecting the various pieces of the URL together. Looks promising.

Coming from a WordPress-integration perspective, my focus is less on how the pieces are collected and more on how those URLs are subsequently built for use in the various CMS contexts, i.e.

1. WordPress admin
2. The Base Page with or without Clean URLs
3. Shortcodes - where URLs are based on the Permalink structure plus query vars
4. Blocks - which in this plugin call the QuickForm classes directly but may use asset:// or assetBuilder:// in future
5. WordPress multilingual plugins (which use the URL scheme in various ways to encode the desired language into it)

For now, it looks as though I can still work on the logic in CRM_Utils_System_WordPress::url() to make it adaptable to the various scenarios that it has to handle... just leaving this here as a reminder in case work is started on URL rendering that does not route via the current $userSystem->url() method(s).

[totten]

Thanks for that last comment @christianwach. Those are some interesting contexts. Thinking about #2 vs #3 -- both are front-end-ish, but #2 is a singular frontend -- and #3 has n-many variations. (EDIT: This was written as a statement, but in my head it was intoned as a question. :) Just trying to make sure I follow the differences.) Which means:

• While rendering web UI for #2 or #3, you could use url('current://...') and expect links with matching structure.
• While processing a background job (say, a sending a stock email with a link to update a recurring contribution), it's much easier to link to #2 than #3. For #2, you can simply interpret url('frontend://civicrm/contribute/updaterecur') as https://example.com/civicrm/contribute/updaterecur. But if you want the emails to point to one of n-many WP pages that support shortcodes, then you need to edit the email-template (or define some complicated override).

[colemanw]

I think we've sat on this one long enough and it has good test coverage. Let's start using it!