Skip to content

Commit

Permalink
Fixed issues found during alerts snagging session (#1649)
Browse files Browse the repository at this point in the history
  • Loading branch information
hortha authored Nov 5, 2024
1 parent f63222b commit 26ed784
Show file tree
Hide file tree
Showing 94 changed files with 3,532 additions and 267 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,6 @@ public void Configure(EntityTypeBuilder<AlertType> builder)
new AlertType { AlertTypeId = new Guid("38db7946-2dbf-408e-bc48-1625829e7dfe"), AlertCategoryId = new Guid("cbf7633f-3904-407d-8371-42a473fa641f"), Name = "Restricted by the Secretary of State - Not Permitted to work as teacher", DqtSanctionCode = "B2B", ProhibitionLevel = ProhibitionLevel.Teaching, InternalOnly = true, IsActive = false },
new AlertType { AlertTypeId = new Guid("18e04dcb-fb86-4b05-8d5d-ff9c5da738dd"), AlertCategoryId = new Guid("cbf7633f-3904-407d-8371-42a473fa641f"), Name = "Restricted by the Secretary of State - Permitted to work as teacher", DqtSanctionCode = "B2A", ProhibitionLevel = ProhibitionLevel.Teaching, InternalOnly = true, IsActive = false },
new AlertType { AlertTypeId = new Guid("241eeb78-fac7-4c77-8059-c12e93dc2fae"), AlertCategoryId = new Guid("38df5a00-94ab-486f-8905-d5b2eac04000"), Name = "Section 128 barring direction", DqtSanctionCode = "T7", ProhibitionLevel = ProhibitionLevel.LeadershipPositions, InternalOnly = false, IsActive = true, DisplayOrder = 1 },
new AlertType { AlertTypeId = new Guid("7924fe90-483c-49f8-84fc-674feddba848"), AlertCategoryId = new Guid("227b75e5-bb98-496c-8860-1baea37aa5c6"), Name = "Secretary of State decision- no prohibition", DqtSanctionCode = "T6", ProhibitionLevel = ProhibitionLevel.None, InternalOnly = false, IsActive = true, DisplayOrder = 1 });
new AlertType { AlertTypeId = new Guid("7924fe90-483c-49f8-84fc-674feddba848"), AlertCategoryId = new Guid("227b75e5-bb98-496c-8860-1baea37aa5c6"), Name = "Secretary of State decision - no prohibition", DqtSanctionCode = "T6", ProhibitionLevel = ProhibitionLevel.None, InternalOnly = false, IsActive = true, DisplayOrder = 1 });
}
}

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
using System;
using Microsoft.EntityFrameworkCore.Migrations;

#nullable disable

namespace TeachingRecordSystem.Core.DataStore.Postgres.Migrations
{
/// <inheritdoc />
public partial class AlertTypeTypoFix : Migration
{
/// <inheritdoc />
protected override void Up(MigrationBuilder migrationBuilder)
{
migrationBuilder.UpdateData(
table: "alert_types",
keyColumn: "alert_type_id",
keyValue: new Guid("7924fe90-483c-49f8-84fc-674feddba848"),
column: "name",
value: "Secretary of State decision - no prohibition");
}

/// <inheritdoc />
protected override void Down(MigrationBuilder migrationBuilder)
{
migrationBuilder.UpdateData(
table: "alert_types",
keyColumn: "alert_type_id",
keyValue: new Guid("7924fe90-483c-49f8-84fc-674feddba848"),
column: "name",
value: "Secretary of State decision- no prohibition");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -980,7 +980,7 @@ protected override void BuildModel(ModelBuilder modelBuilder)
DqtSanctionCode = "T6",
InternalOnly = false,
IsActive = true,
Name = "Secretary of State decision- no prohibition",
Name = "Secretary of State decision - no prohibition",
ProhibitionLevel = 0
});
});
Expand Down
14 changes: 14 additions & 0 deletions TeachingRecordSystem/src/TeachingRecordSystem.Core/TrsUriHelper.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
namespace TeachingRecordSystem.Core;

public static class TrsUriHelper
{
public static bool TryCreateWebsiteUri(string? uriString, out Uri? uri)
{
var isValidUri = Uri.TryCreate(uriString, UriKind.Absolute, out var uri2) ||
Uri.TryCreate("http://" + uriString, UriKind.Absolute, out uri2) &&
(uri2.Scheme == "http" || uri2.Scheme == "https");

uri = uri2;
return isValidUri;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
global using TeachingRecordSystem.UiCommon.FormFlow;
global using ColumnSet = Microsoft.Xrm.Sdk.Query.ColumnSet;
global using CrmTask = TeachingRecordSystem.Core.Dqt.Models.Task;
global using UiDefaults = TeachingRecordSystem.SupportUi.Pages.Common.UiDefaults;
Original file line number Diff line number Diff line change
Expand Up @@ -32,22 +32,22 @@
<govuk-summary-list-row>
<govuk-summary-list-row-key>Link</govuk-summary-list-row-key>
<govuk-summary-list-row-value>
@if (Model.Link is not null)
{
<a href="@Model.Link" class="govuk-link" rel="noreferrer noopener" target="_blank">@($"{Model.Link} (opens in new tab)")</a>
}
else
{
<span use-empty-fallback></span>
}
@if (Model.Link is not null)
{
<a href="@Model.LinkUri" class="govuk-link" rel="noreferrer noopener" target="_blank">@($"{Model.Link} (opens in new tab)")</a>
}
else
{
<span use-empty-fallback></span>
}
</govuk-summary-list-row-value>
<govuk-summary-list-row-actions>
<govuk-summary-list-row-action href="@LinkGenerator.AlertAddLink(Model.PersonId, Model.JourneyInstance!.InstanceId, fromCheckAnswers: true)" visually-hidden-text="link">Change</govuk-summary-list-row-action>
</govuk-summary-list-row-actions>
</govuk-summary-list-row>
<govuk-summary-list-row>
<govuk-summary-list-row-key>Start date</govuk-summary-list-row-key>
<govuk-summary-list-row-value>@Model.StartDate.ToString("d MMMM yyyy")</govuk-summary-list-row-value>
<govuk-summary-list-row-value>@Model.StartDate.ToString(UiDefaults.DateOnlyDisplayFormat)</govuk-summary-list-row-value>
<govuk-summary-list-row-actions>
<govuk-summary-list-row-action href="@LinkGenerator.AlertAddStartDate(Model.PersonId, Model.JourneyInstance!.InstanceId, fromCheckAnswers: true)" visually-hidden-text="start date">Change</govuk-summary-list-row-action>
</govuk-summary-list-row-actions>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ public class CheckAnswersModel(
IFileService fileService,
IClock clock) : PageModel
{
private static readonly TimeSpan _fileUrlExpiresAfter = TimeSpan.FromMinutes(15);

public JourneyInstance<AddAlertState>? JourneyInstance { get; set; }

[FromQuery]
Expand All @@ -33,6 +31,8 @@ public class CheckAnswersModel(

public string? Link { get; set; }

public Uri? LinkUri { get; set; }

public DateOnly StartDate { get; set; }

public AddAlertReasonOption AddReason { get; set; }
Expand Down Expand Up @@ -111,12 +111,13 @@ public override async Task OnPageHandlerExecutionAsync(PageHandlerExecutingConte
AlertTypeName = JourneyInstance!.State.AlertTypeName;
Details = JourneyInstance!.State.Details;
Link = JourneyInstance!.State.Link;
LinkUri = TrsUriHelper.TryCreateWebsiteUri(Link, out var linkUri) ? linkUri : null;
StartDate = JourneyInstance!.State.StartDate!.Value;
AddReason = JourneyInstance!.State.AddReason!.Value;
AddReasonDetail = JourneyInstance!.State.AddReasonDetail;
EvidenceFileName = JourneyInstance.State.EvidenceFileName;
UploadedEvidenceFileUrl = JourneyInstance!.State.EvidenceFileId is not null ?
await fileService.GetFileUrl(JourneyInstance!.State.EvidenceFileId!.Value, _fileUrlExpiresAfter) :
await fileService.GetFileUrl(JourneyInstance!.State.EvidenceFileId!.Value, AlertDefaults.FileUrlExpiry) :
null;

await next();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
<form action="@LinkGenerator.AlertAddDetails(Model.PersonId, Model.JourneyInstance!.InstanceId, Model.FromCheckAnswers)" method="post">
<span class="govuk-caption-l">Add an alert - @Model.PersonName</span>

<govuk-character-count asp-for="Details" max-length="@DetailsModel.DetailsMaxLength" data-testid="details">
<govuk-character-count asp-for="Details" max-length="AlertDefaults.DetailMaxCharacterCount" rows="AlertDefaults.DetailTextAreaMinimumRows" data-testid="details">
<govuk-character-count-label class="govuk-label--l" is-page-heading="true">
@ViewBag.Title
</govuk-character-count-label>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ namespace TeachingRecordSystem.SupportUi.Pages.Alerts.AddAlert;
[Journey(JourneyNames.AddAlert), RequireJourneyInstance]
public class DetailsModel(TrsLinkGenerator linkGenerator) : PageModel
{
public const int DetailsMaxLength = 4000;

public JourneyInstance<AddAlertState>? JourneyInstance { get; set; }

[FromQuery]
Expand All @@ -25,7 +23,7 @@ public class DetailsModel(TrsLinkGenerator linkGenerator) : PageModel
[BindProperty]
[Required(ErrorMessage = "Enter details")]
[Display(Description = "For example, include any restrictions it places on a teacher.")]
[MaxLength(DetailsMaxLength, ErrorMessage = "Details must be 4000 characters or less")]
[MaxLength(AlertDefaults.DetailMaxCharacterCount, ErrorMessage = "Details must be 4000 characters or less")]
public string? Details { get; set; }

public void OnGet()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
<govuk-radios-fieldset>
<govuk-radios-fieldset-legend class="govuk-fieldset__legend--l" />
<govuk-radios-hint>
<a href="https://www.gov.uk/government/collections/teacher-misconduct" class="govuk-link" target="_blank" rel="noopener noreferrer">Panel outcomes (opens in a new tab)</a>
include details about the allegation, the panel’s decision and reasons for it. The link will be seen internally.
<a href="https://www.gov.uk/search/all?parent=&keywords=panel+outcome+misconduct&level_one_taxon=&manual=&organisations%5B%5D=teaching-regulation-agency&organisations%5B%5D=national-college-for-teaching-and-leadership&public_timestamp%5Bfrom%5D=&public_timestamp%5Bto%5D=&order=updated-newest" class="govuk-link" target="_blank" rel="noopener noreferrer">Panel outcomes (opens in a new tab)</a>
include details about the allegation, the panel’s decision and reasons for it. The link will only be seen internally.
</govuk-radios-hint>
<govuk-radios-item value="@true">
Yes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,7 @@ public void OnGet()

public async Task<IActionResult> OnPost()
{
if (AddLink == true &&
(!Uri.TryCreate(Link, UriKind.Absolute, out var uri) ||
(uri.Scheme != "http" && uri.Scheme != "https")))
if (AddLink == true && !TrsUriHelper.TryCreateWebsiteUri(Link, out _))
{
ModelState.AddModelError(nameof(Link), "Enter a valid URL");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
<govuk-radios-item value="@true">
Yes
<govuk-radios-item-conditional>
<govuk-character-count asp-for="AddReasonDetail" max-length="@ReasonModel.AddReasonDetailMaxLength" />
<govuk-character-count asp-for="AddReasonDetail" max-length="AlertDefaults.DetailMaxCharacterCount" rows="AlertDefaults.DetailTextAreaMinimumRows" />
</govuk-radios-item-conditional>
</govuk-radios-item>
<govuk-radios-item value="@false">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,6 @@ namespace TeachingRecordSystem.SupportUi.Pages.Alerts.AddAlert;
[Journey(JourneyNames.AddAlert), RequireJourneyInstance]
public class ReasonModel(TrsLinkGenerator linkGenerator, IFileService fileService) : PageModel
{
public const int MaxFileSizeMb = 50;
public const int AddReasonDetailMaxLength = 4000;

private static readonly TimeSpan _fileUrlExpiresAfter = TimeSpan.FromMinutes(15);

public JourneyInstance<AddAlertState>? JourneyInstance { get; set; }

[FromQuery]
Expand All @@ -38,7 +33,7 @@ public class ReasonModel(TrsLinkGenerator linkGenerator, IFileService fileServic

[BindProperty]
[Display(Name = "Add additional detail")]
[MaxLength(AddReasonDetailMaxLength, ErrorMessage = "Additional detail must be 4000 characters or less")]
[MaxLength(AlertDefaults.DetailMaxCharacterCount, ErrorMessage = "Additional detail must be 4000 characters or less")]
public string? AddReasonDetail { get; set; }

[BindProperty]
Expand All @@ -48,7 +43,7 @@ public class ReasonModel(TrsLinkGenerator linkGenerator, IFileService fileServic

[BindProperty]
[EvidenceFile]
[FileSize(MaxFileSizeMb * 1024 * 1024, ErrorMessage = "The selected file must be smaller than 50MB")]
[FileSize(AlertDefaults.MaxFileUploadSizeMb * 1024 * 1024, ErrorMessage = "The selected file must be smaller than 50MB")]
public IFormFile? EvidenceFile { get; set; }

public Guid? EvidenceFileId { get; set; }
Expand All @@ -65,7 +60,7 @@ public async Task OnGet()
HasAdditionalReasonDetail = JourneyInstance!.State.HasAdditionalReasonDetail;
AddReasonDetail = JourneyInstance!.State.AddReasonDetail;
UploadedEvidenceFileUrl = JourneyInstance?.State.EvidenceFileId is not null ?
await fileService.GetFileUrl(JourneyInstance.State.EvidenceFileId.Value, _fileUrlExpiresAfter) :
await fileService.GetFileUrl(JourneyInstance.State.EvidenceFileId.Value, AlertDefaults.FileUrlExpiry) :
null;
UploadEvidence = JourneyInstance?.State.UploadEvidence;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
namespace TeachingRecordSystem.SupportUi.Pages.Alerts;

public static class AlertDefaults
{
public const int MaxFileUploadSizeMb = 50;
public const int DetailMaxCharacterCount = 4000;
public const int DetailTextAreaMinimumRows = 10;
public static TimeSpan FileUrlExpiry { get; } = TimeSpan.FromMinutes(15);
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,37 @@
<govuk-summary-list>
<govuk-summary-list-row>
<govuk-summary-list-row-key>Details</govuk-summary-list-row-key>
<govuk-summary-list-row-value use-empty-fallback>@Model.Alert.Details</govuk-summary-list-row-value>
<govuk-summary-list-row-value>
@if (Model.Alert.Details is not null)
{
<multi-line-text text="@Model.Alert.Details" />
}
else
{
<span use-empty-fallback></span>
}
</govuk-summary-list-row-value>
</govuk-summary-list-row>
<govuk-summary-list-row>
<govuk-summary-list-row-key>Link</govuk-summary-list-row-key>
<govuk-summary-list-row-value use-empty-fallback>
<govuk-summary-list-row-value>
@if (Model.Alert.ExternalLink is not null)
{
<a href="@Model.Alert.ExternalLink" rel="noopener noreferrer" class="govuk-link">@Model.Alert.ExternalLink</a>
<a href="@Model.ExternalLinkUri" class="govuk-link" rel="noreferrer noopener" target="_blank" data-testid="link">@($"{Model.Alert.ExternalLink} (opens in new tab)")</a>
}
else
{
<span data-testid="link" use-empty-fallback></span>
}
</govuk-summary-list-row-value>
</govuk-summary-list-row>
<govuk-summary-list-row>
<govuk-summary-list-row-key>Start date</govuk-summary-list-row-key>
<govuk-summary-list-row-value use-empty-fallback>@Model.Alert.StartDate?.ToString("d MMMM yyyy")</govuk-summary-list-row-value>
<govuk-summary-list-row-value use-empty-fallback>@Model.Alert.StartDate?.ToString(UiDefaults.DateOnlyDisplayFormat)</govuk-summary-list-row-value>
</govuk-summary-list-row>
<govuk-summary-list-row>
<govuk-summary-list-row-key>End date</govuk-summary-list-row-key>
<govuk-summary-list-row-value use-empty-fallback>@Model.Alert.EndDate?.ToString("d MMMM yyyy")</govuk-summary-list-row-value>
<govuk-summary-list-row-value use-empty-fallback>@Model.Alert.EndDate?.ToString(UiDefaults.DateOnlyDisplayFormat)</govuk-summary-list-row-value>
@if (Model.CanEdit)
{
<govuk-summary-list-row-actions>
Expand All @@ -56,5 +69,18 @@
}
</govuk-summary-list-row-value>
</govuk-summary-list-row>
<govuk-summary-list-row>
<govuk-summary-list-row-key>Evidence</govuk-summary-list-row-key>
<govuk-summary-list-row-value>
@if (Model.UploadedEvidenceFileUrl is not null)
{
<a href="@Model.UploadedEvidenceFileUrl" class="govuk-link" rel="noreferrer noopener" target="_blank" data-testid="uploaded-evidence-link">@($"{Model.EvidenceFileName} (opens in new tab)")</a>
}
else
{
<span data-testid="uploaded-evidence-link" use-empty-fallback></span>
}
</govuk-summary-list-row-value>
</govuk-summary-list-row>

</govuk-summary-list>
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using Microsoft.AspNetCore.Mvc.RazorPages;
using TeachingRecordSystem.Core.DataStore.Postgres;
using TeachingRecordSystem.Core.DataStore.Postgres.Models;
using TeachingRecordSystem.Core.Services.Files;
using TeachingRecordSystem.SupportUi.Infrastructure.Filters;
using TeachingRecordSystem.SupportUi.Infrastructure.Security;

Expand All @@ -11,14 +12,21 @@ namespace TeachingRecordSystem.SupportUi.Pages.Alerts;
[CheckAlertExistsFilterFactory(requiredPermission: Permissions.Alerts.Read), ServiceFilter(typeof(RequireClosedAlertFilter))]
public class AlertDetailModel(
IAuthorizationService authorizationService,
TrsDbContext dbContext) : PageModel
TrsDbContext dbContext,
IFileService fileService) : PageModel
{
public Alert? Alert { get; set; }

public Uri? ExternalLinkUri { get; set; }

public string? ChangeReason { get; set; }

public string? ChangeReasonDetail { get; set; }

public string? EvidenceFileName { get; set; }

public string? UploadedEvidenceFileUrl { get; set; }

public bool CanEdit { get; set; }

public async Task OnGet()
Expand All @@ -30,7 +38,9 @@ public async Task OnGet()
$"""
SELECT
payload ->> 'ChangeReason' as change_reason,
payload ->> 'ChangeReasonDetail' as change_reason_detail
payload ->> 'ChangeReasonDetail' as change_reason_detail,
(payload #>> Array['EvidenceFile', 'FileId'])::uuid as evidence_file_id,
payload #>> Array['EvidenceFile', 'Name'] as evidence_file_name
FROM
events
WHERE
Expand All @@ -46,11 +56,17 @@ created DESC

ChangeReason = changeReasonInfo?.ChangeReason;
ChangeReasonDetail = changeReasonInfo?.ChangeReasonDetail;
EvidenceFileName = changeReasonInfo?.EvidenceFileName;
UploadedEvidenceFileUrl = changeReasonInfo?.EvidenceFileId is not null ?
await fileService.GetFileUrl(changeReasonInfo.EvidenceFileId!.Value, AlertDefaults.FileUrlExpiry) :
null;
ExternalLinkUri = TrsUriHelper.TryCreateWebsiteUri(Alert.ExternalLink, out var linkUri) ? linkUri : null;

CanEdit = (await authorizationService.AuthorizeForAlertTypeAsync(
User,
Alert.AlertTypeId,
Permissions.Alerts.Write)) is { Succeeded: true };
}

private record ChangeReasonInfo(string? ChangeReason, string? ChangeReasonDetail);
private record ChangeReasonInfo(string? ChangeReason, string? ChangeReasonDetail, Guid? EvidenceFileId, string? EvidenceFileName);
}
Loading

0 comments on commit 26ed784

Please sign in to comment.