Add support for Primary education phase #1326
Conversation
New (optional) StageTaughtId parameter + updated tests
|
Kudos, SonarCloud Quality Gate passed!
|
| @@ -81,12 +82,12 @@ public class TeachingEventsController : ControllerBase | |||
| var typeIds = request.TypeIds == null ? string.Empty : string.Join(",", request.TypeIds); | |||
|
|
|||
| _metrics.TeachingEventSearchResults | |||
| .WithLabels(typeIds, request.Radius.ToString()) | |||
| .WithLabels(typeIds, String.Format(CultureInfo.InvariantCulture, "{0}", request.Radius)) | |||
There was a problem hiding this comment.
Just for my info, why has String.Format(CultureInfo.InvariantCulture, "{0}" been added ? Seems unrelated to the change in this PR. Might just be my understanding !
There was a problem hiding this comment.
Yes, this change is unrelated to this issue; the change was flagged by the code linter:
using Radius.ToString() does not guarantee the format of the integer->string conversion, it could vary with culture.
Using String.Format with an invariant culture guarantees the format of the string. Its a very minor issue, but one the linter flagged 🤷
| candidate.PastTeachingPositions.Add(new CandidatePastTeachingPosition() | ||
| { | ||
| Id = PastTeachingPositionId, | ||
| SubjectTaughtId = SubjectTaughtId, |
There was a problem hiding this comment.
Is this a Typo ? It looks like self assignment.
There was a problem hiding this comment.
ah yes, you are right, sorry for that - this line is not required. I'll push an update shortly. Thanks
There was a problem hiding this comment.
@asatwal actually, on reflection (and tests) - this parameter SubjectTaughtId is required. Its part of the initialisation of the CandidatePastTeachingPosition instance:
new CandidatePastTeachingPosition()
{
Id = PastTeachingPositionId,
SubjectTaughtId = SubjectTaughtId,
EducationPhaseId = (int)CandidatePastTeachingPosition.EducationPhase.Secondary,
}
which translates to create a new CandidatePastTeachingPosition instance setting it's SubjectTaughtId to the request's SubjectTaughtId. I agree its not terribly clear... but that is the c# syntax.
New (optional) StageTaughtId parameter + updated tests
This adds support for an improved RTTA process to capture interest in primary stage teaching
Requires changes to be pulled through into https://github.com/DFE-Digital/get-into-teaching-api-ruby-client and https://github.com/DFE-Digital/get-into-teaching-api-csharp-client