codeigniter4 / CodeIgniter4

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

docs: Model property $insertID docs is missing #7440

Closed DhPandya closed 1 year ago

DhPandya commented 1 year ago

PHP Version

8.1

CodeIgniter4 Version

4.3.3

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

Windows

Which server did you use?

apache

Database

No response

What happened?

$insertID doc is missing from the model docs.

Steps to Reproduce

N/A

Expected Output

N/A

Anything else?

No response

kenjis commented 1 year ago

Probably it is better to use getInsertID(). https://codeigniter4.github.io/CodeIgniter4/models/model.html#insert

Why do you think it should be documented? How do you use it?

iRedds commented 1 year ago

I believe this property is pointless and should be removed. When inserting data, the new ID must be returned as part of the data. It is not part of the model, since each new query to the database will change the state of this property.

dd ($user->insert(['name' => 'kenjis']));
// [
//     'name' => 'kenjis',
//     'created_at => '1970-01-01'
//     'id' => 1
// ]
kenjis commented 1 year ago

I don't know the Model should return the inserted record or not. At least, the current Model does return insert ID or bool.

As you say, the property is a bit odd, but the getInsertID() method is clearly documented.

You can retrieve the last inserted row’s primary key using the getInsertID() method. https://codeigniter4.github.io/CodeIgniter4/models/model.html#insert

DhPandya commented 1 year ago

I don't know the Model should return the inserted record or not. At least, the current Model does return insert ID or bool.

As you say, the property is a bit odd, but the getInsertID() method is clearly documented.

You can retrieve the last inserted row’s primary key using the getInsertID() method. https://codeigniter4.github.io/CodeIgniter4/models/model.html#insert

Yes you're absolutely right. The getInsertId() is enough for the id. But when we generate the model using CLI the property $InsertId comes with. But there is no use of this property in the System model as well. So i was checking the document and i didn't find it in the document also.

kenjis commented 1 year ago

I sent a PR to remove $insertID in make:model template.

7443