This will not just give us the ability to create JSON callbacks really easily but also for autocomplete paths etc.. too. I know this could be handled in Contrib but I think this would be really nice if we have this in views core.

Here is an initial patch that add the display plugin, and some basic tests.

I would like to add some options around the actual data that is provided in the JSON, first things first.

Files: 
CommentFileSizeAuthor
#101 1819760-101.patch28.92 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 50,754 pass(es).
[ View ]
#101 interdiff.txt11.7 KBdamiankloip
#98 1819760-98.patch27.74 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 49,905 pass(es).
[ View ]
#98 interdiff.txt8.28 KBdamiankloip
#97 1819760-97.patch27.86 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 49,807 pass(es).
[ View ]
#97 interdiff.txt15.93 KBdamiankloip
#92 1819760-92.patch25.48 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 50,394 pass(es).
[ View ]
#90 interdiff.txt2 KBdamiankloip
#89 1819760-89.patch25.48 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 50,442 pass(es).
[ View ]
#87 drupal-1819760-87.patch25.44 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 50,270 pass(es).
[ View ]
#87 interdiff.txt10.74 KBdamiankloip
#83 drupal-1819760-83.patch25.19 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 49,723 pass(es).
[ View ]
#81 drupal-1819760-81.patch25.17 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 49,910 pass(es), 14 fail(s), and 5 exception(s).
[ View ]
#81 interdiff.txt1.96 KBdamiankloip
#77 drupal-1819760-77.patch25 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 49,912 pass(es), 18 fail(s), and 36 exception(s).
[ View ]
#77 interdiff.txt1.26 KBdamiankloip
#75 drupal-1819760-75.patch24.91 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 49,736 pass(es).
[ View ]
#75 interdiff.txt5.13 KBdawehner
#73 1819760-72.patch22.1 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 49,787 pass(es).
[ View ]
#73 interdiff.txt3.72 KBdamiankloip
#64 interdiff.txt3.32 KBdamiankloip
#63 1819760-63.patch21.71 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 49,695 pass(es), 24 fail(s), and 3 exception(s).
[ View ]
#60 1819760-60.patch20.92 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 48,825 pass(es), 6 fail(s), and 0 exception(s).
[ View ]
#60 interdiff.txt1.23 KBdamiankloip
#59 drupal-1819760-58.patch20.9 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 48,764 pass(es), 7 fail(s), and 0 exception(s).
[ View ]
#59 interdiff.txt3.81 KBdawehner
#56 1819760-56.patch21.83 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 48,154 pass(es), 6 fail(s), and 0 exception(s).
[ View ]
#55 1819760-55.patch21.83 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 48,157 pass(es), 4 fail(s), and 0 exception(s).
[ View ]
#55 interdiff.txt3.53 KBdamiankloip
#51 1819760-51.patch21.45 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 48,037 pass(es), 6 fail(s), and 0 exception(s).
[ View ]
#49 drupal-1819760-49.patch20.9 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 48,063 pass(es), 6 fail(s), and 0 exception(s).
[ View ]
#46 1819760-46.patch21.26 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 48,073 pass(es), 6 fail(s), and 12 exception(s).
[ View ]
#45 1819760-45.patch21.77 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 48,074 pass(es), 6 fail(s), and 12 exception(s).
[ View ]
#39 1819760-39.patch18.82 KBlinclark
FAILED: [[SimpleTest]]: [MySQL] 47,628 pass(es), 6 fail(s), and 0 exception(s).
[ View ]
#39 interdiff.txt4.8 KBlinclark
#37 drupal-1819760-37.patch21.62 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 47,699 pass(es).
[ View ]
#37 interdiff.txt1.6 KBdawehner
#30 interdiff.txt849 bytesdawehner
#30 drupal-1819760-30.patch21.28 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 47,684 pass(es), 7 fail(s), and 10 exception(s).
[ View ]
#29 1819760-29.patch17.24 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 47,628 pass(es).
[ View ]
#27 drupal-1819760-27.patch17.02 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 46,955 pass(es).
[ View ]
#24 1819760-24.patch17.11 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 46,875 pass(es).
[ View ]
#21 1819760-21.patch16.96 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 46,470 pass(es).
[ View ]
#19 1819760-19.patch16.93 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 46,489 pass(es).
[ View ]
#16 1819760-16.patch13.5 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 46,476 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#14 drupal-1819760-14.patch12.64 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 46,445 pass(es).
[ View ]
#13 durpal-1819760-13.patch12.52 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 46,417 pass(es), 1 fail(s), and 1 exception(s).
[ View ]
#11 1819760-11.patch11.76 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 46,469 pass(es), 0 fail(s), and 10 exception(s).
[ View ]
#10 1819760-10.patch13.73 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 44,388 pass(es), 102 fail(s), and 8 exception(s).
[ View ]
#9 1819760-9.patch9.05 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 46,395 pass(es), 2 fail(s), and 1 exception(s).
[ View ]
#6 1819760-6.patch11.26 KBfastangel
PASSED: [[SimpleTest]]: [MySQL] 46,049 pass(es).
[ View ]
#4 1819760-4.patch11.09 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 3,188 pass(es).
[ View ]
#1 1819760-1.patch10.75 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 3,187 pass(es).
[ View ]
views-json-display.patch11.3 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 3,187 pass(es).
[ View ]

Comments

damiankloip’s picture

StatusFileSize
new10.75 KB
PASSED: [[SimpleTest]]: [MySQL] 3,187 pass(es).
[ View ]

Changed tests to use views_test_data.

dawehner’s picture

+++ b/lib/Drupal/views/Plugin/views/display/Json.phpundefined
@@ -0,0 +1,239 @@
+  public function executeHookMenu($callbacks) {

Is there a reason not to extend page, this is quite a lot of duplicated code ... There is no problem with doing that, for example Feed is doing this as well.

+++ b/lib/Drupal/views/Plugin/views/display/Json.phpundefined
@@ -0,0 +1,239 @@
+    return new JsonResponse($this->view->result);

The one problem we have with this approach is that flapi fields aren't part of the actual result, but _entity will be, so it could be that this value is quite polluted with unwanted stuff.

+++ b/lib/Drupal/views/Tests/Plugin/DisplayJsonTest.phpundefined
@@ -0,0 +1,57 @@
+    $actual_json = $this->drupalGet('test/json');
+    $this->assertResponse(200);

Can we actually test for the mimetype of the http result?

damiankloip’s picture

Thanks!

I will look at extending Page instead. I think I was just worried about all the menu code at the time. I see your point though, the hook menu code is being duplicated currently. (from IRC): Maybe a new base class that this plugin, Page, and Feed all extend.

Yes, at the moment we are just going to get the raw data contained in $view->result. So indeed, field data would only be available in _entity. I was thinking maybe a follow up issue to deal with the actual data that is being served. We can work on that, maybe have options available for this too.

Looking into this one too.

damiankloip’s picture

StatusFileSize
new11.09 KB
PASSED: [[SimpleTest]]: [MySQL] 3,188 pass(es).
[ View ]

This just adds a test for the content-type in the header and a usesBreadcrumb method.

xjm’s picture

Project:Views» Drupal core
Version:8.x-3.x-dev» 8.x-dev
Component:Code» views.module
Status:Needs review» Needs work
Issue tags:+Needs reroll

Neat patch. Will need a reroll against core.

fastangel’s picture

Status:Needs work» Needs review
StatusFileSize
new11.26 KB
PASSED: [[SimpleTest]]: [MySQL] 46,049 pass(es).
[ View ]

Attached one patch with reroll.

nod_’s picture

Issue tags:+JavaScript

tag since related

damiankloip’s picture

Assigned:Unassigned» damiankloip
Status:Needs review» Postponed

Postponing for now on #1821654: Refactor Page based display plugin classes. This will make this patch make alot more sense.

damiankloip’s picture

Status:Postponed» Active
StatusFileSize
new9.05 KB
FAILED: [[SimpleTest]]: [MySQL] 46,395 pass(es), 2 fail(s), and 1 exception(s).
[ View ]

Just what I have so far.

damiankloip’s picture

Status:Active» Needs review
Issue tags:-Needs reroll
StatusFileSize
new13.73 KB
FAILED: [[SimpleTest]]: [MySQL] 44,388 pass(es), 102 fail(s), and 8 exception(s).
[ View ]

Here is an updated patch. Now uses a Data display type and data row types, so the style plugin varies to render the data format.

Need to change the current tests and add more coverage.

damiankloip’s picture

Status:Needs review» Active
StatusFileSize
new11.76 KB
FAILED: [[SimpleTest]]: [MySQL] 46,469 pass(es), 0 fail(s), and 10 exception(s).
[ View ]

Cleaned up lots of methods we don't need.

dawehner’s picture

Status:Active» Needs review
+++ b/core/modules/system/lib/Drupal/system/Plugin/views/display/Data.phpundefined
@@ -0,0 +1,169 @@
+    $options['style'] = array(
+      'contains' => array(
+        'type' => array('default' => 'json'),
+        'options' => array('default' => array()),
+      ),
+    );
+    $options['row'] = array(
+      'contains' => array(
+        'type' => array('default' => 'data_entity'),
+        'options' => array('default' => array()),
+      ),
+    );
...
+dpm($options);

What about just using $options['style]['contains']['type']['default'] = 'json'; This would look much simpler.

+++ b/core/modules/system/lib/Drupal/system/Plugin/views/display/Data.phpundefined
@@ -0,0 +1,169 @@
+    unset($options['hide_admin_links'], $options['analyze-theme']);

Maybe we should document that.

dawehner’s picture

StatusFileSize
new12.52 KB
FAILED: [[SimpleTest]]: [MySQL] 46,417 pass(es), 1 fail(s), and 1 exception(s).
[ View ]

We went through the patch together and improved it.

dawehner’s picture

StatusFileSize
new12.64 KB
PASSED: [[SimpleTest]]: [MySQL] 46,445 pass(es).
[ View ]

Remove the group form from the style settings.

tim.plunkett’s picture

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/Data.phpundefined
@@ -0,0 +1,129 @@
+ * Definition of Drupal\views\Plugin\views\display\Data.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/row/DataEntityRow.phpundefined
@@ -0,0 +1,41 @@
+ * Definition of Drupal\views\Plugin\views\row\DataEntityRow.

+++ b/core/modules/views/lib/Drupal/views/Tests/Plugin/DisplayJsonTest.phpundefined
@@ -0,0 +1,68 @@
+ * Definition of Drupal\views\Tests\Plugin\DisplayJsonTest.

Should be Contains

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/row/DataFieldRow.phpundefined
@@ -0,0 +1,52 @@
+  public function render($row) {
+    foreach ($this->view->field as $id => $field) {
+      $output[$field->field_alias] = $row->{$field->field_alias};
+    }
+
+    return $output;

This should initialize $output just in case.

damiankloip’s picture

StatusFileSize
new13.5 KB
FAILED: [[SimpleTest]]: [MySQL] 46,476 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Thanks, made those changes, and a few others. Also added some basic functionality to change the field keys in the output data when using field rows.

tim.plunkett’s picture

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/row/DataFieldRow.phpundefined
@@ -0,0 +1,81 @@
+  public function getFieldKeyAlias($id) {

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/style/Json.phpundefined
@@ -0,0 +1,73 @@
+  public function init(ViewExecutable $view, &$display, $options = NULL) {

I didn't notice earlier, but this is missing a docblock.

This looks really cool overall.

Status:Needs review» Needs work

The last submitted patch, 1819760-16.patch, failed testing.

damiankloip’s picture

Status:Needs work» Needs review
StatusFileSize
new16.93 KB
PASSED: [[SimpleTest]]: [MySQL] 46,489 pass(es).
[ View ]

Thanks, I added a doc for that function, that was the patch I tried to post just before we left the Pantheon office :)

Added some more tests.

dawehner’s picture

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/row/DataFieldRow.phpundefined
@@ -0,0 +1,90 @@
+  public function buildOptionsForm(&$form, &$form_state) {

Just for consistency it should call parent::

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/row/DataFieldRow.phpundefined
@@ -0,0 +1,90 @@
+      $options = $this->options;

Nitpick alarm: Why not use $this->options directly? It's just used in two places so this should be fine.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/row/DataFieldRow.phpundefined
@@ -0,0 +1,90 @@
+      $output[$this->getFieldKeyAlias($id)] = $row->{$field->field_alias};

I hope it is okay to call a function per field and per row. In theory this mapping could be precalculated but yeah that's not so important.

+++ b/core/modules/views/lib/Drupal/views/Tests/Plugin/DisplayJsonTest.phpundefined
@@ -0,0 +1,125 @@
+class DisplayJsonTest extends PluginTestBase {

It seems to be for me that it might make sense to split up the tests for the displayJson test from testing for example the fields row plugin. What do you think about that? We could though still reuse the test view.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/row/DataFieldRow.phpundefined
@@ -0,0 +1,90 @@
+  public function buildOptionsForm(&$form, &$form_state) {

Just for consistency it should call parent::

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/row/DataFieldRow.phpundefined
@@ -0,0 +1,90 @@
+      $options = $this->options;

Nitpick alarm: Why not use $this->options directly? It's just used in two places so this should be fine.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/row/DataFieldRow.phpundefined
@@ -0,0 +1,90 @@
+      $output[$this->getFieldKeyAlias($id)] = $row->{$field->field_alias};

I hope it is okay to call a function per field and per row. In theory this mapping could be precalculated but yeah that's not so important.

damiankloip’s picture

StatusFileSize
new16.96 KB
PASSED: [[SimpleTest]]: [MySQL] 46,470 pass(es).
[ View ]

Here are those small changes. I'm definitely not against moving the tests but I'm not sure what to do with them at the moment as they all ind of indirectly test the Data display plugin I guess.

Status:Needs review» Needs work
Issue tags:-JavaScript, -VDC

The last submitted patch, 1819760-21.patch, failed testing.

damiankloip’s picture

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

#21: 1819760-21.patch queued for re-testing.

damiankloip’s picture

StatusFileSize
new17.11 KB
PASSED: [[SimpleTest]]: [MySQL] 46,875 pass(es).
[ View ]

Added a $usesAreas property based on #1829424: Allow area plugins to be disabled for displays.

Status:Needs review» Needs work
Issue tags:-JavaScript, -VDC

The last submitted patch, 1819760-24.patch, failed testing.

dawehner’s picture

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

#24: 1819760-24.patch queued for re-testing.

dawehner’s picture

Status:Needs review» Reviewed & tested by the community
StatusFileSize
new17.02 KB
PASSED: [[SimpleTest]]: [MySQL] 46,955 pass(es).
[ View ]
+++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/Data.phpundefined
@@ -0,0 +1,136 @@
+   *
+   * @var bool
...
+   *
+   * @var bool
...
+   *
+   * @var bool
...
+   *
+   * @var bool

It's a bit nitpicking but i don't think it makes sense to document the type here, as it is already part of the parent.

webchick’s picture

Status:Reviewed & tested by the community» Needs work

YAY! This looks AWESOME!

Default the style plugin to JSON on a "Data" display. Right now it keeps the old default (?) or at any rate the radio is not auto-selected.

When submitting the style form to make it JSON, then I received a blank confirm form.

And after submitting the form, saw:

Notice: Undefined index: style_options in Drupal\views\Plugin\views\display\DisplayPluginBase->submitOptionsForm() (line 2286 ofcore/modules/views/lib/Drupal/views/Plugin/views/display/DisplayPluginBase.php).

However! After that, it actually looks to indeed be outputting JSON. :D

Also, let's make sure to get linclark's sign-off on this, and make sure this isn't going to harm the JSON-LD work.

damiankloip’s picture

Status:Needs work» Needs review
StatusFileSize
new17.24 KB
PASSED: [[SimpleTest]]: [MySQL] 47,628 pass(es).
[ View ]

Thanks for testing!

I removed the empty options form for the style plugin. I think the defaulting of actual plugins is a more widespread issue, because the plugin is inherited, so we get this initial (wrong) default.

I think this should maybe be a separate follow up, as it affects plugins generally?

dawehner’s picture

StatusFileSize
new21.28 KB
FAILED: [[SimpleTest]]: [MySQL] 47,684 pass(es), 7 fail(s), and 10 exception(s).
[ View ]
new849 bytes

Fixed the UX issue that you don't have a proper style/row style selected by default.

damiankloip’s picture

ok, that looks good to me.

linclark’s picture

I talked to damian about this last night and timplunkett about it this morning. This seems to overlap with the work that klausi and I plan to do with REST/serialization. Tim termed it a poormans JSON response system.

My understanding is: It is possible to create a View of entities, which use entity display (view modes) instead of field display. Currently, users can add a Page display, which has a Path setting. The path will respond with an HTML representation of that collection of entities. This patch enables the creation of a separate path which responds with a JSON version of a view. When someone requested that path, a collection of JSON objects would be returned even if the Accept header is "text/html".

Overlap with REST

Part of the intended scope of the REST work is to respond to the original Views path with JSON if the request's Accept header is "application/json". This means that the definition of a separate path should be unnecessary. Klaus is dealing with the routing part of REST, so he can provide more detail. However, I feel pretty safe in saying that the work enabling this hasn't yet been started.

Overlap with serialization

Since the plugin is coupled to the format, it directly creates a JsonResponse object. It would be possible to decouple this and make it so that the data plugin could respond to any format if it used the Serializer service instead. This would depend on #1814864: Provide way to register serialization classes being committed and also require creating a normalizer that creates the expected JSON array structure (which shouldn't be too hard).

Recommendation

I expect a plugin will eventually be unnecessary if the REST work is completed. However, we may want to put this in as a stopgap and re-evaluate once Klaus is further with REST routing. Since Serializer is pretty close to getting committed, we probably do want to move towards using that instead of coupling the plugin to a format, though.

Status:Needs review» Needs work

The last submitted patch, drupal-1819760-30.patch, failed testing.

damiankloip’s picture

Tim termed it a poormans JSON response system.

This is true I guess :)

Since the plugin is coupled to the format, it directly creates a JsonResponse object.

That sounds like an awesome idea! I will check that out. If we could make it even more generic, I'm happy.

This means that the definition of a separate path should be unnecessary

I'm not sure about this; as we are dealing with raw row data (and we have to) for the row plugins, rather than anything rendered, So I think we would maybe have a need to use separate views. Otherwise you will get alot of junk in your output :) Also, we need to make this work with fields too, so a similar principle applies there.

However, I feel pretty safe in saying that the work enabling this hasn't yet been started.

Is this going to get done before feature freeze? Or is this something that is not deemed a feature? If it is not likely to be in this release, I will not go and research it.

linclark’s picture

I'm testing the patch in #29 and getting the following result:

["\u003Cdiv id=\u0022node-1\u0022 class=\u0022node node-article promoted view-mode-full contextual-region clearfix\u0022 role=\u0022article\u0022 about=\u0022\/node\/1\u0022 typeof=\u0022sioc:Item foaf:Document\u0022\u003E\n\n        \u003Ch2 class=\u0022\u0022 property=\u0022dc:title\u0022 datatype=\u0022\u0022\u003E\n      \u003Ca href=\u0022\/node\/1\u0022\u003EThis is title\u003C\/a\u003E\n    \u003C\/h2\u003E\n    \u003Cdiv class=\u0022contextual\u0022\u003E\u003Cul class=\u0022contextual-links\u0022\u003E\u003Cli class=\u0022node-edit odd first\u0022\u003E\u003Ca href=\u0022\/node\/1\/edit?destination=test-json\u0022\u003EEdit\u003C\/a\u003E\u003C\/li\u003E\u003Cli class=\u0022node-delete even last\u0022\u003E\u003Ca href=\u0022\/node\/1\/delete?destination=test-json\u0022\u003EDelete\u003C\/a\u003E\u003C\/li\u003E\u003C\/ul\u003E\u003C\/div\u003E\n      \u003Cdiv class=\u0022meta submitted\u0022\u003E\n            \u003Cspan property=\u0022dc:date dc:created\u0022 content=\u00222012-11-04T13:19:52-08:00\u0022 datatype=\u0022xsd:dateTime\u0022 rel=\u0022sioc:has_creator\u0022\u003ESubmitted by \u003Ca href=\u0022\/user\/1\u0022 rel=\u0022author\u0022 title=\u0022View user profile.\u0022 class=\u0022username\u0022 lang=\u0022\u0022 about=\u0022\/user\/1\u0022 typeof=\u0022sioc:UserAccount\u0022 property=\u0022foaf:name\u0022\u003Eadmin\u003C\/a\u003E on Sun, 11\/04\/2012 - 13:19\u003C\/span\u003E    \u003C\/div\u003E\n  \n  \u003Cdiv class=\u0022content clearfix\u0022 class=\u0022\u0022\u003E\n    \u003Cdiv  class=\u0022field field-name-body field-type-text-with-summary field-label-hidden\u0022\u003E\u003Cdiv class=\u0022field-items\u0022\u003E\u003Cdiv class=\u0022field-item even\u0022 property=\u0022content:encoded\u0022\u003E\u003Cp\u003EBody field\u003C\/p\u003E\n\u003C\/div\u003E\u003C\/div\u003E\u003C\/div\u003E\u003Cdiv  class=\u0022field field-name-field-tags field-type-taxonomy-term-reference field-label-above clearfix\u0022\u003E\u003Ch3 class=\u0022field-label\u0022\u003ETags: \u003C\/h3\u003E\u003Cul class=\u0022links\u0022\u003E\u003Cli class=\u0022taxonomy-term-reference-0\u0022 rel=\u0022dc:subject\u0022\u003E\u003Ca href=\u0022\/taxonomy\/term\/1\u0022 typeof=\u0022skos:Concept\u0022 property=\u0022rdfs:label skos:prefLabel\u0022\u003Ehere\u0026#039;s a tag\u003C\/a\u003E\u003C\/li\u003E\u003C\/ul\u003E\u003C\/div\u003E  \u003C\/div\u003E\n\n  \n  \n\u003C\/div\u003E\n"]

Is this the expected result?

damiankloip’s picture

In a word, no :) That is because we had an issue with the default row plugin, this should be fixed in dawehner's failing test in #30. Otherwise, if you are using the patch in #29, just save the row form.

dawehner’s picture

Title:Add a JSON display plugin» Add a Data display plugin and a json style plugin
Status:Needs work» Needs review
StatusFileSize
new1.6 KB
new21.62 KB
PASSED: [[SimpleTest]]: [MySQL] 47,699 pass(es).
[ View ]

The test configuration have to be updated as well.

Overlap with REST

It is a great idea to respond different depending on the request type. I'm not really sure about the connection
between the configuration of a view and a json result. Do you want, for example, always the full entities or maybe just certain fields (like the patch is providing). I'm also not sure, how to handle aggregated data, but let's discuss that.

I'm not sure about this; as we are dealing with raw row data (and we have to) for the row plugins, rather than anything rendered, So I think we would maybe have a need to use separate views. Otherwise you will get alot of junk in your output :) Also, we need to make this work with fields too, so a similar principle applies there.

Maybe we could provide a sane default but provide a way to override it, if there is more custom handling needed.

Overlap with serialization
Since the plugin is coupled to the format, it directly creates a JsonResponse object. It would be possible to decouple this and make it so that the data plugin could respond to any format if it used the Serializer service instead. This would depend on #1814864: Provide way to register serialization classes being committed and also require creating a normalizer that creates the expected JSON array structure (which shouldn't be too hard).

So in fact the patch currently provides a ultra-light serializer (not sure about the terminology)
which allows to output an array structure (provided by the row definition (for example the full entity or just certain fields))
with a certain format (currently there is just one provided for json but more could be added). The title of the issue got updated
to make it a bit clearer.

Recommendation
I expect a plugin will eventually be unnecessary if the REST work is completed. However, we may want to put this in as a stopgap and re-evaluate once Klaus is further with REST routing. Since Serializer is pretty close to getting committed, we probably do want to move towards using that instead of coupling the plugin to a format, though.

This makes totally sense, replace json with serialized will automatically work.

damiankloip’s picture

@@ -249,8 +249,8 @@ public function attachTo($display_id) {
     $view->setArguments($args);
     $view->setDisplay($this->display['id']);
     if ($this->getOption('inherit_pager')) {
-      $view->display_handler->usesPager = $this->view->display[$display_id]->handler->usesPager();
-      $view->display_handler->setOption('pager', $this->view->display[$display_id]->handler->getOption('pager'));
+      $view->display_handler->usesPager = $this->view->displayHandlers[$display_id]->usesPager();
+      $view->display_handler->setOption('pager', $this->view->displayHandlers[$display_id]->getOption('pager'));

I think we've got some stuff form another patch in there :)

linclark’s picture

StatusFileSize
new4.8 KB
new18.82 KB
FAILED: [[SimpleTest]]: [MySQL] 47,628 pass(es), 6 fail(s), and 0 exception(s).
[ View ]

I've got the patch working with Serializer, just as a proof of concept. It depends on the patch in #1814864: Provide way to register serialization classes.

Since everything is handled using the serializer service, the Json specific serializer in the patch is no longer necessary. I've only kept it in there because having a Style plugin seems to be required.

Once we have content negotiation in place, the 'json' would no longer have to be hard coded, it would be a variable on the request.

The test is failing because it contains the following data. I don't know where it is coming from.

"\\u0000*\\u0000entityType":"node","\\u0000*\\u0000enforceIsNew":null,"\\u0000*\\u0000newRevision":false,

Status:Needs review» Needs work

The last submitted patch, 1819760-39.patch, failed testing.

Crell’s picture

Lin pointed me here. I think there's two separate and valid use cases here, which parallel what's done in HTML.

1) Entity-based. Each row is a full entity. For this case, the JSON-LD serialization work that Lin is doing should be the format that gets used. That neatly solves the entire "collection" listing problem in REST, because "The answer is Views, what's the question?" is still valid and correct. I don't think it needs any configurability beyond which serializer to use, and should be all tricked out and hypermedia-ified (possibly in follow-ups).

2) Field-based. Each row is some combination of fields, like a fielded view in HTML. This likely would not use JSON-LD at all, and would be just a one-off JSON structure that you'd have to code to directly. (That's what we used for the DrupalCon mobile app, actually, way back in late-model Drupal 6, with the Views Datasource module.) I'm OK with this one not being JSON-LD-ified-semantic-yummy given the complexity involved.

Note that while there will usually be a path overlap between different mime type views, that's not guaranteed. That means it will need to be possible to generate a URI that is only capable of outputting JSON-LD data, not HTML.

I haven't dug into the changes for Views in D8 yet, so I defer to those who have on actual implementation.

linclark’s picture

Just a note for those skimming through the issue, my patch doesn't include JSON-LD, but just Serializer integration. I agree that field-based views should not have a JSON-LD representation, but we can still use Serializer for it. I currently have no opinion on how paths/mim type handling should work.

ygerasimov’s picture

Wonderful idea! I would be very happy about this patch gets in as it will abandon services_views module.

We miss validation on the aliases to make sure we don't have same alias for two fields so in output they won't override each other.

damiankloip’s picture

@Crell; Totally, the patch already contains generic row plugins for entities and fields, to just return the raw data that we would need. So they can be used for any data type of plugin but yes, I agree with what you are saying, we should support both types, and hopefully anything else serializer supports that we can get for free, if anything (haven't looked into it a great deal)?

@ygerasimov, yes that is true, but I think we can actually do that as a follow up once we get the initial patch in. I'm not a fan of trying to get all the bells and whistles into one patch.

I'm leaving this assigned to me to work on the integration with serializer, I have done some initial testing, and this now seems to be working ok with Lin's serializer work from other patches. Will post a new patch soon....

damiankloip’s picture

StatusFileSize
new21.77 KB
FAILED: [[SimpleTest]]: [MySQL] 48,074 pass(es), 6 fail(s), and 12 exception(s).
[ View ]

How about something more like this (in theory)? I don't think this will actually work in its current form plus I think entities need to extend the (what is currently) entityNG class before this will work to be able to get properties for the getIterator() method?

The idea of this is to try and get the requested mime type from the request header and use that with the serializer to just return it in that format if it exists?

This also adds a fix to the views edit form controller, and some simple validation for duplicate field key aliases.

damiankloip’s picture

StatusFileSize
new21.26 KB
FAILED: [[SimpleTest]]: [MySQL] 48,073 pass(es), 6 fail(s), and 12 exception(s).
[ View ]

This patch instead.

dawehner’s picture

+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/ViewEditFormController.phpundefined
@@ -864,9 +866,9 @@ public function getFormBucket(ViewUI $view, $type, $display) {
-        'title' => $rearrange_text,
-        'href' => $rearrange_url,
...
+        'title' => t('Rearrange'),
+        'href' => "admin/structure/views/nojs/rearrange/{$view->get('name')}/{$display['id']}/$type",

Shouldn't we still have a different rearrange_text/url for filters?

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/style/Serialize.phpundefined
@@ -0,0 +1,102 @@
+    // @todo get the serializer formats properly. How??
+    $formats = array('json', 'jsonld');

Do we still need the selection, as it is not used at all with the serializer.

damiankloip’s picture

Yeah, we don't need that now, if my new proposal can work. Sorry, I didn't clean up the patch!

Doh. Yes, filters should have different labels. We currently have a problem with the switch statement there, as if it's 'field' but it doesn't use fields it will actually fall into the area switch conditions and use that.

dawehner’s picture

Status:Needs work» Needs review
StatusFileSize
new20.9 KB
FAILED: [[SimpleTest]]: [MySQL] 48,063 pass(es), 6 fail(s), and 0 exception(s).
[ View ]

PHP is so WTF. What do you think about the sort of cleaner fix in this patch?

  /**
+   * Overrides Drupal\views\Plugin\views\style\StylePluginBase::usesFields().
+   */
+  public function usesFields() {
+    if (!empty($this->row_plugin)) {
+      return $this->row_plugin->usesFields();
+    }

Any reason for an override of the logic? Basically the default usesFields method should work here as well. I see your problem. The style plugin should not define to use fields, when it just leverages row plugins.

This patch fixes some smaller things like a missing defineOptions entry, wrong variable names etc. or misnamed files for tests etc.
The interdiff is pretty useless, sorry.

There are remaining test failures, but it seems to be that first node has to be converted to entityNG

Status:Needs review» Needs work

The last submitted patch, drupal-1819760-49.patch, failed testing.

damiankloip’s picture

StatusFileSize
new21.45 KB
FAILED: [[SimpleTest]]: [MySQL] 48,037 pass(es), 6 fail(s), and 0 exception(s).
[ View ]

Yeah, we don't need the usesFields override now. I did have a reason, but now I think you're right, the default is fine! We do need to wait for entityNG conversions, it falls over when it calls getIterator on the entity; basically it needs the property api to return what we need here.

Updated the patch slightly, just thinking of other use cases and data style plugins; How about we add get/setContentType methods on the data plugin that can be used. Serializer will be dynamic but others in the future may want to set this themselves? So the getContentType method in this patch gets an overridden type, if it's set, otherwise returns the request Content-type.

EDIT: Forgot to say, your fix for the getFormBucket switch is good.

damiankloip’s picture

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, 1819760-51.patch, failed testing.

linclark’s picture

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/style/Serialize.phpundefined
@@ -0,0 +1,57 @@
+    return $serializer->serialize($rows, drupal_container()->get('request')->headers->get('Content-type'));

The second parameter should actually be a content type token, like the keys in Request::initializeFormats(). This can currently be retrieved using ContentNegotiation::getContentType, though that will be changing.

<code>
+++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/Data.phpundefined
@@ -0,0 +1,165 @@
+  public function getContentType() {
+    if (isset($this->contentType)) {
+      return $this->contentType;
+    }
+
+    // Return the content type from the request object.
+    return drupal_container()->get('request')->headers->get('Content-type');

The request shouldn't have a Content-type if it's a GET request. It would have an Accept header.

The best way to get the Content-type to use would probably be to pass the content type token I talked about above to Request::getMimeType().

damiankloip’s picture

Status:Needs work» Needs review
StatusFileSize
new3.53 KB
new21.83 KB
FAILED: [[SimpleTest]]: [MySQL] 48,157 pass(es), 4 fail(s), and 0 exception(s).
[ View ]

@linclark, I think this patch addresses your feedback and is hopefully what you meant! Do you have any ideas about how we can test this easily, without entityNG classes?

I know moshe wanted to test this out, this will probably still be quite difficult though I think :/ As we need the use of the property API for entities (getIterator() method mainly, I think).

Then, it would be good if we could provide support for jsonld, json, xml, all with serializer, out of the box :)

damiankloip’s picture

StatusFileSize
new21.83 KB
FAILED: [[SimpleTest]]: [MySQL] 48,154 pass(es), 6 fail(s), and 0 exception(s).
[ View ]

Sorry, this patch instead. Interdiff still applies, except for the commented out serialize line. ooops!

Status:Needs review» Needs work

The last submitted patch, 1819760-56.patch, failed testing.

damiankloip’s picture

dawehner’s picture

StatusFileSize
new3.81 KB
new20.9 KB
FAILED: [[SimpleTest]]: [MySQL] 48,764 pass(es), 7 fail(s), and 0 exception(s).
[ View ]
+++ b/core/modules/views/lib/Drupal/views/Plugin/views/style/Serialize.phpundefined
@@ -0,0 +1,62 @@
+    $serializer = drupal_container()->get('serializer');
+    $request = drupal_container()->get('request');
+    $negotiation = drupal_container()->get('content_negotiation');

Just realized that we don't want to get all this objects on each row but store it on the object.

+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/ViewEditFormController.phpundefined
@@ -817,8 +817,14 @@ public function getFormBucket(ViewUI $view, $type, $display) {
+    $rearrange_url = "admin/structure/views/nojs/rearrange/{$view->get('name')}/{$display['id']}/$type";
+    $rearrange_text = t('Rearrange');
+    $class = 'icon compact rearrange';

@@ -847,10 +854,7 @@ public function getFormBucket(ViewUI $view, $type, $display) {
-      default:
-        $rearrange_url = "admin/structure/views/nojs/rearrange/{$view->get('name')}/{$display['id']}/$type";
-        $rearrange_text = t('Rearrange');

This change is part of another patch so remove it here.

damiankloip’s picture

StatusFileSize
new1.23 KB
new20.92 KB
FAILED: [[SimpleTest]]: [MySQL] 48,825 pass(es), 6 fail(s), and 0 exception(s).
[ View ]

Oh yeah, That's a nice change. Looking good.

the only thing that popped out at me when reviewing now is the getContentType method. Maybe it should be more like this?

damiankloip’s picture

Title:Add a Data display plugin and a json style plugin» Add a Data display plugin and serializer integration
damiankloip’s picture

We can also use the work in #1850704: Available serialization formats to get the available serializer formats if we need to.

damiankloip’s picture

Status:Needs work» Needs review
StatusFileSize
new21.71 KB
FAILED: [[SimpleTest]]: [MySQL] 49,695 pass(es), 24 fail(s), and 3 exception(s).
[ View ]

Added a test to check that empty aliases aren't used (because they were!) so added an array_filter to the fieldAliases on the Field row handler. I have also changed the drupalGet assertions to drupalGetAJAX, can I do that?

I have been testing this with latest patch from #1846000: Add serializer support for JSON and AJAX and seems to work ok for field rows, obviously not yet with entities. Using Drupal's 'ajax' content type too, so we get previews of the data again.

damiankloip’s picture

StatusFileSize
new3.32 KB

And I guess an interdiff would be polite.

Status:Needs review» Needs work

The last submitted patch, 1819760-63.patch, failed testing.

yched’s picture

Might be of interest here: #1678572: Add support for a raw text formatter

In short : it's a request to add a "raw, unfiltered text" formatter in core for text fields, primary use case being views-based data exports.
We "won't fix"ed it on the grounds that:
- the use case is very non-80%, while offering this formatter on "Manage display" for every text field is a handing a loaded gun to all users
- D8 will have better options for data export (that was referring to JSON-LD entity serializtion, which doesn't rely on formatters)

My understanding is that this thread invalidates the second item ?
At any rate, feedback welcome over there :-)

moshe weitzman’s picture

seems to work ok for field rows, obviously not yet with entities

(from #63

Is there a blocker for integrating with entities? Do we think thats a follow-up?

dawehner’s picture

Is there a blocker for integrating with entities? Do we think thats a follow-up?

The problem is that entities would have to support entityNG to be able to test it. We could use the entity_test entity, though the patch to add views integration for that didn't got any review: #1839624: Add a views integration for entity_test.module

damiankloip’s picture

Status:Needs work» Needs review

#63: 1819760-63.patch queued for re-testing.

damiankloip’s picture

See how we get on anyway, We should just need #1846000: Add serializer support for JSON and AJAX now ideally.

damiankloip’s picture

Nice, that has been committed too. WOrking on this.

Status:Needs review» Needs work

The last submitted patch, 1819760-63.patch, failed testing.

damiankloip’s picture

Status:Needs work» Needs review
StatusFileSize
new3.72 KB
new22.1 KB
PASSED: [[SimpleTest]]: [MySQL] 49,787 pass(es).
[ View ]

Ok, I think this is working now (He says...).

Changes:

  • removed 'use Drupal\views\Tests\Plugin\PluginTestBase', we're in the same namespace
  • Added entity_test module to setUp property
  • Changed importing of views config files to new system
  • Changed the Content-type headers to Accept headers
  • Use serializer to generate expected output (Tested in the above dependency issue)
  • Updated yml view files for entity_test
moshe weitzman’s picture

Status:Needs review» Needs work

Thanks for quick reroll. I tested this a lot and have some UI feedback and a possible bug report.

To test, I enabled entity_test.module and created a few entity_test entities. Then I created a new View with a Data display and a path and then requested that path with Accept: application/json and Accept: application/ld+json headers.

  1. When requesting with Accept: application/ld+json, I get
    Fatal error: Call to a member function uri() on a non-object in /Users/mweitzman/htd/d8x/core/modules/jsonld/lib/Drupal/jsonld/JsonldEntityWrapper.php on line 68

    . Is there a different header I should be using? We should add this to the Test, IMO

  2. In Views UI, we should be able to simplify a bit:
    1. Remove Exposed Form
    2. Remove CSS Class
    3. The Show: item has a choice of Entity or Fields. When using Fields, I get no way to add/remove Fields. This may be a limitation of the entity_test views integration?
  3. Once Views rendering is cleaned up, we should remove Header/Footer/Pager as well since they are not applicable to Data Display.
dawehner’s picture

Status:Needs work» Needs review
StatusFileSize
new5.13 KB
new24.91 KB
PASSED: [[SimpleTest]]: [MySQL] 49,736 pass(es).
[ View ]

Thanks for testing, highly welcomed!

The Show: item has a choice of Entity or Fields. When using Fields, I get no way to add/remove Fields. This may be a limitation of the entity_test views integration?

For some odd reason the fixes in #49 and #51 didn't went into the actual current patch. (patched that in again)
This automatically also hides the header/footer/empty text.

Remove Exposed Form
Remove CSS Class

Yeah I totally agree that they don't make sense here! Removed them.

Fatal error: Call to a member function uri() on a non-object in /Users/mweitzman/htd/d8x/core/modules/jsonld/lib/Drupal/jsonld/JsonldEntityWrapper.php on line 68

No idea how this could have happened. If I add a new request in the test it for sure returns the expected data.

moshe weitzman’s picture

Status:Needs review» Reviewed & tested by the community

Thanks for making those changes and for adding testing for JSONLD. We're RTBC now.

damiankloip’s picture

StatusFileSize
new1.26 KB
new25 KB
FAILED: [[SimpleTest]]: [MySQL] 49,912 pass(es), 18 fail(s), and 36 exception(s).
[ View ]

Yep, just tested this, and looks good.

I've just quickly rerolled to remove the exposed block settings too, as they aren't that useful here either.

damiankloip’s picture

Assigned:damiankloip» Unassigned

Unassigning

tim.plunkett’s picture

Issue tags:-JavaScript, -VDC

#77: drupal-1819760-77.patch queued for re-testing.

Status:Reviewed & tested by the community» Needs work
Issue tags:+JavaScript, +VDC

The last submitted patch, drupal-1819760-77.patch, failed testing.

damiankloip’s picture

Status:Needs work» Needs review
StatusFileSize
new1.96 KB
new25.17 KB
FAILED: [[SimpleTest]]: [MySQL] 49,910 pass(es), 14 fail(s), and 5 exception(s).
[ View ]

Let's see how we get on with the fixed init() method.

Status:Needs review» Needs work

The last submitted patch, drupal-1819760-81.patch, failed testing.

damiankloip’s picture

StatusFileSize
new25.19 KB
PASSED: [[SimpleTest]]: [MySQL] 49,723 pass(es).
[ View ]

That's a strange error, but I remember the issue. The patch is still using the config directory, we are using test_views directory now. This should be fine now, and ready to go again.

damiankloip’s picture

Status:Needs work» Needs review
moshe weitzman’s picture

Status:Needs review» Reviewed & tested by the community

Verified to still work. Reminder that this only works for entityNG entities. For now, you have to enable entity_test module and create using entity-test/add in order to see this in action.

ygerasimov’s picture

Status:Reviewed & tested by the community» Needs work

I have spotted some code duplication.

We have defining $content_type in both display and row plugins.

Display plugin:

+  public function getContentType() {
+    if (!isset($this->contentType)) {
+      // Return the content type based on the request object.
+      $negotiation = drupal_container()->get('content_negotiation');
+      $request = drupal_container()->get('request');
+      $content_type = $negotiation->getContentType($request);
+
+      $this->setContentType($request->getMimeType($content_type));
+    }
+
+    return $this->contentType;
+  }

Row plugin:

+    $content_type = $this->negotiation->getContentType($this->request);
+    $this->displayHandler->setContentType($this->request->getMimeType($content_type));

Can't we in row plugin just call $content_type = $this->displayHandler->getContentType();? Then we would not need protected $request and $negotiation variables in row plugin and initializing them in init() method.

damiankloip’s picture

Status:Needs work» Needs review
Issue tags:-VDC
StatusFileSize
new10.74 KB
new25.44 KB
PASSED: [[SimpleTest]]: [MySQL] 50,270 pass(es).
[ View ]

That's a good point, and it can be optimised. Unfortunately, it's not quite as simple as stated above :) as we need both the symfony machine name for the content type and the actual mime type for the response. I think you mean style plugin and not row plugin maybe aswell?

Anyway, let's shake things up a bit.

I have moved the calls to the container to the initDisplay method, and added get/setters for both content and mime types. Also changed a couple of other things and amended some docs while I was there.

damiankloip’s picture

Issue tags:+VDC

Not sure where that went.

damiankloip’s picture

Issue tags:-VDC
StatusFileSize
new25.48 KB
PASSED: [[SimpleTest]]: [MySQL] 50,442 pass(es).
[ View ]

Ok, I spoke to dawehner on IRC, we should make the module in the annotations uniform.

damiankloip’s picture

Issue tags:+VDC
StatusFileSize
new2 KB
ygerasimov’s picture

Status:Needs review» Needs work

Sorry for my terminology. Glad you understood what I meant.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/Data.phpundefined
@@ -0,0 +1,219 @@
+
+    $this->contentType = $negotiation->getContentType($request);
+    $this->mimeType = $request->getMimeType($this->contentType);

Lets use $this->setContentType() and $this->setMimeType() here instead of assigning properties directly.

damiankloip’s picture

Status:Needs work» Needs review
StatusFileSize
new25.48 KB
PASSED: [[SimpleTest]]: [MySQL] 50,394 pass(es).
[ View ]

Whatever, I don't mind either way.

dawehner’s picture

Status:Needs review» Reviewed & tested by the community

So let's get it back to RTBC.

webchick’s picture

Status:Reviewed & tested by the community» Needs work

So, the description of this functionality sounds absolutely amazing. In practice, however, I wasn't able to make it work. I go into these patches blind to emulate how a new user would behave, so I'm sorry if I repeat anything said above.

The first thing I tried was to do a Content view, add a Data display to it, and have it output fields. This caused me to lose my theme (maintenance page), with the errors:

Notice: Undefined property: stdClass::$unknown in Drupal\views\Plugin\views\row\DataFieldRow->render() (line 107 of core/modules/views/lib/Drupal/views/Plugin/views/row/DataFieldRow.php).
• Symfony\Component\Serializer\Exception\UnexpectedValueException: Serialization for the format html is not supported in Symfony\Component\Serializer\Serializer->serialize() (line 78 of /Users/webchick/Sites/8.x/core/vendor/symfony/serializer/Symfony/Component/Serializer/Serializer.php).

I was informed by Damian that this is because the display is set up to expect "EntityNG" entities, and currently in core the only one we have is Entity Test, at least until/unless #1778178: Convert comments to the new Entity Field API is in.

PROBLEM I think we should add some more robust error handling here; either by preventing you from adding a Data display altogether on an unsupported base table, or else at least not dying outright.

I tried manually enabling entity_test module, and had to do some digging to figure out how to add test entities (entity-test/add in case you are wondering :P) so I could test it.

This didn't give me a notice, but it did still give me the HTML error above.

Also, I feel like there must be a place where I can select JSON instead of HTML, but I can't seem to figure out where it is. :(

So.. I'm stuck. Marking needs work for the error handling, and I'm going to need pretty explicit test instructions.

While I'm at it, here are a few small string tweaks I noticed:

Under "Data: Style options":

[ ] Force using fields
If neither the row nor the style plugin supports fields, this field allows to enable them, so you can for example use groupby.

Should be "group by"

I also am not sure I understand what that means. What will be the consequences if I click this, despite choosing "Entity" under "Show"?

Then if I switch "Entity" to "Fields" I get:

"FIELD ID ALIASES
Rename views default field ID's in the output data.
id"

First, ID's should be IDs.

Secondly, "id" should be "Id" (I think? It's weird to have field labels that aren't capitalized)

Lastly, why would I want to do this? :) Can you add some help text here to explain?

===

UPDATE! I talked to Damian about this in IRC and he informed me of a couple of things:

1) I had a wild misunderstanding of what this patch did; I read "Data" and thought "CSV export", "JSON", etc. when in reality this is setting up a web services endpoint (which explains the errors above, since I was accessing it with just a browser, not a carefully crafted request).

PROBLEM Can we rename this display then to something more explicit like "Web services endpoint" so it sounds scary and people don't use it unless that's what they actually want?

2) The reason the path is giving those errors is because that's the default thing Symfony does when you pass it things it doesn't support.

PROBLEM I don't think we can get away with this, because unlike normal web service endpoints, whose URLs will be a MENU_CALLBACK type and never show up in the UI anywhere, these paths are displayed smack in your face on the views listing page and *of course* you're going to click them because that's how you test all of your other views. Suggestion is to replace those ugly PHP errors with something nice like:

"HTML method not supported. Please access this URL with Accept-header: application/json."

...I think that was everything! Whew. This still seems very cool, but it does need a bit more work.

webchick’s picture

Also, given the advanced nature of this, I wonder if this plugin ought to be exposed by the JSON-LD module, rather than Views itself. I think most "weekend hacker" types would be totally lost and end up as befuddled as me. :)

damiankloip’s picture

Assigned:Unassigned» damiankloip

I'm giving this some tlc now.

Just a couple o' quick points;

Should be "group by"

This is text provided by the StylePluginBase, so this should be a separate issue.

Secondly, "id" should be "Id" (I think? It's weird to have field labels that aren't capitalized)

Lastly, why would I want to do this? :) Can you add some help text here to explain?

You might want to change keys for data from their default ones, to fit how someone might want a response to be formed. For example, you might want to cahnge the node status key to published instead, or a custom text field to describe what it is.

The labels are directly from the actual field keys that are and will be used for the data response, so we could do a ucfirst or something? otherwise this might not be accurate?!

EDIT: This can't really live in the jsonld module as that just provides a format for the serializer to use (nutshell). Where as this uses all available formats.

The rest I will work on now :)

damiankloip’s picture

Status:Needs work» Needs review
StatusFileSize
new15.93 KB
new27.86 KB
PASSED: [[SimpleTest]]: [MySQL] 49,807 pass(es).
[ View ]

Ok, here we go...

I have changed:

  • Data plugin is now called WSEndpoint (Web service endpoint in the UI)
  • Fixed not being able to submit the field row form if you leave aliases empty, This is now tested for us as we have more than 1 field in the view
  • Changed the logic in DataFieldRow to account for fields that have no alias, so they can be rendered instead. For thing such as custom text. Also added this to the tests
  • Fixed the preview() for WSEndpoint, the DisplayPLuginBase preview method was using render() we don't want that. I have now overridden preview on the WSEndpoint class and added a test assertion to check the output of this

And maybe a few other things, see interdiff.

damiankloip’s picture

StatusFileSize
new8.28 KB
new27.74 KB
PASSED: [[SimpleTest]]: [MySQL] 49,905 pass(es).
[ View ]

I spoke to Moshe on IRC, him and webchick think RestExport is the best name for the Data class. I'm happy with that.

Here's a reroll with that in. I have reverted the type and plugin names to be data_* still, as they could feasibly be used by any other type of plugin, not just this rest type one.

Interdiff (without file move) attached.

dawehner’s picture

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/RestExport.phpundefined
@@ -13,20 +13,20 @@
+ *   id = "rest_export",
...
+class RestExport extends PathPluginBase {

You know this was written in a way to support more then just rest webservices ... should we just don't tell people that this will also provide a theoretical possibility for CSV support, which is THE format for a lot of people just using excel? With this we end up again with multiple contrib and custom solutions again :(

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/WSEndpoint.phpundefined
@@ -216,4 +217,14 @@ public function render() {
+   * The DisplayPluginBase preview method assumes we will be returning a render
+   * array. The data plugin will already return the serialized string.

Sure, the symfony serializer framework don't have an idea about drupal render arrays, but should maybe the drupal layer use simple render arrays? Not sure whether this is required for caching.

moshe weitzman’s picture

Status:Needs review» Reviewed & tested by the community

I think REST export is a decent name because it encompasses CVS as well as JSON response. Basically, this plugin returns your results in whatever format you requested in your HTTP Accept header (assuming the site supports your format). Thats's REST. I think we are RTBC.

damiankloip’s picture

StatusFileSize
new11.7 KB
new28.92 KB
PASSED: [[SimpleTest]]: [MySQL] 50,754 pass(es).
[ View ]

I spoke to webchick on IRC briefly about this; We are going to move this over the the rest module, so it's not in everyone's faces all the time, also it helps even more with 'scaring' people away who are not sure what the plugin type does. This should still be rtbc.

dawehner’s picture

+1

webchick’s picture

If I have not committed this in 24 hours, please ping me incessantly about it in IRC. :P

Really sorry that this fell off my radar last week. :(

klausi’s picture

+1 from me, too.

I opened #1891202: REST Export Views format should be configurable as follow-up.

damiankloip’s picture

Awesome, thanks klausi!

webchick’s picture

Title:Add a Data display plugin and serializer integration» Add a REST export display plugin and serializer integration.
Status:Reviewed & tested by the community» Fixed

WOOHOO!

ok I gave this one more spin, everything looks great. :) Here's the command I used to test:

curl -H "Accept: application/json" -H "Content-Type: application/json" -X GET http://8x.localhost:8082/resty

Since this feature was RTBC before BAP, I committed and pushed to 8.x. YAY!

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