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

The alias-url of news with external target doesn't redirect #33

Closed
ADoebeling opened this issue Feb 12, 2018 · 6 comments
Closed

The alias-url of news with external target doesn't redirect #33

ADoebeling opened this issue Feb 12, 2018 · 6 comments
Assignees
Labels
Milestone

Comments

@ADoebeling
Copy link

If you create a news-record with an external redirect, the newslist is linking the entry to the external ressource.

The previously generated alias-url of the news-entry doesn't redirect you.
The old url is still available and shows a 1 to the screen:

image

AFAIK this is a bug as the page-module is handeling this case correctly.

@fritzmg
Copy link
Contributor

fritzmg commented Feb 12, 2018

I agree. If source of the news entry is not default, its URL should respond with a 301 status code redirecting to the configured location. I see this more as a feature than a bug fix though.

	if (null === $objArticle)
	{
		throw new PageNotFoundException('Page not found: ' . \Environment::get('uri'));
	}

+	if ('default' !== $objArticle->source && substr($objArticle->url, 0, 7) !== 'mailto:')
+	{
+		throw new RedirectResponseException($this->generateNewsUrl($objArticle), 301);
+	}

	$arrArticle = $this->parseArticle($objArticle);
	$this->Template->articles = $arrArticle;

// edit: may be a 302 code is more suitable though

@leofeyer leofeyer added this to the 4.6.0 milestone Feb 14, 2018
@leofeyer leofeyer self-assigned this Apr 11, 2018
@leofeyer
Copy link
Member

What about mailto: links? The problem still exists for them.

@fritzmg
Copy link
Contributor

fritzmg commented Apr 11, 2018

May be respond with a 404 in this case?

@leofeyer
Copy link
Member

That's a very good idea. We should even do this if the external URL is not a mailto: link, shouldn't we? Because an external news or event does no longer have an internal URL.

@fritzmg
Copy link
Contributor

fritzmg commented Apr 12, 2018

Hm, so you mean responding with a 404 in general, if $objArticle->source !== 'default'? You are right, may be that is the more correct solution anyway. A 301 redirect for a URL that points to a news article which has an external link would not be correct in the sense of: the page/content did not actually "move permanently" to the new URL. It is simply an invalid URL.

@leofeyer
Copy link
Member

Changed in ce44f81.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants