codeigniter4 / CodeIgniter4

Open Source PHP Framework (originally from EllisLab)
https://codeigniter.com/
MIT License
5.38k stars 1.9k forks source link

save() method trying to insert instead of update #1770

Closed nowackipawel closed 5 years ago

nowackipawel commented 5 years ago

This is model

<?php

class TransactionModel extends \CodeIgniter\Model
{
    protected $DBGroup = DB_ZTN_GROUP;
    protected $table = 't_transaction';
    protected $primaryKey = 'tra_id';
    protected $returnType = 'App\Entities\Transaction\Transaction';

    protected $allowedFields = [
        'tra_usr_id',
        'tra_pap_id',
        'tra_sep_id',
        'tra_dsp_id',
        'tra_identifier',
        'tra_is_paid',
        'tra_is_active',
        'tra_amount',
        'tra_currency'
    ];

    protected $validationRules = [
        'tra_usr_id'        => 'permit_empty|is_natural_no_zero|less_than_equal_to[16777215]',
        'tra_pap_id'        => 'required|is_natural_no_zero|less_than_equal_to[255]',
        'tra_sep_id'        => 'required_without[tra_dsp_id]|is_natural_no_zero|less_than_equal_to[255]',
        'tra_dsp_id'        => 'required_without[tra_sep_id]|is_natural_no_zero|less_than_equal_to[65535]',
        'tra_identifier'    => 'required|exact_length[16]|is_unique[t_transaction.tra_identifier,tra_identifier,{tra_identifier}]',
        'tra_is_paid'       => 'required|in_list[0,1]',
        'tra_is_active'     => 'required|in_list[0,1]',
        'tra_amount'        => 'required|decimal|greater_than[0]',
        'tra_currency'      => 'required|regex_match[/[A-Z]{3}/]'
    ];

    public function save($data)
    {
        try
        {
            parent::save($data); // TODO: Change the autogenerated stub
        }
        catch (\mysqli_sql_exception $e)
        {
            return $this->db->getLastQuery(); // this is printing INSERT!!! ?
        }
    }
}

Entity:

<?php namespace App\Entities\Transaction;

use CodeIgniter\Entity;

class Transaction extends Entity
{
    protected $tra_id;
    protected $tra_usr_id;
    protected $tra_pap_id;
    protected $tra_sep_id;
    protected $tra_dsp_id;
    protected $tra_identifier; //wewn.
    protected $tra_is_paid;
    protected $tra_is_active;
    protected $tra_amount; //decimal 7,2
    protected $tra_currency;
    protected $tra_created_at;
    protected $tra_updated_at;

    protected $_options = [
        'casts'   => [
            'tra_id'        => 'integer',
            'tra_usr_id'    => '?integer',
            'tra_pap_id'    => 'integer',
            'tra_sep_id'    => '?integer',
            'tra_dsp_id'    => '?integer',
            'tra_identifier'    => 'string',
            'tra_is_paid'   => 'boolean',
            'tra_is_active' => 'boolean',
            'tra_amount'    => 'double',
            'tra_currency'  => 'string'
        ],
        'dates'   => ['tra_created_at', 'tra_updated_at'],
        'datamap' => [],
    ];

    public function setTraPapId($value)
    {
        switch ($value)
        {
            case 'dotpay':
                $this->tra_pap_id = 10;
                break;
        }

        if(ctype_digit(strval($value)))
            $this->tra_pap_id = (int)$value;
    }
}

And TestCommand (spark)

<?php namespace App\Commands;

use CodeIgniter\CLI\BaseCommand;
use CodeIgniter\CLI\CLI;

class TestCommand extends BaseCommand
{
    protected $group       = 'test';
    protected $name        = 'test:command';
    protected $description = 'Test Command';

    public function run(array $params)
    {

        $transaction_model = (new \TransactionModel());
        $transaction = $transaction_model->where('tra_identifier', 'S.6H-3586729-756')->get()->getFirstRow($transaction_model->returnType);

        d($transaction);

        $transaction->tra_is_paid = $transaction->tra_is_paid ? 0 : 1; //false if true... true if false
        $transaction->tra_is_active = $transaction->tra_is_active ? 0 : 1; //false if true... true if false

        print $transaction_model->save($transaction) . PHP_EOL;
        print 'primaryKEY: ' . $transaction_model->primaryKey . ' is: ' . $transaction->{$transaction_model->primaryKey};
        d($transaction);

    }
}

and its output:

CodeIgniter CLI Tool - Version 4.0.0-alpha.5 - Server-Time: 2019-02-28 19:01:07pm

┌──────────────────────────────────────────────────────────────────────────────────────────────────────┐
│ $transaction                                                                                         │
└──────────────────────────────────────────────────────────────────────────────────────────────────────┘
App\Entities\Transaction\Transaction (15) (
    protected 'tra_amount' -> string (6) "196.00"
    protected 'tra_created_at' -> string (19) "2019-02-28 15:17:46"
    protected 'tra_currency' -> string (3) "PLN"
    protected 'tra_dsp_id' -> string (3) "143"
    protected 'tra_id' -> string (3) "121"
    protected 'tra_identifier' -> string (16) "S.6H-3586729-756"
    protected 'tra_is_active' -> string (1) "0"
    protected 'tra_is_paid' -> string (1) "1"
    protected 'tra_pap_id' -> string (2) "10"
    protected 'tra_sep_id' -> null
    protected 'tra_updated_at' -> string (19) "2019-02-28 18:42:34"
    protected 'tra_usr_id' -> string (4) "1176"
    protected '_options' -> array (3) [
        'casts' => array (10) [
            'tra_id' => string (7) "integer"
            'tra_usr_id' => string (8) "?integer"
            'tra_pap_id' => string (7) "integer"
            'tra_sep_id' => string (8) "?integer"
            'tra_dsp_id' => string (8) "?integer"
            'tra_identifier' => string (6) "string"
            'tra_is_paid' => string (7) "boolean"
            'tra_is_active' => string (7) "boolean"
            'tra_amount' => string (6) "double"
            'tra_currency' => string (6) "string"
        ]
        'dates' => array (2) [
            0 => string (14) "tra_created_at"
            1 => string (14) "tra_updated_at"
        ]
        'datamap' => array (0) []
    ]
    protected '_original' -> array (12) [
        'tra_id' => string (3) "121"
        'tra_usr_id' => string (4) "1176"
        'tra_pap_id' => string (2) "10"
        'tra_sep_id' => null
        'tra_dsp_id' => string (3) "143"
        'tra_identifier' => string (16) "S.6H-3586729-756"
        'tra_is_paid' => string (1) "1"
        'tra_is_active' => string (1) "0"
        'tra_amount' => string (6) "196.00"
        'tra_currency' => string (3) "PLN"
        'tra_created_at' => string (19) "2019-02-28 15:17:46"
        'tra_updated_at' => string (19) "2019-02-28 18:42:34"
    ]
    private '_cast' -> boolean true
)
════════════════════════════════════════════════════════════════════════════════════════════════════════
Called from .../app/Commands/TestCommand.php:19 [App\Commands\TestCommand->run()]
INSERT INTO `t_transaction` (`tra_is_paid`, `tra_is_active`) VALUES (0, 1)
primaryKEY: tra_id is: 121
┌──────────────────────────────────────────────────────────────────────────────────────────────────────┐
│ $transaction                                                                                         │
└──────────────────────────────────────────────────────────────────────────────────────────────────────┘
App\Entities\Transaction\Transaction (15) (
    protected 'tra_amount' -> string (6) "196.00"
    protected 'tra_created_at' -> string (19) "2019-02-28 15:17:46"
    protected 'tra_currency' -> string (3) "PLN"
    protected 'tra_dsp_id' -> string (3) "143"
    protected 'tra_id' -> string (3) "121"
    protected 'tra_identifier' -> string (16) "S.6H-3586729-756"
    protected 'tra_is_active' -> integer 1
    protected 'tra_is_paid' -> integer 0
    protected 'tra_pap_id' -> string (2) "10"
    protected 'tra_sep_id' -> null
    protected 'tra_updated_at' -> string (19) "2019-02-28 18:42:34"
    protected 'tra_usr_id' -> string (4) "1176"
    protected '_options' -> array (3) [
        'casts' => array (10) [
            'tra_id' => string (7) "integer"
            'tra_usr_id' => string (8) "?integer"
            'tra_pap_id' => string (7) "integer"
            'tra_sep_id' => string (8) "?integer"
            'tra_dsp_id' => string (8) "?integer"
            'tra_identifier' => string (6) "string"
            'tra_is_paid' => string (7) "boolean"
            'tra_is_active' => string (7) "boolean"
            'tra_amount' => string (6) "double"
            'tra_currency' => string (6) "string"
        ]
        'dates' => array (2) [
            0 => string (14) "tra_created_at"
            1 => string (14) "tra_updated_at"
        ]
        'datamap' => array (0) []
    ]
    protected '_original' -> array (12) [
        'tra_id' => string (3) "121"
        'tra_usr_id' => string (4) "1176"
        'tra_pap_id' => string (2) "10"
        'tra_sep_id' => null
        'tra_dsp_id' => string (3) "143"
        'tra_identifier' => string (16) "S.6H-3586729-756"
        'tra_is_paid' => string (1) "1"
        'tra_is_active' => string (1) "0"
        'tra_amount' => string (6) "196.00"
        'tra_currency' => string (3) "PLN"
        'tra_created_at' => string (19) "2019-02-28 15:17:46"
        'tra_updated_at' => string (19) "2019-02-28 18:42:34"
    ]
    private '_cast' -> boolean true
)
════════════════════════════════════════════════════════════════════════════════════════════════════════
Called from .../app/Commands/TestCommand.php:26 [App\Commands\TestCommand->run()]

I am wondering why save is trying to produce INSERT instead of UPDATE .. WHERE tra_id = 121 ?

lonnieezell commented 5 years ago

Have you tried debugging it in the Model's save() command yet? I can't recreate your app for test purposes, and we have tests around that which seems to save fine. If you can debug on your end and determine if something actually is a bug at the framework level then I can actually help.

There's only 2 things I can think of that might be going wrong at the framework level: Either It's not including tra_id when it converts it from an entity to an array, or the value of tra_id you're passing in evaluates as empty.

But what you have here isn't really a bug report, it's an ask for help, which should be asked on the forums. If you can confirm it's a bug with the framework, please let us know.

nowackipawel commented 5 years ago

There was a bug in in toRawArray() in this part:

            if ($onlyChanged && ! $this->hasPropertyChanged($key, $value))
            {
                continue;
            }

if tra_id didn't change then it wasnt avaibale in properties which were returned by toRawArray .

nowackipawel commented 5 years ago

@lonnieezell : test what you are pointing out are not the best example for this case; they are created only for \stdClass or arrays ; there are no test which are utilizing entity toRawArray method. I hope my fix wont break that ones... but we will see. I think in PR#1772 there is less assumptions that do not have to be true. In fact, there is only one that I assumed that before inserting value of entity primaryKey was null.

lonnieezell commented 5 years ago

Hm. Thought I had something in place that would always include the primary key. Will check that. Thanks for debugging further.

atishhamte commented 5 years ago

I guess, the issue is fixed with #1829.

lonnieezell commented 5 years ago

It is grabbing the primary key when using entities and toRawArray, and this test proves it's working I believe.