Merge branch 'unlink-tests'

framework/CHANGELOG.md

@@ -48,6 +48,7 @@ Yii Framework 2 Change Log
 - Bug #3681: Fixed problem with AR::findOne() when a default scope joins another table so that PK name becomes ambigous (cebe)
 - Bug #3715: Fixed the bug that using a custom pager/sorter with `GridView` may generate two different pagers/sorters if the layout configures two pagers/sorters (qiangxue)
 - Bug #3716: `DynamicModel::validateData()` does not call `validate()` if the `$rules` parameter is empty (qiangxue)
+- Bug #3725: Fixed the bug that the filtering condition used in relation definition was ignored when calling `ActiveRecord::unlinkAll()`. (qiangxue, cebe)
 - Bug #3738: ActiveField custom error selector not functioning (qiangxue)
 - Bug #3751: Fixed postgreSQL schema data for enum values, do not add values if there are none (makroxyz)
 - Bug #3752: `QueryBuilder::batchInsert()` does not typecast input values (qiangxue)

framework/db/BaseActiveRecord.php

@@ -1232,6 +1232,8 @@ abstract class BaseActiveRecord extends Model implements ActiveRecordInterface
      *
      * @param string $name the case sensitive name of the relationship.
      * @param ActiveRecordInterface $model the model to be unlinked from the current one.
+     * You have to make sure that the model is really related with the current model as this method
+     * does not check this.
      * @param boolean $delete whether to delete the model that contains the foreign key.
      * If false, the model's foreign key will be set null and saved.
      * If true, the model containing the foreign key will be deleted.
@@ -1282,7 +1284,7 @@ abstract class BaseActiveRecord extends Model implements ActiveRecordInterface
         } else {
             $p1 = $model->isPrimaryKey(array_keys($relation->link));
             $p2 = $this->isPrimaryKey(array_values($relation->link));
-            if ($p1 && $p2 || $p2) {
+            if ($p2) {
                 foreach ($relation->link as $a => $b) {
                     $model->$a = null;
                 }
@@ -1348,6 +1350,9 @@ abstract class BaseActiveRecord extends Model implements ActiveRecordInterface
                 $nulls[$a] = null;
                 $condition[$a] = $this->$b;
             }
+            if (!empty($viaRelation->where)) {
+                $condition = ['and', $condition, $viaRelation->where];
+            }
             if (is_array($relation->via)) {
                 /* @var $viaClass ActiveRecordInterface */
                 if ($delete) {
@@ -1379,6 +1384,9 @@ abstract class BaseActiveRecord extends Model implements ActiveRecordInterface
                     $nulls[$a] = null;
                     $condition[$a] = $this->$b;
                 }
+                if (!empty($relation->where)) {
+                    $condition = ['and', $condition, $relation->where];
+                }
                 if ($delete) {
                     $relatedModel::deleteAll($condition);
                 } else {

tests/unit/data/ar/Customer.php

@@ -36,6 +36,17 @@ class Customer extends ActiveRecord
     {
         return $this->hasMany(Order::className(), ['customer_id' => 'id'])->orderBy('id');
     }
+
+    public function getExpensiveOrders()
+    {
+        return $this->hasMany(Order::className(), ['customer_id' => 'id'])->andWhere('total > 50')->orderBy('id');
+    }
+
+    public function getExpensiveOrdersWithNullFK()
+    {
+        return $this->hasMany(OrderWithNullFK::className(), ['customer_id' => 'id'])->andWhere('total > 50')->orderBy('id');
+    }
+
     public function getOrdersWithNullFK()
     {
         return $this->hasMany(OrderWithNullFK::className(), ['customer_id' => 'id'])->orderBy('id');

tests/unit/data/ar/elasticsearch/Customer.php

@@ -35,6 +35,16 @@ class Customer extends ActiveRecord
         return $this->hasMany(Order::className(), ['customer_id' => 'id'])->orderBy('created_at');
     }
 
+    public function getExpensiveOrders()
+    {
+        return $this->hasMany(Order::className(), ['customer_id' => 'id'])->filter(['range' => ['total' => ['gte' => 50]]])->orderBy('id');
+    }
+
+    public function getExpensiveOrdersWithNullFK()
+    {
+        return $this->hasMany(Order::className(), ['customer_id' => 'id'])->filter(['range' => ['total' => ['gte' => 50]]])->orderBy('id');
+    }
+
     public function getOrdersWithNullFK()
     {
         return $this->hasMany(OrderWithNullFK::className(), ['customer_id' => 'id'])->orderBy('created_at');

tests/unit/data/ar/redis/Customer.php

@@ -30,6 +30,22 @@ class Customer extends ActiveRecord
     /**
      * @return \yii\redis\ActiveQuery
      */
+    public function getExpensiveOrders()
+    {
+        return $this->hasMany(Order::className(), ['customer_id' => 'id'])->andWhere("tonumber(redis.call('HGET','order' .. ':a:' .. pk, 'total')) > 50");
+    }
+
+    /**
+     * @return \yii\redis\ActiveQuery
+     */
+    public function getExpensiveOrdersWithNullFK()
+    {
+        return $this->hasMany(OrderWithNullFK::className(), ['customer_id' => 'id'])->andWhere("tonumber(redis.call('HGET','order' .. ':a:' .. pk, 'total')) > 50");
+    }
+
+    /**
+     * @return \yii\redis\ActiveQuery
+     */
     public function getOrdersWithNullFK()
     {
         return $this->hasMany(OrderWithNullFK::className(), ['customer_id' => 'id']);

tests/unit/framework/ar/ActiveRecordTestTrait.php

@@ -794,6 +794,58 @@ trait ActiveRecordTestTrait
         // via table is covered in \yiiunit\framework\db\ActiveRecordTest::testUnlinkAllViaTable()
     }
 
+    public function testUnlinkAllAndConditionSetNull()
+    {
+        /* @var $this TestCase|ActiveRecordTestTrait */
+
+        /* @var $customerClass \yii\db\BaseActiveRecord */
+        $customerClass = $this->getCustomerClass();
+        /* @var $orderClass \yii\db\BaseActiveRecord */
+        $orderClass = $this->getOrderWithNullFKClass();
+
+        // in this test all orders are owned by customer 1
+        $orderClass::updateAll(['customer_id' => 1]);
+        $this->afterSave();
+
+        $customer = $customerClass::findOne(1);
+        $this->assertEquals(3, count($customer->ordersWithNullFK));
+        $this->assertEquals(1, count($customer->expensiveOrdersWithNullFK));
+        $this->assertEquals(3, $orderClass::find()->count());
+        $customer->unlinkAll('expensiveOrdersWithNullFK');
+        $this->assertEquals(3, count($customer->ordersWithNullFK));
+        $this->assertEquals(0, count($customer->expensiveOrdersWithNullFK));
+        $this->assertEquals(3, $orderClass::find()->count());
+        $customer = $customerClass::findOne(1);
+        $this->assertEquals(2, count($customer->ordersWithNullFK));
+        $this->assertEquals(0, count($customer->expensiveOrdersWithNullFK));
+    }
+
+    public function testUnlinkAllAndConditionDelete()
+    {
+        /* @var $this TestCase|ActiveRecordTestTrait */
+
+        /* @var $customerClass \yii\db\BaseActiveRecord */
+        $customerClass = $this->getCustomerClass();
+        /* @var $orderClass \yii\db\BaseActiveRecord */
+        $orderClass = $this->getOrderClass();
+
+        // in this test all orders are owned by customer 1
+        $orderClass::updateAll(['customer_id' => 1]);
+        $this->afterSave();
+
+        $customer = $customerClass::findOne(1);
+        $this->assertEquals(3, count($customer->orders));
+        $this->assertEquals(1, count($customer->expensiveOrders));
+        $this->assertEquals(3, $orderClass::find()->count());
+        $customer->unlinkAll('expensiveOrders', true);
+        $this->assertEquals(3, count($customer->orders));
+        $this->assertEquals(0, count($customer->expensiveOrders));
+        $this->assertEquals(2, $orderClass::find()->count());
+        $customer = $customerClass::findOne(1);
+        $this->assertEquals(2, count($customer->orders));
+        $this->assertEquals(0, count($customer->expensiveOrders));
+    }
+
     public static $afterSaveNewRecord;
     public static $afterSaveInsert;