Heedls 568 edit supervisor

[AlexJacksonDS]

JIRA link

https://softwiretech.atlassian.net/browse/HEEDLS-568

Description

Added a new Edit Supervisor page linked from the View Delegate course cards and the Delegate Progress page

Screenshots

---

Developer checks

(Leave tasks unticked if they haven't been appropriate for your ticket.)

I have:

• Run the formatter and made sure there are no IDE errors.
• Written tests for the changes (accessibility tests, unit tests for controller, data services, services, view models, etc)
• Manually tested my work with and without JavaScript. Full manual testing guidelines can be found here: https://softwiretech.atlassian.net/wiki/spaces/HEE/pages/6703648740/Testing
• Updated/added documentation in Swiki and/or Readme. Links (if any) are below:
  • doc_1_here
• Updated my Jira ticket with information about other parts of the system that were touched as part of the MR and have to be sanity tested to ensure nothing’s broken.
• Scanned over my own MR to ensure everything is as expected.

[AlexJacksonDS]

Updated post show and tell to allow unsetting a supervisor

[JBsoftwire]

Some comments about the use of ! and a question about Single().

[JBsoftwire]

Can we be confident in this ! assertion? Someone might try to come to this page by entering the URL directly, or the state of the data might change during a session, so I think we should handle the null case explicitly.

[AlexJacksonDS]

Yes, because we are protected by the VerifyAdminUserCanAccessProgress ServiceFilter, which handles the progess == null, return NotFound case. I suppose it is technically less efficient since we're doing double database queries, but that seems to be the pattern we've decided with all these service filters.

[JBsoftwire]

That makes sense, I forgot to check the controller attributes. No problem with this, then.

[JBsoftwire]

I assume this must work since the tests pass, but my understanding of the Single() LINQ operator is that it throws if there's not exactly one result, so this can never return null. Can you tell me what I've got wrong here?

[AlexJacksonDS]

The column SupervisorVerificationRequested is a nullable datetime, so if the aspProgress record exists but has null it will return null, hence the DateTime? return type.
As you say, this will throw an exception if there is not exactly one result, but since we are looking it up by aspProgressId and using it in a unit test this is the behaviour we would want. i.e. this will only throw an exception if the aspProgress record gets deleted from the database for some reason.

[davidm-m]

A few very small things