For a better generic testing for entities and views it would make sense to start with a views integration of entity_test module.

CommentFileSizeAuthor
#7 drupal-1839624-7.patch4.56 KBdawehner
#7 interdiff.txt793 bytesdawehner
#1 drupal-1839624-1.patch4.56 KBdawehner

Comments

dawehner’s picture

dawehner
Primary language German
commented
Status: Active » Needs review
StatusFileSize
new drupal-1839624-1.patch4.56 KB

Sadly this patch requires #1783196: Get the views plugin manager from the DIC when possible to work in the actual UI, but beside from that some review would be cool.

dawehner’s picture

dawehner
Primary language German
commented
Issue tags: -VDC

#1: drupal-1839624-1.patch queued for re-testing.

dawehner’s picture

dawehner
Primary language German
commented

#1: drupal-1839624-1.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, drupal-1839624-1.patch, failed testing.

dawehner’s picture

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

#1: drupal-1839624-1.patch queued for re-testing.

moshe weitzman’s picture

moshe weitzman commented
Status: Needs review » Needs work

The odd bit I noticed is that the uuid field is declared as numeric. Our uuids are varchar 128. You could fix that or remove uuid from Views since none of our current entity types expose that column (which is problem considering that we want to use Views in content staging scenarios).

I think this could go in before #1783196: Get the views plugin manager from the DIC when possible if needed. It is valid on its own.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt793 bytes
new drupal-1839624-7.patch4.56 KB

Oh you are absolute right, it should be a string. Not exposing some fields seems to be a no-go (as you never know whether it is needed for some people). Creating a special filter handler for UUID doesn't seem to make sense for me at the moment.

moshe weitzman’s picture

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

Looks good.

dawehner’s picture

dawehner
Primary language German
commented

Thank you very much to pushing issues forward which are important but not recognized by parts of the VDC team.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Needs review

Juuuuust asking.

Can I scope-creep this a little and ask whether or not entity.module could not be providing a lot of this "boilerplate" views integration for all entities? It seems odd to have to write this much copy/paste code across all entity types.

dawehner’s picture

dawehner
Primary language German
commented

You are totally right, this is what we want to do in the longrun, see http://drupal.org/node/1740492#comment-6728924
though the pure amount of dependencies (which have dependencies as well) is interesting.

dawehner’s picture

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

I hope this answer justifies moving back to RTBC

damiankloip’s picture

damiankloip commented

This looks good to go from my point of view. Later down the line, hopefully we won't need this, but now we certainly do I think, for issues like: #1819760: Add a REST export display plugin and serializer integration.. We can then convert the tests to use entity_test module, which in turn uses entityNG which can be serialized properly :) yay.

dries’s picture

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

Committed to 8.x. Thanks.

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