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

Fixing model observers integration #74

Merged
merged 4 commits into from
Dec 11, 2024
Merged

Conversation

yormy
Copy link
Contributor

@yormy yormy commented Oct 15, 2024

When using model observers saved feature, the old dirty flags were reset to nothing. So you were not able to retrieve any old or updated values.

Sometimes this is needed to do some action after saving a changed model

@freekmurze
Copy link
Member

Could you add a test for this?

@yormy
Copy link
Contributor Author

yormy commented Dec 11, 2024

Added the testcase

@freekmurze freekmurze merged commit 481b95a into spatie:main Dec 11, 2024
6 checks passed
@freekmurze
Copy link
Member

Thanks!

@felabrecque
Copy link
Contributor

felabrecque commented Dec 19, 2024

@freekmurze @yormy

Please revert this change.

Loading data from the database will now always have all the encrypted fields marked as dirty.

$model->getDirty() will return them just after loading them from the database.

It's because $model->getRawOriginal() now returns the encrypted data, instead of the decrypted one.

I understand that it might be nice to be able to see the encrypted data in the model, but the model original data should contain the decrypted database data.

My suggestion is to add a new method in the UseCipherSweet trait to get the encrypted data.
And store them in a new variable at decryption time.

Not good !

@freekmurze
Copy link
Member

@yormy could you take a look at this?

@felabrecque If you need a solution fast, I'm open for a PR that fixes this.

@yormy
Copy link
Contributor Author

yormy commented Jan 6, 2025

It seems that ANY change to a record that has encrypted fields mark the fields as dirty, also in the master before my change. It might be a feature to re-encrypt everything on saving, i am not sure about that details of implementation. Can anyone comment ?

felabrecque added a commit to felabrecque/laravel-ciphersweet that referenced this pull request Jan 6, 2025
Fix Model getChanges()
freekmurze added a commit that referenced this pull request Jan 17, 2025
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.

4 participants