Merge pull request #626 from cabinetoffice/EHD-1663-sentry-errors

EHD-1663: Reduce errors in Sentry

Author: jamesgriff

GenderPayGap.WebUI/BackgroundJobs/ScheduledJobs/NotifyUsersAndRetireInactiveAccountsJob.cs

@@ -37,7 +37,11 @@ private void SendReminderEmails()
             DateTime threeYearsAgoMinusSevenDays = threeYearsAgo.AddDays(7);
 
             List<User> usersToSendReminders = dataRepository.GetAll<User>()
-                .Where(u => u.LoginDate >= threeYearsAgoMinusThirtyDays && u.LoginDate < threeYearsAgoMinusThirtyDays.AddDays(1) || u.LoginDate >= threeYearsAgoMinusSevenDays && u.LoginDate < threeYearsAgoMinusSevenDays.AddDays(1) )
+                .Where(u =>
+                    (u.LoginDate >= threeYearsAgoMinusThirtyDays && u.LoginDate < threeYearsAgoMinusThirtyDays.AddDays(1)) ||
+                    (u.LoginDate >= threeYearsAgoMinusSevenDays && u.LoginDate < threeYearsAgoMinusSevenDays.AddDays(1))
+                )
+                .Where(u => !u.HasBeenAnonymised)
                 .ToList();
 
             foreach (User user in usersToSendReminders)

GenderPayGap.WebUI/Controllers/Account/CloseAccountController.cs

@@ -132,7 +132,10 @@ private void RetireUserAccount(User user)
         private void SendAccountClosedEmail(User user)
         {
             // Send email to user informing them of account closure
-            emailSendingService.SendCloseAccountCompletedEmail(user.EmailAddress);
+            if (!user.HasBeenAnonymised)
+            {
+                emailSendingService.SendCloseAccountCompletedEmail(user.EmailAddress);
+            }
         }
 
         private void InformGeoOfOrphanedOrganisations(List<Organisation> orphanedOrganisations)

GenderPayGap.WebUI/Controllers/CompareEmployersController.cs

@@ -33,7 +33,9 @@ public IActionResult AddEmployer(long organisationId, string returnUrl)
             comparisonBasketService.AddToBasket(organisationId);
             comparisonBasketService.SaveComparedEmployersToCookie();
 
-            return LocalRedirect(returnUrl);
+            return Url.IsLocalUrl(returnUrl)
+                ? LocalRedirect(returnUrl)
+                : RedirectToAction("CompareEmployersNoYear", "CompareEmployers");
         }
 
         [HttpGet("/compare-employers/add-js/{organisationId}")]
@@ -59,7 +61,14 @@ public IActionResult RemoveEmployer(long organisationId, string returnUrl)
             comparisonBasketService.RemoveFromBasket(organisationId);
             comparisonBasketService.SaveComparedEmployersToCookie();
 
-            return LocalRedirect(returnUrl);
+            if (Url.IsLocalUrl(returnUrl))
+            {
+                return LocalRedirect(returnUrl);
+            }
+
+            return comparisonBasketService.ComparedEmployers.Any()
+                ? RedirectToAction("CompareEmployersNoYear", "CompareEmployers")
+                : RedirectToAction("SearchPage", "Search");
         }
 
         [HttpGet("/compare-employers/remove-js/{organisationId}")]
@@ -82,7 +91,14 @@ public IActionResult ClearEmployers(string returnUrl)
             comparisonBasketService.ClearBasket();
             comparisonBasketService.SaveComparedEmployersToCookie();
 
-            return LocalRedirect(returnUrl);
+            if (Url.IsLocalUrl(returnUrl))
+            {
+                return LocalRedirect(returnUrl);
+            }
+
+            return comparisonBasketService.ComparedEmployers.Any()
+                ? RedirectToAction("CompareEmployersNoYear", "CompareEmployers")
+                : RedirectToAction("SearchPage", "Search");
         }
 
         [HttpGet("/compare-employers")]

GenderPayGap.WebUI/Controllers/HealthCheckController.cs

@@ -1,5 +1,7 @@
+using GenderPayGap.Core.Classes.Logger;
 using GenderPayGap.Core.Interfaces;
 using GenderPayGap.Database;
+using GenderPayGap.WebUI.ErrorHandling;
 using GenderPayGap.WebUI.ExternalServices.FileRepositories;
 using GenderPayGap.WebUI.Search;
 using Microsoft.AspNetCore.Mvc;
@@ -37,7 +39,8 @@ private void CheckDatabaseConnection()
 
             if (anOrganisation == null)
             {
-                throw new Exception("Could not load an organisation from the database");
+                CustomLogger.Error("Health Check Failed: Could not load an organisation from the database");
+                throw new HealthCheckFailedException();
             }
         }
 
@@ -64,7 +67,8 @@ private void CheckFileConnection()
             }
             catch (Exception e)
             {
-                throw new Exception($"Could not read or write a file: {e.Message}", e);
+                CustomLogger.Error($"Health Check Failed: Could not read or write a file: {e.Message}");
+                throw new HealthCheckFailedException();
             }
         }
 
@@ -73,7 +77,8 @@ private void CheckSearchRepositoryIsLoaded()
             if (SearchRepository.CachedOrganisations == null ||
                 SearchRepository.CachedUsers == null)
             {
-                throw new Exception("SearchRepository was not loaded");
+                CustomLogger.Error("Health Check Failed: SearchRepository was not loaded");
+                throw new HealthCheckFailedException();
             }
         }
     }

GenderPayGap.WebUI/Controllers/ManageOrganisations/RemoveUserFromOrganisationController.cs

@@ -101,13 +101,19 @@ private void RemoveUserFromOrganisation(UserOrganisation userOrganisationToRemov
             registrationRepository.RemoveRegistration(userOrganisationToRemove, actionByUser);
 
             // Email user that has been unregistered
-            emailSendingService.SendRemovedUserFromOrganisationEmail(
-                userToUnregister.EmailAddress,
-                orgToRemove.OrganisationName,
-                userToUnregister.Fullname);
+            if (!userToUnregister.HasBeenAnonymised)
+            {
+                emailSendingService.SendRemovedUserFromOrganisationEmail(
+                    userToUnregister.EmailAddress,
+                    orgToRemove.OrganisationName,
+                    userToUnregister.Fullname);
+            }
 
             // Email the other users of the organisation
-            IEnumerable<string> emailAddressesForOrganisation = orgToRemove.UserOrganisations.Select(uo => uo.User.EmailAddress);
+            IEnumerable<string> emailAddressesForOrganisation = orgToRemove.UserOrganisations
+                .Where(uo => !uo.User.HasBeenAnonymised)
+                .Select(uo => uo.User.EmailAddress);
+            
             foreach (string emailAddress in emailAddressesForOrganisation)
             {
                 emailSendingService.SendRemovedUserFromOrganisationEmail(

GenderPayGap.WebUI/Controllers/ScopeController.cs

@@ -263,6 +263,7 @@ public void SendScopeChangeEmails(Organisation organisation, DateTime reportingY
                 // Find all email addresses associated with the organisation - only the active ones (who have confirmed their PIN)
                 IEnumerable<string> emailAddressesForOrganisation = organisation.UserOrganisations
                     .Where(uo => uo.PINConfirmedDate.HasValue)
+                    .Where(uo => !uo.User.HasBeenAnonymised)
                     .Select(uo => uo.User.EmailAddress);
 
                 // Send email of correct type to each email address associated with organisation

GenderPayGap.WebUI/ErrorHandling/CustomErrorPageExceptions.cs

@@ -85,4 +85,10 @@ public class RegistrationNotFoundException : CustomErrorPageException
         public override string ViewName => "../Errors/RegistrationNotFound";
         public override int StatusCode => 404;
     }
+    
+    public class HealthCheckFailedException : CustomErrorPageException
+    {
+        public override string ViewName => "../Errors/ThereIsAProblemWithTheService";
+        public override int StatusCode => 500;
+    }
 }

GenderPayGap.WebUI/GenderPayGap.WebUI.csproj

@@ -91,7 +91,7 @@
   <ItemGroup>
     <ProjectReference Include="..\GenderPayGap.Core\GenderPayGap.Core.csproj" />
     <ProjectReference Include="..\GenderPayGap.Database\GenderPayGap.Database.csproj" />
-    <PackageReference Include="CabinetOffice.GovUkDesignSystem" Version="2.0.1" />
+    <PackageReference Include="CabinetOffice.GovUkDesignSystem" Version="2.0.2" />
   </ItemGroup>
 
 </Project>

GenderPayGap.WebUI/Helpers/StaticAssetsVersioningHelper.cs

@@ -1,4 +1,5 @@
-using System.Reflection;
+using System.Collections.Concurrent;
+using System.Reflection;
 using System.Text.RegularExpressions;
 using GenderPayGap.Extensions.AspNetCore;
 
@@ -13,7 +14,7 @@ public static class StaticAssetsVersioningHelper
         private const string AppIe8CssRegex = "app-ie8-[^-]*.css";
         private const string AppJsRegex = "app-.*.js";
 
-        private static Dictionary<string, string> cachedFilenames = new Dictionary<string, string>();
+        private static ConcurrentDictionary<string, string> cachedFilenames = new();
 
         public static string GetAppCssFilename() => GetStaticFile(CompiledDirectory, AppCssRegex);
         public static string GetAppIe8CssFilename() => GetStaticFile(CompiledDirectory, AppIe8CssRegex);
@@ -32,11 +33,13 @@ private static string GetStaticFile(string directory, string fileRegex)
                 // cache the filename so we don't need to search a directory for each request
                 string cacheKey = directory + "/" + fileRegex;
 
-                if (!cachedFilenames.ContainsKey(cacheKey))
+                if (cachedFilenames.TryGetValue(cacheKey, out string filename))
                 {
-                    cachedFilenames[cacheKey] = FindMatchingFile(directory, fileRegex);
+                    return filename;
                 }
 
+                cachedFilenames[cacheKey] = FindMatchingFile(directory, fileRegex);
+
                 return cachedFilenames[cacheKey];
             }
         }

GenderPayGap.WebUI/Services/EmailSendingServiceHelpers.cs

@@ -21,6 +21,7 @@ public static void SendUserAddedEmailToExistingUsers(Organisation organisation,
         public static void SendSuccessfulSubmissionEmailToRegisteredUsers(Return postedReturn, string reportLink, string submittedOrUpdated, EmailSendingService emailSendingService)
         {
             IEnumerable<string> emailAddressesForOrganisation = postedReturn.Organisation.UserOrganisations
+                .Where(uo => !uo.User.HasBeenAnonymised)
                 .Select(uo => uo.User.EmailAddress);
 
             foreach (string emailAddress in emailAddressesForOrganisation)