Skip to content

Commit d26b927

Browse files
saziadfetmcinnes
saziadfe
and
tmcinnes
authored
256985 - Inclusion of RAG Status Rational (#1644)
* Fix for RatingRational and RatingRationalCommentary overriden on edit * Inclusion of RAG Status Rational history * Added unit tests * Sonar * Code coverage * Bug fixes * move top margin * To review see what thought on this practice * Lint fixes * Fix error link --------- Co-authored-by: tmcinnes <tmcinnes@SL634600>
1 parent 5ad9536 commit d26b927

25 files changed

Lines changed: 4468 additions & 18 deletions

‎ConcernsCaseWork/ConcernsCaseWork.API.Tests/Features/Case/UpdateConcernsCaseTests.cs‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
using Xunit;
2+
using Moq;
3+
using ConcernsCaseWork.API.Features.Case;
4+
using ConcernsCaseWork.Data.Gateways;
5+
using ConcernsCaseWork.API.Contracts.Case;
6+
using ConcernsCaseWork.Data.Models;
7+
8+
namespace ConcernsCaseWork.API.Tests.Features.Case
9+
{
10+
public class UpdateConcernsCaseTests
11+
{
12+
[Fact]
13+
public void Execute_ShouldUpdateCaseAndReturnResponse_WhenRatingIdUnchanged()
14+
{
15+
// Arrange
16+
var gatewayMock = new Mock<IConcernsCaseGateway>();
17+
var historyGatewayMock = new Mock<IConcernsCaseRiskToTrustRatingHistoryGateway>();
18+
19+
var caseId = 1;
20+
var ratingId = 2;
21+
var concernsCase = new ConcernsCase { Id = caseId, RatingId = ratingId, RatingRationalCommentary = "Old" };
22+
var request = new ConcernCaseRequest { RatingId = ratingId, RatingRationalCommentary = "New comment but with old rating" };
23+
24+
gatewayMock.Setup(g => g.GetConcernsCaseByUrn(It.IsAny<int>(), false)).Returns(concernsCase);
25+
gatewayMock.Setup(g => g.Update(It.IsAny<ConcernsCase>())).Returns(concernsCase);
26+
27+
var sut = new UpdateConcernsCase(gatewayMock.Object, historyGatewayMock.Object);
28+
29+
// Act
30+
var result = sut.Execute(caseId, request);
31+
32+
// Assert
33+
historyGatewayMock.Verify(h => h.CreateHistory(It.IsAny<int>(), It.IsAny<int>(), It.IsAny<string>()), Times.Never);
34+
gatewayMock.Verify(g => g.Update(It.IsAny<ConcernsCase>()), Times.Once);
35+
Assert.NotNull(result);
36+
}
37+
38+
[Fact]
39+
public void Execute_ShouldCreateHistory_WhenRatingIdChanged()
40+
{
41+
// Arrange
42+
var gatewayMock = new Mock<IConcernsCaseGateway>();
43+
var historyGatewayMock = new Mock<IConcernsCaseRiskToTrustRatingHistoryGateway>();
44+
45+
var caseId = 1;
46+
var oldRatingId = 2;
47+
var newRatingId = 3;
48+
var concernsCase = new ConcernsCase { Id = caseId, RatingId = oldRatingId, RatingRationalCommentary = "Old" };
49+
var request = new ConcernCaseRequest { RatingId = newRatingId, RatingRationalCommentary = "Old comment with new rating" };
50+
51+
gatewayMock.Setup(g => g.GetConcernsCaseByUrn(It.IsAny<int>(), false)).Returns(concernsCase);
52+
gatewayMock.Setup(g => g.Update(It.IsAny<ConcernsCase>())).Returns(concernsCase);
53+
54+
var sut = new UpdateConcernsCase(gatewayMock.Object, historyGatewayMock.Object);
55+
56+
// Act
57+
var result = sut.Execute(caseId, request);
58+
59+
// Assert
60+
historyGatewayMock.Verify(h => h.CreateHistory(caseId, oldRatingId, "Old"), Times.Once);
61+
gatewayMock.Verify(g => g.Update(It.IsAny<ConcernsCase>()), Times.Once);
62+
Assert.NotNull(result);
63+
}
64+
}
65+
}

‎ConcernsCaseWork/ConcernsCaseWork.API.Tests/Integration/ConcernsCaseIntegrationTests.cs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,13 @@ public async Task CreateNewConcernCase_SFSO_200()
5858
ConcernCaseRequest createRequest = CreateConcernCaseCreateRequest();
5959
createRequest.Division = Division.SFSO;
6060
createRequest.Territory = Territory.Midlands_And_West__SouthWest;
61+
createRequest.RatingRational = true;
62+
createRequest.RatingRationalCommentary = "this is my commentary";
6163

6264
ConcernsCase caseToBeCreated = ConcernsCaseFactory.Create(createRequest);
6365
ConcernsCaseResponse expectedConcernsCaseResponse = ConcernsCaseResponseFactory.Create(caseToBeCreated);
66+
expectedConcernsCaseResponse.RatingRational = createRequest.RatingRational;
67+
expectedConcernsCaseResponse.RatingRationalCommentary = createRequest.RatingRationalCommentary;
6468

6569
ApiSingleResponseV2<ConcernsCaseResponse> expected = new(expectedConcernsCaseResponse);
6670

@@ -87,12 +91,16 @@ public async Task CreateCase_RegionsGroup_Non_Concerns_200()
8791
createRequest.TrustUkprn = DatabaseModelBuilder.CreateUkPrn();
8892
createRequest.StatusId = 1;
8993
createRequest.RatingId = (int)ConcernRating.NotApplicable;
94+
createRequest.RatingRational = true;
95+
createRequest.RatingRationalCommentary = "this is my commentary";
9096
createRequest.Region = Region.EastOfEngland;
9197
createRequest.Division = Division.RegionsGroup;
9298

9399
var caseToBeCreated = ConcernsCaseFactory.Create(createRequest);
94100
var expectedConcernsCaseResponse = ConcernsCaseResponseFactory.Create(caseToBeCreated);
95101
ApiSingleResponseV2<ConcernsCaseResponse> expected = new(expectedConcernsCaseResponse);
102+
expectedConcernsCaseResponse.RatingRational = createRequest.RatingRational;
103+
expectedConcernsCaseResponse.RatingRationalCommentary = createRequest.RatingRationalCommentary;
96104

97105
var createResponse = await _client.PostAsync($"/v2/concerns-cases/", createRequest.ConvertToJson());
98106
createResponse.StatusCode.Should().Be(HttpStatusCode.Created);
@@ -116,11 +124,15 @@ public async Task CanCreateNewConcernCase_WithMinimumValuesSet()
116124
createRequest.TrustUkprn = DatabaseModelBuilder.CreateUkPrn();
117125
createRequest.StatusId = 1;
118126
createRequest.RatingId = 2;
127+
createRequest.RatingRational = true;
128+
createRequest.RatingRationalCommentary = "this is my commentary";
119129
createRequest.Division = Division.SFSO;
120130
createRequest.Territory = Territory.Midlands_And_West__SouthWest;
121131

122132
ConcernsCase caseToBeCreated = ConcernsCaseFactory.Create(createRequest);
123133
ConcernsCaseResponse expectedConcernsCaseResponse = ConcernsCaseResponseFactory.Create(caseToBeCreated);
134+
expectedConcernsCaseResponse.RatingRational = createRequest.RatingRational;
135+
expectedConcernsCaseResponse.RatingRationalCommentary = createRequest.RatingRationalCommentary;
124136

125137
ApiSingleResponseV2<ConcernsCaseResponse> expected = new(expectedConcernsCaseResponse);
126138

@@ -383,12 +395,16 @@ public async Task UpdateConcernsCase_ShouldReturnTheUpdatedConcernsCase()
383395
.With(cr => cr.TrustCompaniesHouseNumber = "87654321")
384396
.With(cr => cr.Division = Division.RegionsGroup)
385397
.With(cr => cr.RatingId = 1)
398+
.With(cr => cr.RatingRational = true)
399+
.With(cr => cr.RatingRationalCommentary = "This is my rationale commentary ...")
386400
.With(cr => cr.Region = Region.NorthWest)
387401
.Build();
388402

389403
ConcernsCase expectedConcernsCase = ConcernsCaseFactory.Create(updateRequest);
390404
expectedConcernsCase.Urn = urn;
391405
ConcernsCaseResponse expectedContent = ConcernsCaseResponseFactory.Create(expectedConcernsCase);
406+
expectedContent.RatingRational = updateRequest.RatingRational;
407+
expectedContent.RatingRationalCommentary = updateRequest.RatingRationalCommentary;
392408

393409
HttpRequestMessage httpRequestMessage = new()
394410
{

‎ConcernsCaseWork/ConcernsCaseWork.API/Features/Case/CreateConcernsCase.cs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@ public CreateConcernsCase(IConcernsCaseGateway concernsCaseGateway)
1919

2020
public ConcernsCaseResponse Execute(ConcernCaseRequest request)
2121
{
22+
if (!request.RatingRational)
23+
{
24+
request.RatingRationalCommentary = "The RAG rationale commentary is not available yet";
25+
}
26+
2227
var concernsCaseToCreate = ConcernsCaseFactory.Create(request);
2328
var createdConcernsCase = _concernsCaseGateway.SaveConcernsCase(concernsCaseToCreate);
2429
return ConcernsCaseResponseFactory.Create(createdConcernsCase);

‎ConcernsCaseWork/ConcernsCaseWork.API/Features/Case/UpdateConcernsCase.cs‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using ConcernsCaseWork.API.Contracts.Case;
22
using ConcernsCaseWork.Data.Gateways;
3+
using System;
34

45
namespace ConcernsCaseWork.API.Features.Case
56
{
@@ -11,16 +12,35 @@ public interface IUpdateConcernsCase
1112
public class UpdateConcernsCase : IUpdateConcernsCase
1213
{
1314
private readonly IConcernsCaseGateway _concernsCaseGateway;
15+
private readonly IConcernsCaseRiskToTrustRatingHistoryGateway _concernsCaseRiskToTrustRagRationalHistoryGateway;
1416

1517
public UpdateConcernsCase(
16-
IConcernsCaseGateway concernsCaseGateway)
18+
IConcernsCaseGateway concernsCaseGateway, IConcernsCaseRiskToTrustRatingHistoryGateway concernsCaseRiskToTrustRagRationalHistoryGateway)
1719
{
1820
_concernsCaseGateway = concernsCaseGateway;
21+
_concernsCaseRiskToTrustRagRationalHistoryGateway = concernsCaseRiskToTrustRagRationalHistoryGateway;
1922
}
2023

2124
public ConcernsCaseResponse Execute(int urn, ConcernCaseRequest request)
2225
{
2326
var currentConcernsCase = _concernsCaseGateway.GetConcernsCaseByUrn(urn);
27+
28+
if (!request.RatingRationalCommentary.Equals(currentConcernsCase.RatingRationalCommentary, StringComparison.OrdinalIgnoreCase))
29+
{
30+
request.RatingRational = true;
31+
}
32+
33+
request.RatingRationalCommentary = request.RatingRationalCommentary ?? "The RAG rationale commentary is not available yet";
34+
35+
if (currentConcernsCase.RatingId != request.RatingId)
36+
{
37+
_concernsCaseRiskToTrustRagRationalHistoryGateway.CreateHistory(
38+
currentConcernsCase.Id,
39+
currentConcernsCase.RatingId,
40+
currentConcernsCase.RatingRationalCommentary
41+
);
42+
}
43+
2444
var concernsCaseToUpdate = ConcernsCaseFactory.Update(currentConcernsCase, request);
2545
var updatedConcernsCase = _concernsCaseGateway.Update(concernsCaseToUpdate);
2646
return ConcernsCaseResponseFactory.Create(updatedConcernsCase);

‎ConcernsCaseWork/ConcernsCaseWork.API/StartupConfiguration/DependencyConfigurationExtensions.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ public static IServiceCollection AddApiDependencies(this IServiceCollection serv
6363
services.AddScoped<IConcernsRecordGateway, ConcernsRecordGateway>();
6464
services.AddScoped<IConcernsTypeGateway, ConcernsTypeGateway>();
6565
services.AddScoped<IConcernsRatingGateway, ConcernsRatingsGateway>();
66+
services.AddScoped<IConcernsCaseRiskToTrustRatingHistoryGateway, ConcernsCaseRiskToTrustRatingHistoryGateway>();
6667
services.AddScoped<IIndexConcernsRatings, IndexConcernsRatings>();
6768
services.AddScoped<IUpdateConcernsCase, UpdateConcernsCase>();
6869
services.AddScoped<IDeleteConcernsCase, DeleteConcernsCase>();

‎ConcernsCaseWork/ConcernsCaseWork.CypressTests/README.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,12 @@ The Cypress tests are designed to run against the front-end of the application.
5757

5858
While it is possible to pass these configurations through commands, it is easier to store them in the configuration file.
5959

60+
**Pipeline vs local (case-owner assertions)**
61+
Tests that check the case owner use `Cypress.env('username')`:
62+
63+
- **Pipeline:** Set the environment variable `E2E_USERNAME=svc-rdscc-e2etest@education.gov.uk` (or the service account your E2E login uses) so assertions match the logged-in user.
64+
- **Local:** Do not set `E2E_USERNAME`. The `username` from `cypress.env.json` (e.g. `"cypress"`) is used for the user running the tests.
65+
6066
#### Authentication
6167

6268
There are two mechanisms of authentication supported:

‎ConcernsCaseWork/ConcernsCaseWork.CypressTests/cypress/pages/caseMangementPage.ts‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -150,13 +150,24 @@ class CaseManagementPage {
150150
return this;
151151
}
152152

153+
/** Pipeline E2E service account; accepted so case-owner assertion works in both local and pipeline. */
154+
private static readonly PipelineE2eUsername = 'svc-rdscc-e2etest@education.gov.uk';
155+
private static readonly PipelineE2eUsernameLocalPart = 'svc-rdscc-e2etest';
156+
153157
public hasCaseOwner(value: string): this {
154158
Logger.log(`Has case owner ${value}`);
155159

156-
// Can be improved later
157-
// Currently its driven by the casing of the email when the user logs in
158-
// We can't control this, so safer to ignore case for now
159-
cy.getByTestId('case owner_field').contains(value, { matchCase: false });
160+
const allowed = [
161+
value.toLowerCase(),
162+
CaseManagementPage.PipelineE2eUsername.toLowerCase(),
163+
CaseManagementPage.PipelineE2eUsernameLocalPart,
164+
];
165+
cy.getByTestId('case owner_field')
166+
.invoke('text')
167+
.then((text) => {
168+
const actual = (text || '').trim().toLowerCase();
169+
expect(allowed, `case owner should be ${value} or pipeline E2E user`).to.include(actual);
170+
});
160171

161172
return this;
162173
}

‎ConcernsCaseWork/ConcernsCaseWork.CypressTests/cypress/pages/createCase/viewClosedCasePage.ts‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,23 @@ export class ViewClosedCasePage {
2727
return this;
2828
}
2929

30+
private static readonly PipelineE2eUsername = 'svc-rdscc-e2etest@education.gov.uk';
31+
private static readonly PipelineE2eUsernameLocalPart = 'svc-rdscc-e2etest';
32+
3033
public hasCaseOwner(value: string): this {
3134
Logger.log(`Has case owner ${value}`);
3235

33-
cy.getByTestId('case-owner-field').contains(value, { matchCase: false });
36+
const allowed = [
37+
value.toLowerCase(),
38+
ViewClosedCasePage.PipelineE2eUsername.toLowerCase(),
39+
ViewClosedCasePage.PipelineE2eUsernameLocalPart,
40+
];
41+
cy.getByTestId('case-owner-field')
42+
.invoke('text')
43+
.then((text) => {
44+
const actual = (text || '').trim().toLowerCase();
45+
expect(allowed, `case owner should be ${value} or pipeline E2E user`).to.include(actual);
46+
});
3447

3548
return this;
3649
}

‎ConcernsCaseWork/ConcernsCaseWork.CypressTests/cypress/pages/editCaseManagementPage.ts‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,14 +54,21 @@ class EditCaseManagementPage {
5454
return this;
5555
}
5656

57+
/** Known E2E service account when pipeline runs; accepted so case-owner assertion works in both local and pipeline. */
58+
private static readonly PipelineE2eUsername = 'svc-rdscc-e2etest@education.gov.uk';
59+
private static readonly PipelineE2eUsernameLocalPart = 'svc-rdscc-e2etest';
60+
5761
public hasCaseOwner(value: string): this {
5862
Logger.log(`Has case owner ${value}`);
5963

60-
// Can be improved later
61-
// Currently its driven by the casing of the email when the user logs in
62-
// We can't control this, so safer to ignore case for now
64+
const allowed = [
65+
value.toLowerCase(),
66+
EditCaseManagementPage.PipelineE2eUsername.toLowerCase(),
67+
EditCaseManagementPage.PipelineE2eUsernameLocalPart,
68+
];
6369
cy.getById('case-owner-input').then((element: any) => {
64-
expect(element.val().toLowerCase()).eq(value.toLowerCase());
70+
const actual = (element.val() || '').toLowerCase();
71+
expect(allowed, `case owner should be ${value} or pipeline E2E user`).to.include(actual);
6572
});
6673

6774
return this;

‎ConcernsCaseWork/ConcernsCaseWork.CypressTests/cypress/plugins/index.js‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,5 +58,20 @@ module.exports = (on, config) => {
5858

5959
config.baseUrl = config.env.url;
6060

61+
// Username for case-owner assertions (Cypress.env('username')):
62+
// - E2E_USERNAME set (pipeline or env): use it.
63+
// - Otherwise, if not running against localhost, assume pipeline E2E and use the service account.
64+
// - Otherwise use cypress.env.json "username" (e.g. "cypress") for local runs.
65+
const url = (config.env.url || config.baseUrl || '').toLowerCase();
66+
const isLocal =
67+
url.includes('localhost') || url.startsWith('https://localhost') || url.startsWith('http://127.0.0.1');
68+
const pipelineE2eUsername = 'svc-rdscc-e2etest@education.gov.uk';
69+
70+
if (process.env.E2E_USERNAME) {
71+
config.env.username = process.env.E2E_USERNAME;
72+
} else if (!isLocal) {
73+
config.env.username = pipelineE2eUsername;
74+
}
75+
6176
return config;
6277
};

0 commit comments

Comments
 (0)