Skip to content

HEEDLS-548 Exclude centres from self registration #501

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 23, 2021
Merged

HEEDLS-548 Exclude centres from self registration #501

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
300de7d
HEEDLS-548 Exclude centres with kbSelfRegister=0
Jul 21, 2021
9e10035
HEEDLS-548 Add unit test
Jul 22, 2021
994c16c
HEEDLS-548 Modified method name to clarify it gets centres for delega…
Jul 23, 2021
95168ff
HEEDLS-548 changed method name where referenced
Jul 23, 2021
4a3dbd3
HEEDLS-548 Remove active from method name
Jul 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions DigitalLearningSolutions.Data.Tests/DataServices/CentresDataServiceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,26 +74,38 @@ public void Get_centre_name_should_return_null_when_the_centre_does_not_exist()
}

[Test]
public void Get_active_centres_should_contain_an_active_centre()
public void Get_centres_for_delegate_self_registration_should_contain_an_active_centre()
{
// When
var result = centresDataService.GetActiveCentresAlphabetical().ToList();
var result = centresDataService.GetCentresForDelegateSelfRegistrationAlphabetical().ToList();

// Then
result.Contains((2, "North West Boroughs Healthcare NHS Foundation Trust")).Should().BeTrue();
}

[Test]
public void Get_active_centres_should_not_contain_an_inactive_centre()
public void Get_centres_for_delegate_self_registration_should_not_contain_an_inactive_centre()
{
// When
var result = centresDataService.GetActiveCentresAlphabetical().ToList();
var result = centresDataService.GetCentresForDelegateSelfRegistrationAlphabetical().ToList();

// Then
result.Contains((6, "Closed_Basildon and Thurrock University Hospitals NHS Foundation Trust"))
.Should().BeFalse();
}

[Test]
public void
Get_centres_for_delegate_self_registration_should_not_contain_a_centre_where_a_delegate_cannot_self_register()
{
// When
var result = centresDataService.GetCentresForDelegateSelfRegistrationAlphabetical().ToList();

// Then
result.Contains((4, "Brighton and Sussex University Hospitals NHS Trust"))
.Should().BeFalse();
}

[Test]
public void GetCentreDetailsById_should_return_the_correct_values()
{
Expand Down Expand Up @@ -239,7 +251,6 @@ public void UpdateCentreDetails_updates_centre()
var signature = new byte[100];
var logo = new byte[200];


// When
centresDataService.UpdateCentreDetails(2, notifyEmail, bannerText, signature, logo);
var updatedCentre = centresDataService.GetCentreDetailsById(2)!;
Expand Down
5 changes: 3 additions & 2 deletions DigitalLearningSolutions.Data/DataServices/CentresDataService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public interface ICentresDataService
{
string? GetBannerText(int centreId);
string? GetCentreName(int centreId);
IEnumerable<(int, string)> GetActiveCentresAlphabetical();
IEnumerable<(int, string)> GetCentresForDelegateSelfRegistrationAlphabetical();
Centre? GetCentreDetailsById(int centreId);

void UpdateCentreManagerDetails(
Expand Down Expand Up @@ -92,13 +92,14 @@ FROM Centres
return name;
}

public IEnumerable<(int, string)> GetActiveCentresAlphabetical()
public IEnumerable<(int, string)> GetCentresForDelegateSelfRegistrationAlphabetical()
{
var centres = connection.Query<(int, string)>
(
@"SELECT CentreID, CentreName
FROM Centres
WHERE Active = 1
AND kbSelfRegister = 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to put this in the db query level given that this is only used in registration, but I think we should modify the name of the method so it's clear that we're getting the centre options for delegate self registration (rather than a true active centres list, which should technically only have the Active = 1 constraint)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea! I changed the method name to account for this

ORDER BY CentreName"
);
return centres;
Expand Down
4 changes: 2 additions & 2 deletions DigitalLearningSolutions.Web/Controllers/Register/RegisterController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ public class RegisterController : Controller
private readonly ICentresDataService centresDataService;
private readonly ICryptoService cryptoService;
private readonly CustomPromptHelper customPromptHelper;
private readonly IFeatureManager featureManager;
private readonly IJobGroupsDataService jobGroupsDataService;
private readonly IRegistrationService registrationService;
private readonly IUserService userService;
private readonly IFeatureManager featureManager;

public RegisterController(
ICentresDataService centresDataService,
Expand Down Expand Up @@ -331,7 +331,7 @@ private void PopulatePersonalInformationExtraFields(PersonalInformationViewModel
{
model.CentreName = model.Centre.HasValue ? centresDataService.GetCentreName(model.Centre.Value) : null;
model.CentreOptions = SelectListHelper.MapOptionsToSelectListItems(
centresDataService.GetActiveCentresAlphabetical(),
centresDataService.GetCentresForDelegateSelfRegistrationAlphabetical(),
model.Centre
);
}
Expand Down