PC-612 send email poll checking referral follow ups #231
Conversation
…unction to try triggering email.
…for followUpLink automatically (env dependent) and update views/pages based on client feedback.
…are the same entity
CalPinSW
changed the base branch from
develop
to
PC-611-check-referral-follow-up
HerPublicWebsite.BusinessLogic/Services/ReferralFollowUpManager/ReferralFollowUpService.cs
Outdated
Show resolved
Hide resolved
HerPublicWebsite.BusinessLogic/Services/RegularJobs/ReferralFollowUpJobService.cs
Outdated
Show resolved
Hide resolved
HerPublicWebsite.BusinessLogic/Models/ReferralRequestFollowUp.cs
Outdated
Show resolved
Hide resolved
HerPublicWebsite.BusinessLogic/Services/ReferralFollowUpManager/GuidService.cs
Show resolved
Hide resolved
HerPublicWebsite/Controllers/ReferralRequestFollowUpController.cs
Outdated
Show resolved
Hide resolved
HerPublicWebsite/Views/ReferralRequestFollowUp/ReferralRequestFollowUpAlreadyResponded.cshtml
Outdated
Show resolved
Hide resolved
| <a href="@Url.Action(nameof(StaticPagesController.Index), "StaticPages")" class="govuk-link">speak to an advisor</a> for help. | ||
| </p> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Have we agreed this copy? It reads a little oddly to me, but not sure if we have UX resource to agree this with
There was a problem hiding this comment.
Not yet, We originally had copy for this to all be sent in an email, when it was realised this wasn't possible I put these pages together with the aim to show to the client and get some feedback on.
I have screenshots of the pages with their copy on the Jira ticket, which Gareth has taken a look at, but we're still waiting for full confirmation that they are happy with this approach in general.
…ollowUps from manager, then send email
…tch other functions
…int, simplify routes
| var newReferrals = await dataProvider.GetReferralRequestsWithNoFollowUpBeforeDate(endDate); | ||
| foreach (var newReferral in newReferrals) { | ||
| var referralRequestFollowUp = await referralFollowUpManager.CreateReferralRequestFollowUp(newReferral); | ||
| emailSender.SendFollowUpEmail(newReferral, config.AppBaseUrl + "referral-follow-up?token=" + referralRequestFollowUp.Token); |
There was a problem hiding this comment.
Not sure if there's a way we should be doing this without a hardcoded route?
There was a problem hiding this comment.
Hmm - there is if you have an httpcontext, but you won't have access to that here. It'd be nicer to construct this URL using proper methods (e.g. with escaping) but this is fine - we know the token won't contain any special characters.
There was a problem hiding this comment.
I've done a bit more to improve this. But let me know if you want me to change anything else.
| "Attempted to send follow up email with invalid custodian code \"{CustodianCode}\"", | ||
| referralRequest.CustodianCode | ||
| ); | ||
| throw new ArgumentOutOfRangeException |
There was a problem hiding this comment.
What will happen if we throw an error here - will anyone notice? It'll stop this and any other emails being sent.
I suspect the best we can do is just log an error and maybe use a default value instead (even just empty string).
There was a problem hiding this comment.
Yeah good point. I think it would cause issues with other emails being sent. I'll log it but I think we should just skip the email in that case. I think it may look a bit strange in an email.
On consideration, I can't see a situation where this is likely to occur since we're using a referral request generated with an existing custodian code to index the LAs.
| var newReferrals = await dataProvider.GetReferralRequestsWithNoFollowUpBeforeDate(endDate); | ||
| foreach (var newReferral in newReferrals) { | ||
| var referralRequestFollowUp = await referralFollowUpManager.CreateReferralRequestFollowUp(newReferral); | ||
| emailSender.SendFollowUpEmail(newReferral, config.AppBaseUrl + "referral-follow-up?token=" + referralRequestFollowUp.Token); |
There was a problem hiding this comment.
Hmm - there is if you have an httpcontext, but you won't have access to that here. It'd be nicer to construct this URL using proper methods (e.g. with escaping) but this is fine - we know the token won't contain any special characters.
...bsite.BusinessLogic/Services/RegularJobs/ReferralFollowUpNotificationServiceConfiguration.cs
Outdated
Show resolved
Hide resolved
HerPublicWebsite.BusinessLogic/Services/ReferralFollowUpManager/ReferralFollowUpService.cs
Outdated
Show resolved
Hide resolved
… Give more useful log identification.
… follow up emails to referral requests submitted after that date.
CalPinSW
deleted the
PC-612-send-email-poll-checking-referral-follow-ups
branch
This isn't ready to merge yet, but want to get some eyes on. In particular, Need to work out how to include the base route of a controller/action, when calling from a function not inside a controller.
If clicking the email link a second time:
