-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Adds a command to resend acceptance emails #14722
Conversation
PR Summary
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some requests, but this is a great start :)
} else { | ||
Notification::send( | ||
$unacceptedAsset['acceptance']->assignedTo, | ||
new CheckoutAssetNotification($unacceptedAsset['assetItem'], $unacceptedAsset['acceptance']->assignedTo, $unacceptedAsset['assetItem']->adminuser, $unacceptedAsset['acceptance'], '') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see what you're doing here ;) And it's good! This could end up sending a shitload of emails to the same user since this is a new alert tho, and they could have 300 unaccepted things. I feel like a new blade with a rollup would make more product sense, and please also revisit the original issue to make sure that the user-reminder is the actual ask, versus admin notification and/or both.
@Godmartinz three merits deducted for pushing code during a 1:1 ;) |
😆 I lost track of time! |
@snipe few fixes added, going to tackle the new notification instance and then should be good to go 🤞 |
Nice idea, looking forward to when this comes around :) |
@Godmartinz was this intended to be targeted to master btw? |
I think we may actually want to rethink this actually. If we do this right, we can solve several birds with one stone. We'll talk more about it in the dev meeting or on slack. |
@snipe as per the meeting, this was the PR you were referring to yes? |
I think this still could use some polish, but it's better than what we have... :) |
Description
This adds an artisan command option
snipeit:acceptance-reminder
to allow you to resend the acceptance email to all listed on the unaccepted assets report.Fixes #11756 [sc-25560]
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Checklist: