[3075] show schools when a mentor has been deferred or withdrawn#2159
Conversation
|
Review app deployed to https://cpd-ec2-review-2159-web.test.teacherservices.cloud |
thenapking
force-pushed
the
3075-show-schools-when-a-mentor-has-been-deferred-or-withdrawn
branch
from
1865a66 to
2a1c111
Compare
thenapking
force-pushed
the
3075-show-schools-when-a-mentor-has-been-deferred-or-withdrawn
branch
from
41d54b9 to
fb0441a
Compare
| <p class="govuk-body"><%= teacher_name %> is not currently registered for ECTE mentor training with a lead provider.</p> | ||
| <% when :withdrawn %> | ||
| <p class="govuk-body"><%= lead_provider_name %> have told us that <%= teacher_name %>’s ECTE mentor training is paused. Contact them if you think this is an error.</p> | ||
| <% when :deferred %> |
There was a problem hiding this comment.
Check if withdrawn & deferred copy should to be flipped
There was a problem hiding this comment.
Thanks for spotting that. I don't know why I thought it was the other way round.
|
|
||
| def latest_training_period | ||
| @latest_training_period ||= mentor_period_for_school&.latest_training_period | ||
| end |
There was a problem hiding this comment.
This covers latest but what about if the mentor has a future period starting soon or a current one that's deferred/ withdrawn. We have scope :latest_first, -> { order(started_on: "desc") } and latest_training_period uses latest_first so a future TP starting soon will always beat a current one - this was why I opted for current_or_next_training_period || latest_training_period Good to get your thoughts on this one!
There was a problem hiding this comment.
I had wondered about that in your PR. The card specifically said latest_training_period in the ACs which is why I've got that. But to your point, I've checked the API classes for Withdraw and Defer and both of these have validations that prevent deferral or withdrawal of periods which haven't started or would be 0 days in length if deferred/withdrawn. The services for Defer and Withdraw both set the finished_on date for the training_period, either to the deferral date or the already existing end date. So I don't think there can be current or future periods which are deferred/withdrawn, which probably simplifies the logic a bit. If there is a future, or current period, after one which is withdrawn/deferred, then they aren't deferred/withdrawn any more, they have restarted.
There was a problem hiding this comment.
Yeah I agree the API constraints simplify things - I clarified with Product regarding the potential edge cases and given that you can't have two TP at the same school (one would be finished and the new one would begin with immediate effect) it shouldn't be a problem
| expect(rendered_content).to have_css(".govuk-body", text: /Hidden leaf village have told us that .* is not registered for ECTE mentor training with them./) | ||
|
|
||
| expect(rendered_content).not_to have_css("dt.govuk-summary-list__key", text: "Lead provider") | ||
| expect(rendered_content).not_to have_css("dt.govuk-summary-list__key", text: "Delivery partner") |
There was a problem hiding this comment.
I think you need to swap the deferred message here and withdrawn message below on line 127
There was a problem hiding this comment.
Ah sorry, it was the header text of the expectation - I swapped the code and missed that.
| :training_period, :provider_led, :for_mentor, :deferred, | ||
| mentor_at_school_period: mentor_period, | ||
| started_on: start_date, finished_on: nil, | ||
| school_partnership: |
There was a problem hiding this comment.
Here too - I think we add a finished_on date when the TP has been deferred - does this affect what is rendered in the component?
There was a problem hiding this comment.
Now updated so the factory adds finished dates by default to deferred and withdrawn periods. Updated the view logic and this should all now be as expected.
|
thenapking
deleted the
3075-show-schools-when-a-mentor-has-been-deferred-or-withdrawn
branch
|
Review app for PR 2159 was deleted |
2 participants
Context
Add new details to the mentor list page and details page when the mentor's latest training period is deferred or withdrawn.
Changes proposed in this pull request
Guidance to review
Whilst this ticket has some conceptual overlap with #2164, mostly this is about logic for the display which is quite different. In order to remain in sync with that PR, but without branching off it, I have copied across the method added to the TrainingPeriod model. I have not added tests for it here, as I would expect that to be done in that PR. I am also awaiting confirmation on the naming of the method. I have called it
statushere.For product review, I have set the two mentors in Ackley Bridge to have withdrawn and deferred statuses.