At the moment views just use a stdClass object, returned from the PDO query. We can get it to return our own classed object instead. Me and Daniel were talking about this on IRC, and decided ResultRow might be a good place to start.

It would be good for documentation reasons. For now, it would not do a great deal functionally.

Something like this...

CommentFileSizeAuthor
#21 2034947-21.patch32.12 KBdamiankloip
#18 2034947-18.patch32.12 KBdamiankloip
#11 2034947-11.patch32.16 KBdamiankloip
#11 interdiff-2034947-11.txt502 bytesdamiankloip
#8 2034947-8.patch32.02 KBdamiankloip
#8 interdiff-2034947-8.txt26.03 KBdamiankloip
#5 2034947-5.patch5.99 KBdamiankloip
#5 interdiff-2034947-5.txt3.69 KBdamiankloip
#2 2034947-2.patch2.3 KBdamiankloip
#2 interdiff-2034947-2.txt473 bytesdamiankloip
vdc.ResultRow.patch2.27 KBdamiankloip

Comments

dawehner’s picture

dawehner
Primary language German
commented 
+++ b/core/modules/views/lib/Drupal/views/ResultRow.phpundefined
@@ -0,0 +1,29 @@
+   * @var array

So we could typehint something like \Drupal\Core\Entity\EntityInterface[]

damiankloip’s picture

damiankloip commented
StatusFileSize
new interdiff-2034947-2.txt473 bytes
new 2034947-2.patch2.3 KB

Yep, good docs. I will open a follow up to do the type conversions, as there will be alot.

damiankloip’s picture

damiankloip commented

I created #2034979: Typehint Views field handler methods with ResultRow as a followup to typehint all the things.

Status: Needs review » Needs work

The last submitted patch, 2034947-2.patch, failed testing.

damiankloip’s picture

damiankloip commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-2034947-5.txt3.69 KB
new 2034947-5.patch5.99 KB

Ok, we have to make atleast one typehint change...

dawehner’s picture

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

This is definitive if used everywhere a huge DX win! that is public and named exactly like that so we don't have to change like everything.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2034947-5.patch, failed testing.

damiankloip’s picture

damiankloip commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-2034947-8.txt26.03 KB
new 2034947-8.patch32.02 KB

Ok, so there are some render_link methods with \stdClass typed. Let's change those.

I think fixing/adding docblocks and adding public to methods is out of scope of this issue, what do you think?

dawehner’s picture

dawehner
Primary language German
commented

Lets try to keep it as small as possible so +1 if it is green.

Status: Needs review » Needs work

The last submitted patch, 2034947-8.patch, failed testing.

damiankloip’s picture

damiankloip commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-2034947-11.txt502 bytes
new 2034947-11.patch32.16 KB

Oops, missed a use statement. noise - sorry.

dawehner’s picture

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

Let's go!

chx’s picture

chx commented

Hard to find the words for the joy I have over this. stdClass is a sad remnant of an ancient age (like Drupal 7).

With that said, public $_entity = NULL; -- I see it's not this issue, perhaps a followup to rename wholesale?

damiankloip’s picture

damiankloip commented

Yeah :) I see your point, $_entity suggests protected. I think a follow up would be a good plan, as we could potentially get into conflicts if a field was called 'entity' at the moment, unlikely, but could happen I think.

dawehner’s picture

dawehner
Primary language German
commented

Yeah on the longrun having that is a method would be just perfect!

dawehner’s picture

dawehner
Primary language German
commented

Yeah on the longrun having that is a method would be just perfect, especially fetching this information could be deferred until it is needed.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work

Needs a reroll...

git ac https://drupal.org/files/2034947-11.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 32934  100 32934    0     0  32549      0  0:00:01  0:00:01 --:--:-- 55631
error: patch failed: core/modules/user/lib/Drupal/user/Plugin/views/field/Name.php:74
error: core/modules/user/lib/Drupal/user/Plugin/views/field/Name.php: patch does not apply
error: patch failed: core/modules/views/lib/Drupal/views/Plugin/views/query/Sql.php:1424
error: core/modules/views/lib/Drupal/views/Plugin/views/query/Sql.php: patch does not apply
damiankloip’s picture

damiankloip commented
Status: Needs work » Needs review
StatusFileSize
new 2034947-18.patch32.12 KB

There we go.

damiankloip’s picture

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

And back..

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Needs a reroll...

git ac https://drupal.org/files/2034947-18.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 32888  100 32888    0     0  24945      0  0:00:01  0:00:01 --:--:-- 28950
error: patch failed: core/modules/node/lib/Drupal/node/Plugin/views/field/Link.php:51
error: core/modules/node/lib/Drupal/node/Plugin/views/field/Link.php: patch does not apply
damiankloip’s picture

damiankloip commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new 2034947-21.patch32.12 KB

Again!! ok :)

alvar0hurtad0’s picture

alvar0hurtad0
he/him
Primary language Spanish
Location Cáceres
commented
Status: Needs review » Fixed

I applyes and works fine

damiankloip’s picture

damiankloip commented
Status: Fixed » Reviewed & tested by the community

Thanks for testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 08aeb87 and pushed to 8.x. Thanks!

xjm’s picture

xjm
she/her
Primary language English
commented
Title: Change view results to use a classed object » [Change notice] Change view results to use a classed object
Project: Drupal core » Views (for Drupal 7)
Version: 8.x-dev » 8.x-3.x-dev
Component: views.module » Code
Status: Fixed » Active
Issue tags: +Needs change record
damiankloip’s picture

damiankloip commented

How about this? https://drupal.org/node/2059483

We don't need too much because it doesn't do a great deal at the moment, until we typehint where we need to.

dawehner’s picture

dawehner
Primary language German
commented
Status: Active » Fixed

+1

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