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

Record canonical URLs are unique for record and record_locale routes #1315

Merged
merged 47 commits into from Apr 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
7951a40
Admin can duplicate a file
I-Valchev Apr 20, 2020
37c8272
Backend menu caches localized
I-Valchev Apr 20, 2020
e6ee121
Record canonical URLs are unique for record and record_locale routes
I-Valchev Apr 20, 2020
c1e151e
This comma is not needed
I-Valchev Apr 20, 2020
c30c8ef
Get correct route
I-Valchev Apr 20, 2020
0526aae
Homepage is canonical too
I-Valchev Apr 20, 2020
897c2f6
Record link fiter persists current locale
I-Valchev Apr 20, 2020
5fc0afe
Merge pull request #1316 from bolt/bugfix/persist-locale-setting
bobdenotter Apr 21, 2020
c8dc19a
Merge pull request #1313 from bolt/feature/duplicate-files
bobdenotter Apr 22, 2020
22ebba5
Merge pull request #1314 from bolt/bugfix/localized-menu-cache
bobdenotter Apr 22, 2020
0398f88
Update Field.php
JTNMW Apr 22, 2020
1fe909b
Merge pull request #1325 from JTNMW/master
I-Valchev Apr 22, 2020
c8f046a
Invalidate localized menu cache
I-Valchev Apr 23, 2020
ada1ace
Canonical for default locale works
I-Valchev Apr 23, 2020
1001173
cxfix. fix tests
I-Valchev Apr 23, 2020
e05c058
Remove unused Bolt\Entity\Field import
I-Valchev Apr 23, 2020
dc1284b
Tidy up Twig RelatedExtension
I-Valchev Apr 23, 2020
3b7695f
Fix number of arguments
I-Valchev Apr 23, 2020
b7cf9f6
Fix failing test
I-Valchev Apr 23, 2020
3c421fd
Merge pull request #1326 from bolt/bugfix/invalidate-localized-cache
bobdenotter Apr 23, 2020
756564b
Merge pull request #1327 from bolt/remove-unused-import
bobdenotter Apr 23, 2020
3a90758
Update conten updated successfully message
I-Valchev Apr 24, 2020
01f8bc3
Merge pull request #1331 from bolt/bugfix/content-success-message
bobdenotter Apr 25, 2020
83c7b56
Working on NPM bitrot
bobdenotter Apr 27, 2020
62a1b19
CS Fixes
bobdenotter Apr 27, 2020
5ea6150
Make it so `homepage:` accepts a singleton, or a contentType listing
bobdenotter Apr 27, 2020
0ccb707
Updating symfony/webpack-encore to 0.29
bobdenotter Apr 27, 2020
1c4377e
Merge pull request #1335 from bolt/chore/npm-updates-retry
bobdenotter Apr 27, 2020
62f83d3
Merge pull request #1328 from bolt/enhancement/tidy-up-related-extension
bobdenotter Apr 27, 2020
9944bfd
Merge pull request #1336 from bolt/fix/allow-contenttype-listing-for-…
bobdenotter Apr 27, 2020
ee131e5
Better `isHomepage` detection for singletons
bobdenotter Apr 27, 2020
b0aa8c2
Merge pull request #1337 from bolt/fix/better-homepage-detection-in-`…
bobdenotter Apr 28, 2020
8b7d348
Put back canonical override in DetailController
I-Valchev Apr 29, 2020
c00d164
DetailControler canonical override passes locale
I-Valchev Apr 29, 2020
2296f8a
Fix phpstan failing on parser::create
I-Valchev Apr 29, 2020
874899b
Fix name
I-Valchev Apr 29, 2020
cbe2177
Merge pull request #1339 from bolt/tests/fix-failing-phpstan-check
bobdenotter Apr 29, 2020
c506b32
Record canonical URLs are unique for record and record_locale routes
I-Valchev Apr 20, 2020
7eb23b6
This comma is not needed
I-Valchev Apr 20, 2020
9662d81
Get correct route
I-Valchev Apr 20, 2020
023dee5
Homepage is canonical too
I-Valchev Apr 20, 2020
805978b
Canonical for default locale works
I-Valchev Apr 23, 2020
a27abfa
cxfix. fix tests
I-Valchev Apr 23, 2020
78ea71d
Fix failing test
I-Valchev Apr 23, 2020
1e0cd73
Put back canonical override in DetailController
I-Valchev Apr 29, 2020
6d78663
DetailControler canonical override passes locale
I-Valchev Apr 29, 2020
999f67e
Merge branch 'bugfix/canonical-record-urls' of https://github.com/bol…
I-Valchev Apr 29, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 6 additions & 5 deletions config/bolt/config.yaml
Expand Up @@ -60,14 +60,15 @@ omit_backgrounds: true
#favicon: images/favicon-bolt.ico

# The default content to use for the homepage, and the template to render it
# with. This can either be a specific record (like `page/1`) or a listing of
# records (like `entries`). In the chosen homepage_template, you will have
# `record` or `records` at your disposal, depending on the homepage setting.
# with. This can either be a singleton like `homepage`, a specific record (like
# `page/1`) or a listing of records (like `entries`). In the chosen
# homepage_template, you will have `record` or `records` at your disposal,
# depending on the homepage setting.
#
# Note: If you've changed the filename, and your changes do not show up on
# the website, be sure to check for a theme.yml file in your themes
# the website, be sure to check for a theme.yaml file in your themes
# folder. If a template is set there, it will override the setting here.
homepage: homepage/1
homepage: homepage
homepage_template: index.twig

# The default content for the 404 page. Can be an (array of) template names or
Expand Down
1 change: 1 addition & 0 deletions config/services.yaml
Expand Up @@ -23,6 +23,7 @@ services:
# The best practice is to be explicit about your dependencies anyway.
bind: # defines the scalar arguments once and apply them to any service defined/created in this file
$locales: '%app_locales%'
$defaultLocale: '%locale%'
$emailSender: '%app.notifications.email_sender%'
$projectDir: '%kernel.project_dir%'
$publicFolder: '%bolt.public_folder%'
Expand Down
935 changes: 553 additions & 382 deletions package-lock.json

Large diffs are not rendered by default.

18 changes: 9 additions & 9 deletions package.json
Expand Up @@ -32,7 +32,7 @@
"baguettebox.js": "^1.11.1",
"bootbox": "^5.4.0",
"bootstrap": "^4.4.1",
"codemirror": "^5.52.2",
"codemirror": "^5.53.2",
"dropzone": "^5.7",
"flagpack": "^1.0.4",
"jquery": "^3.5.0",
Expand All @@ -45,16 +45,16 @@
"selectize": "^0.12.6",
"simplemde": "^1.11.2",
"stylelint-config-recommended-scss": "^4.2.0",
"stylelint-scss": "^3.17.0",
"terser": "^4.6.11",
"stylelint-scss": "^3.17.1",
"terser": "^4.6.12",
"tinycolor2": "^1.4.1",
"vue": "^2.6.11",
"vue-flatpickr-component": "^8.1.5",
"vue-multiselect": "^2.1.6",
"vue-simplemde": "^1.0.4",
"vue-trumbowyg": "^3.6.0",
"vuedraggable": "^2.23.2",
"vuex": "^3.2.0",
"vuex": "^3.3.0",
"zxcvbn": "^4.4.2"
},
"devDependencies": {
Expand All @@ -64,21 +64,21 @@
"@babel/polyfill": "^7.8.7",
"@babel/preset-env": "^7.9.5",
"@fortawesome/fontawesome-free": "^5.13.0",
"@symfony/webpack-encore": "^0.28.3",
"@symfony/webpack-encore": "^0.29.1",
"@vue/test-utils": "^1.0.0-beta.33",
"ajv-keywords": "^3.4.1",
"autoprefixer": "^9.7.6",
"babel-core": "^7.0.0-bridge.0",
"babel-eslint": "^10.1.0",
"babel-jest": "^25.3.0",
"babel-jest": "^25.4.0",
"eslint": "^6.8.0",
"eslint-config-prettier": "^6.10.1",
"eslint-config-prettier": "^6.11.0",
"eslint-plugin-prettier": "^3.1.3",
"eslint-plugin-standard": "^4.0.1",
"eslint-plugin-vue": "^6.2.2",
"jest": "^25.3.0",
"jest": "^25.4.0",
"jest-serializer-vue": "^2.0.2",
"node-sass": "^4.13.1",
"node-sass": "^4.14.0",
"postcss-loader": "^3.0.0",
"postcss-preset-env": "^6.7.0",
"prettier": "^1.19.1",
Expand Down
2 changes: 1 addition & 1 deletion public/theme/skeleton/partials/_recordfooter.twig
Expand Up @@ -34,7 +34,7 @@
</p>
{% endif %}

{% set related_content_types = record|related_all %}
{% set related_content_types = record|related_by_type %}
{% if related_content_types is not empty %}
<p class="meta">{{ __('general.phrase.related-content') }}
<ul>
Expand Down
55 changes: 51 additions & 4 deletions src/Canonical.php
Expand Up @@ -10,6 +10,9 @@
use Symfony\Component\Routing\Exception\InvalidParameterException;
use Symfony\Component\Routing\Exception\MissingMandatoryParametersException;
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
use Symfony\Component\Routing\Route;
use Symfony\Component\Routing\RouterInterface;
use Tightenco\Collect\Support\Collection;

class Canonical
{
Expand All @@ -34,11 +37,19 @@ class Canonical
/** @var string */
private $path = null;

public function __construct(Config $config, UrlGeneratorInterface $urlGenerator, RequestStack $requestStack)
/** @var RouterInterface */
private $router;

/** @var string */
private $defaultLocale;

public function __construct(Config $config, UrlGeneratorInterface $urlGenerator, RequestStack $requestStack, RouterInterface $router, string $defaultLocale)
{
$this->config = $config;
$this->urlGenerator = $urlGenerator;
$this->request = $requestStack->getCurrentRequest();
$this->router = $router;
$this->defaultLocale = $defaultLocale;

$this->init();
}
Expand Down Expand Up @@ -137,9 +148,13 @@ public function setHost(string $host): void
public function getPath(): string
{
if ($this->path === null) {
$route = $this->request->attributes->get('_route');
$params = $this->request->attributes->get('_route_params');
$canonicalRoute = $this->getCanonicalRoute($route, $params);

$this->path = $this->urlGenerator->generate(
$this->request->attributes->get('_route'),
$this->request->attributes->get('_route_params'),
$canonicalRoute,
$params,
UrlGeneratorInterface::ABSOLUTE_PATH
);
}
Expand All @@ -155,14 +170,46 @@ public function setPath(?string $route = null, array $params = []): void
$route = $this->request->attributes->get('_route');
}

$canonicalRoute = $this->getCanonicalRoute($route, $params);

try {
$this->path = $this->urlGenerator->generate(
$route,
$canonicalRoute,
$params
);
} catch (InvalidParameterException | MissingMandatoryParametersException $e) {
// Just use the current URL /shrug
$this->request->getUri();
}
}

private function getCanonicalRoute(string $route, array &$params = []): string
{
$routes = new Collection($this->router->getRouteCollection()->getIterator());
$currentController = $routes->get($route)->getDefault('_controller');

$routes = collect($routes->filter(function (Route $route) use ($currentController) {
return $route->getDefault('_controller') === $currentController;
})->keys());

// If only one route matched, return that.
if ($routes->count() === 1) {
return $routes->first();
}

// If no locale or locale is not default, get the first route which is named *_locale
if (array_key_exists('_locale', $params) && $params['_locale'] !== $this->defaultLocale) {
return $routes->filter(function (string $name) {
return fnmatch('*locale', $name);
})->first();
}

// Unset _locale so that it is not passed as query param to url.
unset($params['_locale']);

// Otherwise, get the first route that is not *_locale
return $routes->filter(function (string $name) {
return ! fnmatch('*locale', $name);
})->first();
}
}
1 change: 0 additions & 1 deletion src/Configuration/Parser/ContentTypesParser.php
Expand Up @@ -8,7 +8,6 @@
use Bolt\Common\Str;
use Bolt\Configuration\Content\ContentType;
use Bolt\Configuration\Content\FieldType;
use Bolt\Entity\Field;
use Bolt\Enum\Statuses;
use Bolt\Exception\ConfigurationException;
use Tightenco\Collect\Support\Collection;
Expand Down
63 changes: 58 additions & 5 deletions src/Controller/Backend/FileEditController.php
Expand Up @@ -37,11 +37,15 @@ class FileEditController extends TwigAwareController implements BackendZoneInter
/** @var EntityManagerInterface */
private $em;

/** @var Filesystem */
private $filesystem;

public function __construct(CsrfTokenManagerInterface $csrfTokenManager, MediaRepository $mediaRepository, EntityManagerInterface $em)
{
$this->csrfTokenManager = $csrfTokenManager;
$this->mediaRepository = $mediaRepository;
$this->em = $em;
$this->filesystem = new Filesystem();
}

/**
Expand Down Expand Up @@ -109,12 +113,12 @@ public function save(Request $request, UrlGeneratorInterface $urlGenerator): Res
}

/**
* @Route("/delete", name="bolt_file_delete", methods={"POST", "GET"})
* @Route("/file-delete/", name="bolt_file_delete", methods={"POST", "GET"})
*/
public function handleDelete(Request $request): Response
{
try {
$this->validateCsrf($request, 'delete');
$this->validateCsrf($request, 'file-delete');
} catch (InvalidCsrfTokenException $e) {
return new JsonResponse([
'error' => [
Expand All @@ -123,8 +127,6 @@ public function handleDelete(Request $request): Response
], Response::HTTP_FORBIDDEN);
}

$filesystem = new Filesystem();

$locationName = $request->get('location', '');
$path = $request->get('path', '');

Expand All @@ -139,7 +141,40 @@ public function handleDelete(Request $request): Response
$filePath = Path::canonicalize($locationName . '/' . $path);

try {
$filesystem->remove($filePath);
$this->filesystem->remove($filePath);
} catch (\Throwable $e) {
// something wrong happened, we don't need the uploaded files anymore
throw $e;
}

$this->addFlash('success', 'file.delete_success');
return $this->redirectToRoute('bolt_filemanager', ['location' => $locationName]);
}

/**
* @Route("/file-duplicate/", name="bolt_file_duplicate", methods={"POST", "GET"})
*/
public function handleDuplicate(Request $request): Response
{
try {
$this->validateCsrf($request, 'file-duplicate');
} catch (InvalidCsrfTokenException $e) {
return new JsonResponse([
'error' => [
'message' => 'Invalid CSRF token',
],
], Response::HTTP_FORBIDDEN);
}

$locationName = $request->get('location', '');
$path = $request->get('path', '');

$originalFilepath = Path::canonicalize($locationName . '/' . $path);

$copyFilePath = $this->getCopyFilepath($originalFilepath);

try {
$this->filesystem->copy($originalFilepath, $copyFilePath);
} catch (\Throwable $e) {
// something wrong happened, we don't need the uploaded files anymore
throw $e;
Expand All @@ -149,6 +184,24 @@ public function handleDelete(Request $request): Response
return $this->redirectToRoute('bolt_filemanager', ['location' => $locationName]);
}

/**
* @return string Returns the copy file path. E.g. 'files/foal.jpg' -> 'files/foal (1).jpg'
*/
private function getCopyFilepath(string $path): string
{
$copyPath = $path;

$i = 1;
while ($this->filesystem->exists($copyPath)) {
$pathinfo = pathinfo($path);
$basename = basename($pathinfo['basename'], '.' . $pathinfo['extension']) . ' (' . $i . ')';
$copyPath = Path::canonicalize($pathinfo['dirname'] . '/' . $basename . '.' . $pathinfo['extension']);
$i++;
}

return $copyPath;
}

private function verifyYaml(string $yaml): bool
{
$yamlParser = new Parser();
Expand Down
9 changes: 8 additions & 1 deletion src/Controller/Frontend/DetailController.php
Expand Up @@ -10,6 +10,8 @@
use Bolt\Enum\Statuses;
use Bolt\Repository\ContentRepository;
use Bolt\TemplateChooser;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\RequestStack;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
use Symfony\Component\Routing\Annotation\Route;
Expand All @@ -22,10 +24,14 @@ class DetailController extends TwigAwareController implements FrontendZoneInterf
/** @var ContentRepository */
private $contentRepository;

public function __construct(TemplateChooser $templateChooser, ContentRepository $contentRepository)
/** @var Request */
private $request;

public function __construct(TemplateChooser $templateChooser, ContentRepository $contentRepository, RequestStack $requestStack)
{
$this->templateChooser = $templateChooser;
$this->contentRepository = $contentRepository;
$this->request = $requestStack->getCurrentRequest();
}

/**
Expand Down Expand Up @@ -56,6 +62,7 @@ public function record($slugOrId, ?string $contentTypeSlug = null, bool $require
$this->canonical->setPath(null, [
'contentTypeSlug' => $record ? $record->getContentTypeSingularSlug() : null,
'slugOrId' => $record ? $record->getSlug() : null,
'_locale' => $this->request->getLocale(),
]);

return $this->renderSingle($record, $requirePublished);
Expand Down
12 changes: 10 additions & 2 deletions src/Controller/Frontend/HomepageController.php
Expand Up @@ -32,11 +32,19 @@ public function homepage(ContentRepository $contentRepository): Response
{
$homepage = $this->config->get('theme/homepage') ?: $this->config->get('general/homepage');
$params = explode('/', $homepage);
$contentType = $this->config->get('contenttypes/' . $params[0]);

// Perhaps we need a listing instead. If so, forward the Request there
if (! $contentType->get('singleton') && ! isset($params[1])) {
return $this->forward('Bolt\Controller\Frontend\ListingController::listing', [
'contentTypeSlug' => $homepage,
]);
}

// @todo Get $homepage content, using "setcontent"
$record = $contentRepository->findOneBy([
'contentType' => $params[0],
'id' => $params[1],
'contentType' => $contentType->get('slug'),
'id' => $params[1] ?? 1,
]);
if (! $record) {
$record = $contentRepository->findOneBy(['contentType' => $params[0]]);
Expand Down
1 change: 1 addition & 0 deletions src/Entity/Field.php
Expand Up @@ -72,6 +72,7 @@ class Field implements FieldInterface, TranslatableInterface

/**
* @ORM\ManyToOne(targetEntity="Bolt\Entity\Field", cascade={"persist"})
* @ORM\JoinColumn(onDelete="CASCADE")
*/
private $parent;

Expand Down