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

PSP-7296 : Create/Edit Sales Details (Disposition) #3731

Merged
merged 24 commits into from
Jan 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.Logging;
using Pims.Api.Areas.Acquisition.Controllers;
using Pims.Api.Helpers.Exceptions;
using Pims.Api.Models.Concepts.DispositionFile;
using Pims.Api.Models.Concepts.DispositionFile;
using Pims.Api.Policies;
Expand Down Expand Up @@ -346,6 +347,69 @@
return new JsonResult(_mapper.Map<DispositionFileSaleModel>(dispositionSale));
}

[HttpPost("{id:long}/sale")]
[HasPermission(Permissions.DispositionEdit)]
[Produces("application/json")]
[ProducesResponseType(typeof(DispositionFileSaleModel), 201)]
[SwaggerOperation(Tags = new[] { "dispositionfile" })]
[TypeFilter(typeof(NullJsonResultFilter))]
public IActionResult AddDispositionFileSale([FromRoute] long id, [FromBody] DispositionFileSaleModel dispositionFileSale)
{
_logger.LogInformation(
"Request received by Controller: {Controller}, Action: {ControllerAction}, User: {User}, DateTime: {DateTime}",
nameof(DispositionFileController),
nameof(AddDispositionFileSale),
User.GetUsername(),
DateTime.Now);

_logger.LogInformation("Dispatching to service: {Service}", _dispositionService.GetType());

try
{
if (id != dispositionFileSale.DispositionFileId)
{
throw new BadRequestException("Invalid dispositionFileId.");

Check warning on line 371 in source/backend/api/Areas/Disposition/Controllers/DispositionFileController.cs

View check run for this annotation

Codecov / codecov/patch

source/backend/api/Areas/Disposition/Controllers/DispositionFileController.cs#L370-L371

Added lines #L370 - L371 were not covered by tests
}

var dispositionSaleEntity = _mapper.Map<Dal.Entities.PimsDispositionSale>(dispositionFileSale);
var newDispositionSale = _dispositionService.AddDispositionFileSale(dispositionSaleEntity);

return new JsonResult(_mapper.Map<DispositionFileSaleModel>(newDispositionSale));
}
catch (DuplicateEntityException e)
{
return Conflict(e.Message);

Check warning on line 381 in source/backend/api/Areas/Disposition/Controllers/DispositionFileController.cs

View check run for this annotation

Codecov / codecov/patch

source/backend/api/Areas/Disposition/Controllers/DispositionFileController.cs#L379-L381

Added lines #L379 - L381 were not covered by tests
}
}

[HttpPut("{id:long}/sale/{saleId:long}")]
[HasPermission(Permissions.DispositionEdit)]
[Produces("application/json")]
[ProducesResponseType(typeof(DispositionFileSaleModel), 200)]
[SwaggerOperation(Tags = new[] { "dispositionfile" })]
[TypeFilter(typeof(NullJsonResultFilter))]
public IActionResult UpdateDispositionFileSale([FromRoute]long id, [FromRoute]long saleId, [FromBody] DispositionFileSaleModel dispositionFileSale)
{
_logger.LogInformation(
"Request received by Controller: {Controller}, Action: {ControllerAction}, User: {User}, DateTime: {DateTime}",
nameof(DispositionFileController),
nameof(UpdateDispositionFileSale),
User.GetUsername(),
DateTime.Now);

_logger.LogInformation("Dispatching to service: {Service}", _dispositionService.GetType());

if (id != dispositionFileSale.DispositionFileId || dispositionFileSale.Id != saleId)
{
throw new BadRequestException("Invalid dispositionFileId.");

Check warning on line 404 in source/backend/api/Areas/Disposition/Controllers/DispositionFileController.cs

View check run for this annotation

Codecov / codecov/patch

source/backend/api/Areas/Disposition/Controllers/DispositionFileController.cs#L403-L404

Added lines #L403 - L404 were not covered by tests
}

var dispositionSaleEntity = _mapper.Map<Dal.Entities.PimsDispositionSale>(dispositionFileSale);
var updatedSale = _dispositionService.UpdateDispositionFileSale(dispositionSaleEntity);

return new JsonResult(_mapper.Map<DispositionFileSaleModel>(updatedSale));
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding unit tests for controllers is pretty quick, can you please add the minimal amount to allow for test coverage?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

[HttpGet("{id:long}/appraisal")]
[HasPermission(Permissions.DispositionView)]
[Produces("application/json")]
Expand Down
32 changes: 30 additions & 2 deletions source/backend/api/Services/DispositionFileService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public PimsDispositionFile Update(long id, PimsDispositionFile dispositionFile,
if (!userOverrides.Contains(UserOverrideCode.DispositionFileFinalStatus))
{
var doNotAddToStatuses = new List<string>() { EnumDispositionFileStatusTypeCode.COMPLETE.ToString(), EnumDispositionFileStatusTypeCode.ARCHIVED.ToString() };
if(doNotAddToStatuses.Contains(dispositionFile.DispositionFileStatusTypeCode))
if (doNotAddToStatuses.Contains(dispositionFile.DispositionFileStatusTypeCode))
{
throw new UserOverrideException(UserOverrideCode.DispositionFileFinalStatus, "You are changing this file to a non-editable state. Only system administrators can edit the file when set to Archived, Cancelled or Completed state). Do you wish to continue?");
}
Expand Down Expand Up @@ -246,6 +246,34 @@ public PimsDispositionSale GetDispositionFileSale(long dispositionFileId)
return _dispositionFileRepository.GetDispositionFileSale(dispositionFileId);
}

public PimsDispositionSale UpdateDispositionFileSale(PimsDispositionSale dispositionSale)
{
_logger.LogInformation("Updating disposition file Sale with DispositionFileId: {id}", dispositionSale.DispositionSaleId);
_user.ThrowIfNotAuthorized(Permissions.DispositionEdit);

var updatedSale = _dispositionFileRepository.UpdateDispositionFileSale(dispositionSale);
_dispositionFileRepository.CommitTransaction();

return updatedSale;
}

public PimsDispositionSale AddDispositionFileSale(PimsDispositionSale dispositionSale)
{
_logger.LogInformation("Adding disposition file Sale to Disposition File with Id: {id}", dispositionSale.DispositionFileId);
_user.ThrowIfNotAuthorized(Permissions.DispositionEdit);

var dispositionFileParent = _dispositionFileRepository.GetById(dispositionSale.DispositionFileId);
if (dispositionFileParent.PimsDispositionSales.Count > 0)
{
throw new DuplicateEntityException("Invalid Disposition Sale. A Sale has been already created for this Disposition File");
}

_dispositionFileRepository.AddDispositionFileSale(dispositionSale);
_dispositionFileRepository.CommitTransaction();

return dispositionSale;
}

public PimsDispositionAppraisal GetDispositionFileAppraisal(long dispositionFileId)
{
_logger.LogInformation("Getting disposition file appraisal with DispositionFileId: {id}", dispositionFileId);
Expand All @@ -265,7 +293,7 @@ public PimsDispositionAppraisal AddDispositionFileAppraisal(long dispositionFile
throw new BadRequestException("Invalid dispositionFileId.");
}

if(dispositionFileParent.PimsDispositionAppraisals.Count > 0)
if (dispositionFileParent.PimsDispositionAppraisals.Count > 0)
{
throw new DuplicateEntityException("Invalid Disposition Appraisal. An Appraisal has been already created for this Disposition File");
}
Expand Down
6 changes: 5 additions & 1 deletion source/backend/api/Services/IDispositionFileService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,15 @@ public interface IDispositionFileService

PimsDispositionSale GetDispositionFileSale(long dispositionFileId);

PimsDispositionSale AddDispositionFileSale(PimsDispositionSale dispositionSale);

PimsDispositionSale UpdateDispositionFileSale(PimsDispositionSale dispositionSale);

PimsDispositionAppraisal GetDispositionFileAppraisal(long dispositionFileId);

PimsDispositionAppraisal AddDispositionFileAppraisal(long dispositionFileId, PimsDispositionAppraisal dispositionAppraisal);

PimsDispositionAppraisal UpdateDispositionFileAppraisal(long dispositionFileId,long appraisalId, PimsDispositionAppraisal dispositionAppraisal);
PimsDispositionAppraisal UpdateDispositionFileAppraisal(long dispositionFileId, long appraisalId, PimsDispositionAppraisal dispositionAppraisal);

IEnumerable<PimsDispositionChecklistItem> GetChecklistItems(long id);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
using System.Collections.Generic;
using System.Linq;
using Mapster;
using Pims.Api.Models.Concepts.DispositionFile;
using Pims.Dal.Entities;
using Entity = Pims.Dal.Entities;

namespace Pims.Api.Models.Concepts.DispositionFile
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
using System.Collections.Generic;
using Pims.Api.Models.Base;
using Pims.Api.Models.Concepts.File;
using Pims.Api.Models.Concepts.DispositionFile;

/*
* Frontend model
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using System.Collections.Generic;
using System.Linq;
using Mapster;
using Entity = Pims.Dal.Entities;

Expand All @@ -22,8 +24,8 @@ public void Register(TypeAdapterConfig config)
.Map(dest => dest.SppAmount, src => src.SppAmt)
.Map(dest => dest.RemediationAmount, src => src.RemediationAmt)
.Map(dest => dest.DispositionPurchasers, src => src.PimsDispositionPurchasers)
.Map(dest => dest.DispositionPurchaserAgents, src => src.PimsDspPurchAgents)
.Map(dest => dest.DispositionPurchaserSolicitors, src => src.PimsDspPurchSolicitors);
.Map(dest => dest.DispositionPurchaserAgent, src => src.PimsDspPurchAgents.FirstOrDefault())
.Map(dest => dest.DispositionPurchaserSolicitor, src => src.PimsDspPurchSolicitors.FirstOrDefault());

config.NewConfig<DispositionFileSaleModel, Entity.PimsDispositionSale>()
.Map(dest => dest.DispositionSaleId, src => src.Id)
Expand All @@ -40,8 +42,8 @@ public void Register(TypeAdapterConfig config)
.Map(dest => dest.SppAmt, src => src.SppAmount)
.Map(dest => dest.RemediationAmt, src => src.RemediationAmount)
.Map(dest => dest.PimsDispositionPurchasers, src => src.DispositionPurchasers)
.Map(dest => dest.PimsDspPurchAgents, src => src.DispositionPurchaserAgents)
.Map(dest => dest.PimsDspPurchSolicitors, src => src.DispositionPurchaserSolicitors);
.Map(dest => dest.PimsDspPurchAgents, src => src.DispositionPurchaserAgent == null ? null : new List<DispositionSalePurchaserAgentModel> { src.DispositionPurchaserAgent })
.Map(dest => dest.PimsDspPurchSolicitors, src => src.DispositionPurchaserSolicitor == null ? null : new List<DispositionSalePurchaserSolicitorModel> { src.DispositionPurchaserSolicitor });
Comment on lines +45 to +46
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks odd. why would the ef entities be expecting a list but the model only contain one?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Entities per the scaffolding are one to many, (which is not true) the relationship should be 1 to 1.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, thats not good. If we need db changes does Dough know?

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,16 +61,6 @@ public class DispositionFileSaleModel : BaseConcurrentModel
/// </summary>
public decimal? TotalCostAmount { get; set; }

/// <summary>
/// Disposition Sale Net Proceeds Before Spp Amount.
/// </summary>
public decimal? NetProceedsBeforeSppAmount { get; set; }

/// <summary>
/// Disposition Sale Net Proceeds After Spp Amount.
/// </summary>
public decimal? NetProceedsAfterSppAmount { get; set; }

/// <summary>
/// Disposition Sale Spp Amount.
/// </summary>
Expand All @@ -87,13 +77,13 @@ public class DispositionFileSaleModel : BaseConcurrentModel
public IList<DispositionSalePurchaserModel> DispositionPurchasers { get; set; }

/// <summary>
/// get/set - A list of disposition Sale Purchaser(s) Agents.
/// get/set - Disposition Sale Purchaser(s)'s Agents.
/// </summary>
public IList<DispositionSalePurchaserAgentModel> DispositionPurchaserAgents { get; set; }
public DispositionSalePurchaserAgentModel DispositionPurchaserAgent { get; set; }

/// <summary>
/// get/set - A list of disposition Sale Purchaser(s) Solicitors.
/// get/set - Disposition Sale Purchase Solicitor.
/// </summary>
public IList<DispositionSalePurchaserSolicitorModel> DispositionPurchaserSolicitors { get; set; }
public DispositionSalePurchaserSolicitorModel DispositionPurchaserSolicitor { get; set; }
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
using Mapster;
using Pims.Api.Models.Base;
using Pims.Api.Models.Concepts.DispositionFile;
using Entity = Pims.Dal.Entities;

namespace Pims.Api.Models.Concepts.DispositionFile
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
using Mapster;
using Pims.Api.Models.Base;
using Pims.Api.Models.Concepts.DispositionFile;
using Entity = Pims.Dal.Entities;

namespace Pims.Api.Models.Concepts.DispositionFile
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
using Mapster;
using Pims.Api.Models.Base;
using Pims.Api.Models.Concepts.DispositionFile;
using Entity = Pims.Dal.Entities;

namespace Pims.Api.Models.Concepts.DispositionFile
Expand Down
20 changes: 20 additions & 0 deletions source/backend/dal/Repositories/DispositionFileRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,26 @@ public PimsDispositionSale GetDispositionFileSale(long dispositionId)
.Where(x => x.DispositionFileId == dispositionId).FirstOrDefault();
}

public PimsDispositionSale AddDispositionFileSale(PimsDispositionSale dispositionSale)
{
Context.PimsDispositionSales.Add(dispositionSale);

return dispositionSale;
}

public PimsDispositionSale UpdateDispositionFileSale(PimsDispositionSale dispositionSale)
{
var existingSale = Context.PimsDispositionSales
.FirstOrDefault(x => x.DispositionSaleId.Equals(dispositionSale.DispositionSaleId)) ?? throw new KeyNotFoundException();

Context.Entry(existingSale).CurrentValues.SetValues(dispositionSale);
Context.UpdateChild<PimsDispositionSale, long, PimsDispositionPurchaser, long>(p => p.PimsDispositionPurchasers, dispositionSale.Internal_Id, dispositionSale.PimsDispositionPurchasers.ToArray());
Context.UpdateChild<PimsDispositionSale, long, PimsDspPurchAgent, long>(p => p.PimsDspPurchAgents, dispositionSale.Internal_Id, dispositionSale.PimsDspPurchAgents.ToArray());
Context.UpdateChild<PimsDispositionSale, long, PimsDspPurchSolicitor, long>(p => p.PimsDspPurchSolicitors, dispositionSale.Internal_Id, dispositionSale.PimsDspPurchSolicitors.ToArray());

return existingSale;
}

public PimsDispositionAppraisal GetDispositionFileAppraisal(long dispositionId)
{
return Context.PimsDispositionAppraisals.AsNoTracking()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ public interface IDispositionFileRepository : IRepository

PimsDispositionSale GetDispositionFileSale(long dispositionId);

PimsDispositionSale AddDispositionFileSale(PimsDispositionSale dispositionSale);

PimsDispositionSale UpdateDispositionFileSale(PimsDispositionSale dispositionSale);

PimsDispositionAppraisal GetDispositionFileAppraisal(long dispositionId);

PimsDispositionAppraisal AddDispositionFileAppraisal(PimsDispositionAppraisal dispositionAppraisal);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,47 @@ public void UpdateDispositionFile_Success()
// Assert
this._service.Verify(m => m.Update(It.IsAny<long>(), It.IsAny<PimsDispositionFile>(), It.IsAny<IEnumerable<UserOverrideCode>>()), Times.Once());
}

/// <summary>
/// Make a successful request to POST a disposition file Sale to the Disposition File.
/// </summary>
[Fact]
public void AddDispositionFileSale_Success()
{
// Arrange
var dispFileSale = new PimsDispositionSale();
dispFileSale.DispositionFileId = 1;

this._service.Setup(m => m.AddDispositionFileSale(It.IsAny<PimsDispositionSale>())).Returns(dispFileSale);

// Act
var model = _mapper.Map<DispositionFileSaleModel>(dispFileSale);
var result = this._controller.AddDispositionFileSale(1, model);

// Assert
this._service.Verify(m => m.AddDispositionFileSale(It.IsAny<PimsDispositionSale>()), Times.Once());
}

/// <summary>
/// Make a successful request to PUT a disposition file Sale.
/// </summary>
[Fact]
public void UpdateDispositionFileSale_Success()
{
// Arrange
var dispFileSale = new PimsDispositionSale();
dispFileSale.DispositionFileId = 1;
dispFileSale.DispositionSaleId = 10;

this._service.Setup(m => m.UpdateDispositionFileSale(It.IsAny<PimsDispositionSale>())).Returns(dispFileSale);

// Act
var model = _mapper.Map<DispositionFileSaleModel>(dispFileSale);
var result = this._controller.UpdateDispositionFileSale(1, 10, model);

// Assert
this._service.Verify(m => m.UpdateDispositionFileSale(It.IsAny<PimsDispositionSale>()), Times.Once());
}
#endregion
}
}
Loading
Loading