Skip to content

Bug/dsi 8662 organisationservices http404 #210

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

Open
stephan-williamson wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Bug/dsi 8662 organisationservices http404 #210

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4239012
Always return matching user details
Jun 26, 2026
f193317
Fix broken unit tests
Jul 1, 2026
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
13 changes: 9 additions & 4 deletions src/Dfe.SignIn.Core.Contracts/Users/GetUserOrganisationService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public class GetUserOrganisationService
/// <summary>
/// Gets or sets the unique identifier of the organisation.
/// </summary>
public Guid OrganisationId { get; set; }
public Guid? OrganisationId { get; set; }

/// <summary>
/// Gets or sets the name of the organisation.
Expand Down Expand Up @@ -69,7 +69,7 @@ public class GetUserOrganisationService
/// <summary>
/// Gets or sets the organisation status identifier.
/// </summary>
public int StatusId { get; set; }
public int? StatusId { get; set; }

/// <summary>
/// Gets or sets the date the organisation was closed.
Expand Down Expand Up @@ -159,7 +159,7 @@ public class GetUserOrganisationService
/// <summary>
/// Gets or sets the unique identifier of the service.
/// </summary>
public Guid ServiceId { get; set; }
public Guid? ServiceId { get; set; }

/// <summary>
/// Gets or sets the name of the service.
Expand All @@ -184,5 +184,10 @@ public class GetUserOrganisationService
/// <summary>
/// Gets or sets the organisation role identifier.
/// </summary>
public short OrgRoleId { get; set; }
public short? OrgRoleId { get; set; }

/// <summary>
/// True if the user belongs to the passed in service at any active organisation.
/// </summary>
public int IsInService { get; set; }
}
29 changes: 17 additions & 12 deletions src/Dfe.SignIn.PublicApi/MappingExtensions/GetUserOrganisationServiceMapping.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,22 @@ public static class GetUserOrganisationServiceMapping
/// <returns></returns>
public static IEnumerable<GetUserOrganisationServicesResponse> ToUserDtos(this IEnumerable<GetUserOrganisationService> models)
{
var hasService = models.Any(x => x.IsInService == 1);

return models
.GroupBy(x => x.UserId)
.Select(userGroup => {
GetUserOrganisationService u = userGroup.First();
var u = userGroup.First();

return new GetUserOrganisationServicesResponse {
UserId = u.UserId,
UserStatus = u.UserStatus,
Email = u.Email,
FamilyName = u.FamilyName,
GivenName = u.GivenName,

Organisations = userGroup.ToOrganisationDtos()

Organisations = hasService
? userGroup.ToOrganisationDtos()
: []
};
});
}
Expand All @@ -45,12 +47,13 @@ public static IEnumerable<OrganisationDto> ToOrganisationDtos(
this IGrouping<Guid, GetUserOrganisationService> userGroup)
{
return userGroup
.GroupBy(x => x.OrganisationId)
.Where(x => x.OrganisationId.HasValue) // Filter out nulls
.GroupBy(x => x.OrganisationId!.Value) // Use ! to suppress nullable warning
.Select(static orgGroup => {
var o = orgGroup.First();

return new OrganisationDto {
Id = o.OrganisationId,
Id = o.OrganisationId ?? Guid.Empty,
Name = o.OrganisationName,

Category = new CategoryDto {
Expand All @@ -63,10 +66,12 @@ public static IEnumerable<OrganisationDto> ToOrganisationDtos(
Ukprn = o.Ukprn,
EstablishmentNumber = o.EstablishmentNumber,

Status = new StatusDto {
Id = o.StatusId,
Name = EnumHelpers.MapEnum<OrganisationStatus>(o.StatusId).GetDescription()
},
Status = o.StatusId.HasValue
? new StatusDto {
Id = o.StatusId.Value,
Name = EnumHelpers.MapEnum<OrganisationStatus>(o.StatusId).GetDescription()
}
: new StatusDto(),

ClosedOn = o.ClosedOn,
Address = o.Address,
Expand All @@ -88,8 +93,8 @@ public static IEnumerable<OrganisationDto> ToOrganisationDtos(

Services = orgGroup.ToServiceDtos(),

OrgRoleId = o.OrgRoleId,
OrgRoleName = OrganisationRoles.FromId(o.OrgRoleId)?.Name
OrgRoleId = o.OrgRoleId ?? 0, // Fix: Use fallback if null
OrgRoleName = o.OrgRoleId.HasValue ? OrganisationRoles.FromId(o.OrgRoleId.Value)?.Name : null // Fix: Only access .Value if not null
};
});

Expand Down
46 changes: 24 additions & 22 deletions src/Dfe.SignIn.PublicApi/Repository/OrganisationRepository.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,36 +89,38 @@ u.sub AS UserId
,r.[Name] AS RoleName
,r.Code AS RoleCode
,uo.role_id AS OrgRoleId
FROM dbo.user_organisation uo
JOIN dbo.[user] u
ON u.sub = uo.user_id
JOIN dbo.organisation o
ON o.Id = uo.organisation_id
JOIN dbo.user_services us
,CASE WHEN EXISTS (
SELECT *
FROM dbo.user_services us2
JOIN dbo.[service] s2
ON s2.id = us2.service_id
JOIN dbo.[organisation] o2
ON o2.Id = us2.organisation_id
WHERE
us2.organisation_id = uo.organisation_id
AND us2.user_id = uo.user_id
AND s2.clientId = {clientName}
AND o2.[Status] <> 0
)
THEN 1 ELSE 0 END AS IsInService
FROM dbo.[user] u
LEFT OUTER JOIN dbo.user_organisation uo
ON uo.user_id = u.sub
LEFT OUTER JOIN dbo.organisation o
ON o.Id = uo.organisation_id AND o.[Status] <> 0
LEFT OUTER JOIN dbo.user_services us
ON us.organisation_id = uo.organisation_id
AND us.user_id = uo.user_id
LEFT JOIN dbo.[service] s
LEFT OUTER JOIN dbo.[service] s
ON s.id = us.service_id
LEFT JOIN dbo.user_service_roles usr
LEFT OUTER JOIN dbo.user_service_roles usr
ON usr.organisation_id = us.organisation_id
AND usr.service_id = us.service_id
AND usr.user_id = us.user_id
LEFT JOIN dbo.[Role] r
LEFT OUTER JOIN dbo.[Role] r
ON r.Id = usr.role_id
WHERE
uo.user_id = {userId}
AND
o.[status] <> 0
AND EXISTS (
SELECT 1
FROM dbo.user_services us2
JOIN dbo.[service] s2
ON s2.id = us2.service_id
WHERE
us2.organisation_id = uo.organisation_id
AND us2.user_id = uo.user_id
AND s2.clientId = {clientName}
);
u.sub = {userId};
""")
.ToListAsync(cancellationToken);

Expand Down
62 changes: 43 additions & 19 deletions ...fe.SignIn.PublicApi.UnitTests/MappingExtensions/GetUserOrganisationServiceMappingTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,13 @@ namespace Dfe.SignIn.PublicApi.UnitTests.MappingExtensions;
[TestClass]
public class GetUserOrganisationServiceMappingTests
{
private static GetUserOrganisationService CreateModel(
private static GetUserOrganisationService CreateUserOrganisationServiceModel(
Guid userId,
Guid orgId,
string? serviceName,
string? roleName,
string? roleCode)
string? roleCode,
int inService = 1)
{
return new GetUserOrganisationService {
UserId = userId,
Expand All @@ -33,19 +34,42 @@ private static GetUserOrganisationService CreateModel(
RoleName = roleName,
RoleCode = roleCode,

OrgRoleId = OrganisationRoles.Approver.Id
OrgRoleId = OrganisationRoles.Approver.Id,
IsInService = inService
};
}

[TestMethod]
public void ToUserDtos_WhenServiceNotFound_ReturnsSingleUserWithZeroOrganisations()
{
var userId = Guid.NewGuid();
var orgId = Guid.NewGuid();

var models = new[]
{
CreateUserOrganisationServiceModel(userId, orgId, "ServiceA", "Role1", "R1", 0),
CreateUserOrganisationServiceModel(userId, orgId, "ServiceB", "Role2", "R2", 0)
};

var result = models.ToUserDtos().Single();

Assert.IsEmpty(result.Organisations);
Assert.AreEqual(userId, result.UserId);
Assert.AreEqual("test@test.com", result.Email);
Assert.AreEqual("Doe", result.FamilyName);
Assert.AreEqual("John", result.GivenName);
Assert.AreEqual(1, result.UserStatus);
}

[TestMethod]
public void ToUserDtos_GroupsByUser_ReturnsSingleUser()
{
var userId = Guid.NewGuid();

var models = new[]
{
CreateModel(userId, Guid.NewGuid(), "ServiceA", "Role1", "R1"),
CreateModel(userId, Guid.NewGuid(), "ServiceB", "Role2", "R2")
CreateUserOrganisationServiceModel(userId, Guid.NewGuid(), "ServiceA", "Role1", "R1"),
CreateUserOrganisationServiceModel(userId, Guid.NewGuid(), "ServiceB", "Role2", "R2")
};

var result = models.ToUserDtos().ToList();
Expand All @@ -59,7 +83,7 @@ public void ToUserDtos_MapsUserFields_Correctly()
{
var userId = Guid.NewGuid();

var model = CreateModel(userId, Guid.NewGuid(), "ServiceA", "Role1", "R1");
var model = CreateUserOrganisationServiceModel(userId, Guid.NewGuid(), "ServiceA", "Role1", "R1");

var result = new[] { model }.ToUserDtos().Single();

Expand All @@ -77,8 +101,8 @@ public void ToOrganisationDtos_GroupsByOrganisation()

var models = new[]
{
CreateModel(userId, orgId, "ServiceA", "Role1", "R1"),
CreateModel(userId, orgId, "ServiceB", "Role2", "R2")
CreateUserOrganisationServiceModel(userId, orgId, "ServiceA", "Role1", "R1"),
CreateUserOrganisationServiceModel(userId, orgId, "ServiceB", "Role2", "R2")
};

var result = models.ToUserDtos().Single();
Expand All @@ -92,7 +116,7 @@ public void ToOrganisationDtos_MapsOrganisationFields()
var userId = Guid.NewGuid();
var orgId = Guid.NewGuid();

var model = CreateModel(userId, orgId, "ServiceA", "Role1", "R1");
var model = CreateUserOrganisationServiceModel(userId, orgId, "ServiceA", "Role1", "R1");

var org = new[] { model }
.ToUserDtos()
Expand All @@ -114,8 +138,8 @@ public void ToServiceDtos_GroupsByServiceName()

var models = new[]
{
CreateModel(userId, orgId, "ServiceA", "Role1", "R1"),
CreateModel(userId, orgId, "ServiceA", "Role2", "R2")
CreateUserOrganisationServiceModel(userId, orgId, "ServiceA", "Role1", "R1"),
CreateUserOrganisationServiceModel(userId, orgId, "ServiceA", "Role2", "R2")
};

var services = models
Expand All @@ -136,8 +160,8 @@ public void ToServiceDtos_OrdersServices_ByName()

var models = new[]
{
CreateModel(userId, orgId, "BService", "Role1", "R1"),
CreateModel(userId, orgId, "AService", "Role1", "R1")
CreateUserOrganisationServiceModel(userId, orgId, "BService", "Role1", "R1"),
CreateUserOrganisationServiceModel(userId, orgId, "AService", "Role1", "R1")
};

var services = models
Expand All @@ -160,8 +184,8 @@ public void ToRoleDtos_RemovesDuplicateRoles()

var models = new[]
{
CreateModel(userId, orgId, "ServiceA", "Role1", "R1"),
CreateModel(userId, orgId, "ServiceA", "Role1", "R1") // duplicate
CreateUserOrganisationServiceModel(userId, orgId, "ServiceA", "Role1", "R1"),
CreateUserOrganisationServiceModel(userId, orgId, "ServiceA", "Role1", "R1") // duplicate
};

var roles = models
Expand All @@ -184,8 +208,8 @@ public void ToRoleDtos_ExcludesNullRoleNames()

var models = new[]
{
CreateModel(userId, orgId, "ServiceA", null, "R1"),
CreateModel(userId, orgId, "ServiceA", "Role1", "R1")
CreateUserOrganisationServiceModel(userId, orgId, "ServiceA", null, "R1"),
CreateUserOrganisationServiceModel(userId, orgId, "ServiceA", "Role1", "R1")
};

var roles = models
Expand All @@ -208,7 +232,7 @@ public void ToOrganisationDtos_ConvertsLegacyIdToString()
var userId = Guid.NewGuid();
var orgId = Guid.NewGuid();

var model = CreateModel(userId, orgId, "ServiceA", "Role1", "R1");
var model = CreateUserOrganisationServiceModel(userId, orgId, "ServiceA", "Role1", "R1");
model.LegacyId = 12345;

var org = new[] { model }
Expand All @@ -226,7 +250,7 @@ public void ToOrganisationDtos_MapsOrgRoleName_WhenExists()
var userId = Guid.NewGuid();
var orgId = Guid.NewGuid();

var model = CreateModel(userId, orgId, "ServiceA", "Role1", "R1");
var model = CreateUserOrganisationServiceModel(userId, orgId, "ServiceA", "Role1", "R1");

var org = new[] { model }
.ToUserDtos()
Expand Down
Loading