diff --git a/Core/Resgrid.Model/Repositories/IUserProfilesRepository.cs b/Core/Resgrid.Model/Repositories/IUserProfilesRepository.cs index 41c3d27d4..3514aa2d1 100644 --- a/Core/Resgrid.Model/Repositories/IUserProfilesRepository.cs +++ b/Core/Resgrid.Model/Repositories/IUserProfilesRepository.cs @@ -61,5 +61,20 @@ public interface IUserProfilesRepository: IRepository /// Profiles with SecurityPin and LastUpdated already set. /// The cancellation token. Task UpdateSecurityPinsAsync(IEnumerable profiles, CancellationToken cancellationToken = default); + + /// + /// Updates only the MobileNumber/HomeNumber (and LastUpdated) columns for the supplied profiles + /// as a single batched command. Used by the one-off phone-number normalization sweep. + /// + /// Deliberately bypasses UserProfileService.SaveProfileAsync: that treats any change to a + /// number as the user entering a new one and resets the verification state, clears the + /// verification codes and deletes the SMS chatbot identity links. Rewriting a number into its + /// canonical form is not a change of number, so none of that may fire. + /// + /// Callers are responsible for profile cache eviction. + /// + /// Profiles with MobileNumber, HomeNumber and LastUpdated already set. + /// The cancellation token. + Task UpdatePhoneNumbersAsync(IEnumerable profiles, CancellationToken cancellationToken = default); } } diff --git a/Core/Resgrid.Services/SmsService.cs b/Core/Resgrid.Services/SmsService.cs index 1ed165633..48bae9bbf 100644 --- a/Core/Resgrid.Services/SmsService.cs +++ b/Core/Resgrid.Services/SmsService.cs @@ -19,10 +19,12 @@ public class SmsService : ISmsService private readonly IEmailSender _emailSender; private readonly ISubscriptionsService _subscriptionsService; private readonly ICacheProvider _cacheProvider; + private readonly IPhoneNumberProcesserProvider _phoneNumberProcesser; public SmsService(IUserProfileService userProfileService, IGeoLocationProvider geoLocationProvider, ITextMessageProvider textMessageProvider, IDepartmentSettingsService departmentSettingsService, - IEmailSender emailSender, ISubscriptionsService subscriptionsService, ICacheProvider cacheProvider) + IEmailSender emailSender, ISubscriptionsService subscriptionsService, ICacheProvider cacheProvider, + IPhoneNumberProcesserProvider phoneNumberProcesser) { _userProfileService = userProfileService; _geoLocationProvider = geoLocationProvider; @@ -31,6 +33,50 @@ public SmsService(IUserProfileService userProfileService, IGeoLocationProvider g _emailSender = emailSender; _subscriptionsService = subscriptionsService; _cacheProvider = cacheProvider; + _phoneNumberProcesser = phoneNumberProcesser; + } + + + /// + /// The number to hand a direct provider send (Twilio/SignalWire), in E.164. + /// + /// Not : that returns the display form, which strips the + /// leading "+" while keeping the country code. Twilio receives the value verbatim, so the result + /// is not a valid 'To' - fatal for any non-US number, which has no meaning at all without its + /// "+". CommunicationTestService already normalizes off the raw profile number for the same + /// reason; this is the same treatment for the dispatch path. + /// + /// + private string ResolveDirectSendNumber(UserProfile profile) + { + var processed = _phoneNumberProcesser?.Process(profile?.MobileNumber); + + if (processed != null && processed.IsValid && !string.IsNullOrWhiteSpace(processed.InternationalNumber)) + return processed.InternationalNumber; + + // Unparseable: fall back to the previous behaviour rather than dropping the message. The + // provider logs and swallows an invalid 'To', which is what happened before this too. + return profile?.GetPhoneNumber(); + } + + /// + /// The number to embed in a carrier SMS-gateway address, in national form. + /// + /// The CarriersMap templates ("{0}@vtext.com", "{0}@txt.att.net") are US carrier gateways and + /// expect the bare 10-digit number. A stored E.164 value run through + /// yields 11 digits with the leading country code, which + /// addresses a mailbox that does not exist. Every non-US carrier in the map is a direct-send + /// carrier, so the national form is the right one here. + /// + /// + private string ResolveGatewayNumber(UserProfile profile) + { + var processed = _phoneNumberProcesser?.Process(profile?.MobileNumber); + + if (processed != null && processed.IsValid && !string.IsNullOrWhiteSpace(processed.LocalNumber)) + return processed.LocalNumber; + + return profile?.GetPhoneNumber(); } public async Task SendMessageAsync(Message message, string departmentNumber, int departmentId, UserProfile profile = null, Payment payment = null) @@ -55,17 +101,17 @@ public async Task SendMessageAsync(Message message, string departmentNumbe { if (Config.SystemBehaviorConfig.DepartmentsToForceSmsGateway.Contains(departmentId)) { - await _textMessageProvider.SendTextMessage(profile.GetPhoneNumber(), FormatTextForMessage(message.Subject, message.Body, ShouldDiscloseOptOut(profile.UserId)), + await _textMessageProvider.SendTextMessage(ResolveDirectSendNumber(profile), FormatTextForMessage(message.Subject, message.Body, ShouldDiscloseOptOut(profile.UserId)), departmentNumber, (MobileCarriers)profile.MobileCarrier, departmentId, true, false); } else if (Carriers.DirectSendCarriers.Contains((MobileCarriers)profile.MobileCarrier)) { - await _textMessageProvider.SendTextMessage(profile.GetPhoneNumber(), FormatTextForMessage(message.Subject, message.Body, ShouldDiscloseOptOut(profile.UserId)), + await _textMessageProvider.SendTextMessage(ResolveDirectSendNumber(profile), FormatTextForMessage(message.Subject, message.Body, ShouldDiscloseOptOut(profile.UserId)), departmentNumber, (MobileCarriers)profile.MobileCarrier, departmentId, false, false); } else { - email.To.Add(string.Format(Carriers.CarriersMap[(MobileCarriers)profile.MobileCarrier], profile.GetPhoneNumber())); + email.To.Add(string.Format(Carriers.CarriersMap[(MobileCarriers)profile.MobileCarrier], ResolveGatewayNumber(profile))); email.From = new MailAddress(Config.OutboundEmailServerConfig.FromMail, "RGMsg"); email.Subject = message.Subject; @@ -158,7 +204,7 @@ public async Task SendCallAsync(Call call, CallDispatch dispatch, string d // // text = text + " " + call.ShortenedCallUrl; // //} - // await _textMessageProvider.SendTextMessage(profile.GetPhoneNumber(), FormatTextForMessage(call.Name, text, ShouldDiscloseOptOut(profile.UserId)), departmentNumber, (MobileCarriers)profile.MobileCarrier, departmentId, true, true); + // await _textMessageProvider.SendTextMessage(ResolveDirectSendNumber(profile), FormatTextForMessage(call.Name, text, ShouldDiscloseOptOut(profile.UserId)), departmentNumber, (MobileCarriers)profile.MobileCarrier, departmentId, true, true); // if (Config.SystemBehaviorConfig.SendCallsToSmsEmailGatewayAdditionally) // SendCallViaEmailSmsGateway(call, address, profile); @@ -197,7 +243,7 @@ public async Task SendCallAsync(Call call, CallDispatch dispatch, string d // text = text + " " + call.ShortenedCallUrl; //} - await _textMessageProvider.SendTextMessage(profile.GetPhoneNumber(), FormatTextForMessage(call.Name, text, ShouldDiscloseOptOut(profile.UserId)), departmentNumber, (MobileCarriers)profile.MobileCarrier, departmentId, false, true); + await _textMessageProvider.SendTextMessage(ResolveDirectSendNumber(profile), FormatTextForMessage(call.Name, text, ShouldDiscloseOptOut(profile.UserId)), departmentNumber, (MobileCarriers)profile.MobileCarrier, departmentId, false, true); } else { @@ -268,7 +314,7 @@ public async Task SendCancelCallAsync(Call call, CallDispatch dispatch, st text = text + " (" + protocols + ")"; } - await _textMessageProvider.SendTextMessage(profile.GetPhoneNumber(), FormatTextForMessage(call.Name, text, ShouldDiscloseOptOut(profile.UserId)), departmentNumber, (MobileCarriers)profile.MobileCarrier, departmentId, false, true); + await _textMessageProvider.SendTextMessage(ResolveDirectSendNumber(profile), FormatTextForMessage(call.Name, text, ShouldDiscloseOptOut(profile.UserId)), departmentNumber, (MobileCarriers)profile.MobileCarrier, departmentId, false, true); } else { @@ -282,7 +328,7 @@ public async Task SendCancelCallAsync(Call call, CallDispatch dispatch, st private async Task SendCancelCallViaEmailSmsGatewayAsync(Call call, string address, UserProfile profile) { MailMessage email = new MailMessage(); - email.To.Add(string.Format(Carriers.CarriersMap[(MobileCarriers)profile.MobileCarrier], profile.GetPhoneNumber())); + email.To.Add(string.Format(Carriers.CarriersMap[(MobileCarriers)profile.MobileCarrier], ResolveGatewayNumber(profile))); email.From = new MailAddress(Config.OutboundEmailServerConfig.FromMail, "RGCall"); email.Subject = "CANCELLED: " + call.Name; @@ -307,7 +353,7 @@ private async Task SendCancelCallViaEmailSmsGatewayAsync(Call call, string addre private void SendCallViaEmailSmsGateway(Call call, string address, UserProfile profile) { MailMessage email = new MailMessage(); - email.To.Add(string.Format(Carriers.CarriersMap[(MobileCarriers)profile.MobileCarrier], profile.GetPhoneNumber())); + email.To.Add(string.Format(Carriers.CarriersMap[(MobileCarriers)profile.MobileCarrier], ResolveGatewayNumber(profile))); email.From = new MailAddress(Config.OutboundEmailServerConfig.FromMail, "RGCall"); email.Subject = call.Name; @@ -345,16 +391,16 @@ public void SendTroubleAlert(Unit unit, Call call, string unitAddress, string de if (Config.SystemBehaviorConfig.DepartmentsToForceSmsGateway.Contains(departmentId)) { - _textMessageProvider.SendTextMessage(profile.GetPhoneNumber(), FormatTextForMessage("Trouble Alert", text, ShouldDiscloseOptOut(profile.UserId)), departmentNumber, (MobileCarriers)profile.MobileCarrier, departmentId, true, false); + _textMessageProvider.SendTextMessage(ResolveDirectSendNumber(profile), FormatTextForMessage("Trouble Alert", text, ShouldDiscloseOptOut(profile.UserId)), departmentNumber, (MobileCarriers)profile.MobileCarrier, departmentId, true, false); } else if (Carriers.DirectSendCarriers.Contains((MobileCarriers)profile.MobileCarrier)) { - _textMessageProvider.SendTextMessage(profile.GetPhoneNumber(), FormatTextForMessage("Trouble Alert", text, ShouldDiscloseOptOut(profile.UserId)), departmentNumber, (MobileCarriers)profile.MobileCarrier, departmentId, false, false); + _textMessageProvider.SendTextMessage(ResolveDirectSendNumber(profile), FormatTextForMessage("Trouble Alert", text, ShouldDiscloseOptOut(profile.UserId)), departmentNumber, (MobileCarriers)profile.MobileCarrier, departmentId, false, false); } else { MailMessage email = new MailMessage(); - email.To.Add(string.Format(Carriers.CarriersMap[(MobileCarriers)profile.MobileCarrier], profile.GetPhoneNumber())); + email.To.Add(string.Format(Carriers.CarriersMap[(MobileCarriers)profile.MobileCarrier], ResolveGatewayNumber(profile))); email.From = new MailAddress(Config.OutboundEmailServerConfig.FromMail, "RGCall"); email.Subject = text; @@ -381,11 +427,11 @@ public async Task SendTextAsync(string userId, string title, string messag if (Carriers.DirectSendCarriers.Contains((MobileCarriers)profile.MobileCarrier)) { //string departmentNumber = _departmentSettingsService.GetTextToCallNumberForDepartment(departmentId); - await _textMessageProvider.SendTextMessage(profile.GetPhoneNumber(), FormatTextForMessage(title, message, ShouldDiscloseOptOut(profile.UserId)), departmentNumber, (MobileCarriers)profile.MobileCarrier, departmentId, false, false); + await _textMessageProvider.SendTextMessage(ResolveDirectSendNumber(profile), FormatTextForMessage(title, message, ShouldDiscloseOptOut(profile.UserId)), departmentNumber, (MobileCarriers)profile.MobileCarrier, departmentId, false, false); } else { - email.To.Add(string.Format(Carriers.CarriersMap[(MobileCarriers)profile.MobileCarrier], profile.GetPhoneNumber())); + email.To.Add(string.Format(Carriers.CarriersMap[(MobileCarriers)profile.MobileCarrier], ResolveGatewayNumber(profile))); email.From = new MailAddress(Config.OutboundEmailServerConfig.FromMail, "RGNot"); @@ -417,18 +463,18 @@ public async Task SendNotificationAsync(string userId, int departmentId, s { if (Config.SystemBehaviorConfig.DepartmentsToForceSmsGateway.Contains(departmentId)) { - await _textMessageProvider.SendTextMessage(profile.GetPhoneNumber(), FormatNotificationForMessage(message, ShouldDiscloseOptOut(profile.UserId)), + await _textMessageProvider.SendTextMessage(ResolveDirectSendNumber(profile), FormatNotificationForMessage(message, ShouldDiscloseOptOut(profile.UserId)), departmentNumber, (MobileCarriers)profile.MobileCarrier, departmentId, true, false); } else if (Carriers.DirectSendCarriers.Contains((MobileCarriers)profile.MobileCarrier)) { //string departmentNumber = _departmentSettingsService.GetTextToCallNumberForDepartment(departmentId); - await _textMessageProvider.SendTextMessage(profile.GetPhoneNumber(), FormatNotificationForMessage(message, ShouldDiscloseOptOut(profile.UserId)), + await _textMessageProvider.SendTextMessage(ResolveDirectSendNumber(profile), FormatNotificationForMessage(message, ShouldDiscloseOptOut(profile.UserId)), departmentNumber, (MobileCarriers)profile.MobileCarrier, departmentId, false, false); } else { - email.To.Add(string.Format(Carriers.CarriersMap[(MobileCarriers)profile.MobileCarrier], profile.GetPhoneNumber())); + email.To.Add(string.Format(Carriers.CarriersMap[(MobileCarriers)profile.MobileCarrier], ResolveGatewayNumber(profile))); email.From = new MailAddress(Config.OutboundEmailServerConfig.FromMail, "Resgrid"); email.Subject = "Notification"; diff --git a/Core/Resgrid.Services/UserProfileService.cs b/Core/Resgrid.Services/UserProfileService.cs index f9a5bdc94..3de4abcd0 100644 --- a/Core/Resgrid.Services/UserProfileService.cs +++ b/Core/Resgrid.Services/UserProfileService.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Text; using System.Threading; using System.Threading.Tasks; @@ -174,42 +175,105 @@ public void ClearAllUserProfilesFromCache(int departmentId) public async Task GetProfileByMobileNumberAsync(string number) { - string numberToTest = - number.Replace(" ", "").Replace("(", "").Replace(")", "").Replace("+", "").Replace("-", "").Replace(".", "").Trim(); + return await FindProfileByPhoneAsync(number, + _userProfileRepository.GetProfileByMobileNumberAsync, + profile => profile.MobileNumberVerified); + } - var profile = await _userProfileRepository.GetProfileByMobileNumberAsync(numberToTest); + public async Task GetProfileByHomeNumberAsync(string number) + { + return await FindProfileByPhoneAsync(number, + _userProfileRepository.GetProfileByHomeNumberAsync, + profile => profile.HomeNumberVerified); + } - if (profile != null) - return profile; + /// + /// Resolves the profile that owns a phone number, preferring one that has actually proven it. + /// + /// The same number can sit on more than one profile - a stale or secondary account, or someone + /// who mistyped it and never completed verification. A profile that verified the number is the + /// only one that has demonstrated possession, so it wins outright, even over a closer match on + /// the number's shape. Everything else falls back to candidate order (the number exactly as + /// dialled before its country-code variant). + /// + /// + /// Within a single candidate the query does the same ranking, so this only has to arbitrate + /// between candidates. + /// + /// + private static async Task FindProfileByPhoneAsync(string number, + Func> lookup, Func isVerified) + { + UserProfile unverifiedMatch = null; - if (numberToTest.Length == 11 && numberToTest[0] == char.Parse("1")) + foreach (var candidate in PhoneLookupCandidates(number)) { - numberToTest = numberToTest.Remove(0, 1); + var profile = await lookup(candidate); + + if (profile == null) + continue; + + if (isVerified(profile) == true) + return profile; - return await _userProfileRepository.GetProfileByMobileNumberAsync(numberToTest); + // Keep the first one found so a candidate that matches nothing verified still resolves, + // but keep looking in case a later candidate did verify the number. + unverifiedMatch ??= profile; } - return null; + return unverifiedMatch; } - public async Task GetProfileByHomeNumberAsync(string number) + /// + /// The stored numbers a lookup should be tried against, most-specific first. + /// + /// Profiles are saved in E.164 (+12015550123) while inbound SMS and voice hand us the number in + /// whatever shape the carrier used, so a lookup has to cover the country code being present on + /// one side but not the other. The leading "+" is covered by the query itself, which matches the + /// stored value both bare and plus-prefixed. + /// + /// + /// The order matters and the candidates are tried one at a time rather than matched together: + /// 2015550123 and 12015550123 can be two different profiles, and the repository takes + /// FirstOrDefault() with no ORDER BY. Asking for the number exactly as dialled first means the + /// country-code variant is only ever reached as a fallback. + /// + /// + private static IEnumerable PhoneLookupCandidates(string number) { - string numberToTest = - number.Replace(" ", "").Replace("(", "").Replace(")", "").Replace("+", "").Replace("-", "").Replace(".", "").Trim(); + var digits = NormalizePhoneNumber(number); - var profile = await _userProfileRepository.GetProfileByMobileNumberAsync(numberToTest); + // A blank inbound number must never match: the stored column can also be blank and an + // empty-to-empty compare would hand back an arbitrary profile. + if (string.IsNullOrWhiteSpace(digits)) + yield break; - if (profile != null) - return profile; + yield return digits; - if (numberToTest.Length == 11 && numberToTest[0] == char.Parse("1")) - { - numberToTest = numberToTest.Remove(0, 1); + if (digits.Length == 11 && digits[0] == '1') + yield return digits.Substring(1); + else if (digits.Length == 10) + yield return "1" + digits; + } - return await _userProfileRepository.GetProfileByMobileNumberAsync(numberToTest); + /// + /// Reduces a number to bare digits. Inbound numbers arrive formatted in assorted ways + /// ("+1 (201) 555-0123"), and only the digits are comparable against a stored number. + /// + private static string NormalizePhoneNumber(string number) + { + if (string.IsNullOrWhiteSpace(number)) + return null; + + var digits = new StringBuilder(number.Length); + + foreach (var character in number) + { + if (character >= '0' && character <= '9') + digits.Append(character); } - return null; + return digits.ToString(); } public async Task> GetSelectedUserProfilesAsync(List userIds) diff --git a/Repositories/Resgrid.Repositories.DataRepository/DepartmentsRepository.cs b/Repositories/Resgrid.Repositories.DataRepository/DepartmentsRepository.cs index 661a834b8..8198a2143 100644 --- a/Repositories/Resgrid.Repositories.DataRepository/DepartmentsRepository.cs +++ b/Repositories/Resgrid.Repositories.DataRepository/DepartmentsRepository.cs @@ -449,6 +449,7 @@ public async Task GetDepartmentStatsByDepartmentUserIdAsync(int var dynamicParameters = new DynamicParametersExtension(); dynamicParameters.Add("DepartmentId", departmentId); dynamicParameters.Add("UserId", userId); + dynamicParameters.Add("CurrentDate", DateTime.UtcNow); var query = _queryFactory.GetQuery(); diff --git a/Repositories/Resgrid.Repositories.DataRepository/Queries/Departments/SelectDepartmentStatsByUserDidQuery.cs b/Repositories/Resgrid.Repositories.DataRepository/Queries/Departments/SelectDepartmentStatsByUserDidQuery.cs index cfaabac64..237df67d4 100644 --- a/Repositories/Resgrid.Repositories.DataRepository/Queries/Departments/SelectDepartmentStatsByUserDidQuery.cs +++ b/Repositories/Resgrid.Repositories.DataRepository/Queries/Departments/SelectDepartmentStatsByUserDidQuery.cs @@ -21,11 +21,13 @@ public string GetQuery() _sqlConfiguration.ParameterNotation, new string[] { "%DID%", - "%USERID%" + "%USERID%", + "%CURRENTDATE%" }, new string[] { "DepartmentId", - "UserId" + "UserId", + "CurrentDate" }, new string[] { "%MESSAGESTABLE%", diff --git a/Repositories/Resgrid.Repositories.DataRepository/Servers/PostgreSql/PostgreSqlConfiguration.cs b/Repositories/Resgrid.Repositories.DataRepository/Servers/PostgreSql/PostgreSqlConfiguration.cs index a53a7c0f3..39c4a2baf 100644 --- a/Repositories/Resgrid.Repositories.DataRepository/Servers/PostgreSql/PostgreSqlConfiguration.cs +++ b/Repositories/Resgrid.Repositories.DataRepository/Servers/PostgreSql/PostgreSqlConfiguration.cs @@ -1,4 +1,4 @@ -using Resgrid.Config; +using Resgrid.Config; using Resgrid.Repositories.DataRepository.Configs; using Resgrid.Repositories.DataRepository.Queries.Calendar; using Resgrid.Repositories.DataRepository.Queries.Calls; @@ -283,7 +283,8 @@ FROM Departments d SELECT (SELECT COUNT(*) FROM %SCHEMA%.%MESSAGESTABLE% m LEFT OUTER JOIN %SCHEMA%.%MESSAGERECIPIENTSTABLE% mr ON m.MessageId = mr.MessageId - WHERE mr.UserId = %USERID% AND mr.IsDeleted = false AND m.IsDeleted = false) AS UnreadMessageCount, + WHERE mr.UserId = %USERID% AND mr.IsDeleted = false AND mr.ReadOn IS NULL + AND m.IsDeleted = false AND (m.ExpireOn IS NULL OR m.ExpireOn > %CURRENTDATE%)) AS UnreadMessageCount, (SELECT COUNT(*) FROM %SCHEMA%.%CALLTABLENAME% c WHERE c.DepartmentId = %DID% AND c.IsDeleted = false AND c.State = 0) AS OpenCallsCount"; @@ -547,12 +548,30 @@ SELECT COUNT(*) FROM %SCHEMA%.%MESSAGESTABLE% m SELECT %SCHEMA%.%USERPROFILESTABLE%.*, %SCHEMA%.%ASPNETUSERSTABLE%.Email as MembershipEmail, %SCHEMA%.%ASPNETUSERSTABLE%.* FROM %SCHEMA%.%USERPROFILESTABLE% INNER JOIN %SCHEMA%.%ASPNETUSERSTABLE% ON %SCHEMA%.%ASPNETUSERSTABLE%.Id = %SCHEMA%.%USERPROFILESTABLE%.UserId - WHERE MobileNumber = %MOBILENUMBER%"; + WHERE MobileNumber IS NOT NULL AND MobileNumber <> '' + AND MobileNumber IN (%MOBILENUMBER%, '+' || %MOBILENUMBER%) + ORDER BY + CASE + WHEN %SCHEMA%.%USERPROFILESTABLE%.MobileNumberVerified = true THEN 0 + WHEN %SCHEMA%.%USERPROFILESTABLE%.MobileNumberVerified IS NULL THEN 1 + ELSE 2 + END, + %SCHEMA%.%USERPROFILESTABLE%.LastUpdated DESC NULLS LAST, + %SCHEMA%.%USERPROFILESTABLE%.UserProfileId DESC"; SelectProfileByHomeQuery = @" SELECT %SCHEMA%.%USERPROFILESTABLE%.*, %SCHEMA%.%ASPNETUSERSTABLE%.Email as MembershipEmail, %SCHEMA%.%ASPNETUSERSTABLE%.* FROM %SCHEMA%.%USERPROFILESTABLE% INNER JOIN %SCHEMA%.%ASPNETUSERSTABLE% ON %SCHEMA%.%ASPNETUSERSTABLE%.Id = %SCHEMA%.%USERPROFILESTABLE%.UserId - WHERE HomeNumber = %HOMENUMBER%"; + WHERE HomeNumber IS NOT NULL AND HomeNumber <> '' + AND HomeNumber IN (%HOMENUMBER%, '+' || %HOMENUMBER%) + ORDER BY + CASE + WHEN %SCHEMA%.%USERPROFILESTABLE%.HomeNumberVerified = true THEN 0 + WHEN %SCHEMA%.%USERPROFILESTABLE%.HomeNumberVerified IS NULL THEN 1 + ELSE 2 + END, + %SCHEMA%.%USERPROFILESTABLE%.LastUpdated DESC NULLS LAST, + %SCHEMA%.%USERPROFILESTABLE%.UserProfileId DESC"; SelectProfilesByIdsQuery = @" SELECT %SCHEMA%.%USERPROFILESTABLE%.*, %SCHEMA%.%ASPNETUSERSTABLE%.Email as MembershipEmail, %SCHEMA%.%ASPNETUSERSTABLE%.* FROM %SCHEMA%.%USERPROFILESTABLE% diff --git a/Repositories/Resgrid.Repositories.DataRepository/Servers/SqlServer/SqlServerConfiguration.cs b/Repositories/Resgrid.Repositories.DataRepository/Servers/SqlServer/SqlServerConfiguration.cs index f627fb6ad..a12535d92 100644 --- a/Repositories/Resgrid.Repositories.DataRepository/Servers/SqlServer/SqlServerConfiguration.cs +++ b/Repositories/Resgrid.Repositories.DataRepository/Servers/SqlServer/SqlServerConfiguration.cs @@ -1,4 +1,4 @@ -using Resgrid.Repositories.DataRepository.Configs; +using Resgrid.Repositories.DataRepository.Configs; using Resgrid.Repositories.DataRepository.Queries.Calendar; using Resgrid.Repositories.DataRepository.Queries.Calls; using Resgrid.Repositories.DataRepository.Queries.DepartmentGroups; @@ -281,7 +281,8 @@ FROM AspNetUsers u SELECT (SELECT COUNT(*) FROM %SCHEMA%.%MESSAGESTABLE% m LEFT OUTER JOIN %SCHEMA%.%MESSAGERECIPIENTSTABLE% mr ON m.MessageId = mr.MessageId - WHERE mr.[UserId] = %USERID% AND mr.[IsDeleted] = 0 AND m.[IsDeleted] = 0) AS [UnreadMessageCount], + WHERE mr.[UserId] = %USERID% AND mr.[IsDeleted] = 0 AND mr.[ReadOn] IS NULL + AND m.[IsDeleted] = 0 AND (m.[ExpireOn] IS NULL OR m.[ExpireOn] > %CURRENTDATE%)) AS [UnreadMessageCount], (SELECT COUNT(*) FROM %SCHEMA%.%CALLTABLENAME% c WHERE c.[DepartmentId] = %DID% AND c.[IsDeleted] = 0 AND c.[State] = 0) AS [OpenCallsCount]"; @@ -545,12 +546,30 @@ SELECT COUNT(*) FROM %SCHEMA%.%MESSAGESTABLE% m SELECT %SCHEMA%.%USERPROFILESTABLE%.*, %SCHEMA%.%ASPNETUSERSTABLE%.Email as 'MembershipEmail', %SCHEMA%.%ASPNETUSERSTABLE%.* FROM %SCHEMA%.%USERPROFILESTABLE% INNER JOIN %SCHEMA%.%ASPNETUSERSTABLE% ON %SCHEMA%.%ASPNETUSERSTABLE%.Id = %SCHEMA%.%USERPROFILESTABLE%.UserId - WHERE [MobileNumber] = %MOBILENUMBER%"; + WHERE [MobileNumber] IS NOT NULL AND [MobileNumber] <> '' + AND [MobileNumber] IN (%MOBILENUMBER%, '+' + %MOBILENUMBER%) + ORDER BY + CASE + WHEN %SCHEMA%.%USERPROFILESTABLE%.[MobileNumberVerified] = 1 THEN 0 + WHEN %SCHEMA%.%USERPROFILESTABLE%.[MobileNumberVerified] IS NULL THEN 1 + ELSE 2 + END, + %SCHEMA%.%USERPROFILESTABLE%.[LastUpdated] DESC, + %SCHEMA%.%USERPROFILESTABLE%.[UserProfileId] DESC"; SelectProfileByHomeQuery = @" SELECT %SCHEMA%.%USERPROFILESTABLE%.*, %SCHEMA%.%ASPNETUSERSTABLE%.Email as 'MembershipEmail', %SCHEMA%.%ASPNETUSERSTABLE%.* FROM %SCHEMA%.%USERPROFILESTABLE% INNER JOIN %SCHEMA%.%ASPNETUSERSTABLE% ON %SCHEMA%.%ASPNETUSERSTABLE%.Id = %SCHEMA%.%USERPROFILESTABLE%.UserId - WHERE [HomeNumber] = %HOMENUMBER%"; + WHERE [HomeNumber] IS NOT NULL AND [HomeNumber] <> '' + AND [HomeNumber] IN (%HOMENUMBER%, '+' + %HOMENUMBER%) + ORDER BY + CASE + WHEN %SCHEMA%.%USERPROFILESTABLE%.[HomeNumberVerified] = 1 THEN 0 + WHEN %SCHEMA%.%USERPROFILESTABLE%.[HomeNumberVerified] IS NULL THEN 1 + ELSE 2 + END, + %SCHEMA%.%USERPROFILESTABLE%.[LastUpdated] DESC, + %SCHEMA%.%USERPROFILESTABLE%.[UserProfileId] DESC"; SelectProfilesByIdsQuery = @" SELECT %SCHEMA%.%USERPROFILESTABLE%.*, %SCHEMA%.%ASPNETUSERSTABLE%.Email as 'MembershipEmail', %SCHEMA%.%ASPNETUSERSTABLE%.* FROM %SCHEMA%.%USERPROFILESTABLE% diff --git a/Repositories/Resgrid.Repositories.DataRepository/UserProfilesRepository.cs b/Repositories/Resgrid.Repositories.DataRepository/UserProfilesRepository.cs index 1d727bc69..f376dc596 100644 --- a/Repositories/Resgrid.Repositories.DataRepository/UserProfilesRepository.cs +++ b/Repositories/Resgrid.Repositories.DataRepository/UserProfilesRepository.cs @@ -314,6 +314,55 @@ public async Task> GetSelectedUserProfilesAsync(List profiles, CancellationToken cancellationToken = default) + { + try + { + var items = profiles? + .Where(p => p != null) + .Select(p => new { p.MobileNumber, p.HomeNumber, p.LastUpdated, p.UserProfileId }) + .ToList(); + + if (items == null || items.Count == 0) + return; + + var pn = _sqlConfiguration.ParameterNotation; + var sql = DataConfig.DatabaseType == DatabaseTypes.Postgres + ? $"UPDATE {_sqlConfiguration.SchemaName}.userprofiles SET mobilenumber = {pn}MobileNumber, homenumber = {pn}HomeNumber, lastupdated = {pn}LastUpdated WHERE userprofileid = {pn}UserProfileId" + : $"UPDATE {_sqlConfiguration.SchemaName}.[UserProfiles] SET [MobileNumber] = {pn}MobileNumber, [HomeNumber] = {pn}HomeNumber, [LastUpdated] = {pn}LastUpdated WHERE [UserProfileId] = {pn}UserProfileId"; + + // Dapper executes the statement once per item over a single prepared command/connection. + if (_unitOfWork?.Connection == null) + { + using (var conn = _connectionProvider.Create()) + { + await conn.OpenAsync(cancellationToken); + + // No ambient unit-of-work: run the batch atomically so a mid-batch failure rolls + // back rather than leaving the sweep half-applied. + using (var transaction = await conn.BeginTransactionAsync(cancellationToken)) + { + await conn.ExecuteAsync(sql, items, transaction); + await transaction.CommitAsync(cancellationToken); + } + } + } + else + { + // Ambient unit-of-work: participate in the caller's transaction. + var connection = _unitOfWork.CreateOrGetConnection(); + await connection.ExecuteAsync(sql, items, _unitOfWork.Transaction); + } + } + catch (Exception ex) + { + // Rethrow (unlike the read paths): the sweep reports what it wrote, so a silent failure + // would be reported as a successful normalization. + Logging.LogException(ex); + throw; + } + } + public async Task UpdateSecurityPinsAsync(IEnumerable profiles, CancellationToken cancellationToken = default) { try diff --git a/Tests/Resgrid.Tests/Providers/PhoneNumberProcesserProviderFormatTests.cs b/Tests/Resgrid.Tests/Providers/PhoneNumberProcesserProviderFormatTests.cs new file mode 100644 index 000000000..234984fb2 --- /dev/null +++ b/Tests/Resgrid.Tests/Providers/PhoneNumberProcesserProviderFormatTests.cs @@ -0,0 +1,99 @@ +using FluentAssertions; +using NUnit.Framework; +using Resgrid.Providers.NumberProvider; + +namespace Resgrid.Tests.Providers +{ + /// + /// The profile save paths store PhoneNumberResult.InternationalNumber, and the inbound SMS/voice + /// lookup matches that stored value against digits (optionally plus-prefixed). That only works + /// because this provider emits E.164 with no separators - if it ever returned a spaced or dashed + /// international format, every newly saved number would stop resolving on inbound messages. + /// + [TestFixture] + public class PhoneNumberProcesserProviderFormatTests + { + private PhoneNumberProcesserProvider _provider; + + [SetUp] + public void SetUp() => _provider = new PhoneNumberProcesserProvider(); + + [TestCase("+12015550123", null)] + [TestCase("2015550123", "US")] + [TestCase("(201) 555-0123", "US")] + [TestCase("201.555.0123", "US")] + [TestCase("+1 201 555 0123", null)] + public void Process_returns_e164_without_separators(string input, string region) + { + var result = _provider.Process(input, region); + + result.IsValid.Should().BeTrue(); + result.InternationalNumber.Should().Be("+12015550123"); + } + + [TestCase("+61 2 5550 1234", null, "+61255501234")] + [TestCase("+44 20 7946 0958", null, "+442079460958")] + public void Process_returns_e164_without_separators_outside_the_us(string input, string region, string expected) + { + var result = _provider.Process(input, region); + + result.IsValid.Should().BeTrue(); + result.InternationalNumber.Should().Be(expected); + } + + // ── Real stored formats ─────────────────────────────────────────────────────── + // Sampled from the production UserProfiles.MobileNumber column. The normalization sweep + // (Resgrid.Console --NormalizePhoneNumbers) runs each stored value through this provider, so + // these are the shapes it has to be able to convert. + + [TestCase("(270) 555-0101", "+12705550101")] + [TestCase("270-555-0102", "+12705550102")] + [TestCase("270-555-0103", "+12705550103")] + [TestCase("(574) 555-0104", "+15745550104")] + [TestCase("(815) 555-0105", "+18155550105")] + [TestCase("802-555-0106", "+18025550106")] + [TestCase("970-555-0107", "+19705550107")] + [TestCase("(812) 555-0108", "+18125550108")] + [TestCase("(501) 555-0109", "+15015550109")] + public void Process_converts_punctuated_us_numbers_to_e164(string stored, string expected) + { + var result = _provider.Process(stored, "US"); + + result.IsValid.Should().BeTrue(); + result.InternationalNumber.Should().Be(expected); + } + + [TestCase("7135550110", "+17135550110")] + [TestCase("2315550111", "+12315550111")] + [TestCase("8125550108", "+18125550108")] + [TestCase("+17755550112", "+17755550112")] + public void Process_converts_bare_and_already_canonical_us_numbers_to_e164(string stored, string expected) + { + var result = _provider.Process(stored, "US"); + + result.IsValid.Should().BeTrue(); + result.InternationalNumber.Should().Be(expected); + } + + [Test] + public void Process_maps_the_two_stored_formats_of_one_number_onto_the_same_value() + { + // "(812) 555-0108" and "8125550108" sit on different profiles in the sampled data. After the + // sweep they collide on one number, which is what the verified-first ordering in the profile + // lookup exists to arbitrate - and what the sweep reports as a collision. + _provider.Process("(812) 555-0108", "US").InternationalNumber + .Should().Be(_provider.Process("8125550108", "US").InternationalNumber); + } + + [TestCase("(043) 555-0118")] + [TestCase("(041) 555-0119")] + public void Process_rejects_leading_zero_area_codes_under_the_us_region(string stored) + { + // Non-US national formats. The sweep must not rewrite these - it reports them for review + // instead, because guessing a country code would produce a number that dials elsewhere. + var result = _provider.Process(stored, "US"); + + result.IsValid.Should().BeFalse(); + } + } +} diff --git a/Tests/Resgrid.Tests/Repositories/DepartmentStatsQueryTests.cs b/Tests/Resgrid.Tests/Repositories/DepartmentStatsQueryTests.cs new file mode 100644 index 000000000..49c7b6d1c --- /dev/null +++ b/Tests/Resgrid.Tests/Repositories/DepartmentStatsQueryTests.cs @@ -0,0 +1,53 @@ +using FluentAssertions; +using NUnit.Framework; +using Resgrid.Repositories.DataRepository.Configs; +using Resgrid.Repositories.DataRepository.Queries.Departments; +using Resgrid.Repositories.DataRepository.Queries.Messages; +using Resgrid.Repositories.DataRepository.Servers.SqlServer; + +namespace Resgrid.Tests.Repositories +{ + /// + /// The top nav unread badge is fed by the DepartmentStats query, not by the dedicated unread + /// message count query. When the two drift apart the badge counts read (and expired) messages + /// and never goes back down, so pin the unread predicates here. + /// + [TestFixture] + public class DepartmentStatsQueryTests + { + [Test] + public void SqlServer_department_stats_unread_count_only_counts_unread_unexpired_messages() + { + var query = new SelectDepartmentStatsByUserDidQuery(new SqlServerConfiguration()).GetQuery(); + + query.Should().Contain("mr.[ReadOn] IS NULL"); + query.Should().Contain("m.[ExpireOn] IS NULL OR m.[ExpireOn] > @CurrentDate"); + } + + [Test] + public void Postgres_department_stats_unread_count_only_counts_unread_unexpired_messages() + { + var query = new SelectDepartmentStatsByUserDidQuery(new PostgreSqlConfiguration()).GetQuery(); + + query.Should().Contain("mr.ReadOn IS NULL"); + query.Should().Contain("m.ExpireOn IS NULL OR m.ExpireOn > @CurrentDate"); + } + + [Test] + public void Department_stats_unread_predicates_match_the_dedicated_unread_count_query() + { + var configurations = new SqlConfiguration[] { new SqlServerConfiguration(), new PostgreSqlConfiguration() }; + + foreach (var configuration in configurations) + { + var statsQuery = new SelectDepartmentStatsByUserDidQuery(configuration).GetQuery(); + var unreadQuery = new SelectUnreadMessageCountQuery(configuration).GetQuery(); + + var readOnPredicate = configuration is SqlServerConfiguration ? "mr.[ReadOn] IS NULL" : "mr.ReadOn IS NULL"; + + unreadQuery.Should().Contain(readOnPredicate); + statsQuery.Should().Contain(readOnPredicate); + } + } + } +} diff --git a/Tests/Resgrid.Tests/Repositories/ProfileByPhoneQueryTests.cs b/Tests/Resgrid.Tests/Repositories/ProfileByPhoneQueryTests.cs new file mode 100644 index 000000000..96e403084 --- /dev/null +++ b/Tests/Resgrid.Tests/Repositories/ProfileByPhoneQueryTests.cs @@ -0,0 +1,78 @@ +using FluentAssertions; +using NUnit.Framework; +using Resgrid.Repositories.DataRepository.Queries.UserProfiles; +using Resgrid.Repositories.DataRepository.Servers.SqlServer; + +namespace Resgrid.Tests.Repositories +{ + /// + /// Profiles are saved in E.164 (+12015550123) while older rows hold the bare digits, so the lookup + /// has to match the stored value with and without the leading "+". Matching the column directly + /// keeps the predicate sargable - wrapping MobileNumber/HomeNumber in REPLACE() to normalize it + /// would push this hot inbound-webhook lookup toward a scan of UserProfiles. + /// + /// Only the "+" belongs here. The country-code variant is a separate candidate that + /// UserProfileService tries as an ordered second pass, because 2015550123 and 12015550123 can be + /// two different profiles and UserProfilesRepository takes FirstOrDefault() with no ORDER BY. + /// + [TestFixture] + public class ProfileByPhoneQueryTests + { + [Test] + public void SqlServer_profile_by_mobile_matches_the_stored_number_with_and_without_the_plus() + { + var query = new SelectProfileByMobileQuery(new SqlServerConfiguration()).GetQuery(); + + query.Should().Contain("[MobileNumber] IN (@MobileNumber, '+' + @MobileNumber)"); + query.Should().NotContain("REPLACE("); + query.Should().NotContain("'1' + @MobileNumber"); + } + + [Test] + public void SqlServer_profile_by_home_matches_the_stored_number_with_and_without_the_plus() + { + var query = new SelectProfileByHomeQuery(new SqlServerConfiguration()).GetQuery(); + + query.Should().Contain("[HomeNumber] IN (@HomeNumber, '+' + @HomeNumber)"); + query.Should().NotContain("REPLACE("); + query.Should().NotContain("'1' + @HomeNumber"); + } + + [Test] + public void SqlServer_profile_by_mobile_ranks_verified_profiles_first() + { + var query = new SelectProfileByMobileQuery(new SqlServerConfiguration()).GetQuery(); + + // The same number can sit on a stale or mistyped profile as well as the real owner's, and + // the repository takes FirstOrDefault() - without this ORDER BY which one comes back is up + // to the plan. Verified first, then grandfathered (NULL), then never-verified. + query.Should().Contain("ORDER BY"); + query.Should().Contain("WHEN [dbo].UserProfiles.[MobileNumberVerified] = 1 THEN 0"); + query.Should().Contain("WHEN [dbo].UserProfiles.[MobileNumberVerified] IS NULL THEN 1"); + query.Should().Contain("ELSE 2"); + + // Ties still have to resolve to the same row every time. + query.Should().Contain("[dbo].UserProfiles.[UserProfileId] DESC"); + } + + [Test] + public void SqlServer_profile_by_home_ranks_verified_profiles_first() + { + var query = new SelectProfileByHomeQuery(new SqlServerConfiguration()).GetQuery(); + + query.Should().Contain("WHEN [dbo].UserProfiles.[HomeNumberVerified] = 1 THEN 0"); + query.Should().Contain("WHEN [dbo].UserProfiles.[HomeNumberVerified] IS NULL THEN 1"); + query.Should().Contain("[dbo].UserProfiles.[UserProfileId] DESC"); + } + + [Test] + public void SqlServer_profile_by_phone_queries_ignore_blank_stored_numbers() + { + new SelectProfileByMobileQuery(new SqlServerConfiguration()).GetQuery() + .Should().Contain("[MobileNumber] IS NOT NULL AND [MobileNumber] <> ''"); + + new SelectProfileByHomeQuery(new SqlServerConfiguration()).GetQuery() + .Should().Contain("[HomeNumber] IS NOT NULL AND [HomeNumber] <> ''"); + } + } +} diff --git a/Tests/Resgrid.Tests/Services/SmsServiceNumberFormatTests.cs b/Tests/Resgrid.Tests/Services/SmsServiceNumberFormatTests.cs new file mode 100644 index 000000000..4e18ce967 --- /dev/null +++ b/Tests/Resgrid.Tests/Services/SmsServiceNumberFormatTests.cs @@ -0,0 +1,116 @@ +using System.Net.Mail; +using System.Threading.Tasks; +using FluentAssertions; +using Moq; +using NUnit.Framework; +using Resgrid.Model; +using Resgrid.Model.Providers; +using Resgrid.Model.Services; +using Resgrid.Providers.NumberProvider; +using Resgrid.Services; + +namespace Resgrid.Tests.Services +{ + /// + /// Outbound SMS leaves by two transports that need the number in different shapes, and + /// UserProfile.GetPhoneNumber() is the right shape for neither once a number is stored in E.164: + /// it strips the leading "+" but keeps the country code. + /// + /// A direct provider send needs full E.164 - Twilio takes the value verbatim, and a non-US number + /// without its "+" is meaningless. A carrier gateway address ("{0}@vtext.com") needs the bare + /// national number - 11 digits addresses a mailbox that does not exist. + /// + [TestFixture] + public class SmsServiceNumberFormatTests + { + private Mock _textMessageProvider; + private Mock _emailSender; + private SmsService _service; + + [SetUp] + public void SetUp() + { + _textMessageProvider = new Mock(); + _emailSender = new Mock(); + + Resgrid.Config.SystemBehaviorConfig.DoNotBroadcast = false; + Resgrid.Config.SystemBehaviorConfig.DepartmentsToForceSmsGateway.Clear(); + + _service = new SmsService( + new Mock().Object, + new Mock().Object, + _textMessageProvider.Object, + new Mock().Object, + _emailSender.Object, + new Mock().Object, + new Mock().Object, + // The real processor: deterministic, no I/O, and the point of the test is that the + // service asks it for the correct form. + new PhoneNumberProcesserProvider()); + } + + private static UserProfile Profile(string mobileNumber, MobileCarriers carrier) => new UserProfile + { + UserId = "user-1", + MobileNumber = mobileNumber, + MobileCarrier = (int)carrier, + SendMessageSms = true + }; + + private Task SendAsync(UserProfile profile) => + _service.SendMessageAsync(new Message { Subject = "Subject", Body = "Body" }, "+15555550100", 1, profile); + + [TestCase("+12705550101")] + [TestCase("(270) 555-0101")] + [TestCase("2705550101")] + public async Task Direct_send_receives_the_number_in_e164(string stored) + { + // Verizon is a direct-send carrier, so this goes to the provider rather than a gateway. + await SendAsync(Profile(stored, MobileCarriers.Verizon)); + + _textMessageProvider.Verify(x => x.SendTextMessage( + "+12705550101", It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); + } + + [Test] + public async Task Direct_send_keeps_the_plus_on_a_non_us_number() + { + // Without the "+" this number has no meaning to the provider at all. + await SendAsync(Profile("+61255501234", MobileCarriers.Telstra)); + + _textMessageProvider.Verify(x => x.SendTextMessage( + "+61255501234", It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); + } + + [TestCase("+12705550101")] + [TestCase("(270) 555-0101")] + [TestCase("2705550101")] + public async Task Carrier_gateway_receives_the_bare_national_number(string stored) + { + // MetroPCS has no direct-send route, so this addresses the carrier's SMS gateway. + MailMessage sent = null; + _emailSender.Setup(x => x.SendEmail(It.IsAny())) + .Callback(m => sent = m) + .ReturnsAsync(true); + + await SendAsync(Profile(stored, MobileCarriers.MetroPcs)); + + sent.Should().NotBeNull(); + sent.To.Should().ContainSingle().Which.Address.Should().Be("2705550101@mymetropcs.com"); + } + + [Test] + public async Task An_unparseable_number_falls_back_to_the_previous_behaviour() + { + // Nothing valid to send to. Rather than dropping the message on a new code path, it goes + // out exactly as it did before this change and the provider rejects it as it did before. + await SendAsync(Profile("12345", MobileCarriers.Verizon)); + + _textMessageProvider.Verify(x => x.SendTextMessage( + "12345", It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); + } + } +} diff --git a/Tests/Resgrid.Tests/Services/UserProfilePhoneLookupTests.cs b/Tests/Resgrid.Tests/Services/UserProfilePhoneLookupTests.cs new file mode 100644 index 000000000..2de8f5514 --- /dev/null +++ b/Tests/Resgrid.Tests/Services/UserProfilePhoneLookupTests.cs @@ -0,0 +1,204 @@ +using System.Threading.Tasks; +using FluentAssertions; +using Moq; +using NUnit.Framework; +using Resgrid.Model; +using Resgrid.Model.Providers; +using Resgrid.Model.Repositories; +using Resgrid.Services; + +namespace Resgrid.Tests.Services +{ + /// + /// Profiles are persisted in E.164 (+12015550123) by the profile save flow, while inbound SMS and + /// voice hand us the number in whatever shape the carrier used. The query matches the stored value + /// with and without the leading "+"; the service covers the country code being present on one side + /// but not the other, and strips the formatting off the inbound number. + /// + [TestFixture] + public class UserProfilePhoneLookupTests + { + private Mock _repository; + private UserProfileService _service; + + [SetUp] + public void SetUp() + { + _repository = new Mock(); + _service = new UserProfileService(_repository.Object, new Mock().Object, + new Mock().Object); + } + + [TestCase("+12015550123", "12015550123")] + [TestCase("12015550123", "12015550123")] + [TestCase("(201) 555-0123", "2015550123")] + [TestCase("201.555.0123", "2015550123")] + [TestCase(" +1 201 555 0123 ", "12015550123")] + public async Task GetProfileByMobileNumberAsync_strips_formatting_before_hitting_the_repository(string inbound, string expected) + { + var profile = new UserProfile { UserId = "user-1" }; + _repository.Setup(x => x.GetProfileByMobileNumberAsync(expected)).ReturnsAsync(profile); + + var result = await _service.GetProfileByMobileNumberAsync(inbound); + + result.Should().BeSameAs(profile); + _repository.Verify(x => x.GetProfileByMobileNumberAsync(expected), Times.Once); + } + + [Test] + public async Task GetProfileByMobileNumberAsync_falls_back_to_the_number_without_the_country_code() + { + var profile = new UserProfile { UserId = "user-1" }; + _repository.Setup(x => x.GetProfileByMobileNumberAsync("12015550123")).ReturnsAsync((UserProfile)null); + _repository.Setup(x => x.GetProfileByMobileNumberAsync("2015550123")).ReturnsAsync(profile); + + var result = await _service.GetProfileByMobileNumberAsync("+12015550123"); + + result.Should().BeSameAs(profile); + } + + [Test] + public async Task GetProfileByMobileNumberAsync_falls_back_to_the_number_with_the_country_code() + { + var profile = new UserProfile { UserId = "user-1" }; + _repository.Setup(x => x.GetProfileByMobileNumberAsync("2015550123")).ReturnsAsync((UserProfile)null); + _repository.Setup(x => x.GetProfileByMobileNumberAsync("12015550123")).ReturnsAsync(profile); + + var result = await _service.GetProfileByMobileNumberAsync("(201) 555-0123"); + + result.Should().BeSameAs(profile); + } + + [Test] + public async Task GetProfileByMobileNumberAsync_prefers_the_number_exactly_as_dialled() + { + // 2015550123 and 12015550123 can be two different profiles, so with nothing to separate + // them on verification the country-code variant stays a fallback rather than being matched + // alongside the number that was actually dialled. + var exact = new UserProfile { UserId = "exact", MobileNumberVerified = true }; + var variant = new UserProfile { UserId = "variant", MobileNumberVerified = true }; + _repository.Setup(x => x.GetProfileByMobileNumberAsync("12015550123")).ReturnsAsync(exact); + _repository.Setup(x => x.GetProfileByMobileNumberAsync("2015550123")).ReturnsAsync(variant); + + var result = await _service.GetProfileByMobileNumberAsync("+12015550123"); + + result.Should().BeSameAs(exact); + _repository.Verify(x => x.GetProfileByMobileNumberAsync("2015550123"), Times.Never); + } + + // ── Verified profiles win ───────────────────────────────────────────────────── + // The same number can sit on a stale account, a secondary account, or one where it was + // mistyped and never verified. Only a verified profile has proven possession of the number. + + [Test] + public async Task GetProfileByMobileNumberAsync_prefers_a_verified_profile_over_a_closer_number_match() + { + var mistyped = new UserProfile { UserId = "mistyped", MobileNumberVerified = false }; + var owner = new UserProfile { UserId = "owner", MobileNumberVerified = true }; + _repository.Setup(x => x.GetProfileByMobileNumberAsync("12015550123")).ReturnsAsync(mistyped); + _repository.Setup(x => x.GetProfileByMobileNumberAsync("2015550123")).ReturnsAsync(owner); + + var result = await _service.GetProfileByMobileNumberAsync("+12015550123"); + + result.Should().BeSameAs(owner); + } + + [Test] + public async Task GetProfileByMobileNumberAsync_prefers_a_verified_profile_over_a_grandfathered_one() + { + // NULL is the grandfathered pre-verification state, not a verified one. + var grandfathered = new UserProfile { UserId = "grandfathered", MobileNumberVerified = null }; + var owner = new UserProfile { UserId = "owner", MobileNumberVerified = true }; + _repository.Setup(x => x.GetProfileByMobileNumberAsync("12015550123")).ReturnsAsync(grandfathered); + _repository.Setup(x => x.GetProfileByMobileNumberAsync("2015550123")).ReturnsAsync(owner); + + var result = await _service.GetProfileByMobileNumberAsync("+12015550123"); + + result.Should().BeSameAs(owner); + } + + [Test] + public async Task GetProfileByMobileNumberAsync_still_resolves_when_nothing_is_verified() + { + var mistyped = new UserProfile { UserId = "mistyped", MobileNumberVerified = false }; + _repository.Setup(x => x.GetProfileByMobileNumberAsync("12015550123")).ReturnsAsync(mistyped); + _repository.Setup(x => x.GetProfileByMobileNumberAsync("2015550123")).ReturnsAsync((UserProfile)null); + + var result = await _service.GetProfileByMobileNumberAsync("+12015550123"); + + // Callers apply their own verification gate; resolving the profile is not the same as + // trusting it, so an unverified match is still returned rather than swallowed. + result.Should().BeSameAs(mistyped); + } + + [Test] + public async Task GetProfileByMobileNumberAsync_stops_at_the_first_verified_profile() + { + var owner = new UserProfile { UserId = "owner", MobileNumberVerified = true }; + _repository.Setup(x => x.GetProfileByMobileNumberAsync("12015550123")).ReturnsAsync(owner); + + await _service.GetProfileByMobileNumberAsync("+12015550123"); + + _repository.Verify(x => x.GetProfileByMobileNumberAsync("2015550123"), Times.Never); + } + + [Test] + public async Task GetProfileByHomeNumberAsync_prefers_a_verified_profile() + { + var secondary = new UserProfile { UserId = "secondary", HomeNumberVerified = false }; + var owner = new UserProfile { UserId = "owner", HomeNumberVerified = true }; + _repository.Setup(x => x.GetProfileByHomeNumberAsync("12015550123")).ReturnsAsync(secondary); + _repository.Setup(x => x.GetProfileByHomeNumberAsync("2015550123")).ReturnsAsync(owner); + + var result = await _service.GetProfileByHomeNumberAsync("+12015550123"); + + result.Should().BeSameAs(owner); + } + + [TestCase(null)] + [TestCase("")] + [TestCase(" ")] + [TestCase("+-() .")] + public async Task GetProfileByMobileNumberAsync_never_matches_on_a_blank_number(string inbound) + { + var result = await _service.GetProfileByMobileNumberAsync(inbound); + + result.Should().BeNull(); + _repository.Verify(x => x.GetProfileByMobileNumberAsync(It.IsAny()), Times.Never); + } + + [Test] + public async Task GetProfileByHomeNumberAsync_queries_the_home_number_not_the_mobile_number() + { + var profile = new UserProfile { UserId = "user-1" }; + _repository.Setup(x => x.GetProfileByHomeNumberAsync("12015550123")).ReturnsAsync(profile); + + var result = await _service.GetProfileByHomeNumberAsync("+1 (201) 555-0123"); + + result.Should().BeSameAs(profile); + _repository.Verify(x => x.GetProfileByMobileNumberAsync(It.IsAny()), Times.Never); + } + + [Test] + public async Task GetProfileByHomeNumberAsync_falls_back_to_the_number_without_the_country_code() + { + var profile = new UserProfile { UserId = "user-1" }; + _repository.Setup(x => x.GetProfileByHomeNumberAsync("12015550123")).ReturnsAsync((UserProfile)null); + _repository.Setup(x => x.GetProfileByHomeNumberAsync("2015550123")).ReturnsAsync(profile); + + var result = await _service.GetProfileByHomeNumberAsync("+12015550123"); + + result.Should().BeSameAs(profile); + } + + [TestCase(null)] + [TestCase("")] + public async Task GetProfileByHomeNumberAsync_never_matches_on_a_blank_number(string inbound) + { + var result = await _service.GetProfileByHomeNumberAsync(inbound); + + result.Should().BeNull(); + _repository.Verify(x => x.GetProfileByHomeNumberAsync(It.IsAny()), Times.Never); + } + } +} diff --git a/Tools/Resgrid.Console/Commands/HelpCommand.cs b/Tools/Resgrid.Console/Commands/HelpCommand.cs index 9d56e300f..db010061b 100644 --- a/Tools/Resgrid.Console/Commands/HelpCommand.cs +++ b/Tools/Resgrid.Console/Commands/HelpCommand.cs @@ -25,6 +25,7 @@ public async Task ExecuteMainAsync(string[] args, CancellationToken ca logger.LogInformation("--DbUpdate || --UpdateDb :: Updates the Resgrid Database"); logger.LogInformation("--GenOidcCerts :: Generates the OIDC Certificates"); logger.LogInformation("--MigrateDocsDb :: Migrates the Resgrid Docs Database"); + logger.LogInformation("--NormalizePhoneNumbers [--Apply] [--DepartmentId=1] :: Rewrites stored profile phone numbers to E.164. Dry run unless --Apply is passed"); logger.LogInformation("--OidcUpdate :: Updates the Resgrid OIDC Database"); logger.LogInformation("--ResetPassword -- --UserId=[GUID] --Password=[PASSWORD] :: Resets the password for a user"); logger.LogInformation("--SecurityRefresh :: Refreshes the Resgrid Security Matrix Cache"); diff --git a/Tools/Resgrid.Console/Commands/NormalizePhoneNumbersCommand.cs b/Tools/Resgrid.Console/Commands/NormalizePhoneNumbersCommand.cs new file mode 100644 index 000000000..907566310 --- /dev/null +++ b/Tools/Resgrid.Console/Commands/NormalizePhoneNumbersCommand.cs @@ -0,0 +1,284 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging; +using Resgrid.Console.Models; +using Resgrid.Framework; +using Resgrid.Model; +using Resgrid.Model.Providers; +using Resgrid.Model.Repositories; +using Resgrid.Model.Services; + +namespace Resgrid.Console.Commands +{ + /// + /// One-off sweep that rewrites stored profile phone numbers into the canonical E.164 form the + /// save flow already produces (+12015550123). + /// + /// Most existing rows were written before the phone validation in EditUserProfile/AddPerson + /// existed, so they hold whatever the user typed - "(270) 555-0101", "270-555-0102". Inbound SMS + /// and voice resolve the sender by comparing against the stored number, and those formats match + /// nothing, so those users cannot be identified by a text or a call. Every current write path + /// validates and stores E.164, so this only has to run once. + /// + /// + /// Dry run by default: it reports what it would change and writes nothing. Pass --Apply to + /// commit. Scope to a single department with --DepartmentId=N. + /// + /// + public sealed class NormalizePhoneNumbersCommand( + ILogger logger, + IDepartmentsService departmentsService, + IUserProfilesRepository userProfilesRepository, + IUserProfileService userProfileService, + IAddressService addressService, + IPhoneNumberProcesserProvider phoneNumberProcesser) : ICommandService + { + private sealed record Change(int DepartmentId, string UserId, string Field, string From, string To); + + private sealed record Skip(int DepartmentId, string UserId, string Field, string Value, string Reason); + + public async Task ExecuteMainAsync(string[] args, CancellationToken cancellationToken) + { + var apply = args.Any(a => a.Equals("--Apply", StringComparison.OrdinalIgnoreCase)); + var departmentFilter = ParseDepartmentId(args); + + logger.LogInformation("Resgrid Phone Number Normalization"); + logger.LogInformation(apply + ? "Mode: APPLY - matching profiles will be updated." + : "Mode: DRY RUN - nothing will be written. Pass --Apply to commit."); + + try + { + var departments = await departmentsService.GetAllAsync(); + + if (departments == null || departments.Count == 0) + { + logger.LogWarning("No departments found, nothing to do."); + return ExitCode.Success; + } + + if (departmentFilter.HasValue) + { + departments = departments.Where(d => d.DepartmentId == departmentFilter.Value).ToList(); + + if (departments.Count == 0) + { + logger.LogError("Department {DepartmentId} was not found.", departmentFilter.Value); + return ExitCode.Failed; + } + } + + var changes = new List(); + var skips = new List(); + var scanned = 0; + + foreach (var department in departments) + { + cancellationToken.ThrowIfCancellationRequested(); + + // Includes disabled and deleted members: their rows are still matched by an inbound + // lookup, so leaving them unnormalized leaves the same identification hole open. + var profiles = await userProfilesRepository + .GetAllUserProfilesForDepartmentIncDisabledDeletedAsync(department.DepartmentId); + + if (profiles == null) + continue; + + // Resolved once per department rather than per profile: it is the same lookup for + // everyone in it. + var departmentRegion = await CountryIsoAsync(department.AddressId); + + var pending = new List(); + + foreach (var profile in profiles) + { + scanned++; + + var region = await ResolveRegionAsync(profile, departmentRegion); + + var mobile = Normalize(profile.MobileNumber, "MobileNumber", region, department.DepartmentId, profile.UserId, skips); + var home = Normalize(profile.HomeNumber, "HomeNumber", region, department.DepartmentId, profile.UserId, skips); + + if (mobile == null && home == null) + continue; + + if (mobile != null) + { + changes.Add(new Change(department.DepartmentId, profile.UserId, "MobileNumber", profile.MobileNumber, mobile)); + profile.MobileNumber = mobile; + } + + if (home != null) + { + changes.Add(new Change(department.DepartmentId, profile.UserId, "HomeNumber", profile.HomeNumber, home)); + profile.HomeNumber = home; + } + + profile.LastUpdated = DateTime.UtcNow; + pending.Add(profile); + } + + if (pending.Count == 0) + continue; + + if (apply) + { + await userProfilesRepository.UpdatePhoneNumbersAsync(pending, cancellationToken); + + // The repository write bypasses the service, so evict the profile caches by hand. + // Profiles are cached for 14 days - without this the old value stays live. + foreach (var updated in pending) + userProfileService.ClearUserProfileFromCache(updated.UserId); + + userProfileService.ClearAllUserProfilesFromCache(department.DepartmentId); + } + + logger.LogInformation("Department {DepartmentId} ({Name}): {Count} profile(s) {Action}.", + department.DepartmentId, department.Name, pending.Count, apply ? "updated" : "would be updated"); + } + + Report(scanned, changes, skips, apply); + } + catch (OperationCanceledException) + { + logger.LogWarning("Cancelled. Any department already committed stays committed."); + return ExitCode.Failed; + } + catch (Exception ex) + { + Logging.LogException(ex, "There was an error running the phone number normalization"); + logger.LogError(ex, "Phone number normalization failed."); + return ExitCode.Failed; + } + + return ExitCode.Success; + } + + /// + /// Returns the canonical form when the stored value should be rewritten, or null to leave it + /// alone (blank, already canonical, or not parseable as a real number). + /// + private string Normalize(string number, string field, string region, int departmentId, string userId, + List skips) + { + if (string.IsNullOrWhiteSpace(number)) + return null; + + var result = phoneNumberProcesser.Process(number, region); + + if (result == null || !result.IsValid || string.IsNullOrWhiteSpace(result.InternationalNumber)) + { + // Never guess. A number that does not parse to a real one - a truncated entry, or a + // national format whose country cannot be resolved from the profile's address - is + // reported for a human to look at rather than rewritten into something that would + // dial somewhere else. + skips.Add(new Skip(departmentId, userId, field, number, "does not parse to a valid number")); + return null; + } + + return string.Equals(result.InternationalNumber, number, StringComparison.Ordinal) + ? null + : result.InternationalNumber; + } + + /// + /// The country to interpret a national-format number against. Without one, a stored + /// "270-555-0102" cannot be resolved to a country code at all. + /// + /// Follows EditUserProfile - the home (physical) address country, then the mailing address - + /// and falls back to the department's own address when the profile has neither, or when the + /// two disagree. A member whose physical and mailing addresses sit in different countries + /// gives no reliable answer on its own, so the department the number was issued under is the + /// better authority than picking one of the two arbitrarily. + /// + /// + private async Task ResolveRegionAsync(UserProfile profile, string departmentRegion) + { + var physical = await CountryIsoAsync(profile.HomeAddressId); + var mailing = await CountryIsoAsync(profile.MailingAddressId); + + if (physical != null && mailing != null && + !string.Equals(physical, mailing, StringComparison.OrdinalIgnoreCase)) + return departmentRegion; + + return physical ?? mailing ?? departmentRegion; + } + + private async Task CountryIsoAsync(int? addressId) + { + if (!addressId.HasValue) + return null; + + var address = await addressService.GetAddressByIdAsync(addressId.Value); + + return address == null ? null : PhoneRegionHelper.ToIso(address.Country); + } + + private static int? ParseDepartmentId(string[] args) + { + var argument = args.FirstOrDefault(a => a.StartsWith("--DepartmentId=", StringComparison.OrdinalIgnoreCase)); + + if (argument == null) + return null; + + return int.TryParse(argument.Split('=', 2)[1], out var departmentId) ? departmentId : null; + } + + private void Report(int scanned, List changes, List skips, bool apply) + { + logger.LogInformation("-----------------------------------------"); + logger.LogInformation("Profiles scanned: {Scanned}", scanned); + logger.LogInformation("Numbers {Action}: {Count}", apply ? "rewritten" : "to rewrite", changes.Count); + logger.LogInformation("Numbers skipped: {Count}", skips.Count); + + // After normalization two profiles can land on the same number - the production data + // already holds the same number in two formats on different rows. The inbound lookup + // prefers a verified profile, but these are worth a human look. + var collisions = changes + .Where(c => c.Field == "MobileNumber") + .GroupBy(c => c.To) + .Where(g => g.Select(c => c.UserId).Distinct().Count() > 1) + .ToList(); + + if (collisions.Count > 0) + { + logger.LogWarning("{Count} mobile number(s) end up on more than one profile:", collisions.Count); + + foreach (var collision in collisions) + logger.LogWarning(" {Number} -> {UserIds}", collision.Key, + string.Join(", ", collision.Select(c => c.UserId).Distinct())); + } + + if (skips.Count > 0) + { + var path = Path.Combine(Directory.GetCurrentDirectory(), "phone-normalization-skipped.csv"); + var lines = new List { "DepartmentId,UserId,Field,Value,Reason" }; + + lines.AddRange(skips.Select(s => string.Join(",", + s.DepartmentId.ToString(), + Csv(s.UserId), + Csv(s.Field), + Csv(s.Value), + Csv(s.Reason)))); + + System.IO.File.WriteAllLines(path, lines); + logger.LogInformation("Skipped numbers written to {Path} for review.", path); + } + + if (!apply && changes.Count > 0) + logger.LogInformation("Re-run with --Apply to commit these changes."); + } + + private static string Csv(string value) + { + if (string.IsNullOrEmpty(value)) + return "\"\""; + + return "\"" + value.Replace("\"", "\"\"") + "\""; + } + } +} diff --git a/Tools/Resgrid.Console/Program.cs b/Tools/Resgrid.Console/Program.cs index f75fe44c9..9feef83fc 100644 --- a/Tools/Resgrid.Console/Program.cs +++ b/Tools/Resgrid.Console/Program.cs @@ -108,6 +108,7 @@ static async Task Main(string[] args) services.AddKeyedTransient("GenOidcCertsCommand"); services.AddKeyedTransient("MigrateDocsDbCommand"); services.AddKeyedTransient("CleanUtf8Command"); + services.AddKeyedTransient("NormalizePhoneNumbersCommand"); services.AddKeyedTransient("OidcUpdateCommand"); services.AddKeyedTransient("SecurityRefreshCommand"); services.AddKeyedTransient("HelpCommand"); diff --git a/Tools/Resgrid.Console/Services/ApplicationHostedService.cs b/Tools/Resgrid.Console/Services/ApplicationHostedService.cs index 7239c2fc3..9660c968d 100644 --- a/Tools/Resgrid.Console/Services/ApplicationHostedService.cs +++ b/Tools/Resgrid.Console/Services/ApplicationHostedService.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -31,6 +31,7 @@ public sealed class ApplicationHostedService : IHostedService, IDisposable private ICommandService _genOidcCertsCommand; private ICommandService _migrateDocsDbCommand; private ICommandService _cleanUtf8Command; + private ICommandService _normalizePhoneNumbersCommand; private ICommandService _oidcUpdateCommand; private ICommandService _securityRefreshCommand; private ICommandService _helpCommand; @@ -57,6 +58,7 @@ public ApplicationHostedService( [FromKeyedServices("GenOidcCertsCommand")] ICommandService genOidcCertsCommand, [FromKeyedServices("MigrateDocsDbCommand")] ICommandService migrateDocsDbCommand, [FromKeyedServices("CleanUtf8Command")] ICommandService cleanUtf8Command, + [FromKeyedServices("NormalizePhoneNumbersCommand")] ICommandService normalizePhoneNumbersCommand, [FromKeyedServices("OidcUpdateCommand")] ICommandService oidcUpdateCommand, [FromKeyedServices("SecurityRefreshCommand")] ICommandService securityRefreshCommand, [FromKeyedServices("HelpCommand")] ICommandService helpCommand) @@ -70,6 +72,7 @@ public ApplicationHostedService( _genOidcCertsCommand = genOidcCertsCommand; _migrateDocsDbCommand = migrateDocsDbCommand; _cleanUtf8Command = cleanUtf8Command; + _normalizePhoneNumbersCommand = normalizePhoneNumbersCommand; _oidcUpdateCommand = oidcUpdateCommand; _securityRefreshCommand = securityRefreshCommand; _helpCommand = helpCommand; @@ -197,6 +200,8 @@ private async Task ExecuteMainAsync(string[] args, CancellationToken c return await _migrateDocsDbCommand.ExecuteMainAsync(args, cancellationToken).ConfigureAwait(false); else if (args.Contains("--CleanUtf8")) return await _cleanUtf8Command.ExecuteMainAsync(args, cancellationToken).ConfigureAwait(false); + else if (args.Contains("--NormalizePhoneNumbers")) + return await _normalizePhoneNumbersCommand.ExecuteMainAsync(args, cancellationToken).ConfigureAwait(false); else if (args.Contains("--OidcUpdate")) return await _oidcUpdateCommand.ExecuteMainAsync(args, cancellationToken).ConfigureAwait(false); else if (args.Contains("--SecurityRefresh"))