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.

Sign up for GitHub

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

Jump to bottom

Integrity check for templates and more #21

Open
ghost opened this issue Aug 21, 2018 · 14 comments
Open

Integrity check for templates and more #21

ghost opened this issue Aug 21, 2018 · 14 comments
Labels
feature help wanted Issues and PRs which are looking for volunteers to complete them.

Comments

@ghost
Copy link

ghost commented Aug 21, 2018

Issue by @fritzmg
August 17th, 2018, 12:05 GMT

In Contao 3 you were able to select a variety of custom template for articles, including mod_article_plain and mod_article_teaser which were removed in Contao 4.

However, this will lead to an

Could not find template "mod_article_plain"

error after upgrading to Contao 4 if these template were previously set as a customTpl manually (for whatever reason) and no such template is present as a custom template.

Thus may be Version400Update should check whether or not a custom template with these names are present and if not, tl_module.customTpl should be emptied for these records.
@ghost ghost added the bug label Aug 21, 2018
@ghost ghost mentioned this issue Aug 21, 2018
Closed
@leofeyer leofeyer added this to the 4.4.22 milestone Aug 21, 2018
@leofeyer
Copy link
Member

I'm actually not sure if this is the right way to go. Don't we want those errors to occur so the webmaster notices it? Otherwise it would just fail silently.

@fritzmg
Copy link
Contributor

fritzmg commented Aug 27, 2018

There is a similar situation with the j_slider template for example. If you update from 3.5 to 4.4 (?) the same error will occur for that template in the front end, since it does not exist anymore.

Imho that's fine though, since updating your page layout and template files yourself is part of the regular, upgrade process. The same is still true for those article templates, however it's far easier to just edit and save page layouts. With the article templates you'd have to go through each article manually (if you are not very tech savvy and don't fix it directly in the database with one query 😉).

@leofeyer leofeyer removed this from the 4.4.22 milestone Aug 27, 2018
@leofeyer leofeyer added the up for discussion Issues and PRs which will be discussed in our monthly Mumble calls. label Aug 27, 2018
@fritzmg
Copy link
Contributor

fritzmg commented Aug 27, 2018

May be instead of trying to fix stuff like that for the user, we could introduce some sort of analyzer to help users migrating such things? In case of an upgrade it could analyze and show the user, what he has to do manually. e.g. it could detect whether a page layout still has old core templates selected, that don't exist anymore, same for the article template selection.

@dmolineus
Copy link
Contributor

I'm actually not sure if this is the right way to go. Don't we want those errors to occur so the webmaster notices it? Otherwise it would just fail silently.

Sidenote: There's some inconsistency how missing elements are handled. Missing templates throw an exception, missing modules / content elements only add an error message. Would be nice if both cases are handled equally.

@leofeyer
Copy link
Member

As discussed in Mumble on August 30th, we do not want to migrate the templates, because there are too many edge cases in which this can cause problems.

we could introduce some sort of analyzer to help users migrating such things? In case of an upgrade it could analyze and show the user, what he has to do manually.

This is a good idea. Can we add this to the Contao check? Or a separate tool? We have to be able to run the tool before we actually update the installation.

@leofeyer leofeyer added feature and removed bug up for discussion Issues and PRs which will be discussed in our monthly Mumble calls. labels Aug 30, 2018
@leofeyer
Copy link
Member

@ausi suggested to add checks in the Version400Update class, which generate notices.

@fritzmg
Copy link
Contributor

fritzmg commented Aug 30, 2018

My suggestion would also be to add those checks to Version400Update.

@leofeyer leofeyer added bug and removed feature labels Aug 30, 2018
@leofeyer leofeyer added this to the 4.4.24 milestone Aug 30, 2018
@leofeyer
Copy link
Member

leofeyer commented Sep 3, 2018 •
edited

Like this?

@asaage
Copy link

asaage commented Sep 3, 2018 •
edited

It would be nice if this appears in a separate fieldset/legend and covers not just "old" templates but checks for existence of any manually assigned template.

@leofeyer leofeyer self-assigned this Sep 3, 2018
leofeyer added a commit that referenced this issue Sep 3, 2018
@leofeyer
Show a hint in the install tool if there are old custom templates (see …
9fe6884 
…#21)
@leofeyer
Copy link
Member

leofeyer commented Sep 3, 2018

Fixed in 9fe6884.

@leofeyer leofeyer closed this as completed Sep 3, 2018
@leofeyer
Copy link
Member

leofeyer commented Sep 3, 2018

@asaage This seems way out of scope for the install tool, don't you think?

@fritzmg
Copy link
Contributor

fritzmg commented Sep 3, 2018

I would also add the tl_layout template, that have changed (e.g. j_slider to js_slider etc.) or been removed.

@asaage
Copy link

asaage commented Sep 3, 2018

OK It probably is way out of scope for the install tool...
But sometimes i have to deal with installations that contain tons of templates in the template-folder not knowing which of them are in use - and where they are being used.
Just thought it might probably be easier to provide a generic solution than the mapping of obsolete templates per update.

@leofeyer
Copy link
Member

leofeyer commented Sep 3, 2018 •
edited

@fritzmg These templates are adjusted automatically.

Just thought it might probably be easier to provide a generic solution than the mapping of obsolete templates per update.

It is most likely not easier but it makes a lot more sense. Frankly, I am thinking about reverting 9fe6884, because the note will only be shown once during the version 4.0.0 update and I don't know if it is helpful at this time.

A generic solution would allow to retrieve the information at any time. And it could be enhanced with more integrity checks.

@contao/developers /cc

leofeyer added a commit that referenced this issue Sep 3, 2018
@leofeyer
Show a hint in the install tool if there are old custom templates (see …
a7a459b 
…#21)
@leofeyer leofeyer reopened this Sep 3, 2018
leofeyer added a commit that referenced this issue Sep 3, 2018
@leofeyer
Revert 'Show a hint in the install tool if there are old custom templ…
add3f00 
…ates (see #21)'
@leofeyer leofeyer removed this from the 4.4.24 milestone Sep 3, 2018
@leofeyer leofeyer added feature and removed bug labels Sep 3, 2018
@leofeyer leofeyer changed the title handle mod_article_teaser and mod_article_plain selection Integrity check for templates and more Sep 3, 2018
@leofeyer leofeyer added the help wanted Issues and PRs which are looking for volunteers to complete them. label Jun 12, 2020
@leofeyer leofeyer removed their assignment Jun 12, 2020
leofeyer pushed a commit that referenced this issue Aug 4, 2020
@qzminski
Fix a potential error if the URL has percentage in it (see #619)
0a7b13d 
Description
-----------

With @Toflar we have discovered by accident that if a URL is double-encoded (for some reason, doesn't matter) the Contao's `RouteProvider` will eventually throw an error trying to query a database.

```
URL original: drachenlochmuseum-v%25c3%25a4ttis.html
URL decoded: drachenlochmuseum-v%c3%a4ttis.html
URL decoded 2nd time: drachenlochmuseum-vättis.html
```

The decoded URL is used in the database query and that fails because the database driver would like to replace wildcards `%c` with parameters that were not provided.

Stack trace:

```
Exception: Too few arguments to build the query string
#27 vendor/contao/core-bundle/src/Resources/contao/library/Contao/Database/Statement.php(304): replaceWildcards
#26 vendor/contao/core-bundle/src/Resources/contao/library/Contao/Database/Statement.php(249): execute
#25 vendor/contao/core-bundle/src/Resources/contao/library/Contao/Model.php(1102): find
#24 vendor/contao/core-bundle/src/Resources/contao/library/Contao/Model.php(973): findBy
#23 vendor/contao/core-bundle/src/Framework/Adapter.php(38): __call
#22 vendor/contao/core-bundle/src/Routing/RouteProvider.php(493): findPages
#21 vendor/contao/core-bundle/src/Routing/RouteProvider.php(88): getRouteCollectionForRequest
#20 vendor/contao/core-bundle/src/Routing/LegacyRouteProvider.php(43): getRouteCollectionForRequest
#19 vendor/symfony-cmf/routing/src/NestedMatcher/NestedMatcher.php(141): matchRequest
#18 vendor/contao/core-bundle/src/Routing/Matcher/LegacyMatcher.php(69): matchRequest
#17 vendor/symfony-cmf/routing/src/DynamicRouter.php(271): matchRequest
#16 vendor/symfony-cmf/routing/src/ChainRouter.php(188): doMatch
#15 vendor/symfony-cmf/routing/src/ChainRouter.php(158): matchRequest
#14 vendor/symfony/http-kernel/EventListener/RouterListener.php(115): onKernelRequest
#13 vendor/symfony/event-dispatcher/EventDispatcher.php(212): doDispatch
#12 vendor/symfony/event-dispatcher/EventDispatcher.php(44): dispatch
#11 vendor/symfony/http-kernel/HttpKernel.php(126): handleRaw
#10 vendor/symfony/http-kernel/HttpKernel.php(67): handle
#9 vendor/symfony/http-kernel/Kernel.php(198): handle
#8 vendor/symfony/http-kernel/HttpCache/SubRequestHandler.php(85): handle
#7 vendor/symfony/http-kernel/HttpCache/HttpCache.php(448): forward
#6 vendor/symfony/framework-bundle/HttpCache/HttpCache.php(57): forward
#5 vendor/symfony/http-kernel/HttpCache/HttpCache.php(420): fetch
#4 vendor/contao/manager-bundle/src/HttpKernel/ContaoCache.php(46): fetch
#3 vendor/symfony/http-kernel/HttpCache/HttpCache.php(317): lookup
#2 vendor/symfony/http-kernel/HttpCache/HttpCache.php(192): handle
#1 vendor/friendsofsymfony/http-cache/src/SymfonyCache/EventDispatchingHttpCache.php(98): handle
#0 web/app.php(58): null
```

Commits
-------

8ae2582 Fix a potential error if the URL has percentage in it
8caaf25 Fix unit tests
509f762 Correctly encode the page aliases
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature help wanted Issues and PRs which are looking for volunteers to complete them.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@dmolineus @leofeyer @asaage @fritzmg