Problem/Motivation

'alternate' links in CE API output have a URL pointing to the backend, e.g.

(imagine example JSON here: contains "href": "https://lupus-decoupled.ddev.site/....", should be "href": "https://lupus-nuxt.ddev.site/....")

These 'alternate' links (to the original node as well as its existing translations) are only added to the output of a node, if the node has a translation.

Steps to reproduce

  • add translation to node
  • view API output for node

Proposed resolution

See MR, test-only and with proposed fix.

Remaining tasks

Review. Please note:

  • MR !74 is meant for review/merging -- but tests fail, just like they do on 1.x, until #3409409: Respect installations with base paths gets merged
  • therefore MR!76 is exactly the same commits, but with #3409409 merged in, to prove the fix
  • and MR!73 is the first test-only commit, but with #3409409 merged in, to prove the test failed before the fix.

#3409409

CommentFileSizeAuthor
#7 3412044-alternate-links.patch1.49 KBroderik

Issue fork lupus_decoupled-3412044

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

roderik created an issue. See original summary.

roderik opened merge request !73

roderik opened merge request !74

roderik’s picture

roderik
Primary language Dutch
Location Amsterdam,NL / Budapest,HU
commented
Status: Active » Needs review

Created 1 MR with phpunit test only (fail), 1 MR with the fix on top of it.

I had to include all of #3409409: Respect installations with base paths in order to make the phpunit output make sense, so for a review, you'll need to completely disregard the changes made to files modules/lupus_decoupled_ce_api/src/BackendApiRequest.php and (new) tests/src/Unit/LupusDecoupledBackendApiRequestTest.php (i.e. the first 10 commits)

Only look at

  • tests/src/Functional/LupusDecoupledApiResponseTest.php (same commit == same changes in both MRs)
  • modules/lupus_decoupled_ce_api/src/PathProcessor/LupusFrontendPathProcessor.php (in 'real' MR)
roderik’s picture

roderik
Primary language Dutch
Location Amsterdam,NL / Budapest,HU
commented
Status: Needs review » Needs work

I guess not "needs review" until I comment the fix...

roderik’s picture

roderik
Primary language Dutch
Location Amsterdam,NL / Budapest,HU
commented
Status: Needs work » Needs review

It's fine, I don't think it needs public comments because it was commented elsewhere internally.

roderik’s picture

roderik
Primary language Dutch
Location Amsterdam,NL / Budapest,HU
commented
StatusFileSize
new 3412044-alternate-links.patch1.49 KB

Patch file with just the actual code change from this issue - not the phpunit test or the stuff from #3409409: Respect installations with base paths

roderik opened merge request !76

roderik’s picture

roderik
Primary language Dutch
Location Amsterdam,NL / Budapest,HU
commented
Issue summary: View changes
roderik’s picture

roderik
Primary language Dutch
Location Amsterdam,NL / Budapest,HU
commented

I did not change the code since I originally opened the merge requests...

However, I realized that the original "tests green" MR is not for merging, because it contains #3409409: Respect installations with base paths.

So I created separate merge requests:

  • to prove the fix still works / stay 'green', but not for merging) which includes #3409409
  • for review merging (but not green)
fago’s picture

fago
Primary language German
Location Vienna
commented
Title: 'alternate' links point to backend » 'alternate' link metatags do not point the frontend
roderik’s picture

roderik
Primary language Dutch
Location Amsterdam,NL / Budapest,HU
commented
Issue summary: View changes

  • fago committed a761b814 on 1.x authored by roderik 
    Issue #3412044 by roderik, fago: 'alternate' link metatags do not point...
fago’s picture

fago
Primary language German
Location Vienna
commented
Status: Needs review » Fixed

thanks, reviewed, green and merged!

Status: Fixed » Closed (fixed)

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