Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PageSelectionResolver does not resolve NODE_TYPE_INTERNAL_LINK and NODE_TYPE_EXTERNAL_LINK urls #62

Closed
alexandersch opened this issue Nov 30, 2020 · 3 comments · Fixed by #98
Assignees
Labels
bug Error or unexpected behavior of already existing functionality

Comments

@alexandersch
Copy link

alexandersch commented Nov 30, 2020

The pageSelectionResolver does not resolve the correct URL when the structure's node type is NODE_TYPE_INTERNAL_LINK or NODE_TYPE_EXTERNAL_LINK. For me it just returns the internal path in the JSON response.

This is the default output right now:

"link": {
    "id": "6491f713-e30d-48b5-8e25-fe19e09d3582",
    "type": "page",
    "template": "default",
    "content": {
        "title": "GCTV",
        "url": "/gctv"
    },
    "view": {
        "title": [],
        "url": []
    },
    "author": "2",
    "authored": "2020-09-04T13:42:12+0200",
    "changer": 2,
    "changed": "2020-09-04T14:26:09+0200",
    "creator": 9,
    "created": "2020-07-13T13:42:12+0200"
}

What I would like to see is that url resolves the redirect URL like this:

"link": {
    "id": "6491f713-e30d-48b5-8e25-fe19e09d3582",
    "type": "page",
    "template": "default",
    "content": {
        "title": "GCTV",
        "url": "https://www.gctv.com"
    },
    "view": {
        "title": [],
        "url": []
    },
    "author": "2",
    "authored": "2020-09-04T13:42:12+0200",
    "changer": 2,
    "changed": "2020-09-04T14:26:09+0200",
    "creator": 9,
    "created": "2020-07-13T13:42:12+0200"
}

Example template:

<?xml version="1.0" ?>
<template xmlns="http://schemas.sulu.io/template/template"
          xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
          xsi:schemaLocation="http://schemas.sulu.io/template/template http://schemas.sulu.io/template/template-1.0.xsd">

    <key>test</key>

    <view>pages/default</view>
    <controller>Sulu\Bundle\HeadlessBundle\Controller\HeadlessWebsiteController::indexAction</controller>

    <meta>
        <title lang="en">Test Page</title>
    </meta>

    <properties>

        <property name="title" type="text_line" mandatory="true">
            <meta>
                <title lang="en">Title</title>
            </meta>
            <params>
                <param name="headline" value="true"/>
            </params>

            <tag name="sulu.rlp.part"/>
        </property>

        <property name="url" type="resource_locator" mandatory="true">
            <meta>
                <title lang="en">Slug</title>
            </meta>
            <tag name="sulu.rlp"/>
        </property>

        <property name="link" type="single_page_selection">
            <meta>
                <title lang="en">Link</title>
            </meta>
        </property>

    </properties>
</template>
@alexandersch alexandersch changed the title pageSelectionResolver does not resolve redirect URl\s pageSelectionResolver does not resolve redirect URL's Nov 30, 2020
@alexander-schranz alexander-schranz changed the title pageSelectionResolver does not resolve redirect URL's PageSelectionResolver does not resolve NODE_TYPE_INTERNAL_LINK and NODE_TYPE_EXTERNAL_LINK urls Dec 9, 2020
@alexander-schranz alexander-schranz added the bug Error or unexpected behavior of already existing functionality label Dec 9, 2020
@alexander-schranz
Copy link
Member

Thank you for report. We definitely should find a way doing this in the contentQueryBuilder at current state with your fix in sulu/sulu#5600 atleast the redirect should be done correctly.

@niklasnatter
Copy link
Contributor

niklasnatter commented Dec 9, 2020

Yes, we should definitely provide some information about the target if a page is an internal link or external link. I am not sure if the content object is the correct place for this, because that object usually contains the resolved data of the selected page. I also think we should implement this somehow similar to the default Sulu implementation.

For reference, in a fresh Sulu installation without any bundles, the following property results in the following data:

<property name="pages" type="page_selection">
    <meta>
        <title>Pages</title>
    </meta>

    <params>
        <param name="properties" type="collection">
            <param name="title" value="title"/>
            <param name="url" value="url"/>
        </param>
    </params>
</property>
"pages" => array:3 [▼
  0 => array:20 [▼
    "id" => "ff6e57bf-59e9-44c4-a8a0-e1e1565f3713"
    "uuid" => "ff6e57bf-59e9-44c4-a8a0-e1e1565f3713"
    "nodeType" => 1
    "path" => "/normal-page"
    "changed" => DateTime @1607515641 {#3674 ▶}
    "changer" => 1
    "created" => DateTime @1606314684 {#3677 ▶}
    "publishedState" => true
    "published" => DateTime @1606314684 {#3666 ▶}
    "creator" => 1
    "title" => "Normal Page"
    "locale" => "en"
    "webspaceKey" => "example"
    "template" => "default"
    "parent" => "09d5d72e-8e3c-44fe-bf50-fecfbc0146c6"
    "url" => "/normal-page"
    "urls" => array:1 [▼
      "en" => "/normal-page"
    ]
    "author" => "1"
    "authored" => DateTime @1606314683 {#3669 ▶}
    "order" => 20
  ]
  1 => array:20 [▼
    "id" => "8c9d007c-45dc-4094-8e4b-4d4c1579f443"
    "uuid" => "8c9d007c-45dc-4094-8e4b-4d4c1579f443"
    "nodeType" => 2
    "path" => "/internal-link"
    "changed" => DateTime @1607515806 {#4300 ▶}
    "changer" => 1
    "created" => DateTime @1606897107 {#4297 ▶}
    "publishedState" => true
    "published" => DateTime @1606897107 {#4309 ▶}
    "creator" => 1
    "title" => "Internal Link Target"
    "locale" => "en"
    "webspaceKey" => "example"
    "template" => "default"
    "parent" => "09d5d72e-8e3c-44fe-bf50-fecfbc0146c6"
    "url" => "/internal-link-target"
    "urls" => array:1 [▼
      "en" => "/internal-link-target"
    ]
    "author" => "1"
    "authored" => DateTime @1606897106 {#4292 ▶}
    "order" => 70
  ]
  2 => array:20 [▼
    "id" => "13cc6cfd-6d02-4565-9c92-7270b1d71093"
    "uuid" => "13cc6cfd-6d02-4565-9c92-7270b1d71093"
    "nodeType" => 4
    "path" => "/external-link"
    "changed" => DateTime @1606720220 {#4422 ▶}
    "changer" => 1
    "created" => DateTime @1606720205 {#4386 ▶}
    "publishedState" => true
    "published" => DateTime @1606720205 {#4413 ▶}
    "creator" => 1
    "title" => "External Link"
    "locale" => "en"
    "webspaceKey" => "example"
    "template" => "default"
    "parent" => "09d5d72e-8e3c-44fe-bf50-fecfbc0146c6"
    "url" => "/external-link"
    "urls" => array:1 [▼
      "en" => "/external-link"
    ]
    "author" => "1"
    "authored" => DateTime @1606720205 {#4381 ▶}
    "order" => 60
  ]
]

@niklasnatter
Copy link
Contributor

Thanks for reporting! Should be fixed by #98

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Error or unexpected behavior of already existing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants