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

Allow overriding the CheckFailedNotification class #102

Closed
wants to merge 1 commit into from

Conversation

moisish
Copy link
Contributor

@moisish moisish commented Jun 29, 2022

At the moment you can change the config file notifications.notifications but the class is harcoded on the RunHealthChecksCommand command and it's not possible to use your own

@freekmurze
Copy link
Member

Using the first key seems a bit dirty to me. In the latest version, I've made it easy to override the notification class name to use.

@freekmurze freekmurze closed this Jun 29, 2022
@moisish
Copy link
Contributor Author

moisish commented Jun 30, 2022

Hello @freekmurze

Since you are using an array on the config file, it's the only way to use array_first.

I can see you added a method getFailedNotificationClass but that means we need to override the RunHealthChecksCommand class.
Why do we defined the class on the config file if you cannot change it in one place?

@moisish
Copy link
Contributor Author

moisish commented Jun 30, 2022

Another solution will be to change the config file and split the class and channels something like

'channels' => [
            'mail',
            'slack'
        ],
'notification' => Spatie\Health\Notifications\CheckFailedNotification::class

Let me know if this solution is acceptable and I will submit a new PR

@Nonslas
Copy link

Nonslas commented Sep 2, 2023

I had same issue, should I make new command extend RunHealthChecksCommand to override method getFailedNotificationClass?

@moisish
Copy link
Contributor Author

moisish commented Sep 2, 2023

@freekmurze can we reopen this please?

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