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

Not found page must return HTTP 404 #71

Closed
nick-denry opened this issue May 13, 2018 · 4 comments
Closed

Not found page must return HTTP 404 #71

nick-denry opened this issue May 13, 2018 · 4 comments

Comments

@nick-denry
Copy link
Contributor

nick-denry commented May 13, 2018

Hi!

  1. I'm pretty sure that custom "not found" page should return HTTP 404 to be not indexed by SE. Now it's return HTTP 200.

  2. It's also better not to use 301 redirect to "not found" page, but display it's content with 404 code.
    Otherwise it will be redirect 301 to HTTP 404 (301 -> 404).

@nadar nadar self-assigned this May 16, 2018
@nadar
Copy link
Member

nadar commented May 16, 2018

  1. You mean the page which can be set by the cms as 404. I agree!
  2. This is not possible, as we do not render the erroring page (cause we don't know what triggers the error) so we have to redirect to the defined error page, but return the error page as 404 is a good idea though

@nick-denry
Copy link
Contributor Author

nick-denry commented May 17, 2018

  1. This is not possible, as we do not render the erroring page (cause we don't know what triggers the error) so we have to redirect to the defined error page

I don't understand. Assume there is some event or condition when LUYA redirects to predefined 404 page, i.e. "no route" or "model not found". Why we can't just display predefined page content and throw 404 instead of redirect to it?

Could you point me where redirect happens?

@dven84
Copy link
Contributor

dven84 commented May 18, 2018

nick-denry, the problem can be solved like this

 'cms' => [
             ...
            'errorViewFile' => '@app/views/error/index.php',
        ],
  ...
  'errorHandler' => [
            'errorAction' => 'cms/error/index',
         ...
        ],

in views/error/index.php:

...
<?php if ($error_page = \luya\cms\models\NavItem::findOne(['alias' => $exception->statusCode])): ?>
            <?= $error_page->type->renderPlaceholder('content') ?>
<?php else: ?>
          <h1>Error: <?= $exception->statusCode ?></h1>
           <p><?= $exception->getMessage() ?></p>
<?php endif; ?>
...

@nadar
Copy link
Member

nadar commented Jun 4, 2018

@nick-denry Maybe i have an idea how to fix this problem and output the content from the page instead of redirecting, this would fix problem 1 and 2.

@dven84 Thanks for providing us your solution, this is indeed also a good idea to cover all kind of exceptions!

Fix:
Use getModel() of nav item in order to return content and assign to data in response component.

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

No branches or pull requests

3 participants