Part of meta-issue #1856630: [Change notice] [META] Rename Views methods to core standards

CommentFileSizeAuthor
#7 views.core-change_get_aggregation_info-2002892-06.patch5.28 KBtvlooy
#6 views-rename_get_aggregation_info-2002892-6.patch5.28 KBJeroenT
#2 views.core-change_get_aggregation_info-2002892-02.patch4.52 KBtoddtomlinson
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

toddtomlinson’s picture

toddtomlinson CreditAttribution: toddtomlinson commented
Assigned: Unassigned » toddtomlinson
toddtomlinson’s picture

toddtomlinson CreditAttribution: toddtomlinson commented
Assigned: toddtomlinson » Unassigned
Status: Active » Needs review
FileSize
views.core-change_get_aggregation_info-2002892-02.patch4.52 KB

Patch attached to change get_aggregation_info to getAggregationInfo.

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

The last submitted patch, views.core-change_get_aggregation_info-2002892-02.patch, failed testing.

oenie’s picture

oenie CreditAttribution: oenie commented
Status: Needs work » Needs review
Issue tags: +Novice, +VDC

#2: views.core-change_get_aggregation_info-2002892-02.patch queued for re-testing.

oenie’s picture

oenie CreditAttribution: oenie commented
Status: Needs review » Needs work 
+++ b/core/modules/views/lib/Drupal/views/Plugin/views/query/QueryPluginBase.phpundefined
@@ -86,7 +86,7 @@ function add_signature(ViewExecutable $view) { }
+  function getAggregationInfo() { }

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/query/Sql.phpundefined
@@ -1647,7 +1647,7 @@ function add_signature(ViewExecutable $view) {
+  function getAggregationInfo() {

Add public access modifier in front of the functions to adher to the new OOP standards.

core/modules/views_ui/lib/Drupal/views_ui/Form/Ajax/ConfigItem.php, line 220:

$aggregate = $executable->query->get_aggregation_info();

And it seems you forgot a reference to the method in a different module.

JeroenT’s picture

JeroenT
Primary language Dutch
Location 🇧🇪
CreditAttribution: JeroenT commented
Status: Needs work » Needs review
FileSize
views-rename_get_aggregation_info-2002892-6.patch5.28 KB

Patch attached to change get_aggregation_info to getAggregationInfo.

tvlooy’s picture

tvlooy CreditAttribution: tvlooy commented
FileSize
views.core-change_get_aggregation_info-2002892-06.patch5.28 KB

Duplicate patch. Sorry!

oenie’s picture

oenie CreditAttribution: oenie commented

Looks fine to me now ! Once the patch passed testbot, it's RTBC for me.

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn commented
Status: Needs review » Reviewed & tested by the community

No mentions of get_aggregation_info any more and applies cleanly. Passed testbot.

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn commented

No mentions of get_aggregation_info and applies cleanly. Passed testbot.

alexpott’s picture

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

Committed b06ef6d and pushed to 8.x. Thanks!

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