View API Output links point to frontend

Thanks. The first commit is a test, which is now failing. Hopefully a fix follows soon.

I feel like I'm taking a stab in the dark / retroactively establishing 'rules' for the rewriting of URLs, to fit whatever works.

Well, a little tweak worked / at least passes all the tests: do not change the base URL if $options['base_url'] is already set.

(We must still change things if $options['base_url'] is not set yet but $options['absolute'] is set to TRUE -- otherwise many things start failing.)

I'll create a MR but won't set "Review" state before it has run through our internal test suite - and who knows what things that will trigger.

The MR introduces a new URL option 'lupus_decoupled_skip_rewrite'. I want to state clearly why (even if only for my future self):

• In a non-API request == backend HTML page, we need* to rewrite links to frontend destinations. In practice: we need the node-view links in /admin/content to point to the frontend.
• We have no easy way to distinguish between these links, and a call to BaseUrlProvider::getAdminBaseUrl(), which must not be rewritten.
  • We used to distinguish between them by only rewriting links that had $option['entity'] set, but we also need to rewrite the following, for which it is practically impossible to always set $option['entity']:
    • absolute links in API requests - for 'alternate' links, per 🐛 'alternate' link metatags do not point the frontend Fixed
    • links to the front page in API requests - for breadcrumb links (in the json+ld section), per 🐛 'breadcrumb' metatags do not point the frontend Fixed

There is another solution that tiptoes around the above and passes all our current tests** + also fixes the "View API Output" links:

-    if (isset($options['route']) && in_array($options['route']->getPath(), $this->frontendPaths) && $request) {
+    if (isset($options['route']) && in_array($options['route']->getPath(), $this->frontendPaths) && empty($options['base_url']) && $request) {
       $absolute = $options['absolute'] ?? FALSE;
-      if ($absolute || !$this->isApiResponse($request)) {
+      if ($this->isApiResponse($request) ? $absolute : isset($options['entity'])) {

As in:

• For non-API responses, only rewrite links where isset($options['entity']); this is equal to the 2023 situation.
• For API responses, only rewrite absolute links - .

Apparently we don't currently need BaseUrlProvider::getAdminBaseUrl() in API responses - but this still feels fishy to me. It feels clearer to introduce a new URL option 'lupus_decoupled_skip_rewrite' option to prevent that (hopefully to be used only inside of BaseUrlProvider).

For comparison, the current proposed code change is:

-    if (isset($options['route']) && in_array($options['route']->getPath(), $this->frontendPaths) && $request) {
+    if (isset($options['route']) && in_array($options['route']->getPath(), $this->frontendPaths)
+      && empty($options['lupus_decoupled_skip_rewrite']) && empty($options['base_url']) && $request) {

where && empty($options['base_url']) could also be left out and replaced by setting $options['lupus_decoupled_skip_rewrite'] explicitly for "View API output".

--

* I'm treating this "need" as a given: the code which rewrites links to frontend destinations has existed since Oct 2022. As far as I've been able to see, we could theoretically do without it, but that would mean

• the frontend destination links not being visible in users' browsers in e.g. admin/content, which is arguably a UX degredation
• extra redirects from the backend to the frontend destination, whenever such a link is clicked.
• some code change needed in FrontendRedirectSubscriber; I haven't checked those details.

** I haven't checked our internal test suite with this yet.

thx. I think the analysis and changes make sense.

Only think I think we should improve is the naming of the option "lupus_decoupled_skip_rewrite", because without knowing this issues, the name would not tell me what it is about. What rewrite? In the end, the flag is about to force the baseURL to the backend (or frontend?).

What about supporting the flag 'lupus_decoupled_frontend' => FALSE or 'lupus_decoupled_rewrite_frontend' => FALSE ?

Note: This is related: https://www.drupal.org/project/lupus_decoupled/issues/3346436 →

I have an in-grown reservation against parameters that are unset and default-TRUE ... but that doesn't matter much here because we don't actually want this one to be used much.

Implemented as 'lupus_decoupled_rewrite_frontend' => FALSE. Tests are green.

> I have an in-grown reservation against parameters that are unset and default-TRUE ... but that doesn't matter much here because we don't actually want this one to be used much.

:D I dislike parameters which have a negation in the naming.. ;-) We can bike-shed!

Anyway, this is good to go, merged, thank you!

Automatically closed - issue fixed for 2 weeks with no activity.