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

EZP-31130: Ensured content variable is always present in Custom Template Twig view #84

Merged
merged 2 commits into from
Dec 10, 2019

Conversation

alongosz
Copy link
Member

@alongosz alongosz commented Nov 13, 2019

Question Answer
JIRA issue EZP-31130
Bug/Improvement yes
New feature no
Target version v1.1.x for eZ Platform 2.5.x LTS
BC breaks no
Tests pass yes
Doc needed no, doc is already aligned

This PR introduces small change - ensuring that content Twig variable is always present in Custom Template (Tag, Style) Twig view.

eZ Platform DocBook schema (.rng) currently allows ezcontent tag to be not present in eztemplate. While AdminUI ensures ezcontent is always set, even if empty, a DocBook comming from migrated XmlText might not have this tag sometimes. This resulted in content Twig variable being not present in a Twig view for a given Custom Template, which yields a fatal error both on a front site and in AdminUI Location View.

There were 3 directions to solve that issue:

  1. Force a developer to check for existence of that variable.
  2. Add ezcontent tag during migration from XmlText to RichText.
  3. Make content variable always present in a Twig view for Custom Template.

The second option was rejected because RNG schema defines ezcontent as optional.
The first option yields bad DX.

Currently this PR implements the third option, so a developer, instead of writing:

{% if content is defined %}{{ content|raw }}{% endif %}

can simply write:

{{ content|raw }}

which is also consistent with the doc that we provide.

QA

  • Sanity checks for creating Custom Tags with or without content.
  • Test: Create a Custom Tag, find it in ezcontentobject_attributes, remove <content>...</content> markup from XML, save, clear cache, observe the mentioned error on a front site.

TODO:

@alongosz alongosz added the Improvement New feature or request label Nov 13, 2019
@alongosz alongosz requested review from adamwojs and vidarl November 13, 2019 16:21
@alongosz alongosz added the DX Developer Experience improvement label Nov 13, 2019
Copy link
Member

@vidarl vidarl left a comment

Choose a reason for hiding this comment

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

Looks fine and works here ;)

@vidarl
Copy link
Member

vidarl commented Nov 14, 2019

FYI : I just realized that AdminUI does NOT ensure <ezcontent/>is set on inline custom tags if they have no content

@alongosz
Copy link
Member Author

FYI : I just realized that AdminUI does NOT ensure <ezcontent/>is set on inline custom tags if they have no content

Hmm, ok. Just one more reason for the change proposed here 👍

@alongosz alongosz changed the title Ensured content variable is always present in Custom Template Twig view EZP-31130: Ensured content variable is always present in Custom Template Twig view Nov 14, 2019
@alongosz alongosz force-pushed the custom-templates-always-set-content branch from 7016f63 to 351c24c Compare December 5, 2019 14:12
@katarzynazawada katarzynazawada self-assigned this Dec 6, 2019
@lserwatka lserwatka merged commit 859410e into ezsystems:1.1 Dec 10, 2019
@lserwatka
Copy link
Member

You can merge it up.

@alongosz alongosz deleted the custom-templates-always-set-content branch December 11, 2019 09:11
@alongosz
Copy link
Member Author

You can merge it up.

Done via 0b1045f.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DX Developer Experience improvement Improvement New feature or request QA approved
Development

Successfully merging this pull request may close these issues.

5 participants