October: SoftDeletes in Lists are not being filter out

Created on 22 Mar 2017  路  6Comments  路  Source: octobercms/october

Expected behavior

When rendering a list, where columns have a relation to a model with a SoftDeleteTrait, I expect that the model adheres to the SoftDelete.
So, soft deleted relations will NOT be visible in a list.

Actual behavior

When defining a relation to a model with a SoftDeleteTrait, the soft deleted record will still show up in the column of the list.

Even adding a seperated scope, besides the scope that is added automatically for a soft delete doesn't change anything to the list rendering.

Reproduce steps

Model: Sections

<?php

namespace Models;

use Model;
use October\Rain\Database\Traits\Validation;

/**
 * Model
 */
class Sections extends Model
{
    use Validation;

    /*
     * Validation
     */
    /**
     * @var array
     */
    public $rules = [];

    /*
     * Disable timestamps by default.
     * Remove this line if timestamps are defined in the database table.
     */
    /**
     * @var bool
     */
    public $timestamps = false;

    /**
     * @var string The database table used by the model.
     */
    public $table = 'hornbach_eventplanner_sections';

    /**
     *
     */
    public function beforeCreate()
    {
        $this->available = $this->total;
    }

    /**
     * @var array
     */
    public $hasMany = [
        'clients' => [
            'Models\Clients'
        ],
    ];

    /**
     * @var array The following models have multiple records for one section
     */
    public $belongsToMany = [
        'eventdates' => [
            'Models\Eventdates',
            'table' => 'eventdates_sections',
        ],
    ];

}

Model: Clients

<?php

namespace Models;

use Model;
use October\Rain\Database\Traits\Validation;
use October\Rain\Database\Traits\SoftDelete;

/**
 * Model
 */
class Clients extends Model
{
    use Validation, SoftDelete;

    /**
     * Date columns for model.
     *
     * @var array $dates
     */
    protected $dates = ['deleted_at'];

    /*
     * Validation
     */
    /**
     * @var array
     */
    public $rules = [];

    /*
     * Mass assignment
     */

    /**
     * @var array
     */
    protected $fillable = ['edit_link'];

    /*
     * Disable timestamps by default.
     * Remove this line if timestamps are defined in the database table.
     */
    /**
     * @var bool
     */
    public $timestamps = true;

    /**
     * @var string The database table used by the model.
     */
    public $table = 'clients';

    /**
     * @var array
     */
    public $belongsTo = [
        'events' => ['Models\Events'],
        'sections' => ['Models\Sections'],
        'branches' => ['Models\Branches'],
        'eventdates' => ['Models\Eventdates'],
        'section' => ['Models\Sections'],
    ];

    /**
     * @var array
     */
    public $hasMany = [
        'clientdata' => ['Models\ClientData'],
        'clientfile' => ['Models\ClientFile'],
        'payments' => ['Models\Payments'],
    ];

    /**
     * @var array
     */
    public $hasOne = [
        'branche' => ['Models\Branches'],
    ];
}

Sections --> Columns.yaml

columns:
    name:
        label: 'lang.name'
        type: text
    total:
        label: Total 
        type: text
    available:
        label: Available
        type: text
    customername:
        label: Cusomer
        relation: clients
        select: concat(firstname, ' ', lastname)
    email:
        label: E-mail
        relation: clients
        type: text
        select: email

The list is being generated correctly, but the SoftDeleted relations will still show up in the list, in this case in the columns customername and email which are both related to 'clients'.

Also adding a condition in the model 'sections' under 'hasMany' has no influence on the result.

I've narrowed it down to the file: modules/backend/widgets/Lists.php.
On line 455 explicits ignores using scopes in the count query, resulting in not using the SoftDelete scope.

$countQuery = $relationObj->getRelationCountQuery($relationObj->getRelated()->newQueryWithoutScopes(), $query);

When changing newQueryWithoutScopes to newQuery, the scope for SoftDelete is being used.

Result before:

SELECT
  `sections`.*,
  (SELECT group_concat(concat(firstname, ' ', lastname) SEPARATOR ', ')
   FROM `clients`
   WHERE `clients`.`sections_id` = `sections`.`id`) AS `klantnaam`,
  (SELECT group_concat(email SEPARATOR ', ')
   FROM `clients`
   WHERE `clients`.`sections_id` = `sections`.`id`) AS `email`,
  `sections`.`eventdates_id`                                   AS `pivot_eventdates_id`,
  `sections`.`sections_id`                                     AS `pivot_sections_id`
FROM `sections`
  INNER JOIN `sections`
    ON `sections`.`id` = `sections`.`sections_id`
WHERE `sections`.`eventdates_id` = 47
ORDER BY `name` DESC

Result after changing the line in Lists.php

SELECT
  `sections`.*,
  (SELECT group_concat(concat(firstname, ' ', lastname) SEPARATOR ', ')
   FROM `clients`
   WHERE `clients`.`deleted_at` IS NULL AND
         `clients`.`sections_id` = `sections`.`id`) AS `klantnaam`,
  (SELECT group_concat(email SEPARATOR ', ')
   FROM `clients`
   WHERE `clients`.`deleted_at` IS NULL AND
         `clients`.`sections_id` = `sections`.`id`) AS `email`,
  `eventdates_sections`.`eventdates_id`                                   AS `pivot_eventdates_id`,
  `eventdates_sections`.`sections_id`                                     AS `pivot_sections_id`
FROM `sections`
  INNER JOIN `eventdates_sections`
    ON `sections`.`id` = `eventdates_sections`.`sections_id`
WHERE `eventdates_sections`.`eventdates_id` = 47
ORDER BY `name` DESC
October build

396

Medium Archived Review Needed Bug

All 6 comments

@intheweb Thank you for the detailed report!

@daftspunk Why is the query for columns where relation is set (so querying the relation) initialized without any scopes in the first place? Is it going to break anything if we change it to use the default scopes that would otherwise normally be applied?

@daftspunk @LukeTowers looking into the GIT commits on the Lists.php file I came across https://github.com/octoberrain/backend/commit/2ead4f467cc2eeeb85a04dd75f2da651e4482288

Which leads to https://github.com/rainlab/blog-plugin/issues/171

Does this rings bells? :)
How can I help?

I think it makes sense that these relational queries are not limited by constraints because it reduces the complexity of the query. I know that doesn't quite help your situation. We'll need to explore an option that allows the best of both worlds. Keeping in mind, in some cases the user may want the soft deleted records to be displayed.

This definitely needs some review. If you can help us to help you, please submit this as a PR to the test plugin and also consider making a contribution to the project, which would free up more time for us to be able look at this for you.

@daftspunk Currently checking out how to add this to the test plugin (like the setup of it!)
Because it's my first time trying to help: can you point me in the right direction.

I'll will be working on it this week.

Where can I, or should I, add the soft delete trait to?
To an existing part of the test plugin or a complete new section.

Anywhere is fine really, preferably somewhere that is logical and makes sense in a real world situation. We try to keep the test plugin as meaningful as possible, so be creative and avoid adding things like "test" and "foo bar" if you can. Thanks!

Closing as it has been over a month since any activity on this occurred.

Was this page helpful?
0 / 5 - 0 ratings