Problem/Motivation

Functions inside database.inc are deprecated and need to be replaced with their respective alternatives.

Proposed resolution

Replace db_and() and db_or() calls with Drupal\Core\Database\Query\Condition objects as specified in the function documentation.

Remaining tasks

User interface changes

API changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Crell’s picture

In concept I'm fine with this. I'd prefer something a bit less verbose than andConditionGroup(), but removing the wrapper functions is fine by me.

tim.plunkett’s picture

Could just as easily be marked @deprecated. As long as we replace the usages in core, doesn't matter much to me.

jibran’s picture

Issue tags: +API clean-up

+1 for the change.

Could just as easily be marked @deprecated.

I see no purpose of this issue then :).

jibran’s picture

Issue summary: View changes

Updated issue summary.

disasm’s picture

Status: Active » Needs review
FileSize
25.53 KB

first go at this.

Status: Needs review » Needs work

The last submitted patch, drupal8.database-system.2117305-4.patch, failed testing.

disasm’s picture

Status: Needs work » Needs review
FileSize
24.05 KB

undoing one views file that just doesn't have a query object accessible to the methods creating the conditions. It's probably going to need a refactor of some sort.

Status: Needs review » Needs work

The last submitted patch, drupal8.database-system.2117305-6.patch, failed testing.

disasm’s picture

Status: Needs work » Needs review
FileSize
2.46 KB
26.5 KB

got a fix for views. The issue with search is the object returned by db_query does not have the method orConditionGroup on it. That hasn't been addressed yet.

The last submitted patch, drupal8.database-system.2117305-8.patch, failed testing.

Anonymous’s picture

Issue summary: View changes

Updated issue summary.

chx’s picture

> The issue with search is the object returned by db_query does not have the method orConditionGroup on it

Care to tell me a little more? db_query returns a statement, there's no more conditions on that. Where / what should I look at?

sun’s picture

Title: Get rid of db_or / db_and » Remove db_or() and db_and()
Issue summary: View changes
Issue tags: +@deprecated
andypost’s picture

+++ w/core/includes/database.inc
@@ -534,6 +534,8 @@ function db_next_id($existing_id = 0) {
+ * @deprecated use $query->orConditionGroup()

@@ -544,6 +546,8 @@ function db_or() {
+ * @deprecated use $query->andConditionGroup()

Should be marked when to remove, suppose 9.0

JeroenT’s picture

Title: Remove db_or() and db_and() » Remove usages of db_or() and db_and().
Issue tags: +Needs issue summary update
JeroenT’s picture

Issue summary: View changes
Issue tags: -Needs issue summary update
JeroenT’s picture

Version: 8.0.x-dev » 9.x-dev
Status: Needs work » Postponed
Crell’s picture

Version: 9.x-dev » 8.0.x-dev
Status: Postponed » Needs work

If they're marked deprecated that means working to remove their usage is entirely OK and encouraged for Drupal 8.

Mile23’s picture

Version: 8.0.x-dev » 8.1.x-dev
Issue tags: +Needs issue summary update
Parent issue: » #2319859: [META] Deprecate contents of database.inc

Setting to 8.1.x because I think that's where code cleanup type stuff is supposed to happen. Happy to be mistaken.

Setting parent issue to #2319859: [META] Deprecate contents of database.inc

Note that db_or() and db_and() have simple replacements spelled out in their @deprecation docs, so I'm adding the 'needs issue summary update' tag.

 * @deprecated as of Drupal 8.0.x, will be removed in Drupal 9.0.0. Create
 *   a \Drupal\Core\Database\Query\Condition object, specifying an AND
 *   conjunction: new Condition('AND');
[..]
 * @deprecated as of Drupal 8.0.x, will be removed in Drupal 9.0.0. Create
 *   a \Drupal\Core\Database\Query\Condition object, specifying an XOR
 *   conjunction: new Condition('XOR');

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

ieguskiza’s picture

Status: Needs work » Needs review
FileSize
22.33 KB

Hi,
I'm uploading a new patch based on the comment on #17: replacing db_and and db_or with Drupal\Core\Database\Query\Condition objects.

ieguskiza’s picture

Title: Remove usages of db_or() and db_and(). » Replace usages of db_or() and db_and().
Issue summary: View changes
Issue tags: -@deprecated, -Needs issue summary update

I have cleaned up the tags for the issue and tried to rewrite the summary in order to reflect the problem and solution.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

daffie’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll
cilefen’s picture

jofitz’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
22.33 KB

Re-roll.

daffie’s picture

Status: Needs review » Reviewed & tested by the community

All code changes are good.
All instances of the usage of db_and() and db_or() are replaced.
The patch looks good to me.
Two minor documentation nitpicks can be fixed on commit.

  1. +++ b/core/modules/search/src/ViewsSearchQuery.php
    @@ -74,8 +74,9 @@ function conditionReplaceString($search, $replace, &$condition) {
    +          // As conditions can have subconditions,
    +          // for example new Condition('OR'),
    +          // the function has to be called recursively.
    

    Documentation should be filled out to the 80 character line.

  2. +++ b/core/modules/views/src/ManyToOneHelper.php
    @@ -269,7 +270,7 @@ public function addFilter() {
         // add_condition determines whether a single expression is enough(FALSE) or the
    -    // conditions should be added via an db_or()/db_and() (TRUE).
    +    // conditions should be added via an new Condition('OR')/new Condition('AND') (TRUE).
    

    The no documentation beyond the 80 character line rule is broken.

xjm’s picture

Status: Reviewed & tested by the community » Needs work

Let's fix those docs errors in the patch. :) Thanks!

Pavan B S’s picture

Status: Needs work » Needs review
FileSize
22.44 KB
1.42 KB

Applying the patch as suggested in the comment #26 , please review .

jofitz’s picture

Status: Needs review » Reviewed & tested by the community

Only documentation changes so setting back to RTBC.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 23d140f and pushed to 8.4.x. Thanks!

diff --git a/core/modules/search/src/ViewsSearchQuery.php b/core/modules/search/src/ViewsSearchQuery.php
index 95aaab8..e61a7e0 100644
--- a/core/modules/search/src/ViewsSearchQuery.php
+++ b/core/modules/search/src/ViewsSearchQuery.php
@@ -74,8 +74,8 @@ function conditionReplaceString($search, $replace, &$condition) {
       $conditions =& $condition['field']->conditions();
       foreach ($conditions as $key => &$subcondition) {
         if (is_numeric($key)) {
-          // As conditions can have subconditions,for example
-          // new Condition('OR'),the function has to be called recursively.
+          // As conditions can be nested, the function has to be called
+          // recursively.
           $this->conditionReplaceString($search, $replace, $subcondition);
         }
       }
diff --git a/core/modules/views/src/ManyToOneHelper.php b/core/modules/views/src/ManyToOneHelper.php
index 3afc196..d9ed2ca 100644
--- a/core/modules/views/src/ManyToOneHelper.php
+++ b/core/modules/views/src/ManyToOneHelper.php
@@ -269,9 +269,8 @@ public function addFilter() {
       $options['group'] = 0;
     }
 
-    // add_condition determines whether a single expression is enough(FALSE)
-    // or the conditions should be added via an new Condition('OR')/
-    // new Condition('AND') (TRUE).
+    // If $add_condition is set to FALSE, a single expression is enough. If it
+    // is set to TRUE, conditions will be added.
     $add_condition = TRUE;
     if ($operator == 'not') {
       $value = NULL;

These original comments are a bit odd I've tried to make them a bit simpler and less about the code and more about what is actually relevant.

  • alexpott committed 23d140f on 8.4.x
    Issue #2117305 by disasm, Pavan B S, ieguskiza, Jo Fitzgerald: Replace...

Status: Fixed » Closed (fixed)

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