Fixes #3725: Fixed the bug that the filtering condition used in relation…

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/framework/ar/ActiveRecordTestTrait.php

@@ -728,54 +728,6 @@ trait ActiveRecordTestTrait
         $this->assertEquals(2, count($order->orderItems));
     }
 
-    public function testUnlinkAndConditionSetNull()
-    {
-        /* @var $this TestCase|ActiveRecordTestTrait */
-
-        /* @var $customerClass \yii\db\ActiveRecordInterface */
-        $customerClass = $this->getCustomerClass();
-        /* @var $orderClass \yii\db\ActiveRecordInterface */
-        $orderClass = $this->getOrderClass();
-
-        // in this test all orders are owned by customer 1
-        $orderClass::updateAll(['customer_id' => 1]);
-        $this->afterSave();
-
-        // removing a model that is not related should not work
-        $customer = $customerClass::findOne(1);
-        $this->assertEquals(3, count($customer->ordersWithNullFK));
-        $this->assertEquals(1, count($customer->expensiveOrdersWithNullFK));
-        $customer->unlink('expensiveOrdersWithNullFK', $customer->orders[2], false);
-        $this->afterSave();
-        $customer = $customerClass::findOne(1);
-        $this->assertEquals(3, count($customer->ordersWithNullFK));
-        $this->assertEquals(1, count($customer->expensiveOrdersWithNullFK));
-    }
-
-    public function testUnlinkAndConditionDelete()
-    {
-        /* @var $this TestCase|ActiveRecordTestTrait */
-
-        /* @var $customerClass \yii\db\ActiveRecordInterface */
-        $customerClass = $this->getCustomerClass();
-        /* @var $orderClass \yii\db\ActiveRecordInterface */
-        $orderClass = $this->getOrderClass();
-
-        // in this test all orders are owned by customer 1
-        $orderClass::updateAll(['customer_id' => 1]);
-        $this->afterSave();
-
-        // removing a model that is not related should not work
-        $customer = $customerClass::findOne(1);
-        $this->assertEquals(3, count($customer->orders));
-        $this->assertEquals(1, count($customer->expensiveOrders));
-        $customer->unlink('expensiveOrders', $customer->orders[2], true);
-        $this->afterSave();
-        $customer = $customerClass::findOne(1);
-        $this->assertEquals(3, count($customer->orders));
-        $this->assertEquals(1, count($customer->expensiveOrders));
-    }
-
     public function testUnlinkAll()
     {
         /* @var $customerClass \yii\db\ActiveRecordInterface */
@@ -849,7 +801,7 @@ trait ActiveRecordTestTrait
         /* @var $customerClass \yii\db\BaseActiveRecord */
         $customerClass = $this->getCustomerClass();
         /* @var $orderClass \yii\db\BaseActiveRecord */
-        $orderClass = $this->getOrderClass();
+        $orderClass = $this->getOrderWithNullFKClass();
 
         // in this test all orders are owned by customer 1
         $orderClass::updateAll(['customer_id' => 1]);
@@ -860,9 +812,12 @@ trait ActiveRecordTestTrait
         $this->assertEquals(1, count($customer->expensiveOrdersWithNullFK));
         $this->assertEquals(3, $orderClass::find()->count());
         $customer->unlinkAll('expensiveOrdersWithNullFK');
-        $this->assertEquals(2, count($customer->ordersWithNullFK));
+        $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()
@@ -883,9 +838,12 @@ trait ActiveRecordTestTrait
         $this->assertEquals(1, count($customer->expensiveOrders));
         $this->assertEquals(3, $orderClass::find()->count());
         $customer->unlinkAll('expensiveOrders', true);
-        $this->assertEquals(2, count($customer->orders));
+        $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;