Skip to content

Show withdrawn deferred ect to schools#2164

Open
ericaporter wants to merge 14 commits intomainfrom
show-withdrawn-deferred-ect-to-schools
Open

Show withdrawn deferred ect to schools#2164
ericaporter wants to merge 14 commits intomainfrom
show-withdrawn-deferred-ect-to-schools

Conversation

@ericaporter
Copy link
Contributor

@ericaporter ericaporter commented Feb 5, 2026

Ticket here

Context

This PR covers updates to the school-facing UI when an ECT’s latest training period at the school has been withdrawn or deferred by a lead provider via the API.

Previously, these states were not surfaced consistently across the ECT list and individual ECT details pages. This PR ensures withdrawn and deferred training statuses are displayed with the correct messaging, actions, and priority whilst leaving the underlying ECT at school period unchanged.

Changes proposed in this pull request

  • Updated components to derive withdrawn and deferred states from the latest training period at the school, rather than from ECT-level attributes
  • Added explicit handling for withdrawn vs deferred training periods in:
    • ECT list card components
    • ECT profile summary components
    • Training details components
  • Added conditional rendering to:
    • show withdrawn-specific warning copy, status tags and CTAs only when the latest training period is withdrawn,
    • retain standard training summary rows for deferred training periods.
  • Implemented precedence rules so that:
    • 'withdrawn' and 'deferred' training statuses override 'mentor required',
    • existing 'leaving school' and 'exempt' logic continues to take priority over withdrawn/deferred
  • Ensured lead provider and delivery partner details are:
    • hidden for withdrawn training periods
    • preserved for deferred training periods

Guidance to review

@github-actions
Copy link

github-actions bot commented Feb 5, 2026

Review app deployed to https://cpd-ec2-review-2164-web.test.teacherservices.cloud

end
end

def school_training_status
Copy link
Contributor

@thenapking thenapking Feb 9, 2026

Choose a reason for hiding this comment

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

I think we could just name this status. training is redundant as this is a TrainingPeriod. school is also not relevant because the TrainingPeriod is linked to the ect/mentor_at_school_period, so it's not varying in this instance. It's also missing a test.

Copy link
Contributor Author

@ericaporter ericaporter Feb 11, 2026

Choose a reason for hiding this comment

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

Yep that's fair. I originally went with school_training_status because we’ve got a couple of 'status' concepts floating around (participant/induction/training-period etc) and this one is specifically 'what schools should see'. Happy to rename to something like status_for_school ... we use training_status elsewhere

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I understand your concern here. status_for_school doesn't really communicate the purpose of this as the naming suggests it's more to do with the view - what the school sees - but that might not be clear in the context. training_period.training_status is just a repetition, though if we can't think of anything better we can do that. I can see training_status used in the readme for example.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the tests. They are fine obvs.

Copy link
Contributor

@thenapking thenapking left a comment

Choose a reason for hiding this comment

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

I think this is sound, and meets the ACs, but needs a little clean-up in places.

@ericaporter ericaporter force-pushed the show-withdrawn-deferred-ect-to-schools branch from afc51d1 to 3a40196 Compare February 16, 2026 08:33
@sonarqubecloud
Copy link

Copy link
Contributor

@thenapking thenapking left a comment

Choose a reason for hiding this comment

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

Great. thanks for all the updates

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants