Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(content): Add error handling + logging to content route #150

Merged
merged 3 commits into from
Aug 28, 2024
Merged

feat(content): Add error handling + logging to content route #150

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9ca88d1
feat(content): Add error handling + logging to content route
jimwashbrook Aug 28, 2024
af09d71
tests(content): Test missing slug message
jimwashbrook Aug 28, 2024
43fca65
chore: Use constant for error action name
jimwashbrook Aug 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 31 additions & 11 deletions src/Dfe.ContentSupport.Web/Controllers/ContentController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ namespace Dfe.ContentSupport.Web.Controllers;

[Route("/content")]
[AllowAnonymous]
public class ContentController(IContentService contentService, ILayoutService layoutService)
public class ContentController(IContentService contentService, ILayoutService layoutService, ILogger<ContentController> logger)
: Controller
{
public const string ErrorActionName = "error";

public async Task<IActionResult> Home()
{
var defaultModel = new CsPage
Expand All @@ -28,24 +30,42 @@ public async Task<IActionResult> Home()
return View(defaultModel);
}



[HttpGet("{slug}/{page?}")]
public async Task<IActionResult> Index(string slug, string page = "", bool isPreview = false, [FromQuery] List<string>? tags = null)
{
if (!ModelState.IsValid) return RedirectToAction("error");
if (string.IsNullOrEmpty(slug)) return RedirectToAction("error");
if (!ModelState.IsValid)
{
logger.LogError("Invalid model state received for {Controller} {Action} with slug {Slug}", nameof(ContentController), nameof(Index), slug);
return RedirectToAction(ErrorActionName);
}

if (string.IsNullOrEmpty(slug))
{
logger.LogError("No slug received for C&S {Controller} {Action}", nameof(ContentController), nameof(Index));
return RedirectToAction(ErrorActionName);
}

var resp = await contentService.GetContent(slug, isPreview);
if (resp is null) return RedirectToAction("error");
try
{
var resp = await contentService.GetContent(slug, isPreview);
if (resp is null)
{
logger.LogError("Failed to load content for C&S page {Slug}; no content received.", slug);
return RedirectToAction(ErrorActionName);
}

resp = layoutService.GenerateLayout(resp, Request, page);
ViewBag.tags = tags;
resp = layoutService.GenerateLayout(resp, Request, page);
ViewBag.tags = tags;

return View("CsIndex", resp);
return View("CsIndex", resp);
}
catch (Exception ex)
{
logger.LogError(ex, "Error loading C&S content page {Slug}", slug);
return RedirectToAction(ErrorActionName);
}
}


public IActionResult Privacy()
{
return View();
Expand Down
47 changes: 45 additions & 2 deletions tests/Dfe.ContentSupport.Web.Tests/Controllers/ContentControllerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,18 @@
using Dfe.ContentSupport.Web.Models.Mapped;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.Logging;

namespace Dfe.ContentSupport.Web.Tests.Controllers;

public class ContentControllerTests
{
private readonly Mock<ILogger<ContentController>> _loggerMock = new();
private readonly Mock<IContentService> _contentServiceMock = new();

private ContentController GetController()
{
return new ContentController(_contentServiceMock.Object, new LayoutService());
return new ContentController(_contentServiceMock.Object, new LayoutService(), _loggerMock.Object);
}


Expand All @@ -36,6 +38,15 @@ public async Task Index_NoSlug_Returns_ErrorAction()

result.Should().BeOfType<RedirectToActionResult>();
(result as RedirectToActionResult)!.ActionName.Should().BeEquivalentTo("error");

_loggerMock.Verify(
x => x.Log(
LogLevel.Error,
It.IsAny<EventId>(),
It.Is<It.IsAnyType>((o, t) => o.ToString() != null && o.ToString()!.StartsWith($"No slug received for C&S {nameof(ContentController)} {nameof(sut.Index)}")),
It.IsAny<Exception>(),
It.IsAny<Func<It.IsAnyType, Exception?, string>>()),
Times.Once);
}

[Fact]
Expand All @@ -53,15 +64,47 @@ public async Task Index_Calls_Service_GetContent()
[Fact]
public async Task Index_NullResponse_ReturnsErrorAction()
{
var slug = "slug";
_contentServiceMock.Setup(o => o.GetContent(It.IsAny<string>(), It.IsAny<bool>()))
.ReturnsAsync((CsPage?)null);

var sut = GetController();

var result = await sut.Index("slug");
var result = await sut.Index(slug);

result.Should().BeOfType<RedirectToActionResult>();
(result as RedirectToActionResult)!.ActionName.Should().BeEquivalentTo("error");

_loggerMock.Verify(x => x.Log(
LogLevel.Error,
It.IsAny<EventId>(),
It.Is<It.IsAnyType>((o, t) => o.ToString() != null && o.ToString()!.Equals($"Failed to load content for C&S page {slug}; no content received.")),
It.IsAny<Exception>(),
It.IsAny<Func<It.IsAnyType, Exception?, string>>()),
Times.Once);
}

[Fact]
public async Task Index_ExceptionThrown_ReturnsErrorAction()
{
var slug = "slug";
_contentServiceMock.Setup(o => o.GetContent(It.IsAny<string>(), It.IsAny<bool>()))
.ThrowsAsync(new Exception("An exception occurred loading content"));

var sut = GetController();

var result = await sut.Index(slug);

result.Should().BeOfType<RedirectToActionResult>();
(result as RedirectToActionResult)!.ActionName.Should().BeEquivalentTo("error");

_loggerMock.Verify(x => x.Log(
LogLevel.Error,
It.IsAny<EventId>(),
It.Is<It.IsAnyType>((o, t) => o.ToString() != null && o.ToString()!.Equals($"Error loading C&S content page {slug}")),
It.IsAny<Exception>(),
It.IsAny<Func<It.IsAnyType, Exception?, string>>()),
Times.Once);
}

[Fact]
Expand Down
17 changes: 9 additions & 8 deletions tests/Dfe.ContentSupport.Web.Tests/Dfe.ContentSupport.Web.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,21 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="coverlet.collector" Version="6.0.0"/>
<PackageReference Include="FluentAssertions" Version="7.0.0-alpha.3"/>
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.8.0"/>
<PackageReference Include="Moq" Version="4.20.70"/>
<PackageReference Include="xunit" Version="2.5.3"/>
<PackageReference Include="xunit.runner.visualstudio" Version="2.5.3"/>
<PackageReference Include="coverlet.collector" Version="6.0.0" />
<PackageReference Include="FluentAssertions" Version="7.0.0-alpha.3" />
<PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="8.0.1" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.8.0" />
<PackageReference Include="Moq" Version="4.20.70" />
<PackageReference Include="xunit" Version="2.5.3" />
<PackageReference Include="xunit.runner.visualstudio" Version="2.5.3" />
</ItemGroup>

<ItemGroup>
<Using Include="Xunit"/>
<Using Include="Xunit" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\..\src\Dfe.ContentSupport.Web\Dfe.ContentSupport.Web.csproj"/>
<ProjectReference Include="..\..\src\Dfe.ContentSupport.Web\Dfe.ContentSupport.Web.csproj" />
</ItemGroup>

</Project>
Loading