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.

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

Story/241800 user redirect #983

Merged
merged 16 commits into from
Mar 5, 2025
Merged

Conversation

DrewAire
Copy link
Collaborator

@DrewAire DrewAire commented Feb 27, 2025

Overview

Addresses ticket #241800

Adds a new controller filter MaintainUrlOnKeyNotFoundAttribute which looks for KeyNotFoundExceptions returned from the Contentful data context and returns the NotFoundError view page if content is missing. Doing it this way achieves the desired goal of leaving the user at the URL where it failed so that they can refresh.

I also fixed some of the warnings that pop up in GitHub tests.

Changes

Major

  • MaintainUrlOnKeyNotFoundAttribute filter added at a global level to show the NotFoundError view when we pick up a KeyNotFoundException
  • ContentController.Index no longer redirects when the slug is empty, instead raising a KeyNotFoundException which routes through the new filter
  • PagesController.GetByRoute no longer redirects when the page is null, instead returning the NotFoundError view

Minor

  • GetLatestResponsesQuery class no longer reassigns the db value to a private readonly variable
  • Removed uses of magic strings e.g. in UserHelper.cs where the constant string can just reference ClaimConstants.Organisation
  • Removed properties which are already defined in the parent class in ContentItem and ContentItemBase
  • Fixed warning on establishmentId string in OnUserInformationReceivedEvent.cs
  • Changes to ServiceExceptionHandlerMiddleware to make better use of dependency injection instead of pulling services out in Program.cs and passing them down
  • Renamed some test variables from sut to controller

Non-functional changes (e.g. documentation)

  • Removed references to Moq - now NSubstitute

How to review the PR

@jag-nahl-airelogic, this is going to be a pain to test. I managed to stop the entities getting returned by adding the following in ContentfulRepository on line 36:

        var missingContent = false;
        if (missingContent)
        {
            return [];
        }

Checklist

Delete any rows that do not apply to the PR.

  • [v] Title uses Angular commit convention
  • [v] PR targets development branch
  • [v] Unit tests have been added/updated
  • [v] Documentation has been updated where relevant

@DrewAire DrewAire force-pushed the story/241800-user-redirect branch from f76d234 to 1e514f5 Compare February 27, 2025 23:55
…content is missing without changing the URL.
@DrewAire DrewAire force-pushed the story/241800-user-redirect branch from 27bfdf1 to 6b96c2b Compare February 27, 2025 23:57
github-actions bot and others added 6 commits February 27, 2025 23:59
… story/241800-user-redirect

# Conflicts:
#	src/Dfe.PlanTech.Web/Controllers/ContentController.cs
#	src/Dfe.PlanTech.Web/Controllers/PagesController.cs
#	src/Dfe.PlanTech.Web/Helpers/MaintainUrlOnKeyNotFoundAttribute.cs
#	tests/Dfe.PlanTech.Web.UnitTests/Controllers/ContentControllerTests.cs
#	tests/Dfe.PlanTech.Web.UnitTests/Controllers/PagesControllerTests.cs
@jag-nahl-airelogic jag-nahl-airelogic self-requested a review March 4, 2025 09:50
@jag-nahl-airelogic
Copy link
Collaborator

All checks passed, approved! Great job 👍

@DrewAire DrewAire merged commit 94dc77b into development Mar 5, 2025
11 checks passed
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.

3 participants