Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: haveInDatabase tear down use row values if primary key values are available #44

Merged

Conversation

JesusTheHun
Copy link
Contributor

Right now Db::addInsertedRow() uses the lastInsertId to remove the row during tear down.
In some case the primary key is not auto incremented but another column is.

// auto_increment_not_on_pk : [id: int, counter: int auto_increment]
$I->haveInDatabase('auto_increment_not_on_pk', ['id' => 777]);
// created row : [id: 777, counter: 1]

The row being referenced with ['id' => 1] because $driver->lastInsertId() returns 1, and the module assumes it's the PK value.

This PR fixes this behaviour by checking if the provided values cover the primary key. If so, it uses those value, if not it follows the default behaviour.

@JesusTheHun JesusTheHun changed the base branch from master to 2.x December 1, 2022 18:40
Copy link
Member

@Arhell Arhell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Naktibalda
Copy link
Member

Thank you

@Naktibalda Naktibalda merged commit d0554b7 into Codeception:2.x Dec 3, 2022
@YaakovR
Copy link

YaakovR commented Dec 6, 2022

@JesusTheHun What about using haveInDatabase on a table that does not have auto increment on any column?
Currently, it gives this error:
[TypeError] Codeception\Lib\Driver\Db::lastInsertId(): Return value must be of type string, bool returned

@Naktibalda
Copy link
Member

yes, it is a bug, please raise pull request to fix it.

@JesusTheHun
Copy link
Contributor Author

@YaakovR can you tell me what db you are using ? I tried with MySQL which (sadly) returns 0 when no id has been generated.

@YaakovR
Copy link

YaakovR commented Dec 8, 2022

@JesusTheHun SQL Server

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants