Problem/Motivation

Reopening this issue as 3334147 was closed but the problem has not been resolved as missing patch #4.

Proposed resolution

Attached patch works as expected and should be released.

3334147_3.patch

CommentFileSizeAuthor
#5 entity_extra_field_3469612-5.patch2.19 KBdroath
Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

coaston created an issue. See original summary.

anybody’s picture

Status: Active » Postponed (maintainer needs more info)

@coaston how do you come to this conclusion?

Comparing the commit from the linked issue
https://git.drupalcode.org/project/entity_extra_field/-/commit/94a2ab3dc...
to the latest branch:
https://git.drupalcode.org/project/entity_extra_field/-/blob/2.1.x/src/P...

it's present!

starlight-sparkle’s picture

it's present!

Coaston means to say that the original commit is incomplete, not that the commit is missing.

The MR created by jnettik implemented the fix in a different way from the patch file submitted by artyom. Namely:

  • The MR implementation simply calls Element::isEmpty()
  • The patch file implementation additionally checks if the plugin is a View, then executes the view to check if the rendered output is empty

This is significant in the special case of views because as explained:

ViewExecutable will build a render array regardless of whether it has anything to display, because the view is not executed until render time, and hence just checking the render array it not reliable, which is all Element::isEmpty() does.

The Drupal Core views block display plugin (\Drupal\views\Plugin\views\display\Block::execute()) suggests that the correct way to check is to actually execute the view with arguments, then check $view->getDisplay()->outputIsEmpty(), which is how the plugin implements its "Hide block if the view output is empty" option.

joelsteidl’s picture

I can confirm that this patch solved the issue of a View block with empty output. I will take a look at the approach and see if it can be improved.

droath’s picture

StatusFileSize
new2.19 KB

Upon further reflection on this issue, there are two potential solutions for using Entity Extra Fields to render Views. A developer can implement them using either the Views or the Block plugin within the Entity Extra Field module. I currently do not recommend supporting views being rendered in the block plugin when we have a Views plugin available. I've added some additional code to the Views Plugin within the Entity Extra Field module to address this issue. Please let me know if this addresses the issue you're personally experiencing.

droath’s picture

Status: Postponed (maintainer needs more info) » Needs review
coaston’s picture

Status: Needs review » Reviewed & tested by the community

Thank you, your patch works as expected with the latest 2.1.0-rc4 version. No issue found. I think it is ready to be merged.

anybody’s picture

Status: Reviewed & tested by the community » Needs work

@coaston thanks for the RTBC. Could you please create a MR from the patch? Thanks!

divyansh.gupta’s picture

Working on it!!

divyansh.gupta’s picture

Status: Needs work » Needs review

Converted the patch to MR,
please review!!

anybody’s picture

Thanks for the MR. Back to NR, would be nice if someone here could test it (or even better add a test)!

Code-wise this LGTM, still there might be an impact on performance, as each view is now executed twice? Do we have a way to improve that or are there better ways?
Otherwise I'd be fine to merge this after review. Thank you! :)

anybody’s picture

Assigned: Unassigned » grevil
grevil’s picture

Code-wise this LGTM, still there might be an impact on performance, as each view is now executed twice

Yes, this is indeed a problem, but I am unsure how to fix it. As @starlight-sparkle in #3 correctly says, that this is probably the only way to check whether the output is really empty or not.

I thought about caching, but I don't think that is a good option in this context...

grevil’s picture

Assigned: grevil » Unassigned
Status: Needs review » Reviewed & tested by the community

I guess this is the best approach. Code LGTM!

grevil’s picture

Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, please review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, please credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.