From: Tim Otten <totten@civicrm.org>
Date: Wed, 26 Jul 2023 00:13:27 +0000 (-0700)
Subject: Civi::url() - Allow building new URL from empty. Update comments.
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=d5a81c8140dcc91332792a1691a5ec8f72fa061a;p=civicrm-core.git

Civi::url() - Allow building new URL from empty. Update comments.
---

diff --git a/Civi.php b/Civi.php
index 2ee7ccad10..7c86845541 100644
--- a/Civi.php
+++ b/Civi.php
@@ -219,21 +219,35 @@ class Civi {
   }
 
   /**
-   * Construct a URL based on a logical service address. URL building follows a few rules:
+   * Construct a URL based on a logical service address. For example:
    *
-   * 1. You should initialize with a baseline URL (e.g. 'frontend://civicrm/profile/view?id=123&gid=456').
+   *   Civi::url('frontend://civicrm/user?reset=1');
+   *
+   *   Civi::url()
+   *     ->setScheme('frontend')
+   *     ->setPath(['civicrm', 'user'])
+   *     ->setQuery(['reset' => 1])
+   *
+   * URL building follows a few rules:
+   *
+   * 1. You may initialize with a baseline URL.
    * 2. The scheme indicates the general type of URL ('frontend://', 'backend://', 'asset://', 'assetBuilder://').
-   * 3. The URL object provides getters, setters, and adders (e.g. `getScheme()`, `setPath(...)`, `addQuery(...)`)
+   * 3. The result object provides getters, setters, and adders (e.g. `getScheme()`, `setPath(...)`, `addQuery(...)`)
    * 4. Strings are raw. Arrays are auto-encoded. (`addQuery('name=John+Doughnut')` or `addQuery(['name' => 'John Doughnut'])`)
    * 5. You may use variable expressions (`id=[contact]&gid=[profile]`).
    * 6. The URL can be cast to string (aka `__toString()`).
    *
-   * Here are several examples:
+   * If you are converting from `CRM_Utils_System::url()` to `Civi::url()`, then be sure to:
+   *
+   * - Pay attention to the scheme (eg 'current://' vs 'frontend://')
+   * - Pay attention to HTML escaping, as the behavior changed:
+   *      - Civi::url() returns plain URLs (eg "id=100&gid=200") by default
+   *      - CRM_Utils_System::url() returns HTML-escaped URLs (eg "id=100&amp;gid=200") by default
    *
-   * Ex: Link to constituent's dashboard (specifically on frontend UI)
-   *   $url = Civi::url('frontend://civicrm/user?reset=1');
+   * Here are several examples:
    *
    * Ex: Link to constituent's dashboard (on frontend UI or backend UI -- based on the active scheme of current page-view)
+   *   $url = Civi::url('current://civicrm/user?reset=1');
    *   $url = Civi::url('//civicrm/user?reset=1');
    *
    * Ex: Link to constituent's dashboard (with method calls - good for dynamic options)
@@ -263,13 +277,13 @@ class Civi {
    *   $url = Civi::url('frontend://civicrm/ajax/api4/[entity]/[action]')
    *      ->addVars(['entity' => 'Foo', 'action' => 'bar']);
    *
-   * @param string $logicalUri
+   * @param string|null $logicalUri
    *   Logical URI. The scheme of the URI may be one of:
    *     - 'frontend://' (Front-end page-route for constituents)
    *     - 'backend://' (Back-end page-route for staff)
-   *     - 'service://` (Web-service page-route for automated integrations; aka webhooks and IPNs)
+   *     - 'service://' (Web-service page-route for automated integrations; aka webhooks and IPNs)
    *     - 'current://' (Whichever UI is currently active)
-   *     - 'default://'(Whichever UI is recorded in the metadata)
+   *     - 'default://' (Whichever UI is recorded in the metadata)
    *     - 'asset://' (Static asset-file; see \Civi::paths())
    *     - 'assetBuilder://' (Dynamically-generated asset-file; see \Civi\Core\AssetBuilder)
    *     - 'ext://' (Static asset-file provided by an extension)
@@ -282,11 +296,10 @@ class Civi {
    *   - 't': text (aka `setHtmlEscape(FALSE)`)
    *   - 's': ssl (aka `setSsl(TRUE)`)
    *   - 'c': cache code for resources (aka Civi::resources()->addCacheCode())
-   *   FIXME: Should we have a flag for appending 'resCacheCode'?
    * @return \Civi\Core\Url
    *   URL object which may be modified or rendered as text.
    */
-  public static function url(string $logicalUri, ?string $flags = NULL): \Civi\Core\Url {
+  public static function url(?string $logicalUri = NULL, ?string $flags = NULL): \Civi\Core\Url {
     return new \Civi\Core\Url($logicalUri, $flags);
   }
 
diff --git a/Civi/Core/Url.php b/Civi/Core/Url.php
index 70eb99bbee..7d7ee73cf4 100644
--- a/Civi/Core/Url.php
+++ b/Civi/Core/Url.php
@@ -116,17 +116,39 @@ final class Url implements \JsonSerializable {
   private $varsCallback;
 
   /**
-   * @param string $logicalUri
+   * @param string|null $logicalUri
    * @param string|null $flags
    * @see \Civi::url()
    */
-  public function __construct(string $logicalUri, ?string $flags = NULL) {
+  public function __construct(?string $logicalUri = NULL, ?string $flags = NULL) {
+    if ($logicalUri !== NULL) {
+      $this->useUri($logicalUri);
+    }
+    if ($flags !== NULL) {
+      $this->useFlags($flags);
+    }
+  }
+
+  /**
+   * Parse a logical URI.
+   *
+   * @param string $logicalUri
+   * @return void
+   */
+  protected function useUri(string $logicalUri): void {
     if ($logicalUri[0] === '/') {
+      // Scheme-relative path implies a preferences to inherit current scheme.
       $logicalUri = 'current:' . $logicalUri;
     }
     elseif ($logicalUri[0] === '[') {
       $logicalUri = 'asset://' . $logicalUri;
     }
+    // else: Should we fill in scheme when there is NO indicator (eg $logicalUri===`civicrm/event/info')?
+    // It could be a little annoying to write `frontend://` everywhere. It's not hard to add this.
+    // But it's ambiguous whether `current://` or `default://` is the better interpretation.
+    // I'd sooner vote for something explicit but short -- eg aliases (f<=>frontend; d<=>default)
+    //   - `Civi::url('f://civicrm/event/info')`
+    //   - `Civi::url('civicrm/event/info', 'f')`.
 
     $parsed = parse_url($logicalUri);
     $this->scheme = $parsed['scheme'] ?? NULL;
@@ -138,10 +160,6 @@ final class Url implements \JsonSerializable {
     $fragmentParts = isset($parsed['fragment']) ? explode('?', $parsed['fragment'], 2) : [];
     $this->fragment = $fragmentParts[0] ?? NULL;
     $this->fragmentQuery = $fragmentParts[1] ?? NULL;
-
-    if ($flags !== NULL) {
-      $this->useFlags($flags);
-    }
   }
 
   /**
diff --git a/tests/phpunit/Civi/Core/UrlTest.php b/tests/phpunit/Civi/Core/UrlTest.php
index 4bbbb8812f..3548458b28 100644
--- a/tests/phpunit/Civi/Core/UrlTest.php
+++ b/tests/phpunit/Civi/Core/UrlTest.php
@@ -153,4 +153,36 @@ class UrlTest extends \CiviUnitTestCase {
     }
   }
 
+  public function testFunkyStartPoints(): void {
+    $baseline = (string) \Civi::url('frontend://civicrm/event/info?id=1');
+    $this->assertStringContainsString('event/info', $baseline);
+
+    $alternatives = [
+      // Start with nothing!
+      \Civi::url()
+        ->setScheme('frontend')
+        ->setPath(['civicrm', 'event', 'info'])
+        ->addQuery(['id' => 1]),
+
+      // Start with nothing! And build it backwards!
+      \Civi::url()
+        ->addQuery(['id' => 1])
+        ->addPath('civicrm')->addPath('event')->addPath('info')
+        ->setScheme('frontend'),
+
+      // Start with just the scheme
+      \Civi::url('frontend:')
+        ->addPath('civicrm/event/info')
+        ->addQuery('id=1'),
+
+      // Start with just the path
+      \Civi::url('civicrm/event/info')
+        ->setScheme('frontend')
+        ->addQuery(['id' => 1]),
+    ];
+    foreach ($alternatives as $key => $alternative) {
+      $this->assertEquals($baseline, (string) $alternative, "Alternative #$key should match baseline");
+    }
+  }
+
 }