Skip to content

Commit 9fa61df

Browse files
committed
IDAM-528 Refactor
1 parent 9d1af90 commit 9fa61df

6 files changed

Lines changed: 44 additions & 29 deletions

File tree

NICE.Identity.Authorisation.WebAPI/Controllers/InactiveAccountsController.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ public class InactiveAccountsController : ControllerBase
1717
{
1818
private readonly ILogger<InactiveAccountsController> _logger;
1919
private readonly IUsersService _usersService;
20-
private const int yearsToKeepInActiveAccounts = 3;
20+
private const int yearsToKeepInactiveAccounts = 3;
2121

2222
public InactiveAccountsController(ILogger<InactiveAccountsController> logger, IUsersService usersService)
2323
{
@@ -38,7 +38,7 @@ public async Task<IActionResult> DeleteAllOverAgeWithNotification()
3838
{
3939
try
4040
{
41-
await _usersService.DeleteInActiveAccountsOlderThan(notify: true, yearsToKeepInActiveAccounts);
41+
await _usersService.DeleteInactiveAccountsOlderThan(notify: true, yearsToKeepInactiveAccounts);
4242
return Ok();
4343
}
4444
catch (Exception e)

NICE.Identity.Authorisation.WebAPI/Repositories/Context.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -318,12 +318,13 @@ public int AddUsersToRole(IEnumerable<User> users, int roleId)
318318

319319
#endregion
320320

321-
public IEnumerable<User> GetInActiveUsersOverAge(int yearsToKeepInActiveAcounts)
321+
public IEnumerable<User> GetInActiveUsersOverAge(int yearsToKeepInactiveAcounts)
322322
{
323-
var dateToKeepAccountsFrom = DateTime.UtcNow.AddYears(-yearsToKeepInActiveAcounts);
323+
var dateToKeepAccountsFrom = DateTime.UtcNow.AddYears(-yearsToKeepInactiveAcounts);
324324

325325
return Users.Where(u => u.HasVerifiedEmailAddress &&
326-
u.LastLoggedInDate.HasValue && u.LastLoggedInDate.Value <= dateToKeepAccountsFrom);
326+
u.LastLoggedInDate.HasValue && u.LastLoggedInDate.Value <= dateToKeepAccountsFrom
327+
&& !u.EmailAddress.Contains("@nice.org.uk"));
327328
}
328329
}
329330
}

NICE.Identity.Authorisation.WebAPI/Services/EmailService.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,23 +12,23 @@ namespace NICE.Identity.Authorisation.WebAPI.Services
1212
public interface IEmailService
1313
{
1414
void SendPendingAccountRemovalNotifications(IList<string> toEmailAddresses);
15-
void SendInActiveAccountRemovalNotifications(IList<string> toEmailAddresses);
15+
void SendInactiveAccountRemovalNotifications(IList<string> toEmailAddresses);
1616
}
1717

1818
public class EmailService : IEmailService
1919
{
2020
private readonly string _notificationEmailHTMLPath;
2121
private readonly string _notificationEmailTextPath;
22-
private readonly string _inActiveNotificationEmailHTMLPath;
23-
private readonly string _inActiveNotificationEmailTextPath;
22+
private readonly string _inactiveNotificationEmailHTMLPath;
23+
private readonly string _inactiveNotificationEmailTextPath;
2424

2525
public EmailService(IWebHostEnvironment webHostEnvironment)
2626
{
2727
var pathToEmails = Path.Combine(webHostEnvironment.ContentRootPath, "Emails");
2828
_notificationEmailHTMLPath = Path.Combine(pathToEmails, "account_removal.html");
2929
_notificationEmailTextPath = Path.Combine(pathToEmails, "account_removal.txt");
30-
_inActiveNotificationEmailHTMLPath = Path.Combine(pathToEmails, "inactive_account_removal.html");
31-
_inActiveNotificationEmailTextPath = Path.Combine(pathToEmails, "inactive_account_removal.txt");
30+
_inactiveNotificationEmailHTMLPath = Path.Combine(pathToEmails, "inactive_account_removal.html");
31+
_inactiveNotificationEmailTextPath = Path.Combine(pathToEmails, "inactive_account_removal.txt");
3232
}
3333

3434
public void SendPendingAccountRemovalNotifications(IList<string> toEmailAddresses)
@@ -60,7 +60,7 @@ public void SendPendingAccountRemovalNotifications(IList<string> toEmailAddresse
6060
}
6161

6262

63-
public void SendInActiveAccountRemovalNotifications(IList<string> toEmailAddresses)
63+
public void SendInactiveAccountRemovalNotifications(IList<string> toEmailAddresses)
6464
{
6565
if (toEmailAddresses == null || !toEmailAddresses.Any())
6666
return;
@@ -79,8 +79,8 @@ public void SendInActiveAccountRemovalNotifications(IList<string> toEmailAddress
7979
}
8080

8181
var bodyBuilder = new BodyBuilder();
82-
bodyBuilder.HtmlBody = File.ReadAllText(_inActiveNotificationEmailHTMLPath);
83-
bodyBuilder.TextBody = File.ReadAllText(_inActiveNotificationEmailTextPath);
82+
bodyBuilder.HtmlBody = File.ReadAllText(_inactiveNotificationEmailHTMLPath);
83+
bodyBuilder.TextBody = File.ReadAllText(_inactiveNotificationEmailTextPath);
8484
var messageBody = bodyBuilder.ToMessageBody();
8585

8686
const string subject = "Inactive account removal";

NICE.Identity.Authorisation.WebAPI/Services/UsersService.cs

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public interface IUsersService
3131
IList<UserRole> UpdateRolesForUser(int userId, List<UserRole> userRolesToUpdate);
3232
Task<int> DeleteAllUsers();
3333
Task DeleteRegistrationsOlderThan(bool notify, int daysToKeepPendingRegistration);
34-
Task DeleteInActiveAccountsOlderThan(bool notify, int yearsToKeepPendingRegistration);
34+
Task DeleteInactiveAccountsOlderThan(bool notify, int yearsToKeepPendingRegistration);
3535
IList<User> GetUsersByOrganisationId(int organisationId);
3636
UsersAndJobIdsForOrganisation GetUsersAndJobIdsByOrganisationId(int organisationId);
3737
}
@@ -503,38 +503,38 @@ public async Task DeleteRegistrationsOlderThan(bool notify, int daysToKeepPendin
503503
_logger.LogWarning($"DeleteRegistrationsOlderThan - Total records deleted : {recordsDeleted}");
504504
}
505505

506-
public async Task DeleteInActiveAccountsOlderThan(bool notify, int yearsToKeepInActiveAcounts)
506+
public async Task DeleteInactiveAccountsOlderThan(bool notify, int yearsToKeepInactiveAcounts)
507507
{
508-
_logger.LogWarning($"DeleteInActiveAccountsOlderThan - Deleting Inactive Accounts Older Than {yearsToKeepInActiveAcounts} years."); //extra logging here in order to verify that the scheduled task is running via kibana.
508+
_logger.LogWarning($"DeleteInActiveAccountsOlderThan - Deleting Inactive Accounts Older Than {yearsToKeepInactiveAcounts} years."); //extra logging here in order to verify that the scheduled task is running via kibana.
509509

510-
var allUsersWithInActiveAccountsOverAge = _context.GetInActiveUsersOverAge(yearsToKeepInActiveAcounts).ToList();
510+
var allUsersWithInactiveAccountsOverAge = _context.GetInActiveUsersOverAge(yearsToKeepInactiveAcounts).ToList();
511511

512-
if (!allUsersWithInActiveAccountsOverAge.Any())
512+
if (!allUsersWithInactiveAccountsOverAge.Any())
513513
{
514514
_logger.LogWarning("DeleteInActiveAccountsOlderThan - No records found to delete. exiting");
515515
return;
516516
}
517517

518-
var uniqueEmailAddresses = allUsersWithInActiveAccountsOverAge.Select(u => u.EmailAddress).Distinct().ToList();
519-
if (uniqueEmailAddresses.Count() != allUsersWithInActiveAccountsOverAge.Count())
518+
var uniqueEmailAddresses = allUsersWithInactiveAccountsOverAge.Select(u => u.EmailAddress).Distinct().ToList();
519+
if (uniqueEmailAddresses.Count() != allUsersWithInactiveAccountsOverAge.Count())
520520
{
521521
_logger.LogWarning("Inactive accounts exist for the same email address.");
522522
}
523523

524524
//1. delete user accounts: allUsersWithInActiveAccountsOverAge
525-
var recordsDeleted = await _context.DeleteUsers(allUsersWithInActiveAccountsOverAge);
525+
var recordsDeleted = await _context.DeleteUsers(allUsersWithInactiveAccountsOverAge);
526526

527527
//2. delete user account in auth0
528-
foreach (var user in allUsersWithInActiveAccountsOverAge)
528+
foreach (var user in allUsersWithInactiveAccountsOverAge)
529529
{
530530
await _providerManagementService.DeleteUser(user.NameIdentifier);
531531
}
532532

533533
//3. send notification to the email addresses, one email per email address.
534534
if (notify)
535-
_emailService.SendInActiveAccountRemovalNotifications(uniqueEmailAddresses);
535+
_emailService.SendInactiveAccountRemovalNotifications(uniqueEmailAddresses);
536536

537-
_logger.LogWarning($"DeleteInActiveAccountsOlderThan - Total records deleted: {recordsDeleted}");
537+
_logger.LogWarning($"DeleteInactiveAccountsOlderThan - Total records deleted: {recordsDeleted}");
538538

539539
}
540540

NICE.Identity.Test/Infrastructure/MockUserService.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public Task DeleteRegistrationsOlderThan(bool notify, int daysToKeepPendingRegis
8383
throw new NotImplementedException();
8484
}
8585

86-
public Task DeleteInActiveAccountsOlderThan(bool notify, int yearsToKeepPendingRegistration)
86+
public Task DeleteInactiveAccountsOlderThan(bool notify, int yearsToKeepPendingRegistration)
8787
{
8888
throw new NotImplementedException();
8989
}

NICE.Identity.Test/UnitTests/Authorisation.WebAPI/Services/UserServiceTests.cs

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -699,13 +699,14 @@ public async Task TestPendingRegistrationDeletion()
699699
}
700700

701701
[Fact]
702-
public async Task TestInActiveAccounts()
702+
public async Task TestInactiveAccounts()
703703
{
704704
//Arrange
705705
var context = GetContext();
706706
var userService = new UsersService(context, _logger.Object, _providerManagementService.Object, null);
707707
const string user1NameIdentifier = "auth|user1";
708708
const string user2NameIdentifier = "auth|user2";
709+
const string user3NameIdentifier = "auth|user3";
709710

710711
userService.CreateUser(new ApiModels.User
711712
{
@@ -716,6 +717,8 @@ public async Task TestInActiveAccounts()
716717
HasVerifiedEmailAddress = true,
717718
LastLoggedInDate = DateTime.UtcNow
718719
});
720+
721+
719722
userService.CreateUser(new ApiModels.User
720723
{
721724
NameIdentifier = user2NameIdentifier,
@@ -725,6 +728,16 @@ public async Task TestInActiveAccounts()
725728
HasVerifiedEmailAddress = true,
726729
LastLoggedInDate = DateTime.UtcNow.AddYears(-3)
727730
});
731+
732+
userService.CreateUser(new ApiModels.User
733+
{
734+
NameIdentifier = user3NameIdentifier,
735+
FirstName = "User not to be deleted",
736+
LastName = "",
737+
EmailAddress = "user3@nice.org.uk",
738+
HasVerifiedEmailAddress = true,
739+
LastLoggedInDate = DateTime.UtcNow.AddYears(-3)
740+
});
728741
context.Users.Single(u => u.NameIdentifier == user2NameIdentifier).LastLoggedInDate = DateTime.UtcNow.AddYears(-3);
729742
context.SaveChanges();
730743
//now add some related data to test that related entities are deleted too.
@@ -737,11 +750,12 @@ public async Task TestInActiveAccounts()
737750
context.SaveChanges();
738751

739752
//Act
740-
await userService.DeleteInActiveAccountsOlderThan(notify: false, yearsToKeepInActiveAcounts: 3);
753+
await userService.DeleteInactiveAccountsOlderThan(notify: false, yearsToKeepInactiveAcounts: 3);
741754

742755
//Assert
743-
context.Users.Count().ShouldBe(1);
744-
context.Users.Single().NameIdentifier.ShouldBe(user1NameIdentifier);
756+
context.Users.Count().ShouldBe(2);
757+
context.Users.Where(u => u.NameIdentifier == user1NameIdentifier).ShouldNotBeNull();
758+
context.Users.Where(u => u.NameIdentifier == user3NameIdentifier).ShouldNotBeNull();
745759
context.Jobs.Count().ShouldBe(0);
746760
}
747761

0 commit comments

Comments
 (0)