From 064a31cc9d4b17acda8dc1b28bb373f7d18b564d Mon Sep 17 00:00:00 2001 From: WILDE Date: Mon, 9 Jan 2023 12:17:18 +0000 Subject: [PATCH 1/2] Fix for unexpected project note duplication bug. --- .gitignore | 3 + .../AcademisationContext.cs | 8 +-- .../ProjectAggregate/ProjectNoteState.cs | 1 + .../ProjectUpdateDataCommand.cs | 3 +- .../LegacyProjectAddNoteCommandTests.cs | 59 +++++++++++++++---- ...mies.Academisation.Service.UnitTest.csproj | 1 + .../Project/LegacyProjectAddNoteCommand.cs | 21 +++++-- 7 files changed, 71 insertions(+), 25 deletions(-) diff --git a/.gitignore b/.gitignore index a48d585a4..fa687ccad 100644 --- a/.gitignore +++ b/.gitignore @@ -384,3 +384,6 @@ terraform.rc* !terraform.tfvars.example .terraform/ backend.vars + +# Stackify +Stackify.json \ No newline at end of file diff --git a/Dfe.Academies.Academisation.Data/AcademisationContext.cs b/Dfe.Academies.Academisation.Data/AcademisationContext.cs index 36eb1172c..6818c78b9 100644 --- a/Dfe.Academies.Academisation.Data/AcademisationContext.cs +++ b/Dfe.Academies.Academisation.Data/AcademisationContext.cs @@ -1,16 +1,12 @@ -using System.Reflection.Emit; -using Dfe.Academies.Academisation.Data.ConversionAdvisoryBoardDecisionAggregate; +using Dfe.Academies.Academisation.Data.ConversionAdvisoryBoardDecisionAggregate; using Dfe.Academies.Academisation.Data.ProjectAggregate; using Dfe.Academies.Academisation.Domain.ApplicationAggregate; using Dfe.Academies.Academisation.Domain.ApplicationAggregate.Schools; using Dfe.Academies.Academisation.Domain.ApplicationAggregate.Trusts; -using Dfe.Academies.Academisation.Domain.ConversionAdvisoryBoardDecisionAggregate; -using Dfe.Academies.Academisation.Domain.Core.ApplicationAggregate; using Dfe.Academies.Academisation.Domain.SeedWork; using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.ChangeTracking; using Microsoft.EntityFrameworkCore.Metadata.Builders; -using Microsoft.IdentityModel.Tokens; namespace Dfe.Academies.Academisation.Data; @@ -32,6 +28,7 @@ public AcademisationContext(DbContextOptions options) : ba public DbSet FormTrusts { get; set; } = null!; // done public DbSet Projects { get; set; } = null!; + public DbSet ProjectNotes { get; set; } = null!; public DbSet ConversionAdvisoryBoardDecisions { get; set; } = null!; public override int SaveChanges() @@ -162,6 +159,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) private static void ConfigureProject(EntityTypeBuilder projectConfiguration) { projectConfiguration.ToTable("Project", DEFAULT_SCHEMA); + projectConfiguration .HasMany(x => x.Notes) .WithOne() diff --git a/Dfe.Academies.Academisation.Data/ProjectAggregate/ProjectNoteState.cs b/Dfe.Academies.Academisation.Data/ProjectAggregate/ProjectNoteState.cs index a0b708b83..3e9715290 100644 --- a/Dfe.Academies.Academisation.Data/ProjectAggregate/ProjectNoteState.cs +++ b/Dfe.Academies.Academisation.Data/ProjectAggregate/ProjectNoteState.cs @@ -10,5 +10,6 @@ public class ProjectNoteState public string? Note { get; set; } public string? Author { get; set; } public DateTime? Date { get; set; } + public int ProjectId { get; set; } } } diff --git a/Dfe.Academies.Academisation.Data/ProjectAggregate/ProjectUpdateDataCommand.cs b/Dfe.Academies.Academisation.Data/ProjectAggregate/ProjectUpdateDataCommand.cs index 6d441c041..705d74003 100644 --- a/Dfe.Academies.Academisation.Data/ProjectAggregate/ProjectUpdateDataCommand.cs +++ b/Dfe.Academies.Academisation.Data/ProjectAggregate/ProjectUpdateDataCommand.cs @@ -19,7 +19,8 @@ public async Task Execute(IProject project) await _context.Projects.SingleAsync(p => p.Id == project.Id); - _context.ReplaceTracked(projectState); + _context.ReplaceTracked(projectState) + .Collection(x => x.Notes).IsModified = false; await _context.SaveChangesAsync(); } diff --git a/Dfe.Academies.Academisation.Service.UnitTest/Commands/Legacy/Project/LegacyProjectAddNoteCommandTests.cs b/Dfe.Academies.Academisation.Service.UnitTest/Commands/Legacy/Project/LegacyProjectAddNoteCommandTests.cs index c1a86caec..a50399aae 100644 --- a/Dfe.Academies.Academisation.Service.UnitTest/Commands/Legacy/Project/LegacyProjectAddNoteCommandTests.cs +++ b/Dfe.Academies.Academisation.Service.UnitTest/Commands/Legacy/Project/LegacyProjectAddNoteCommandTests.cs @@ -1,4 +1,8 @@ -using Dfe.Academies.Academisation.Core; +using AutoFixture; +using Dfe.Academies.Academisation.Core; +using Dfe.Academies.Academisation.Data; +using Dfe.Academies.Academisation.Data.ProjectAggregate; +using Dfe.Academies.Academisation.Data.UnitTest.Contexts; using Dfe.Academies.Academisation.Domain.Core.ProjectAggregate; using Dfe.Academies.Academisation.IData.ProjectAggregate; using Dfe.Academies.Academisation.IDomain.ProjectAggregate; @@ -12,20 +16,36 @@ namespace Dfe.Academies.Academisation.Service.UnitTest.Commands.Legacy.Project { public class LegacyProjectAddNoteCommandTests { - private readonly Mock _projectGetDataQuery; - private readonly Mock _projectUpdateDataCommand; private readonly LegacyProjectAddNoteModel _addNoteModel; + private readonly AcademisationContext _context; + private readonly Mock _projectGetDataQuery; public LegacyProjectAddNoteCommandTests() { + ProjectState projectState = new Fixture().Create(); + + var testProjectContext = new TestProjectContext(); + _context = testProjectContext.CreateContext(); + + _context.Projects.Add(projectState); + _context.SaveChanges(); + _projectGetDataQuery = new Mock(); - _projectUpdateDataCommand = new Mock(); - _addNoteModel = new LegacyProjectAddNoteModel("Subject", "Note", "Author", DateTime.Today, 1234); + _addNoteModel = new LegacyProjectAddNoteModel( + "Subject", + "Note", + "Author", + DateTime.Today, + projectState.Id + ); } private LegacyProjectAddNoteCommand System_under_test() { - return new LegacyProjectAddNoteCommand(_projectGetDataQuery.Object, _projectUpdateDataCommand.Object); + return new LegacyProjectAddNoteCommand( + _projectGetDataQuery.Object, + _context + ); } [Fact] @@ -38,11 +58,19 @@ public async Task Should_return_not_found_result_if_the_specified_project_does_n LegacyProjectAddNoteCommand command = System_under_test(); CommandResult result = - await command.Execute(_addNoteModel); - - _projectUpdateDataCommand.Verify(x => x.Execute(It.IsAny()), Times.Never); + await command.Execute(_addNoteModel); result.Should().BeOfType(); + + _context.ProjectNotes.Should().NotContainEquivalentOf( + new ProjectNoteState + { + Date = _addNoteModel.Date, + Subject = _addNoteModel.Subject, + Author = _addNoteModel.Author, + Note = _addNoteModel.Note, + ProjectId = _addNoteModel.ProjectId + }, x => x.Excluding(q => q.Id)); } [Fact] @@ -58,8 +86,15 @@ public async Task Should_add_the_new_note_to_the_project() await command.Execute(_addNoteModel with { ProjectId = project.Id }); - project.Details.Notes - .Should().ContainEquivalentOf(new ProjectNote("Subject", "Note", "Author", _addNoteModel.Date)); + _context.ProjectNotes.Should().ContainEquivalentOf( + new ProjectNoteState + { + Date = _addNoteModel.Date, + Subject = _addNoteModel.Subject, + Author = _addNoteModel.Author, + Note = _addNoteModel.Note, + ProjectId = _addNoteModel.ProjectId + }, x => x.Excluding(q => q.Id)); } [Fact] @@ -75,8 +110,6 @@ public async Task Should_save_the_amended_project() CommandResult result = await command.Execute(_addNoteModel with { ProjectId = project.Id }); - _projectUpdateDataCommand.Verify(x => x.Execute(project), Times.Once()); - result.Should().BeOfType(); } } diff --git a/Dfe.Academies.Academisation.Service.UnitTest/Dfe.Academies.Academisation.Service.UnitTest.csproj b/Dfe.Academies.Academisation.Service.UnitTest/Dfe.Academies.Academisation.Service.UnitTest.csproj index 2162bd0f5..c5b84ed8c 100644 --- a/Dfe.Academies.Academisation.Service.UnitTest/Dfe.Academies.Academisation.Service.UnitTest.csproj +++ b/Dfe.Academies.Academisation.Service.UnitTest/Dfe.Academies.Academisation.Service.UnitTest.csproj @@ -21,6 +21,7 @@ + diff --git a/Dfe.Academies.Academisation.Service/Commands/Legacy/Project/LegacyProjectAddNoteCommand.cs b/Dfe.Academies.Academisation.Service/Commands/Legacy/Project/LegacyProjectAddNoteCommand.cs index 4a4e31539..96d6850c5 100644 --- a/Dfe.Academies.Academisation.Service/Commands/Legacy/Project/LegacyProjectAddNoteCommand.cs +++ b/Dfe.Academies.Academisation.Service/Commands/Legacy/Project/LegacyProjectAddNoteCommand.cs @@ -1,21 +1,22 @@ using Dfe.Academies.Academisation.Core; +using Dfe.Academies.Academisation.Data; +using Dfe.Academies.Academisation.Data.ProjectAggregate; using Dfe.Academies.Academisation.IData.ProjectAggregate; using Dfe.Academies.Academisation.IDomain.ProjectAggregate; using Dfe.Academies.Academisation.IService.Commands.Legacy.Project; using Dfe.Academies.Academisation.IService.ServiceModels.Legacy.ProjectAggregate; -using Dfe.Academies.Academisation.Service.Extensions; namespace Dfe.Academies.Academisation.Service.Commands.Legacy.Project { public class LegacyProjectAddNoteCommand : ILegacyProjectAddNoteCommand { private readonly IProjectGetDataQuery _projectGetDataQuery; - private readonly IProjectUpdateDataCommand _projectUpdateDataCommand; + private readonly AcademisationContext _context; - public LegacyProjectAddNoteCommand(IProjectGetDataQuery projectGetDataQuery, IProjectUpdateDataCommand projectUpdateDataCommand) + public LegacyProjectAddNoteCommand(IProjectGetDataQuery projectGetDataQuery, AcademisationContext context) { _projectGetDataQuery = projectGetDataQuery; - _projectUpdateDataCommand = projectUpdateDataCommand; + _context = context; } public async Task Execute(LegacyProjectAddNoteModel model) @@ -24,8 +25,16 @@ public async Task Execute(LegacyProjectAddNoteModel model) if (project is null) return new NotFoundCommandResult(); - project.Details.Notes.Add(model.ToProjectNote()); - await _projectUpdateDataCommand.Execute(project); + _context.ProjectNotes.Add(new ProjectNoteState + { + Author = model.Author, + Date = model.Date, + Subject = model.Subject, + Note = model.Note, + ProjectId = model.ProjectId + }); + + await _context.SaveChangesAsync(); return new CommandSuccessResult(); } From 49f34c0976f43b5b2481a936e183f06f58a94cec Mon Sep 17 00:00:00 2001 From: WILDE Date: Mon, 9 Jan 2023 13:40:58 +0000 Subject: [PATCH 2/2] Ignore nulls. --- .../ProjectAggregate/ProjectUpdateDataCommand.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dfe.Academies.Academisation.Data/ProjectAggregate/ProjectUpdateDataCommand.cs b/Dfe.Academies.Academisation.Data/ProjectAggregate/ProjectUpdateDataCommand.cs index 705d74003..b293c8373 100644 --- a/Dfe.Academies.Academisation.Data/ProjectAggregate/ProjectUpdateDataCommand.cs +++ b/Dfe.Academies.Academisation.Data/ProjectAggregate/ProjectUpdateDataCommand.cs @@ -20,7 +20,7 @@ public async Task Execute(IProject project) await _context.Projects.SingleAsync(p => p.Id == project.Id); _context.ReplaceTracked(projectState) - .Collection(x => x.Notes).IsModified = false; + .Collection(x => x.Notes!).IsModified = false; await _context.SaveChangesAsync(); }