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: Ability to disable auditable #35

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

Conversation

GoatFreezy
Copy link
Contributor

Add an attribute to disable auditable on the fly (like timestamps)
Can be usefull for easier batch or command processing.

@yajra
Copy link
Owner

yajra commented Mar 4, 2025

Thanks for the contribution. I think this is already covered in the framework via https://laravel.com/docs/11.x/eloquent#saving-a-single-model-without-events?

@GoatFreezy
Copy link
Contributor Author

Hello, sorry for the late answer.

Yes it's globally covered by the framework, but if you want to still use other events and only prevent auditable events to trigger it might come handy as much as $timestamps allows it and doesn't stop other events from being fired.

I had an issue with php stan which I discovered here and I can't find a way to make him understand that my attribute does exists

@yajra
Copy link
Owner

yajra commented Mar 21, 2025

@GoatFreezy got your point and makes sense to me now. Can you please resolve the conflicts?

In addition, the auditable property might cause a potential breaking change since some columns might be named like it. Maybe we should use something else or maybe similar to how the framework does it.

Model::withoutAudits(fn() => ...)

@GoatFreezy GoatFreezy force-pushed the feat/disable-auditable branch from 5fd30e7 to 9a6c92a Compare March 24, 2025 22:46
@GoatFreezy
Copy link
Contributor Author

Hello,
i've resolved the conflict and tried to implement a method withoutAudits()
I still have issue with phpstan no resolving my method can you help me out ?

@obyajra
Copy link

obyajra commented Mar 25, 2025

I think the issue with static analysis will be fixed via generics. We can work on it on another PR.

I will review the changes as soon as I can. Thanks!

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.

3 participants