From 9688b3dc2ddd0bb6bb03ff658b06a02e9eb2c0f0 Mon Sep 17 00:00:00 2001 From: Truls Henrik <39614013+trulshj@users.noreply.github.com> Date: Mon, 6 Jan 2025 09:29:17 +0100 Subject: [PATCH] chore: clean up AgreementController (#558) --- backend/Api/Agreements/AgreementController.cs | 104 +++++++----------- backend/Api/Agreements/AgreementModels.cs | 18 ++- .../DatabaseContext/ApplicationContext.cs | 35 +++--- .../ApplicationContextModelSnapshot.cs | 28 ++--- frontend/src/actions/agreementActions.ts | 10 +- 5 files changed, 85 insertions(+), 110 deletions(-) diff --git a/backend/Api/Agreements/AgreementController.cs b/backend/Api/Agreements/AgreementController.cs index 3b74dd9e..53a17766 100644 --- a/backend/Api/Agreements/AgreementController.cs +++ b/backend/Api/Agreements/AgreementController.cs @@ -5,9 +5,10 @@ using Infrastructure.DatabaseContext; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; +using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Caching.Memory; -namespace Api.Projects; +namespace Api.Agreements; [Authorize] [Route("/v0/{orgUrlKey}/agreements")] @@ -18,14 +19,16 @@ public class AgreementController( IOrganisationRepository organisationRepository, IAgreementsRepository agreementsRepository) : ControllerBase { + private const string SelectedOrganizationNotFound = "Selected organization not found"; + [HttpGet] - [Route("get/{agreementId}")] + [Route("{agreementId:int}")] public async Task> GetAgreement([FromRoute] string orgUrlKey, [FromRoute] int agreementId, CancellationToken ct) { var selectedOrg = await organisationRepository.GetOrganizationByUrlKey(orgUrlKey, ct); - if (selectedOrg is null) return BadRequest("Selected org not found"); + if (selectedOrg is null) return NotFound(SelectedOrganizationNotFound); var agreement = await agreementsRepository.GetAgreementById(agreementId, ct); @@ -43,28 +46,21 @@ public async Task> GetAgreement([FromRoute] str Notes: agreement.Notes, Options: agreement.Options, PriceAdjustmentProcess: agreement.PriceAdjustmentProcess, - Files: agreement.Files.Select(f => new FileReferenceReadModel( - FileName: f.FileName, - BlobName: f.BlobName, - UploadedOn: f.UploadedOn, - UploadedBy: f.UploadedBy ?? "Unknown" - )).ToList() + Files: agreement.Files.Select(f => new FileReferenceReadModel(f)).ToList() ); return Ok(responseModel); } [HttpGet] - [Route("get/engagement/{engagementId}")] + [Route("engagement/{engagementId:int}")] public async Task>> GetAgreementsByEngagement([FromRoute] string orgUrlKey, [FromRoute] int engagementId, CancellationToken ct) { var selectedOrg = await organisationRepository.GetOrganizationByUrlKey(orgUrlKey, ct); - if (selectedOrg is null) return BadRequest("Selected org not found"); + if (selectedOrg is null) return NotFound(SelectedOrganizationNotFound); var agreements = await agreementsRepository.GetAgreementsByEngagementId(engagementId, ct); - if (agreements is null || !agreements.Any()) return NotFound(); - var responseModels = agreements.Select(agreement => new AgreementReadModel( AgreementId: agreement.Id, Name: agreement.Name, @@ -77,29 +73,22 @@ public async Task>> GetAgreementsByEngagem Notes: agreement.Notes, Options: agreement.Options, PriceAdjustmentProcess: agreement.PriceAdjustmentProcess, - Files: agreement.Files.Select(f => new FileReferenceReadModel( - FileName: f.FileName, - BlobName: f.BlobName, - UploadedOn: f.UploadedOn, - UploadedBy: f.UploadedBy ?? "Unknown" - )).ToList() + Files: agreement.Files.Select(f => new FileReferenceReadModel(f)).ToList() )).ToList(); return Ok(responseModels); } [HttpGet] - [Route("get/customer/{customerId}")] + [Route("customer/{customerId:int}")] public async Task>> GetAgreementsByCustomer([FromRoute] string orgUrlKey, [FromRoute] int customerId, CancellationToken ct) { var selectedOrg = await organisationRepository.GetOrganizationByUrlKey(orgUrlKey, ct); - if (selectedOrg is null) return BadRequest("Selected org not found"); + if (selectedOrg is null) return NotFound(SelectedOrganizationNotFound); var agreements = await agreementsRepository.GetAgreementsByCustomerId(customerId, ct); - if (agreements is null || !agreements.Any()) return NotFound(); - var responseModels = agreements.Select(agreement => new AgreementReadModel( AgreementId: agreement.Id, Name: agreement.Name, @@ -112,21 +101,15 @@ public async Task>> GetAgreementsByCustome Notes: agreement.Notes, Options: agreement.Options, PriceAdjustmentProcess: agreement.PriceAdjustmentProcess, - Files: agreement.Files.Select(f => new FileReferenceReadModel( - FileName: f.FileName, - BlobName: f.BlobName, - UploadedOn: f.UploadedOn, - UploadedBy: f.UploadedBy ?? "Unknown" - )).ToList() + Files: agreement.Files.Select(f => new FileReferenceReadModel(f)).ToList() )).ToList(); return Ok(responseModels); } [HttpPost] - [Route("create")] public async Task> Post([FromRoute] string orgUrlKey, - [FromBody] AgreementWriteModel body, CancellationToken ct) + [FromBody] AgreementWriteModel body, CancellationToken cancellationToken) { if (!ModelState.IsValid) { @@ -139,24 +122,26 @@ public async Task> Post([FromRoute] string orgU return BadRequest(ModelState); } - var selectedOrg = await organisationRepository.GetOrganizationByUrlKey(orgUrlKey, ct); + var selectedOrg = await organisationRepository.GetOrganizationByUrlKey(orgUrlKey, cancellationToken); if (selectedOrg is null) - return BadRequest("Selected organization not found"); + return NotFound(SelectedOrganizationNotFound); Customer? customer = null; if (body.CustomerId != null) { - customer = await context.Customer.FindAsync(body.CustomerId.Value); + customer = await context.Customer.FirstOrDefaultAsync(c => c.Id == body.CustomerId.Value, + cancellationToken); if (customer == null) - return BadRequest("Customer not found"); + return NotFound("Customer not found"); } Engagement? engagement = null; if (body.EngagementId != null) { - engagement = await context.Project.FindAsync(body.EngagementId.Value); + engagement = + await context.Project.FirstOrDefaultAsync(e => e.Id == body.EngagementId.Value, cancellationToken); if (engagement is null) - return BadRequest("Engagement not found"); + return NotFound("Engagement not found"); } var agreement = new Agreement @@ -182,7 +167,7 @@ public async Task> Post([FromRoute] string orgU }).ToList() }; - await agreementsRepository.AddAgreementAsync(agreement, ct); + await agreementsRepository.AddAgreementAsync(agreement, cancellationToken); var responseModel = new AgreementReadModel( Name: agreement.Name, @@ -196,12 +181,7 @@ public async Task> Post([FromRoute] string orgU Notes: agreement.Notes, Options: agreement.Options, PriceAdjustmentProcess: agreement.PriceAdjustmentProcess, - Files: agreement.Files.Select(f => new FileReferenceReadModel( - FileName: f.FileName, - BlobName: f.BlobName, - UploadedOn: f.UploadedOn, - UploadedBy: f.UploadedBy ?? "Unknown" - )).ToList() + Files: agreement.Files.Select(f => new FileReferenceReadModel(f)).ToList() ); cache.Remove($"consultantCacheKey/{orgUrlKey}"); @@ -209,9 +189,9 @@ public async Task> Post([FromRoute] string orgU } [HttpPut] - [Route("update/{agreementId}")] + [Route("{agreementId:int}")] public async Task> Put([FromRoute] string orgUrlKey, - [FromRoute] int agreementId, [FromBody] AgreementWriteModel body, CancellationToken ct) + [FromRoute] int agreementId, [FromBody] AgreementWriteModel body, CancellationToken cancellationToken) { if (!ModelState.IsValid) { @@ -224,20 +204,19 @@ public async Task> Put([FromRoute] string orgUr return BadRequest(ModelState); } - var selectedOrg = await organisationRepository.GetOrganizationByUrlKey(orgUrlKey, ct); + var selectedOrg = await organisationRepository.GetOrganizationByUrlKey(orgUrlKey, cancellationToken); if (selectedOrg is null) - return BadRequest("Selected organization not found"); + return NotFound("Selected organization not found"); - var agreement = await agreementsRepository.GetAgreementById(agreementId, ct); + var agreement = await agreementsRepository.GetAgreementById(agreementId, cancellationToken); if (agreement is null) return NotFound("Agreement not found"); - Customer? customer = null; if (body.CustomerId is not null) { - customer = await context.Customer.FindAsync(body.CustomerId); + var customer = await context.Customer.FirstOrDefaultAsync(c => c.Id == body.CustomerId, cancellationToken); if (customer is null) - return BadRequest("Customer not found"); + return NotFound("Customer not found"); agreement.CustomerId = body.CustomerId; agreement.Customer = customer; @@ -248,12 +227,12 @@ public async Task> Put([FromRoute] string orgUr agreement.Customer = null; } - Engagement? engagement = null; if (body.EngagementId is not null) { - engagement = await context.Project.FindAsync(body.EngagementId); + var engagement = + await context.Project.FirstOrDefaultAsync(e => e.Id == body.EngagementId, cancellationToken); if (engagement is null) - return BadRequest("Engagement not found"); + return NotFound("Engagement not found"); agreement.EngagementId = body.EngagementId; agreement.Engagement = engagement; @@ -280,7 +259,7 @@ public async Task> Put([FromRoute] string orgUr UploadedBy = f.UploadedBy ?? "Unknown" }).ToList(); - await agreementsRepository.UpdateAgreementAsync(agreement, ct); + await agreementsRepository.UpdateAgreementAsync(agreement, cancellationToken); var responseModel = new AgreementReadModel( AgreementId: agreement.Id, @@ -294,12 +273,7 @@ public async Task> Put([FromRoute] string orgUr Notes: agreement.Notes, Options: agreement.Options, PriceAdjustmentProcess: agreement.PriceAdjustmentProcess, - Files: agreement.Files.Select(f => new FileReferenceReadModel( - FileName: f.FileName, - BlobName: f.BlobName, - UploadedOn: f.UploadedOn, - UploadedBy: f.UploadedBy ?? "Unknown" - )).ToList() + agreement.Files.Select(f => new FileReferenceReadModel(f)).ToList() ); cache.Remove($"consultantCacheKey/{orgUrlKey}"); @@ -308,11 +282,11 @@ public async Task> Put([FromRoute] string orgUr } [HttpDelete] - [Route("delete/{agreementId}")] + [Route("{agreementId:int}")] public async Task Delete([FromRoute] string orgUrlKey, [FromRoute] int agreementId, CancellationToken ct) { var selectedOrg = await organisationRepository.GetOrganizationByUrlKey(orgUrlKey, ct); - if (selectedOrg is null) return BadRequest("Selected org not found"); + if (selectedOrg is null) return NotFound(SelectedOrganizationNotFound); var agreement = await agreementsRepository.GetAgreementById(agreementId, ct); if (agreement is null) return NotFound(); @@ -328,7 +302,7 @@ public async Task Delete([FromRoute] string orgUrlKey, [FromRoute] public async Task>> GetPriceAdjustmentIndexes([FromRoute] string orgUrlKey, CancellationToken ct) { var selectedOrg = await organisationRepository.GetOrganizationByUrlKey(orgUrlKey, ct); - if (selectedOrg is null) return BadRequest("Selected org not found"); + if (selectedOrg is null) return NotFound(SelectedOrganizationNotFound); var priceAdjustmentIndexes = await agreementsRepository.GetPriceAdjustmentIndexesAsync(ct); diff --git a/backend/Api/Agreements/AgreementModels.cs b/backend/Api/Agreements/AgreementModels.cs index 96cddf7e..e38fc16d 100644 --- a/backend/Api/Agreements/AgreementModels.cs +++ b/backend/Api/Agreements/AgreementModels.cs @@ -1,6 +1,8 @@ -using System; -using System.Collections.Generic; using System.ComponentModel.DataAnnotations; +using Core.Agreements; +// ReSharper disable NotAccessedPositionalProperty.Global + +namespace Api.Agreements; public record AgreementReadModel( int AgreementId, @@ -16,12 +18,19 @@ public record AgreementReadModel( string? PriceAdjustmentProcess, List Files ); + public record FileReferenceReadModel( string FileName, string BlobName, DateTime UploadedOn, string? UploadedBy -); +) +{ + public FileReferenceReadModel(FileReference fileReference) : this(fileReference.FileName, fileReference.BlobName, + fileReference.UploadedOn, fileReference.UploadedBy ?? "Unknown") + { + } +} public record AgreementWriteModel( string? Name, @@ -49,9 +58,10 @@ public IEnumerable Validate(ValidationContext validationContex } +// ReSharper disable once ClassNeverInstantiated.Global public record FileReferenceWriteModel( string FileName, string BlobName, DateTime UploadedOn, string? UploadedBy -); +); \ No newline at end of file diff --git a/backend/Infrastructure/DatabaseContext/ApplicationContext.cs b/backend/Infrastructure/DatabaseContext/ApplicationContext.cs index c06bbb60..25b2c1b9 100644 --- a/backend/Infrastructure/DatabaseContext/ApplicationContext.cs +++ b/backend/Infrastructure/DatabaseContext/ApplicationContext.cs @@ -13,25 +13,20 @@ namespace Infrastructure.DatabaseContext; -public class ApplicationContext : DbContext +public class ApplicationContext(DbContextOptions options) : DbContext(options) { - public ApplicationContext(DbContextOptions options) : base(options) - { - } - - public DbSet Consultant { get; set; } = null!; - public DbSet Competence { get; set; } = null!; - public DbSet CompetenceConsultant { get; set; } = null!; - public DbSet Department { get; set; } = null!; - public DbSet Organization { get; set; } = null!; - public DbSet Absence { get; set; } = null!; - - public DbSet PlannedAbsence { get; set; } = null!; - public DbSet Vacation { get; set; } = null!; - public DbSet Customer { get; set; } = null!; - public DbSet Project { get; set; } = null!; - public DbSet Staffing { get; set; } = null!; - public DbSet Agreements { get; set; } = null!; + public DbSet Consultant { get; init; } = null!; + public DbSet Competence { get; init; } = null!; + public DbSet CompetenceConsultant { get; init; } = null!; + public DbSet Department { get; init; } = null!; + public DbSet Organization { get; init; } = null!; + public DbSet Absence { get; init; } = null!; + public DbSet PlannedAbsence { get; init; } = null!; + public DbSet Vacation { get; init; } = null!; + public DbSet Customer { get; init; } = null!; + public DbSet Project { get; init; } = null!; + public DbSet Staffing { get; init; } = null!; + public DbSet Agreements { get; init; } = null!; protected override void ConfigureConventions(ModelConfigurationBuilder configurationBuilder) @@ -135,10 +130,6 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) .Property(v => v.EndDate) .HasConversion(); - /*modelBuilder.Entity() - .HasMany(v => v.CompetenceConsultant) - .WithMany();*/ - modelBuilder.Entity() .Property(c => c.TransferredVacationDays) .HasDefaultValue(0); diff --git a/backend/Infrastructure/Migrations/ApplicationContextModelSnapshot.cs b/backend/Infrastructure/Migrations/ApplicationContextModelSnapshot.cs index b4fc0430..0b56970d 100644 --- a/backend/Infrastructure/Migrations/ApplicationContextModelSnapshot.cs +++ b/backend/Infrastructure/Migrations/ApplicationContextModelSnapshot.cs @@ -45,7 +45,7 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.HasIndex("OrganizationId"); - b.ToTable("Absence"); + b.ToTable("Absence", (string)null); }); modelBuilder.Entity("Core.Agreements.Agreement", b => @@ -92,7 +92,7 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.HasIndex("EngagementId"); - b.ToTable("Agreements"); + b.ToTable("Agreements", (string)null); }); modelBuilder.Entity("Core.Consultants.Competence", b => @@ -106,7 +106,7 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.HasKey("Id"); - b.ToTable("Competence"); + b.ToTable("Competence", (string)null); b.HasData( new @@ -148,7 +148,7 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.HasIndex("CompetencesId"); - b.ToTable("CompetenceConsultant"); + b.ToTable("CompetenceConsultant", (string)null); }); modelBuilder.Entity("Core.Consultants.Consultant", b => @@ -192,7 +192,7 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.HasIndex("DepartmentId"); - b.ToTable("Consultant"); + b.ToTable("Consultant", (string)null); b.HasData( new @@ -228,7 +228,7 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.HasIndex("OrganizationId", "Name") .IsUnique(); - b.ToTable("Customer"); + b.ToTable("Customer", (string)null); }); modelBuilder.Entity("Core.Engagements.Engagement", b => @@ -258,7 +258,7 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.HasIndex("CustomerId", "Name") .IsUnique(); - b.ToTable("Project"); + b.ToTable("Project", (string)null); }); modelBuilder.Entity("Core.Organizations.Department", b => @@ -282,7 +282,7 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.HasIndex("OrganizationId"); - b.ToTable("Department"); + b.ToTable("Department", (string)null); b.HasData( new @@ -321,7 +321,7 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.HasKey("Id"); - b.ToTable("Organization"); + b.ToTable("Organization", (string)null); b.HasData( new @@ -354,7 +354,7 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.HasIndex("ConsultantId"); - b.ToTable("PlannedAbsence"); + b.ToTable("PlannedAbsence", (string)null); }); modelBuilder.Entity("Core.Staffings.Staffing", b => @@ -375,7 +375,7 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.HasIndex("ConsultantId"); - b.ToTable("Staffing"); + b.ToTable("Staffing", (string)null); }); modelBuilder.Entity("Core.Vacations.Vacation", b => @@ -396,7 +396,7 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.HasIndex("ConsultantId"); - b.ToTable("Vacation"); + b.ToTable("Vacation", (string)null); }); modelBuilder.Entity("Core.Absences.Absence", b => @@ -422,7 +422,7 @@ protected override void BuildModel(ModelBuilder modelBuilder) .HasForeignKey("EngagementId") .OnDelete(DeleteBehavior.Restrict); - b.OwnsMany("Core.Agreements.FileReference", "Files", b1 => + b.OwnsMany("Core.Agreements.Agreement.Files#Core.Agreements.FileReference", "Files", b1 => { b1.Property("Id") .ValueGeneratedOnAdd() @@ -451,7 +451,7 @@ protected override void BuildModel(ModelBuilder modelBuilder) b1.HasIndex("AgreementId"); - b1.ToTable("FileReference"); + b1.ToTable("FileReference", (string)null); b1.WithOwner() .HasForeignKey("AgreementId"); diff --git a/frontend/src/actions/agreementActions.ts b/frontend/src/actions/agreementActions.ts index baf87687..330e9971 100644 --- a/frontend/src/actions/agreementActions.ts +++ b/frontend/src/actions/agreementActions.ts @@ -135,7 +135,7 @@ export async function getAgreementsForProject( ) { try { const res = await fetchWithToken( - `${orgUrlKey}/agreements/get/engagement/${projectId}`, + `${orgUrlKey}/agreements/engagement/${projectId}`, ); let agreementsWithDateTypes: Agreement[] = []; @@ -154,7 +154,7 @@ export async function getAgreementsForCustomer( ) { try { const res = await fetchWithToken( - `${orgUrlKey}/agreements/get/customer/${customerId}`, + `${orgUrlKey}/agreements/customer/${customerId}`, ); let agreementsWithDateTypes: Agreement[] = []; @@ -170,7 +170,7 @@ export async function getAgreementsForCustomer( export async function updateAgreement(agreement: Agreement, orgUrlKey: string) { try { const res = await putWithToken( - `${orgUrlKey}/agreements/update/${agreement.agreementId}`, + `${orgUrlKey}/agreements/${agreement.agreementId}`, agreement, ); @@ -190,7 +190,7 @@ export async function createAgreement( ) { try { const res = await postWithToken( - `${orgUrlKey}/agreements/create`, + `${orgUrlKey}/agreements`, agreement, ); let agreementWithDateTypes: Agreement | null = null; @@ -206,7 +206,7 @@ export async function createAgreement( export async function deleteAgreement(agreementId: number, orgUrlKey: string) { try { const res = await deleteWithToken( - `${orgUrlKey}/agreements/delete/${agreementId}`, + `${orgUrlKey}/agreements/${agreementId}`, ); return res;