Download & Extend
Issues

Introduce a generic entity bundle filter handler

Posted by damiankloip on January 17, 2013 at 1:19pm
Project:Drupal core
Version:8.x-dev
Component:views.module
Category:task
Priority:normal
Assigned:damiankloip
Status:closed (fixed)
Issue tags:VDC

Issue Summary

In D7 we have an entity type filter handler that we can use for bundle filtering, Let's port that to D8. Then nodes/taxonomy etc... can just use this and we can remove any others.

AttachmentSizeStatusTest resultOperations
vdc.entity-type-filter.patch5.34 KBIdleFAILED: [[SimpleTest]]: [MySQL] 50,708 pass(es), 0 fail(s), and 113 exception(s).View details
Login or register to post comments

Comments

#1

Posted by damiankloip on January 17, 2013 at 1:21pm
Issue tags:+Needs tests

Probably needs tests too.

#2

Posted by dawehner on January 17, 2013 at 1:30pm
Issue tags:-Needs tests

There are just nitpicks:

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/filter/EntityType.phpundefined
@@ -0,0 +1,85 @@
+ * Definition of views_handler_filter_entity_bundle

ups :)

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/filter/EntityType.phpundefined
@@ -0,0 +1,85 @@
+  public $entityType;
...
+  public $entityInfo;

This should be better protected

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/filter/EntityType.phpundefined
@@ -0,0 +1,85 @@
+    // Set the entity type from the definition data, if it's there.
+    if (isset($this->definition['entity_type'])) {
+      $this->entityType = $this->definition['entity_type'];
+    }

Should we just ignore it, if it's not needed/used here?

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/filter/EntityType.phpundefined
@@ -0,0 +1,85 @@
+        $options[$type] = t($info['label']);

Isn't the label already runned through t()?

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/filter/EntityType.phpundefined
@@ -0,0 +1,85 @@
+    $this->ensureMyTable();

A comment might be helpful.

#3

Posted by System Message on January 17, 2013 at 2:22pm
Status:needs review» needs work

The last submitted patch, vdc.entity-type-filter.patch, failed testing.

#4

Posted by damiankloip on January 17, 2013 at 2:26pm
Status:needs work» needs review

Thanks for the review! Here are those changes, see interdiff.

AttachmentSizeStatusTest resultOperations
1891214-3.patch5.18 KBIdleFAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/views/lib/Drupal/views/Plugin/views/filter/EntityType.php.View details
interdiff.txt2.13 KBIgnored: Check issue status.NoneNone

#5

Posted by damiankloip on January 17, 2013 at 2:27pm

Sorry, complete fail on the protected properties, here we go.

AttachmentSizeStatusTest resultOperations
1891214-4.patch5.17 KBIdleFAILED: [[SimpleTest]]: [MySQL] 50,679 pass(es), 0 fail(s), and 110 exception(s).View details

#6

Posted by System Message on January 17, 2013 at 3:15pm
Status:needs review» needs work

The last submitted patch, 1891214-4.patch, failed testing.

#7

Posted by damiankloip on January 17, 2013 at 3:58pm
Status:needs work» needs review
AttachmentSizeStatusTest resultOperations
1891214-7.patch5.17 KBIdlePASSED: [[SimpleTest]]: [MySQL] 50,767 pass(es).View details
interdiff.txt704 bytesIgnored: Check issue status.NoneNone

#8

Posted by dawehner on January 17, 2013 at 11:18pm
Issue tags:+Needs tests

Ups

#9

Posted by damiankloip on January 22, 2013 at 9:38am

Here are some tests too, I have also modified entity_test module to use a bundle too.

AttachmentSizeStatusTest resultOperations
d8.entity-type-filter-with-tests.patch14.15 KBIdleFAILED: [[SimpleTest]]: [MySQL] 50,634 pass(es), 21 fail(s), and 66 exception(s).View details

#10

Posted by System Message on January 22, 2013 at 10:26am
Status:needs review» needs work

The last submitted patch, d8.entity-type-filter-with-tests.patch, failed testing.

#11

Posted by damiankloip on January 22, 2013 at 10:39am
Assigned to:Anonymous» damiankloip

Need to fix the other tests that rely on entity_test module.

#12

Posted by dawehner on January 22, 2013 at 6:25pm

+++ b/core/modules/system/tests/modules/entity_test/lib/Drupal/entity_test/Plugin/Core/Entity/EntityTest.phpundefined
@@ -64,6 +73,13 @@ class EntityTest extends EntityNG {
+   * The bundle type.

You might be able to improve that comment.

Additional isn't the type a scalar value, like "test_1" aka. a string?

+++ b/core/modules/views/lib/Drupal/views/Tests/Entity/FilterEntityTest.phpundefined
@@ -0,0 +1,99 @@
+class FilterEntityTest extends ViewTestBase {

I'm wondering whether we could use unit tests for that.

+++ b/core/modules/views/lib/Drupal/views/Tests/Entity/FilterEntityTest.phpundefined
@@ -0,0 +1,99 @@
+   * @todo.

He, at least that's honest :)

#13

Posted by damiankloip on January 23, 2013 at 7:58am
Status:needs work» needs review

I have re worked this to use node instead for now. Adding a bundle to entity_test module requires quite alot of work with alot of tests having to be changed.

Here is the handler with tests using node as the base table, and some custom bundles. I think this means we can't use unittests too, which is a shame.

What do you think?

AttachmentSizeStatusTest resultOperations
1891214-13.patch10.84 KBIdlePASSED: [[SimpleTest]]: [MySQL] 50,691 pass(es).View details

#14

Posted by dawehner on January 23, 2013 at 8:53am
Status:needs review» reviewed & tested by the community
Issue tags:-Needs tests

Let's get that in, we have tests.

We can always try to rework existing tests to use DrupalUnitTests

#15

Posted by chx on January 23, 2013 at 8:55am
Status:reviewed & tested by the community» needs review
Issue tags:+Needs tests

Why are you calling the bundle filter entity type filter? You are not filtering on entity type (that wouldn't make any sense!)

#16

Posted by chx on January 23, 2013 at 8:56am
Status:needs review» needs work
Issue tags:-Needs tests

Sorry x-post but also , IMO this is CNW.

#17

Posted by damiankloip on January 23, 2013 at 10:08am
Status:needs work» needs review

Yep, fair comments. I have changed the filter class to Bundle, and also the test class to FilterEntityBundleTest.

AttachmentSizeStatusTest resultOperations
1891214-17.patch10.77 KBIdlePASSED: [[SimpleTest]]: [MySQL] 50,691 pass(es).View details
interdiff.txt1.89 KBIgnored: Check issue status.NoneNone

#18

Posted by dawehner on January 23, 2013 at 8:08pm
Status:needs review» reviewed & tested by the community

Noone knows yet what will happen if we have proper typed data support in views, but for now this looks really good!

#19

Posted by dawehner on January 23, 2013 at 9:05pm
Title:Introduce a generic entity type filter handler» Introduce a generic entity bundle filter handler

Fix title

#20

Posted by chx on January 24, 2013 at 5:01pm

That's great.( I almost got a heart attack from the earlier title thinking Views invented some super base that can be filtered on entity type. Phew.)

#21

Posted by damiankloip on January 24, 2013 at 5:20pm

AHAHA :) Sorry about that...

#22

Posted by dawehner on January 24, 2013 at 5:24pm

So that's what people always wanted to have: support for UNION.

#23

Posted by catch on January 29, 2013 at 1:12pm
Status:reviewed & tested by the community» fixed

Looks great. I had to write one of these in Drupal 7 for a custom entity type and couldn't believe it didn't exist there. Committed/pushed to 8.x.

#24

Posted by damiankloip on January 29, 2013 at 3:20pm

Super.

#25

Posted by tim.plunkett on January 29, 2013 at 4:02pm
Title:Introduce a generic entity bundle filter handler» HEAD BROKEN: Introduce a generic entity bundle filter handler

This was never retested after #1822458: Move dynamic parts (view modes, bundles) out of entity_info(), and broke HEAD.

git revert 44cedfa

#26

Posted by webchick on January 29, 2013 at 4:07pm
Status:fixed» needs work

Rolled back.

#27

Posted by webchick on January 29, 2013 at 4:10pm
Title:HEAD BROKEN: Introduce a generic entity bundle filter handler» Introduce a generic entity bundle filter handler

#28

Posted by damiankloip on January 29, 2013 at 4:14pm
Title:Introduce a generic entity bundle filter handler» HEAD BROKEN: Introduce a generic entity bundle filter handler
Status:needs work» fixed

Here is a patch that unbreaks it.

AttachmentSizeStatusTest resultOperations
1891214-unbreak-HEAD.patch2.65 KBIdleFAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.View details

#29

Posted by damiankloip on January 29, 2013 at 4:15pm
Status:fixed» needs review

ok, a new patch.

AttachmentSizeStatusTest resultOperations
1891214-29.patch10.85 KBIdlePASSED: [[SimpleTest]]: [MySQL] 49,138 pass(es).View details

#30

Posted by damiankloip on January 29, 2013 at 4:16pm
Title:HEAD BROKEN: Introduce a generic entity bundle filter handler» Introduce a generic entity bundle filter handler

oops

#31

Posted by System Message on January 29, 2013 at 6:43pm
Status:needs review» needs work

The last submitted patch, 1891214-29.patch, failed testing.

#32

Posted by damiankloip on January 29, 2013 at 7:17pm
Status:needs work» needs review

#29: 1891214-29.patch queued for re-testing.

#33

Posted by dawehner on January 30, 2013 at 6:04pm
Status:needs review» reviewed & tested by the community

I guess we can move that back to RTBC?

#34

Posted by catch on February 4, 2013 at 2:44pm

#29: 1891214-29.patch queued for re-testing.

#35

Posted by catch on February 6, 2013 at 12:43pm
Status:reviewed & tested by the community» fixed

Let's try that again. Committed/pushed to 8.x.

#36

Posted by System Message on February 20, 2013 at 12:50pm
Status:fixed» closed (fixed)

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

nobody click here