Behaviour of getInsertID() when using save()

When using the CI Model for basic CRUD, does `getInsertID()` return null when a preceding `save()` did an UPDATE rather than INSERT? Or does it return the affected primary key regardless? This detail would be a great addition to that method's documentation entry.

getInsertId() is in system/BaseModel.php

This is all that I can find on it.

PHP Code:

    /**
    * Returns last insert ID or 0.
    *
    * @return int|string
    */
    public function getInsertID()
    {
        return is_numeric($this->insertID) ? (int) $this->insertID : $this->insertID;
    } 


What did you Try? What did you Get? What did you Expect?

Joined CodeIgniter Community 2009.  ( Skype: insitfx )

getInsertId() is in system/BaseModel.php

This is all that I can find on it.

PHP Code:

    /**
    * Returns last insert ID or 0.
    *
    * @return int|string
    */
    public function getInsertID()
    {
        return is_numeric($this->insertID) ? (int) $this->insertID : $this->insertID;
    } 


That function is just a getter for the BaseModel property. The property is set when Model::doInsert() actually performs a query:

PHP Code:

// If insertion succeeded then save the insert ID
        if ($result) {
            $this->insertID = ! $this->useAutoIncrement ? $row[$this->primaryKey] : $this->db->insertID();
        } 


Unfortunately, Model::update() does not set this property:

PHP Code:

public function update($id = null, $row = null): bool
    {
        if (is_bool($id)) {
            throw new InvalidArgumentException('update(): argument #1 ($id) should not be boolean.');
        }

        if (is_numeric($id) || is_string($id)) {
            $id = [$id];
        }

        $row = $this->transformDataToArray($row, 'update');

        // Validate data before saving.
        if (! $this->skipValidation && ! $this->validate($row)) {
            return false;
        }

        // Must be called first, so we don't
        // strip out updated_at values.
        $row = $this->doProtectFields($row);

        // doProtectFields() can further remove elements from
        // $row, so we need to check for empty dataset again
        if ($row === []) {
            throw DataException::forEmptyDataset('update');
        }

        $row = $this->setUpdatedField($row, $this->setDate());

        $eventData = [
            'id'  => $id,
            'data' => $row,
        ];

        if ($this->tempAllowCallbacks) {
            $eventData = $this->trigger('beforeUpdate', $eventData);
        }

        $eventData = [
            'id'    => $id,
            'data'  => $eventData['data'],
            'result' => $this->doUpdate($id, $eventData['data']),
        ];

        if ($this->tempAllowCallbacks) {
            $this->trigger('afterUpdate', $eventData);
        }

        $this->tempAllowCallbacks = $this->allowCallbacks;

        return $eventData['result'];
    } 


And if we look at Model:ave(), it's dumb to the primary key ID altogether. 

PHP Code:

public function save($row): bool
    {
        if ((array) $row === []) {
            return true;
        }

        if ($this->shouldUpdate($row)) {
            $response = $this->update($this->getIdValue($row), $row);
        } else {
            $response = $this->insert($row, false);

            if ($response !== false) {
                $response = true;
            }
        }

        return $response;
    } 


I guess the answer to my question is that the behaviour is inconsistent. The $this->insertID is only set when save() does an insert. I suppose it makes sense when thinking about the update() and insert() functions being called explicitly (if updating, one knows the ID already), but the point of the helper is to be able to shove a record into it without having to determine if it's new or not. If I have to write the logic to determine if an ID exists, a pretty common need for performing further inserts that use it as a FK, it defeats the purpose of save().

How opposed would we be to having update() save the affected ID so that save() works better? I can PR if there is no reason not to...

I have to agree with you on the inconsistent nature of it.

A framework should be consistent through out.

I would go to GitHub and put in an issue on it and see if they would like to take your PR.

What did you Try? What did you Get? What did you Expect?

Joined CodeIgniter Community 2009.  ( Skype: insitfx )