Support from Acquia helps fund testing for Drupal Acquia logo

Comments

heddn’s picture

sillygwailo’s picture

Taking this with baldwinlouie.

sillygwailo’s picture

Status: Active » Needs review
FileSize
1.31 KB

Patch attached, no testing done, method only appears in one place.

dawehner’s picture

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/filter/FilterPluginBase.phpundefined
@@ -175,7 +175,7 @@ public function canExpose() { return TRUE; }
+  function canBuildGroup() {

This should be marked as protected

These are the only places the method is used.

Status: Needs review » Needs work

The last submitted patch, 2002416-rename-can_build_group.patch, failed testing.

sillygwailo’s picture

Status: Needs work » Needs review
FileSize
1.32 KB

Re-rolled as protected.

Status: Needs review » Needs work

The last submitted patch, 2002416-rename-can_build_group.patch, failed testing.

JeroenT’s picture

rename can_build_group to canBuildGroup.

JeroenT’s picture

Status: Needs work » Needs review

marking as needs review.

Status: Needs review » Needs work

The last submitted patch, views-rename_can_build_group-2002416-8.patch, failed testing.

mari3.14’s picture

Status: Needs work » Needs review
FileSize
1.32 KB

Patch re-rolled, tested locally, let's see if I am lucky

oenie’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me now !

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

Needs a re-roll

curl http://drupal.org/files/rename-can_build_group-views-2002416-11.patch | git a
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  1348  100  1348    0     0    481      0  0:00:02  0:00:02 --:--:--   513
error: patch failed: core/modules/views/lib/Drupal/views/Plugin/views/filter/FilterPluginBase.php:175
error: core/modules/views/lib/Drupal/views/Plugin/views/filter/FilterPluginBase.php: patch does not apply
jibran’s picture

Status: Needs work » Needs review
FileSize
0 bytes

Reroll
Conflict

++<<<<<<< HEAD
 +  function can_build_group() {
 +    return $this->isExposed() && (count($this->operatorOptions()) > 0);
++=======
+   protected function canBuildGroup() {
+     return $this->isExposed() && (count($this->operator_options()) > 0);
++>>>>>>> 11

Resloved

-   function can_build_group() {
 -  protected function canBuildGroup() {
 -    return $this->isExposed() && (count($this->operator_options()) > 0);
++  function canBuildGroup() {
 +    return $this->isExposed() && (count($this->operatorOptions()) > 0);
sillygwailo’s picture

Empty patch? Re-rolled.

Status: Needs review » Needs work

The last submitted patch, 2002416-rename-can_build_group.patch, failed testing.

mari3.14’s picture

Status: Needs work » Needs review
FileSize
1.31 KB

Re-rolling the patch following suggestions

[I am learning about patching but I do not think this is going to be the one; I am happy to try again if someone tells me what needs changing please]

dawehner’s picture

Status: Needs review » Needs work
+++ b/core/modules/views/lib/Drupal/views/Plugin/views/filter/FilterPluginBase.phpundefined
@@ -175,7 +175,7 @@ public function canExpose() { return TRUE; }
+  function canBuildGroup() {

Should be a protected method.

sillygwailo’s picture

Re-rolled with protected.

sillygwailo’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 2002416-rename-can_build_group.patch, failed testing.

dawehner’s picture

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/filter/FilterPluginBase.phpundefined
@@ -197,14 +197,14 @@ public function buildOptionsForm(&$form, &$form_state) {
-    if ($this->can_build_group()) {
+    if ($this->canBuildBroup()) {

You made a typo here: Should be canBuildGroup instead

JeroenT’s picture

Status: Needs work » Needs review
FileSize
1.32 KB

Renamed can_build_group to canBuildGroup. Added protected access modifier as mentioned by dawehner.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Seems fine.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 7d31103 and pushed to 8.x. Thanks!

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