I've got some troubles when use query aggregation.

First of all, there is notice message:

Notice: Undefined property: view::$dom_id в функции template_preprocess_views_view() (строка 116 в файле /usr/local/www/*****.ru/www/sites/all/modules/views/theme/theme.inc).

And the second - I use hook_views_pre_render(&$view) to override header area depends on current contextual filter argument. But there is no argument at all. =(

Files: 
CommentFileSizeAuthor
#6 1791304-6.patch580 bytesdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 59 pass(es).
[ View ]
#4 0001-Issue-1791304-Call-pre_execute-before-render-when-qu.patch845 bytesmarvil07
PASSED: [[SimpleTest]]: [MySQL] 59 pass(es).
[ View ]
#4 0001-Issue-1791304-Set-is_attachment-to-avoid-dom_id-on-t.patch953 bytesmarvil07
PASSED: [[SimpleTest]]: [MySQL] 59 pass(es).
[ View ]
#2 1791304-2-field_view_adding_arg_and_dom_id.patch810 bytesholdmann
PASSED: [[SimpleTest]]: [MySQL] 59 pass(es).
[ View ]
#1 1791304-1-field_view_adding_arg_and_dom_id.patch783 bytesholdmann
PASSED: [[SimpleTest]]: [MySQL] 59 pass(es).
[ View ]
field_view_field_screen.png89.28 KBholdmann

Comments

Status:Active» Needs review
StatusFileSize
new783 bytes
PASSED: [[SimpleTest]]: [MySQL] 59 pass(es).
[ View ]

Here some patch

StatusFileSize
new810 bytes
PASSED: [[SimpleTest]]: [MySQL] 59 pass(es).
[ View ]

Oops, some troubles with spaces and tabs

I'm getting the same notice.

This seems to be a related issue: #1556174: $dom_id issues with upgrade

I'm currently investigating the module if there should be a $child_view->pre_execute() implemented, as dawehner stated in the mentioned issue. Simply adding the dom_id might not be enough.

Assigned:holdmann» Unassigned
StatusFileSize
new953 bytes
PASSED: [[SimpleTest]]: [MySQL] 59 pass(es).
[ View ]
new845 bytes
PASSED: [[SimpleTest]]: [MySQL] 59 pass(es).
[ View ]

I am not sure what is the right solution here, so I propose several options to let maintainers, which are more familiar with the module decide. Notice all make the warning disappear.

Run $child_view->pre_execute()

This sounds like the more rational idea, since views maintainers mention on #1556174-10: $dom_id issues with upgrade that we should do that. Basically that method set dom_id in the right way.

Not sure yet what exactly to pass as argument, but it should not matter since we are not executing the child view when aggregation is used (is executed on pre_render() instead).

The patch from comment 2

+++ b/views_field_view_handler_field_view.inc
@@ -426,6 +426,9 @@ class views_field_view_handler_field_view extends views_handler_field {
             $child_view->result = $results;
+            // Set agrument and dom_id related to current view
+            $child_view->args = array($values->{$this->view->base_field});
  • args here is assumed as one value, but we can pass several values as arguments to the child view, so this is probably wrong.
  • dom_id data member here is set directly, views let set it via hook_views_pre_view(), so that seems to be wrong too.

Set $child_view->is_attachment to TRUE

The mentioned warning happens on views theme/theme.inc when $view->is_attachment is empty, which is originally used to prevent attached views to show a dom_id, which they do not have.
This is hack I guess, but setting it to TRUE by pass that template variable assignment, so it also seems to fix the problem.

After a quick review Daniel mentioned if we should use preview() method.
I think that method should not be used in that portion of views_field_view. render() method is used directly instead, so calling preview() does not make sense on the aggregate query option of views_field_view, which tries to avoid executing the query many times, and executes it once on pre_execute().

StatusFileSize
new580 bytes
PASSED: [[SimpleTest]]: [MySQL] 59 pass(es).
[ View ]

Well, yeah. preview will pre_render the view too. So Daniel wanted to just do something like this?

@damiankloip: Yes, and as mentioned, that doe snot make sense in the context of the vies_field_view aggregation option, so I am each time more convinced that the right solution is to run pre_execute() as the first patch on comment 4.

Version:7.x-1.0» 8.x-1.x-dev
Status:Needs review» Patch (to be ported)

Awesome!