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

implement sitemap and error handling of seo-bundle #352

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
42 changes: 42 additions & 0 deletions app/Resources/views/error/index.html.twig
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
{% extends 'base.html.twig' %}

{% block content %}
<h1>Oops! An Error Occurred</h1>
<h2>The server returned a "{{ status_code }} {{ status_text }}".</h2>
<p>If you see this page, it means your sandbox is not correctly set up.
Please see the README file in the sandbox root folder and if you can't figure out
what is wrong, ask us on freenode irc #symfonycmf or the mailinglist [email protected].
</p>

<p>If you are seeing this page as the result of an edit in the admin tool, please report what you were doing
to our <a href="https://github.com/symfony-cmf/cmf-sandbox/issues/new">ticket system</a>,
so that we can add means to prevent this issue in the future. But to get things working again
for now, please just <a href="{{ app.request.getSchemeAndHttpHost() }}/reloadfixtures.php">click here</a>
to reload the data fixtures.
</p><p style='color:red;'>
<strong>Detected the following problem</strong>:
{{ exception.getMessage() }}

<h3>Suggested pages</h3>
<div class="alert alert-info clearfix">
<p>
This page is rendered by the
<code>SuggestionProviderController</code>
of the CmfSeoBundle. This way, usefull suggestions can be shown to your users.
</p>

<a class="docref" href="http://symfony.com/doc/current/cmf/bundles/seo/error_pages.html">
<i class="glyphicon glyphicon-chevron-right"></i>Read about this feature in the CMF documentation.
</a>
</div>
{% for group, list in best_matches if list is not empty %}
Copy link
Member

@wouterj wouterj Nov 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please include the blue "docref" boxes we use throughout the sandbox:

<div class="alert  alert-info  clearfix">
    <p>This page is rendered by the <code>SuggestionProviderController</code> of the CmfSeoBundle. This way, usefull suggestions can be shown to your users.</p>

    <a class="docref" href="http://symfony.com/doc/current/cmf/bundles/seo/error_pages.html"><i class="glyphicon glyphicon-chevron-right"></i>Read about this feature in the CMF documentation.</a>
</div>

<h4>{{ group|capitalize }}</h4>
<ul>
{% for match in list %}
<li><a href="{{ path(match.id) }}">{{ match.content.title }}</a></li>
{% endfor %}
</ul>
{% else %}
<h4>No suggestions found</h4>
{% endfor %}
{% endblock %}
20 changes: 20 additions & 0 deletions app/Resources/views/sitemap/index.html.twig
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{% extends 'base.html.twig' %}

{% block content %}
<h1>Sitemap</h1>
<p>
The sitemap feature allows to give an overview of the content.
The content document decides whether it should be displayed on a sitemap or not.
The sitemap of the symfony-cmf sandbox is relatively flat because the content URL structure is flat.
If you have deeper nested content, the sitemap is organized along the nested structure.<br />
This information is arranged for human users. For search engines, the sitemap also exists in
an <a href="{{ url('cmf_seo_sitemap', {_format: 'xml', sitemap: 'sitemap'}) }}">xml format</a>.
</p>
<ul class="cmf-sitemap">
{% for url in urls %}
<li{% if url.depth is not null %} class="indent-{{ url.depth }}"{% endif %}>
<a href="{{ url.location }}" title="{{ url.label }}">{{ url.label }}</a>
</li>
{% endfor %}
</ul>
{% endblock %}
17 changes: 17 additions & 0 deletions app/Resources/views/sitemap/index.xml.twig
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?xml version="1.0" encoding="UTF-8"?>
<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
{% for url in urls %}
<url>
<loc>{{ url.location }}</loc>
{% if url.lastModification %}
<lastmod>{{ url.lastModification }}</lastmod>
{% endif %}
<changefreq>{{ url.changeFrequency }}</changefreq>
{% if url.alternateLocales is defined and url.alternateLocales|length > 0 %}
{% for locale in url.alternateLocales %}
<xhtml:link rel="alternate" hreflang="{{ locale.hrefLocale }}" href="{{ locale.href }}"/>
{% endfor %}
{% endif %}
</url>
{% endfor %}
</urlset>
19 changes: 18 additions & 1 deletion app/config/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ framework:
twig:
debug: '%kernel.debug%'
strict_variables: '%kernel.debug%'
exception_controller: 'FOS\RestBundle\Controller\ExceptionController::showAction'
exception_controller: cmf_seo.error.suggestion_provider.controller:listAction
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this also handle encoding? like when i did a json request, do i get back a json error or a html page with error information?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, need to have a look into the code or test it, would be nice.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I propose to use the same strategy the TwigBundle uses: Use $request->getRequestFormat() to determine the format. This way, we support all tools in the environment to determine the request format. (e.g. FOSRestBundle, custom things, _format route placeholder, etc)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but this should not have to happen here, right? is an issue of seo-bundle


# Assetic Configuration
assetic:
Expand Down Expand Up @@ -113,6 +113,23 @@ cmf_seo:
title: 'CMF Sandbox - %%content_title%%'
description: 'The Content Management Framework. %%content_description%%'
alternate_locale: ~
error:
enable_parent_provider: true
enable_sibling_provider: true
templates:
html: ":error/index.html.twig"
exclusion_rules:
- { path: 'excluded' }
sitemap:
defaults:
default_change_frequency: never
templates:
xml: ':sitemap/index.xml.twig'
html: ':sitemap/index.html.twig'
configurations:
sitemap: ~
frequent:
default_change_frequency: always

cmf_menu:
voters:
Expand Down
4 changes: 4 additions & 0 deletions app/config/routing.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,7 @@ block_cache:
cmf_resource:
resource: '@CmfResourceRestBundle/Resources/config/routing.yml'
prefix: /admin

sitemaps:
prefix: /sitemaps
resource: "@CmfSeoBundle/Resources/config/routing/sitemap.xml"
7 changes: 0 additions & 7 deletions app/config/services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,6 @@ services:
class: AppBundle\Controller\ContentController
parent: cmf_content.controller

app.exception_listener:
class: AppBundle\EventListener\SandboxExceptionListener
calls:
- [setContainer, ['@service_container']]
tags:
- { name: kernel.event_subscriber }

app.twig.menu_extension:
class: AppBundle\Twig\MenuExtension
arguments: ['@knp_menu.helper', '@knp_menu.matcher']
Expand Down
2 changes: 2 additions & 0 deletions src/AppBundle/DataFixtures/PHPCR/LoadMenuData.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ public function load(ObjectManager $manager)
$this->createMenuNode($manager, $seo, 'simple-seo-example', array('en' => 'Seo-Simple-Content'), $manager->find(null, "$content_path/simple-seo-example"));
$this->createMenuNode($manager, $seo, 'demo-seo-extractor', array('en' => 'Seo-Extractor'), $manager->find(null, "$content_path/demo-seo-extractor"));
$this->createMenuNode($manager, $seo, 'simple-seo-property', array('en' => 'Seo-Extra-Properties'), $manager->find(null, "$content_path/simple-seo-property"));
$this->createMenuNode($manager, $seo, 'seo-error-pages', array('en' => 'Seo-Error-Pages'), $manager->find(null, "$content_path/seo-error-pages"));
$this->createMenuNode($manager, $seo, 'seo-sitemap', 'Sitemap', null, '/sitemaps/sitemap.html');

$this->createMenuNode($manager, $main, 'routing-auto-item', array('en' => 'Auto routing example', 'de' => 'Auto routing beispiel', 'fr' => 'Auto routing exemple'), $manager->find(null, "$content_path/news/RoutingAutoBundle generates routes!"));

Expand Down
1 change: 1 addition & 0 deletions src/AppBundle/DataFixtures/PHPCR/LoadNewsData.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ public function load(ObjectManager $manager)
See the routing auto <a href="https://github.com/symfony-cmf/cmf-sandbox/blob/master/src/AppBundle/Resources/config/cmf_routing_auto.yml">configuration file</a> to see how this works.
EOT
);
$news->setIsVisibleForSitemap(true);

$manager->persist($news);
$manager->flush();
Expand Down
5 changes: 5 additions & 0 deletions src/AppBundle/DataFixtures/PHPCR/LoadRoutingData.php
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,11 @@ public function load(ObjectManager $manager)
$seo->setPosition($home, 'demo-seo-extractor');
$seo->setContent($manager->find(null, "$content_path/demo-seo-extractor"));
$manager->persist($seo);

$seo = new Route();
$seo->setPosition($home, 'seo-error-pages');
$seo->setContent($manager->find(null, "$content_path/seo-error-pages"));
$manager->persist($seo);
}

// demo features of routing
Expand Down
7 changes: 7 additions & 0 deletions src/AppBundle/DataFixtures/PHPCR/LoadStaticPageData.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use PHPCR\Util\NodeHelper;
use AppBundle\Document\DemoSeoContent;
use Symfony\Cmf\Bundle\SeoBundle\Doctrine\Phpcr\SeoMetadata;
use Symfony\Cmf\Bundle\SeoBundle\SitemapAwareInterface;
use Symfony\Component\DependencyInjection\ContainerAwareTrait;
use Symfony\Component\DependencyInjection\ContainerAwareInterface;
use Symfony\Component\Yaml\Parser;
Expand Down Expand Up @@ -95,6 +96,10 @@ public function load(ObjectManager $manager)
$this->loadBlock($manager, $page, $name, $block);
}
}

if ($page instanceof SitemapAwareInterface) {
$page->setIsVisibleForSitemap(true);
}
}

//add a loading for a simple seo aware page
Expand All @@ -114,6 +119,7 @@ public function load(ObjectManager $manager)
EOH
);
$seoDemo->setParentDocument($parent);
$seoDemo->setIsVisibleForSitemap(true);

$seoMetadata = new SeoMetadata();
$seoMetadata->setTitle('Simple seo example');
Expand Down Expand Up @@ -146,6 +152,7 @@ public function load(ObjectManager $manager)
$seoMetadata->addExtraName('robots', 'index, follow');
$seoMetadata->addExtraProperty('og:title', 'extra title');
$seoDemo->setSeoMetadata($seoMetadata);
$seoDemo->setIsVisibleForSitemap(true);
$manager->persist($seoDemo);

$manager->bindTranslation($seoDemo, 'en');
Expand Down
31 changes: 30 additions & 1 deletion src/AppBundle/Document/DemoClassContent.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace AppBundle\Document;

use Doctrine\ODM\PHPCR\Mapping\Annotations as PHPCRODM;
use Symfony\Cmf\Bundle\SeoBundle\SitemapAwareInterface;
use Symfony\Component\Validator\Constraints as Assert;
use Symfony\Cmf\Component\Routing\RouteReferrersReadInterface;

Expand All @@ -20,7 +21,7 @@
*
* @PHPCRODM\Document(referenceable=true)
*/
class DemoClassContent implements RouteReferrersReadInterface
class DemoClassContent implements RouteReferrersReadInterface, SitemapAwareInterface
{
/**
* to create the document at the specified location. read only for existing documents.
Expand Down Expand Up @@ -62,6 +63,13 @@ class DemoClassContent implements RouteReferrersReadInterface
*/
public $routes;

/**
* @var bool
*
* @PHPCRODM\Field(type="boolean", property="visible_for_sitemap")
*/
private $isVisibleForSitemap;

public function getName()
{
return $this->name;
Expand Down Expand Up @@ -117,4 +125,25 @@ public function getRoutes()
{
return $this->routes->toArray();
}

/**
* Decision whether a document should be visible
* in sitemap or not.
*
* @param $sitemap
*
* @return bool
*/
public function isVisibleInSitemap($sitemap)
{
return $this->isVisibleForSitemap;
}

/**
* @param bool $isVisibleForSitemap
*/
public function setIsVisibleForSitemap($isVisibleForSitemap)
{
$this->isVisibleForSitemap = $isVisibleForSitemap;
}
}
31 changes: 30 additions & 1 deletion src/AppBundle/Document/DemoSeoContent.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Symfony\Cmf\Bundle\ContentBundle\Doctrine\Phpcr\StaticContent;
use Symfony\Cmf\Bundle\SeoBundle\SeoAwareInterface;
use Symfony\Cmf\Bundle\SeoBundle\Doctrine\Phpcr\SeoMetadata;
use Symfony\Cmf\Bundle\SeoBundle\SitemapAwareInterface;

/**
* That example class uses the extractors for the creation of the SeoMetadata.
Expand All @@ -23,7 +24,7 @@
*
* @author Maximilian Berghoff <[email protected]>
*/
class DemoSeoContent extends StaticContent implements SeoAwareInterface
class DemoSeoContent extends StaticContent implements SeoAwareInterface, SitemapAwareInterface
{
/**
* @var SeoMetadata
Expand All @@ -32,6 +33,13 @@ class DemoSeoContent extends StaticContent implements SeoAwareInterface
*/
protected $seoMetadata;

/**
* @var bool
*
* @PHPCRODM\Field(type="boolean", property="visible_for_sitemap")
*/
private $isVisibleForSitemap;

public function __construct()
{
$this->seoMetadata = new SeoMetadata();
Expand All @@ -53,4 +61,25 @@ public function setSeoMetadata($seoMetadata)
{
$this->seoMetadata = $seoMetadata;
}

/**
* Decision whether a document should be visible
* in sitemap or not.
*
* @param $sitemap
*
* @return bool
*/
public function isVisibleInSitemap($sitemap)
{
return $this->isVisibleForSitemap;
}

/**
* @param bool $isVisibleForSitemap
*/
public function setIsVisibleForSitemap($isVisibleForSitemap)
{
$this->isVisibleForSitemap = $isVisibleForSitemap;
}
}
6 changes: 1 addition & 5 deletions src/AppBundle/Document/DemoSeoExtractor.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,7 @@
*
* @author Maximilian Berghoff <[email protected]>
*/
class DemoSeoExtractor extends DemoSeoContent implements
TitleReadInterface,
DescriptionReadInterface,
OriginalUrlReadInterface,
KeywordsReadInterface
class DemoSeoExtractor extends DemoSeoContent implements TitleReadInterface, DescriptionReadInterface, OriginalUrlReadInterface, KeywordsReadInterface
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rather disable this fixer and keep this multiline to make it readable. put disabled: [single_line_class_definition] into .styleci.yml

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh. we should use that everywhere, cause i saw that on seo-bundle yesterday too. from which coding style is that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouter added it in one of the bundles, can't remember which one

{
/**
* {@inheritdoc}
Expand Down
Loading