snipe / snipe-it

A free open source IT asset/license management system
https://snipeitapp.com
GNU Affero General Public License v3.0
10.36k stars 3.06k forks source link

Checkin Notificaton twice #5186

Closed ArcheNoah77 closed 6 years ago

ArcheNoah77 commented 6 years ago

Expected Behavior

When i checked out an asset to a user he should get an notification e-mail.


Actual Behavior

When i checked out an asset to a user, he get an notification e-mail but i get another notification e-mail to our IT Mail-Account. So every time we checked out an asset, we got an e-mail in copy... But only since the update to 4.1.14


Please confirm you have done the following before posting your bug report:


Provide answers to these questions:

Please do not post an issue without answering the related questions above. If you have opened a different issue and already answered these questions, answer them again, once for every ticket. It will be next to impossible for us to help you.

https://snipe-it.readme.io/docs/getting-help

ArcheNoah77 commented 6 years ago

bild itam I get this everytim, i checked in or out an asset.

tantonw commented 6 years ago

I'm getting the same thing on checkin, two emails. One is correct, the other is the sample notification ArcheNoah posted. I think the proper email is coming from:

snipe-it/app/Http/Controllers/AssetsController.php

and the strange default email is coming from:

snipe-it/app/Notifications/CheckinNotification.php

snipe commented 6 years ago

Huh. That definitely shouldn't be happening. I'll take a look.

snipe commented 6 years ago

In CheckoutNotification, if you remove this entire block, does that resolve your issue?

/**
     * Get the mail representation of the notification.
     *
     * @param  mixed  $notifiable
     * @return \Illuminate\Notifications\sMessages\MailMessage
     */
    public function toMail($notifiable)
    {
        return (new MailMessage)
                    ->line('The introduction to the notification.')
                    ->action('Notification Action', 'https://laravel.com')
                    ->line('Thank you for using our application!');
    }
snipe commented 6 years ago

Actually, until we fix it properly, you should probably just delete this part:

 return (new MailMessage)
                    ->line('The introduction to the notification.')
                    ->action('Notification Action', 'https://laravel.com')
                    ->line('Thank you for using our application!');

Otherwise it may yell that no mail method was specified.

ArcheNoah77 commented 6 years ago

When i delete the part, the "Checkin Notification" E-Mail doesn't be sent. But when i checkin a asset, i get "Whooops, an error...". I have to reload the page and all is okay.

But the other E-Mail (Copy of the Mail, the User get, when i checked out an asset to him) still be sent to out IT Mail Account....

tantonw commented 6 years ago

Yea, commenting out those lines in CheckinNotification causes a "whoops" on asset checkin:

Trying to get property of non-object

in MailChannel.php (line 71) at HandleExceptions->handleError(8, 'Trying to get property of non-object', '/opt/rh/httpd24/root/var/www/snipeit/vendor/laravel/framework/src/Illuminate/Notifications/Channels/MailChannel.php', 71, array('message' => null))in MailChannel.php (line 71) at MailChannel->buildView(null)in MailChannel.php (line 58) at MailChannel->send(object(Setting), object(CheckinNotification))in NotificationSender.php (line 113) at NotificationSender->sendToNotifiable(object(Setting), '8da41308-f626-42ba-8fbf-ff952c46284f', object(CheckinNotification), 'mail')in NotificationSender.php (line 89) at NotificationSender->sendNow(object(Collection), object(CheckinNotification))in NotificationSender.php (line 64) at NotificationSender->send(object(Collection), object(CheckinNotification))in ChannelManager.php (line 36) at ChannelManager->send(object(Setting), object(CheckinNotification))in RoutesNotifications.php (line 18) at Setting->notify(object(CheckinNotification))in Loggable.php (line 123) at Asset->logCheckin(object(User), '')in AssetsController.php (line 580) at AssetsController->postCheckin(object(AssetCheckinRequest), '53', null) at call_user_func_array(array(object(AssetsController), 'postCheckin'), array(object(AssetCheckinRequest), 'assetId' => '53', null))in Controller.php (line 55) at Controller->callAction('postCheckin', array(object(AssetCheckinRequest), 'assetId' => '53', null))in ControllerDispatcher.php (line 44) at ControllerDispatcher->dispatch(object(Route), object(AssetsController), 'postCheckin')in Route.php (line 203) at Route->runController()in Route.php (line 160) at Route->run()in Router.php (line 572)

--snip--

I also tried just "return (new MailMessage);" but that still sends a mostly empty email in addition to the correct checkin email.

snipe commented 6 years ago

The generic email should no longer be being sent out on checkin - still investigating the duplicate checkout emails