Support from Acquia helps fund testing for Drupal Acquia logo

Comments

phenaproxima’s picture

Assigned: Unassigned » phenaproxima
phenaproxima’s picture

Assigned: phenaproxima » Unassigned
Status: Active » Needs review
FileSize
737 bytes
munizjor’s picture

Status: Needs review » Reviewed & tested by the community

Ran successful test creating a new view.

Status: Reviewed & tested by the community » Needs work
Issue tags: -Novice, -VDC

The last submitted patch, views-2002270-addHaving.patch, failed testing.

chustedde’s picture

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

#2: views-2002270-addHaving.patch queued for re-testing.

oenie’s picture

Status: Needs review » Needs work
+++ b/core/modules/views/lib/Drupal/views/Plugin/views/query/Sql.phpundefined
@@ -915,7 +915,7 @@ function add_where_expression($group, $snippet, $args = array()) {
+  function addHaving($group, $field, $value = NULL, $operator = NULL) {

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

zschmid’s picture

Status: Needs work » Needs review
FileSize
1.11 KB

adding public access modifier

oenie’s picture

Status: Needs review » Reviewed & tested by the community

Looks fine to me now !

alexpott’s picture

Status: Reviewed & tested by the community » Needs review

This function does not seem to be called anywhere... is it needed?

dawehner’s picture

Yes that's true there is no place currently in core which calls this function.

To be honest I can't think of a usecase of a having expression without using a snippet (like addHavingExpression() allows you)
as you always need an aggregation function. I would go with dropping support for it.

aspilicious’s picture

Title: Rename Views method add_having() to addHaving() » Remove views method add_having()
Status: Needs review » Needs work

:)

vito_a’s picture

Assigned: Unassigned » vito_a
vito_a’s picture

Assigned: vito_a » Unassigned
Status: Needs work » Needs review
Issue tags: +CodeSprintUA
FileSize
2.3 KB

removing it

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

If we really see that someone is using it we can readd it. This will not break any api in the future.

webchick’s picture

Title: Remove views method add_having() » Change notice: Remove views method add_having()
Project: Drupal core » Views (for Drupal 7)
Version: 8.x-dev » 8.x-3.x-dev
Component: views.module » Code
Status: Reviewed & tested by the community » Active
Issue tags: +Needs change record

Committed and pushed to 8.x. Thanks!

Moving to Views for the change notice.

xjm’s picture

Title: Change notice: Remove views method add_having() » Remove views method add_having()
Project: Views (for Drupal 7) » Drupal core
Version: 8.x-3.x-dev » 8.x-dev
Component: Code » views.module
Status: Active » Fixed
Issue tags: -Needs change record

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