A use case for this is in #1808542: Allow area handlers to override the page title when used in an empty area.. Overriding the title only really makes sense to use as an empty area handler.

Think of how views_fetch_fields would fetch handlers for a base_table etc...

We could add this 'type' to the plugin definition or the views_data. If no type/types are specified, then any area handler should be assumed E.g. Text.

Questions

Where should we put the logic to filter this? Plugin manager level? after we have got the list of definitions or data?

CommentFileSizeAuthor
#17 views-1808670-17.patch4.04 KBdawehner
#15 views-1808670-15.patch4.04 KBdawehner
#14 views-1808670-14.patch4.05 KBdawehner
#9 views-1808670-8.patch4.07 KBdawehner
#7 1808670-7.patch2.23 KBdamiankloip
#5 1808670-5.patch2.93 KBdamiankloip
#3 1808670-3.patch3.74 KBdamiankloip
#2 1808670-2-array.patch3.95 KBdamiankloip
#1 views-1808670-1.patch3.25 KBdawehner

Comments

dawehner’s picture

dawehner
Primary language German
commented
Status: Active » Needs review
StatusFileSize
new views-1808670-1.patch3.25 KB

What about something like this?

damiankloip’s picture

damiankloip commented
StatusFileSize
new 1808670-2-array.patch3.95 KB

Yes something like this is kind of what I was thinking. I think this is the right stage to filter available handlers. How about we make it work for arrays too? I also changed the docs slightly. Hope that's ok.

Also, we don't have any tests for views_fetch_fields I don't think? It would be a good idea, as it's quite important.

damiankloip’s picture

damiankloip commented
StatusFileSize
new 1808670-3.patch3.74 KB

@dawehner had a better idea of casting the value to an array and just checking that.

dawehner’s picture

dawehner
Primary language German
commented

Let's write a proper test coverage for this function #1809608: Write test coverage for views_fetch_fields

damiankloip’s picture

damiankloip commented
StatusFileSize
new 1808670-5.patch2.93 KB

tim suggested switching the $sub_type parameter to be last, let's do that.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Needs work 
+++ b/views_ui/admin.incundefined
@@ -1746,7 +1746,7 @@ function views_ui_config_item_form($form, &$form_state) {
+        $base_fields = views_fetch_fields($view->storage->base_table, $form_state['type'], $form_state['type'], $view->display_handler->useGroupBy());

Then let's switch it :)

damiankloip’s picture

damiankloip commented
Status: Needs work » Needs review
StatusFileSize
new 1808670-7.patch2.23 KB

Ahaha

dawehner’s picture

dawehner
Primary language German
commented

Here is a test based on #1809608: Write test coverage for views_fetch_fields

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new views-1808670-8.patch4.07 KB

.

Status: Needs review » Needs work

The last submitted patch, views-1808670-8.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented

Well sure, the other patch didn't got in yet.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
Issue tags: -VDC

#9: views-1808670-8.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +VDC

The last submitted patch, views-1808670-8.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
StatusFileSize
new views-1808670-14.patch4.05 KB

Just a rerole.

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new views-1808670-15.patch4.04 KB

Rerolled with a better assertion message.

damiankloip’s picture

damiankloip commented 
+++ b/lib/Drupal/views/Tests/ViewsDataTest.phpundefined
@@ -110,6 +126,16 @@ class ViewsDataTest extends ViewTestBase {
+      $expected_keys = array_walk($expected['header'], function(&$item) {
+        $item = "views_test_data.$item";

Should this be checking against the expected footer? $expected[$sub_type] instead?

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new views-1808670-17.patch4.04 KB

You are totally right here, let's see whether this test run works fine as well.

damiankloip’s picture

damiankloip commented
Status: Needs review » Reviewed & tested by the community

That's better :)

dawehner’s picture

dawehner
Primary language German
commented
Status: Reviewed & tested by the community » Fixed

Great, committed and pushed

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

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Updated issue summary.