fuel / orm

Fuel PHP Framework - Fuel v1.x ORM
http://fuelphp.com/docs/packages/orm/intro.html
152 stars 96 forks source link

Soft delete: records appearing even when deleted_at is not null (FuelPHP 1.6) #256

Closed mikelexp closed 11 years ago

mikelexp commented 11 years ago

Model:

class Model_Product extends \Orm\Model_Soft
{
    protected static $_properties = array(
        'id',
        'name',
        'active',
        'deleted_at',
    );

    protected static $_soft_delete = array(
        "mysql_timestamp" => false
    );

}

Orm Query:

$products = \Model_Product::query()
    ->where("active", 1)
    ->order_by("name")
    ->get();

The resulting $products array contains all items, even those that have been marked as deleted correctly with a unix timestamp.

This started to happen with FuelPHP 1.6, with 1.5.3 it worked fine.

emlynwest commented 11 years ago

I am not able to reproduce this on 1.6/master or 1.7/develop. What database schema are you using?

mikelexp commented 11 years ago

These are the real schemas and models:

CREATE TABLE `rrhh_busquedas` (
  `id` int(11) NOT NULL AUTO_INCREMENT,
  `nombre` varchar(255) DEFAULT NULL,
  `resumen` text,
  `descripcion` text,
  `url` varchar(255) DEFAULT NULL,
  `orden` int(11) DEFAULT NULL,
  `activo` tinyint(1) NOT NULL DEFAULT '0',
  `created_at` int(11) DEFAULT NULL,
  `updated_at` int(11) DEFAULT NULL,
  `deleted_at` int(11) DEFAULT NULL,
  PRIMARY KEY (`id`)
) ENGINE=MyISAM AUTO_INCREMENT=4 DEFAULT CHARSET=utf8;
CREATE TABLE `rrhh_busquedas_envios` (
  `id` int(11) NOT NULL AUTO_INCREMENT,
  `rrhh_busqueda_id` int(11) DEFAULT NULL,
  `nyap` varchar(255) DEFAULT NULL,
  `email` varchar(255) DEFAULT NULL,
  `telefono` varchar(255) DEFAULT NULL,
  `url` varchar(255) DEFAULT NULL,
  `cv` varchar(255) DEFAULT NULL,
  `fecha` datetime DEFAULT NULL,
  `created_at` int(11) DEFAULT NULL,
  `updated_at` int(11) DEFAULT NULL,
  `deleted_at` int(11) DEFAULT NULL,
  PRIMARY KEY (`id`)
) ENGINE=MyISAM AUTO_INCREMENT=4 DEFAULT CHARSET=utf8;
<?php

class Model_Rrhh_Busqueda extends \Orm\Model_Soft
{
    protected static $_properties = array(
        'id',
        'nombre',
        'resumen',
        'descripcion',
        'url',
        'orden',
        'activo',
        'created_at',
        'updated_at',
        'deleted_at',
    );

    protected static $_table_name = "rrhh_busquedas";

    protected static $_has_many = array(
        "envios" => array(
            "model_to" => "Model_Rrhh_Busquedas_Envio",
            "cascade_delete" => true,
        ),
    );

    protected static $_soft_delete = array(
        "mysql_timestamp" => false
    );

    protected static $_observers = array(
        'Orm\Observer_CreatedAt' => array(
            'events' => array('before_insert'),
            'mysql_timestamp' => false,
        ),
        'Orm\Observer_UpdatedAt' => array(
            'events' => array('before_save'),
            'mysql_timestamp' => false,
        ),
    );

    public static function activar($id) {

        $item = Model_Rrhh_Busqueda::find($id);

        if (!$item)
            throw new Exception("La búsqueda no existe");

        $item->activo = $item->activo ? false : true;
        $item->save();

        return $item->activo ? "desactivar" : "activar";

    }

}
<?php

class Model_Rrhh_Busquedas_Envio extends \Orm\Model_Soft
{
    protected static $_properties = array(
        'id',
        'rrhh_busqueda_id',
        'nyap',
        'email',
        'telefono',
        'url',
        'cv',
        'fecha',
        'created_at',
        'updated_at',
        'deleted_at',
    );

    protected static $_table_name = "rrhh_busquedas_envios";

    protected static $_belongs_to = array(
        "busqueda" => array(
            "model_to" => "Model_Rrhh_Busqueda",
        ),
    );

    protected static $_soft_delete = array(
        "mysql_timestamp" => false
    );

    protected static $_observers = array(
        'Orm\Observer_CreatedAt' => array(
            'events' => array('before_insert'),
            'mysql_timestamp' => false,
        ),
        'Orm\Observer_UpdatedAt' => array(
            'events' => array('before_save'),
            'mysql_timestamp' => false,
        ),
    );

    public static function activar($id) {

        $item = Model_Admin::find($id);

        if (!$item)
            throw new Exception("El administrador no existe");

        $item->activo = $item->activo ? false : true;
        $item->save();

        return $item->activo ? "desactivar" : "activar";

    }

}
emlynwest commented 11 years ago

I am sorry but I am still not able to reproduce this error. Are you sure you have the latest version of the orm?

renoguyon commented 11 years ago

I'm facing exactly the same issue!

In addition, when I try to delete an item which has a relation (eg, a belongs_to relation), I systematically get an error like this (even with cascade delete set to false):

Notice!

Fuel\Core\PhpErrorException [ Notice ]: Trying to get property of non-object PKGPATH/orm/classes/belongsto.php @ line 159

Thanks for your help!

emlynwest commented 11 years ago

I'll try and have a look into this more over the weekend then. Work has me pretty busy right now.

@renoguyon Thanks for the feedback, what version of the orm are you using? Lots of issues with belongs_to have been resolved recently.

renoguyon commented 11 years ago

Hi @stevewest I was using 1.6/master, the issue with belongs_to is now solve since I have upgraded to 1.7/develop.

Regarding the soft delete query results, I did a little comparison between 1.5 and 1.6, and found a way to solve the problem.

Let's have a look at orm/classes/model/soft.php

In 1.5.3 there was a condition to add a where clause to the ORM query if "filter status" is true:

public static function query($options=array())
{
    if (static::get_filter_status())
    {
        //Make sure we are filtering out soft deleted items
        $deleted_column = static::soft_delete_property('deleted_field', static::$_default_field_name);
        $options['where'][] = array($deleted_column, null);
    }

    return parent::query($options);
}

In 1.6, we are calling a Query_Soft::set_soft_filter() method which allow to modify join results but does not seem to modify where clause.

public static function query($options = array())
{
    $query = Query_Soft::forge(get_called_class(), static::connection(), $options);

    if (static::get_filter_status())
    {
        //Make sure we are filtering out soft deleted items
        $query->set_soft_filter(static::soft_delete_property('deleted_field', static::$_default_field_name));
    }

    return $query;
}

A way to solve the issue is to add a where condition like:

public static function query($options = array())
{
    $query = Query_Soft::forge(get_called_class(), static::connection(), $options);

    if (static::get_filter_status())
    {
        //Make sure we are filtering out soft deleted items
        $deleted_column = static::soft_delete_property('deleted_field', static::$_default_field_name);
        $query->set_soft_filter($deleted_column);
        $query->where(array($deleted_column, null));
    }

    return $query;
}
emlynwest commented 11 years ago

That should do it. Let me know if you have any more problems.

mikelexp commented 11 years ago

Thanks Steve! Until 1.7 is stable, I'll be using @renoguyon 's fix on the 1.6 branch.

emlynwest commented 11 years ago

@mikelexp 1.7 is as stable as 1.6 at the moment. There's no new feature so only extra bug fixes are in 1.7