Skip to content

Fix AuditCustom event not respecting audit.enabled and audit.console #1048

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.

Sign up for GitHub

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

Closed
rrodrigofranco wants to merge 3 commits into from
Closed

Fix AuditCustom event not respecting audit.enabled and audit.console #1048

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c89896d
Fix AuditCustom event not respecting audit.enabled and audit.console …
rrodrigofranco Mar 26, 2026
7378cf8
Avoid breaking change by using method_exists instead of adding isAudi…
rrodrigofranco Mar 26, 2026
55a7178
Check isAuditingEnabled in readyForAuditing to fix all manual dispatc…
rrodrigofranco Mar 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/Listeners/RecordCustomAudit.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ class RecordCustomAudit
{
public function handle(AuditCustom $event): void
{
if (method_exists($event->model, 'isAuditingEnabled') && ! $event->model::isAuditingEnabled()) {
return;
}

Auditor::execute($event->model);
}
}
35 changes: 35 additions & 0 deletions tests/Functional/AuditingTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -946,6 +946,41 @@ public function test_can_audit_any_custom_event(): void
]);
}

public function test_custom_event_does_not_audit_when_auditing_is_disabled(): void
{
config(['audit.enabled' => false]);

$article = Article::factory()->create();
$article->auditEvent = 'whateverYouWant';
$article->isCustomEvent = true;
$article->auditCustomOld = ['customExample' => 'Anakin Skywalker'];
$article->auditCustomNew = ['customExample' => 'Darth Vader'];

$auditCountBefore = Audit::where('auditable_type', Article::class)->count();

Event::dispatch(new AuditCustom($article));

$this->assertSame($auditCountBefore, Audit::where('auditable_type', Article::class)->count());
}

public function test_custom_event_does_not_audit_when_running_in_console_without_console_flag(): void
{
App::shouldReceive('runningInConsole')->andReturn(true);
config(['audit.enabled' => true, 'audit.console' => false]);

$article = Article::factory()->create();
$article->auditEvent = 'whateverYouWant';
$article->isCustomEvent = true;
$article->auditCustomOld = ['customExample' => 'Anakin Skywalker'];
$article->auditCustomNew = ['customExample' => 'Darth Vader'];

$auditCountBefore = Audit::where('auditable_type', Article::class)->count();

Event::dispatch(new AuditCustom($article));

@erikn69 erikn69 Mar 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

presumably this needs to check self::isAuditingEnabled() because the other two disabled flags aren't related to the config?

As I recall, that method was only for boot; it shouldn't be used at runtime. For runtime, there are $auditingDisabled and $auditingGloballyDisabled.

I can see the problem, but it would also happen in regular audits, since the event is being triggered manually.

Event::dispatch(new \OwenIt\Auditing\Events\DispatchAudit($article));

I think it needs a different approach, this has never been tested and apparently has always been present

@rrodrigofranco rrodrigofranco Mar 26, 2026 •
edited by erikn69
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@erikn69 you're right, the same issue exists for DispatchAudit when dispatched manually. The root cause is that readyForAuditing() doesn't check the config, it only checks the runtime flags.

Would the preferred fix be to add the config check inside readyForAuditing() itself? Something like:

public function readyForAuditing(): bool
{
    if (static::$auditingDisabled || Models\Audit::$auditingGloballyDisabled) {
        return false;
    }

    if (App::runningInConsole() && ! Config::get('audit.console', false)) {
        return false;
    }

    if (! Config::get('audit.enabled', true)) {
        return false;
    }
    // ...
}

That would fix both AuditCustom and DispatchAudit in a single place, rather than patching each listener individually.

@erikn69 erikn69 Mar 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As I recall, that method was only for boot; it shouldn't be used at runtime

Try #1050

@rrodrigofranco rrodrigofranco Mar 26, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@erikn69 Thanks for the guidance and for opening #1050 with an alternative approach. I'll close this PR in favor of yours.


$this->assertSame($auditCountBefore, Audit::where('auditable_type', Article::class)->count());
}

public function test_can_audit_custom_audit_model_implementation(): void
{
$audit = null;
Expand Down