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

[8.x] Customize type column in database notifications using databaseType me… #39811

Conversation

mvaliolahi
Copy link
Contributor

By this pull request, we can customize the type column in the database notifications.

By default, Laravel use class_name($notification) for the type column, and this can be undesirable on the client-side.

class InvoiceNotification extends Notification implements ShouldQueue {

    public function via($notifiable)
    {
        return ['database'];
    }

    public function databaseType() 
    {
        return 'DAILY';
    }

@mvaliolahi mvaliolahi force-pushed the feature/cutomize-notification-type-database-driver branch from cd9b283 to 1d15564 Compare November 29, 2021 12:04
@driesvints
Copy link
Member

driesvints commented Nov 29, 2021

Are you completely sure this actually works? Have you tested this in a real Laravel application? I've been trying to do this myself a couple of times but there was always a snag I ran into. Can't exactly remember which ones though 🤔

@driesvints driesvints changed the title Customize type column in database notifications using databaseType me… [8.x] Customize type column in database notifications using databaseType me… Nov 29, 2021
@mvaliolahi
Copy link
Contributor Author

Yes, I test it, and works for me.

@taylorotwell
Copy link
Member

Thanks for your pull request to Laravel!

Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include.

If possible, please consider releasing your code as a package so that the community can still take advantage of your contributions!

If you feel absolutely certain that this code corrects a bug in the framework, please "@" mention me in a follow-up comment with further explanation so that GitHub will send me a notification of your response.

@mvaliolahi
Copy link
Contributor Author

mvaliolahi commented Nov 29, 2021

@taylorotwell Thanks for the reply, This PR is similar to Relation::enforceMorphMap and I am not convinced!

Notification record in DB is like below:

{
"notifications": [
	{
		"id" : "",
		"type" : "App\\Notifications\\LikeNotification",
		"notifiable_type" : "",
		"notifiable_id" :,
		"data" : "{}",
		"read_at" : ,
		"created_at" : "",
		"updated_at" : ""
	},
]}

@taylorotwell taylorotwell reopened this Nov 29, 2021
@taylorotwell taylorotwell merged commit a8f9d65 into laravel:8.x Nov 29, 2021
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.

None yet

3 participants