Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

FileSize
1.97 KB

This is a start to allow to configure any kind of label using the grouped filter functionality.

dawehner’s picture

Status: Active » Needs review
FileSize
5.67 KB
6.73 KB

Let's add proper test + fix some notices.

jibran’s picture

+++ b/core/modules/views_ui/lib/Drupal/views_ui/Tests/FilterBooleanWebTest.phpundefined
@@ -0,0 +1,90 @@
+    debug($display['display_options']['filters']['age']);

I think this is by mistake or missing assertion.

dawehner’s picture

FileSize
619 bytes
6.67 KB

Fixed that.

dawehner’s picture

Fixed that.

damiankloip’s picture

FileSize
6.96 KB
11.88 KB

Here is a test for the actual logic of the filter value. Things are slightly broken atm.

damiankloip’s picture

FileSize
2.81 KB
12.13 KB
damiankloip’s picture

FileSize
4.99 KB

Fixed up based on your feedback, thanks Daniel.

Sorry, missed the interdiff.

EDIT: This is the wrong patch. #7 is good. posted on the wrong issue.

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

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

ParisLiakos’s picture

Status: Needs work » Needs review

#7: 1998192-7.patch queued for re-testing.

damiankloip’s picture

#8: 1998192-8.patch queued for re-testing.

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

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

damiankloip’s picture

Status: Needs work » Needs review
FileSize
12.13 KB

We just need to retest #7.

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

The last submitted patch, 1998192-7.patch, failed testing.

xjm’s picture

Status: Needs work » Needs review

#13: 1998192-7.patch queued for re-testing.

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

The last submitted patch, 1998192-7.patch, failed testing.

damiankloip’s picture

Status: Needs work » Needs review
FileSize
2.52 KB
12.08 KB

I *think* this should fix the tests now. Let's see..

dawehner’s picture

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/filter/BooleanOperator.phpundefined
@@ -165,10 +197,23 @@ public function defaultExposeOptions() {
+  /**
+   * @param string $field
+   */
+  protected function queryOpBoolean($field) {

Sorry .. needs docs.

+++ b/core/modules/views/lib/Drupal/views/Tests/Handler/FilterBooleanOperatorTest.phpundefined
@@ -0,0 +1,97 @@
+ * Tests the core Drupal\views\Plugin\views\filter\InOperator handler.

Could get one @see as well.

+++ b/core/modules/views/lib/Drupal/views/Tests/ViewTestData.phpundefined
@@ -112,6 +112,13 @@ public static function schemaDefinition() {
+        'status' => array(
+          'description' => "The status of this record",
+          'type' => 'int',
+          'unsigned' => TRUE,
+          'not null' => TRUE,
+          'default' => 0,
+        ),

@@ -219,6 +226,19 @@ public static function viewsData() {
+    $data['views_test_data']['status'] = array(
+      'title' => t('Status'),
+      'help' => t('The status of this record'),
+      'field' => array(
+        'id' => 'boolean',
+      ),
+      'filter' => array(
+        'id' => 'boolean',
+      ),
+      'sort' => array(
+        'id' => 'standard',
+      ),

@@ -232,30 +252,35 @@ public static function dataSet() {
+        'status' => 1,
...
+        'status' => 0,
...
+        'status' => 1,
...
+        'status' => 0,
...
+        'status' => 1,

So for the UI test we alter and for the unit test we add this feel all the time? This seems unconsistent. I would personally vote to alter both times.

+++ b/core/modules/views_ui/lib/Drupal/views_ui/Tests/FilterBooleanWebTest.phpundefined
@@ -0,0 +1,84 @@
+ * @see \Drupal\views\Plugin\views\filter\BooleanOperator

yeah!!

damiankloip’s picture

I'm not sure about altering the views test data to add this handler, as for things like ID we always have that field around, status we don't so altering wouldn't help? not unless we already had a handler on the view, but it would use Boolean anyways.

Status: Needs review » Needs work

The last submitted patch, 1998192-19.patch, failed testing.

damiankloip’s picture

Status: Needs work » Needs review
FileSize
534 bytes
534 bytes
damiankloip’s picture

FileSize
12.82 KB

Sorry, with the right patch and not two interdiffs.

dawehner’s picture

I still think that if we add boolean fields to all beatles we should leverage it in both tests.

dawehner’s picture

Status: Needs review » Needs work

:(

damiankloip’s picture

Status: Needs work » Needs review
FileSize
1.91 KB
12.44 KB

Sorry, I see exactly what you mean now. We should use the status handler in the UI test instead of altering the data on the age field :)

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

I'm glad you understood me. Perfect! (Once the testbot liked us).

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

Needs a reroll...

curl https://drupal.org/files/1998192-25.patch | git a
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 12742  100 12742    0     0   9052      0  0:00:01  0:00:01 --:--:-- 10600
error: patch failed: core/modules/views/lib/Drupal/views/Plugin/views/filter/BooleanOperator.php:96
error: core/modules/views/lib/Drupal/views/Plugin/views/filter/BooleanOperator.php: patch does not apply
damiankloip’s picture

Status: Needs work » Needs review
FileSize
12.45 KB

rerolled

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Back to RTBC, yeah!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 0e8571d and pushed to 8.x. Thanks!

dawehner’s picture

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