Skip to content

Fix update method. #10

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix update method. #10

wants to merge 2 commits into from

Conversation

chriscarpenter12
Copy link

I noticed my update method wasn't working either. It was updating the main table, but erroring out on the versioned table. Using a duplicate key of 1 for every update no matter how many versions. Looks like getVersionColumn() needed to be changed to getLatestVersionColumn().

I also added code that fetches the old version row and merges in the new updates. The way the old code was wrote it would only insert a versioned row with just the updated values, and null out the existing data. I believe this was the desired behavior.

@markusjwetzel
Copy link
Contributor

All data for updating is present in the Eloquent model. Why do you need an additional database call?

@chriscarpenter12
Copy link
Author

It has access to the data in the main table, but it does not contain the data for the version table. $affectedRecords = $this->getAffectedRecords(); only returns columns in the main table. That is why I added the additional query.

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.

2 participants