Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Problem/Motivation
With any fields using the views Field field handler, which is a lot after all the conversions to re-use field formatters, you cannot use the field row plugin and raw output. It will always be the rendered value.
Proposed resolution
Fix the DataFieldRow plugin so it works with fields using the Field handler too. The problem comes about as the Field handler never sets the field_alias property, which the DataFieldRow plugin checks for.
Remaining tasks
Tests
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#27 | 2574077-27.patch | 7.17 KB | charginghawk |
#25 | interdiff-2574077-25.txt | 87 bytes | charginghawk |
#25 | 2574077-25.patch | 7.17 KB | charginghawk |
| |||
#22 | interdiff-2574077-22.txt | 851 bytes | damiankloip |
#22 | 2574077-22.patch | 7.18 KB | damiankloip |
Comments
Comment #2
damiankloip CreditAttribution: damiankloip commentedComment #3
damiankloip CreditAttribution: damiankloip commentedDamn, empty patch. Here is a patch just with a potential fix.
Comment #4
damiankloip CreditAttribution: damiankloip commentedWith some test coverage.
Comment #7
damiankloip CreditAttribution: damiankloip commentedI tihnk this change is ok, fields have changed sooo much since we added this originally. It doesn't make sense for the field handler with no actual field alias used, as the entity value is fetched directly.
Comment #8
dawehnerThe document is outdated now, this is for sure. Is there a reason why we ever did that? IMHO this was a bad idea in the first place, this is for sure! Yeah that comment was horrible and pointless and all the time.
ah so this doesn't change anything fundemental
Comment #9
damiankloip CreditAttribution: damiankloip commentedSorry, forgot about this, updated comment on the patch!
Comment #10
dawehnerThank you @damiankloip
Comment #11
charginghawk CreditAttribution: charginghawk at Genuine commentedThis patched worked for me - thanks! There was an issue where if the value was an array, the output would simply be "Array". Maybe something should be thrown in, along the lines of...
That'll give a serialized array output, instead of just "Array".
Comment #12
catchIf #11 is right look like we should keep going here and fix that case as well.
Comment #13
dawehnerWell yeah the question is actually, what do you want to do in case there is an array/object, kinda tricky indeed.
Comment #14
damiankloip CreditAttribution: damiankloip commentedIf we just removed the unnecessary sanitizeValue() this would not be an issue.
Comment #15
dawehnerSo what about just doing that then? Did anyone relied on that behaviour? Its rest, they should take care about escaping on the frontend code, if they really use REST for frontend stuff.
Comment #16
damiankloip CreditAttribution: damiankloip commentedYes, it makes no sense to me. It's ridiculous, not sure why it was there to start with (my fault I think).
Comment #17
dawehnerI guess we are fine that people have to escape when they output. It is simply the right thing to do.
Comment #18
alexpottI think we should add tests for the change in #16 both for an array value and a value with XSS.
Comment #19
damiankloip CreditAttribution: damiankloip commentedAdded some more coverage for these cases.
Comment #22
damiankloip CreditAttribution: damiankloip commentedWhoops, added the assertion text in the wrong parens! Let's try that again.
Comment #24
dawehnerThe fix is super clean, this is for sure.
Comment #25
charginghawk CreditAttribution: charginghawk at Genuine commentedI think there's a case for using
$value = $row->_entity->$id;
instead of$value = $field->getValue($row);
.In my use case, I have a normalizer plugin for a specific field type (see #2614292: Add normalizers for Field Collection Items (to enable REST/JSON API support)). Using the "Show: Entity" option, it gets routed through DataEntityRow, which just returns
return $row->_entity;
, and correctly outputs the field per my normalizer plugin.However, if I use "Show: Fields" with this patch, it sidesteps my normalizer plugin completely. I think the
->getValue($row)
departs from the serializing process.So, I think the entity and field formatters should handle this logic the same way, and I'm submitting a patch for that.
Comment #27
charginghawk CreditAttribution: charginghawk at Genuine commentedPatch failed to apply... :(
Let's try this again.
Comment #29
damiankloip CreditAttribution: damiankloip commentedSorry, a change like that almost certainly needs to be a follow up IMO. I think you are maybe getting confused about 'fields' in views. Not all of these will be a field API field or whatnot. This is about a views field, not necessarily a field on an entity. This could be any field defined in views data.
In that light the patch in #22 and the RTBC in #24 is good I think.
Comment #30
dawehnerYeah fields make just more sense.
Comment #31
damiankloip CreditAttribution: damiankloip commentedJust to be clear, RTBC is for patch in #22.
Comment #32
alexpott@charginghawk - I agree with @damiankloip - let's explore #25 in a new issue.
Committed #22 569de54 and pushed to 8.0.x and 8.1.x. Thanks!
Comment #35
charginghawk CreditAttribution: charginghawk at Genuine commentedPer #32 I've created #2637312: REST views: using DataFieldRow's "raw_output" option causes entity reference fields to output NULL.
Comment #36
damiankloip CreditAttribution: damiankloip commentedThanks charginghawk!