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

IBX-9415: Added support for ContentAwareInterface in ibexa_* Twig functions #467

Open
wants to merge 9 commits into
base: 4.6
Choose a base branch
from

Conversation

wiewiurdp
Copy link

@wiewiurdp wiewiurdp commented Jan 27, 2025

🎫 Issue IBX-9415

Related PRs:

Description:

Added support for ContentAwareInterface objects (e.g. product) to following Twing functions

  • ibexa_render_field
  • ibexa_field_value - core
  • ibexa_field
  • ibexa_field_name
  • ibexa_field_description
  • ibexa_field_is_empty
  • ibexa_has_field
  • ibexa_content_field_identifier_first_filled_image
  • ibexa_content_name
  • ibexa_render_content

For QA:

It needs to be chcked if all theses functions can instead of content as a first argument also work with for example product

Documentation:

Docs for all theses functions should be changed that they are working also with ContentAwareInterfaced objects

@wiewiurdp wiewiurdp requested a review from a team January 27, 2025 13:05
@wiewiurdp wiewiurdp added Doc needed The changes require some documentation Ready for review labels Jan 27, 2025
@@ -4,6 +4,7 @@
{% extends 'templates/base.html.twig' %}
{% block content %}
{{ ez_render_field( nooverride, 'testfield' ) }}
{{ ibexa_render_field( contentaware, 'testfield' ) }}
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldnt you be testing {{ ez_render_field( contentaware, 'testfield' ) }} here and in other ez_ fixtures?

new TwigFunction(
'ibexa_field_is_empty',
[$this, 'isFieldEmpty']
),
new TwigFunction(
Copy link
Contributor

Choose a reason for hiding this comment

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

minor, but there is no need to change it position in the array and complicate eventual merge up where all ez_ functions are dropped.

@ViniTou ViniTou changed the title IBX-9415 Support for ContentAwareInterface in ibexa_* Twig functions added IBX-9415: Support for ContentAwareInterface in ibexa_* Twig functions added Jan 28, 2025
@alongosz alongosz changed the title IBX-9415: Support for ContentAwareInterface in ibexa_* Twig functions added IBX-9415: Added support for ContentAwareInterface in ibexa_* Twig functions Jan 31, 2025
* @param string $forcedLanguage Locale we want the content name translation in (e.g. "fre-FR"). Null by default (takes current locale)
*
* @throws \Ibexa\Core\Base\Exceptions\InvalidArgumentType When $content is not a valid Content or ContentInfo object.
* @throws \Ibexa\Core\Base\Exceptions\InvalidArgumentType When $content is not a valid Content, ContentInfo or ContentAwareInterface object.
Copy link
Member

Choose a reason for hiding this comment

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

punctuation, use Oxford comma please (yes, the punctuation here was incorrect before):

Suggested change
* @throws \Ibexa\Core\Base\Exceptions\InvalidArgumentType When $content is not a valid Content, ContentInfo or ContentAwareInterface object.
* @throws \Ibexa\Core\Base\Exceptions\InvalidArgumentType When $content is not a valid Content, ContentInfo, or ContentAwareInterface object.

@alongosz alongosz requested a review from a team February 4, 2025 12:21
Copy link

sonarqubecloud bot commented Feb 4, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Doc needed The changes require some documentation Ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants