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-7295 : Create/Edit Appraisal and Assessment #3709

Merged
merged 9 commits into from
Jan 15, 2024
Merged
Show file tree
Hide file tree
Changes from 7 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 @@ -323,6 +323,80 @@
return new JsonResult(_mapper.Map<DispositionFileSaleModel>(dispositionSale));
}

[HttpGet("{id:long}/appraisal")]
[HasPermission(Permissions.DispositionView)]
[Produces("application/json")]
[ProducesResponseType(typeof(DispositionFileAppraisalModel), 200)]
[SwaggerOperation(Tags = new[] { "dispositionfile" })]
[TypeFilter(typeof(NullJsonResultFilter))]
public IActionResult GetDispositionFileAppraisal([FromRoute] long id)
{
_logger.LogInformation(
"Request received by Controller: {Controller}, Action: {ControllerAction}, User: {User}, DateTime: {DateTime}",
nameof(DispositionFileController),
nameof(GetDispositionFileAppraisal),
User.GetUsername(),
DateTime.Now);

Check warning on line 339 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#L333-L339

Added lines #L333 - L339 were not covered by tests

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

Check warning on line 341 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#L341

Added line #L341 was not covered by tests

var dispositionSale = _dispositionService.GetDispositionFileAppraisal(id);
return new JsonResult(_mapper.Map<DispositionFileAppraisalModel>(dispositionSale));
}

Check warning on line 345 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#L343-L345

Added lines #L343 - L345 were not covered by tests

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

Check warning on line 360 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#L354-L360

Added lines #L354 - L360 were not covered by tests

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

Check warning on line 362 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#L362

Added line #L362 was not covered by tests

try
{
var dispositionAppraisalEntity = _mapper.Map<Dal.Entities.PimsDispositionAppraisal>(dispositionFileAppraisal);
var newDispositionAppraisal = _dispositionService.AddDispositionFileAppraisal(id, dispositionAppraisalEntity);

Check warning on line 367 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#L365-L367

Added lines #L365 - L367 were not covered by tests

return new JsonResult(_mapper.Map<DispositionFileAppraisalModel>(newDispositionAppraisal));

Check warning on line 369 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#L369

Added line #L369 was not covered by tests
}
catch (DuplicateEntityException e)
{
return Conflict(e.Message);

Check warning on line 373 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#L371-L373

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

Check warning on line 375 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#L375

Added line #L375 was not covered by tests

[HttpPut("{id:long}/appraisal/{appraisalId:long}")]
[HasPermission(Permissions.DispositionEdit)]
[Produces("application/json")]
[ProducesResponseType(typeof(DispositionFileAppraisalModel), 200)]
[SwaggerOperation(Tags = new[] { "dispositionfile" })]
[TypeFilter(typeof(NullJsonResultFilter))]
public IActionResult UpdateDispositionFileAppraisal([FromRoute] long id, [FromRoute] long appraisalId, [FromBody] DispositionFileAppraisalModel dispositionFileAppraisal)
Copy link
Collaborator

Choose a reason for hiding this comment

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

To co-exist with our existing patterns I'd be tempted to remove the add endpoint entirely. In general when editing sub-entities we consider that an update. We only use post endpoints for first class entities like files or persons. sub-entities are all handled with puts, and that single put handles add/edit/delete (allthough in this case it'll just handle add/edit).

{
_logger.LogInformation(
"Request received by Controller: {Controller}, Action: {ControllerAction}, User: {User}, DateTime: {DateTime}",
nameof(DispositionFileController),
nameof(UpdateDispositionFileAppraisal),
User.GetUsername(),
DateTime.Now);

Check warning on line 390 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#L384-L390

Added lines #L384 - L390 were not covered by tests

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

Check warning on line 392 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#L392

Added line #L392 was not covered by tests

var dispositionAppraisalEntity = _mapper.Map<Dal.Entities.PimsDispositionAppraisal>(dispositionFileAppraisal);
var updatedOffer = _dispositionService.UpdateDispositionFileAppraisal(id, appraisalId, dispositionAppraisalEntity);

Check warning on line 395 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#L394-L395

Added lines #L394 - L395 were not covered by tests

return new JsonResult(_mapper.Map<DispositionFileAppraisalModel>(updatedOffer));
}

Check warning on line 398 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#L397-L398

Added lines #L397 - L398 were not covered by tests

#endregion
}
}
1 change: 0 additions & 1 deletion source/backend/api/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
using System.Diagnostics.CodeAnalysis;
using CommandLine;
using CommandLine.Text;
using Microsoft.AspNetCore;
using Microsoft.AspNetCore.Hosting;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.Hosting;
Expand Down
53 changes: 49 additions & 4 deletions source/backend/api/Services/DispositionFileService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,17 @@
using Microsoft.Extensions.Logging;
using Pims.Api.Constants;
using Pims.Api.Helpers.Exceptions;
using Pims.Core.Extensions;
using Pims.Api.Models.CodeTypes;
using Pims.Core.Exceptions;
using Pims.Core.Extensions;
using Pims.Dal.Constants;
using Pims.Dal.Entities;
using Pims.Dal.Entities.Extensions;
using Pims.Dal.Entities.Models;
using Pims.Dal.Exceptions;
using Pims.Dal.Helpers;
using Pims.Dal.Helpers.Extensions;
using Pims.Dal.Repositories;
using Pims.Dal.Security;
using Pims.Dal.Entities.Extensions;
using Pims.Api.Models.CodeTypes;

namespace Pims.Api.Services
{
Expand Down Expand Up @@ -209,6 +207,53 @@
return _dispositionFileRepository.GetDispositionFileSale(dispositionFileId);
}

public PimsDispositionAppraisal GetDispositionFileAppraisal(long dispositionFileId)
{
_logger.LogInformation("Getting disposition file appraisal with DispositionFileId: {id}", dispositionFileId);
_user.ThrowIfNotAuthorized(Permissions.DispositionView);

return _dispositionFileRepository.GetDispositionFileAppraisal(dispositionFileId);
}

Check warning on line 216 in source/backend/api/Services/DispositionFileService.cs

View check run for this annotation

Codecov / codecov/patch

source/backend/api/Services/DispositionFileService.cs#L215-L216

Added lines #L215 - L216 were not covered by tests

public PimsDispositionAppraisal AddDispositionFileAppraisal(long dispositionFileId, PimsDispositionAppraisal dispositionAppraisal)
Copy link
Collaborator

Choose a reason for hiding this comment

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

see other comment on add/update.

{
_logger.LogInformation("Adding disposition file offer to Disposition File with Id: {id}", dispositionFileId);
_user.ThrowIfNotAuthorized(Permissions.DispositionEdit);

var dispositionFileParent = _dispositionFileRepository.GetById(dispositionFileId);
if (dispositionFileId != dispositionAppraisal.DispositionFileId || dispositionFileParent is null)
{
throw new BadRequestException("Invalid dispositionFileId.");
}

if(dispositionFileParent.PimsDispositionAppraisals.Count > 0)
{
throw new DuplicateEntityException("Invalid Disposition Appraisal. An Appraisal has been already created for this Disposition File");
}

var newAppraisal = _dispositionFileRepository.AddDispositionFileAppraisal(dispositionAppraisal);
_dispositionFileRepository.CommitTransaction();

return newAppraisal;
}

public PimsDispositionAppraisal UpdateDispositionFileAppraisal(long dispositionFileId, long appraisalId, PimsDispositionAppraisal dispositionAppraisal)
{
_logger.LogInformation("Updating disposition file Appraisal with DispositionFileId: {id}", dispositionFileId);
_user.ThrowIfNotAuthorized(Permissions.DispositionEdit);

Check warning on line 243 in source/backend/api/Services/DispositionFileService.cs

View check run for this annotation

Codecov / codecov/patch

source/backend/api/Services/DispositionFileService.cs#L241-L243

Added lines #L241 - L243 were not covered by tests

var dispositionFileParent = _dispositionFileRepository.GetById(dispositionFileId);

Check warning on line 245 in source/backend/api/Services/DispositionFileService.cs

View check run for this annotation

Codecov / codecov/patch

source/backend/api/Services/DispositionFileService.cs#L245

Added line #L245 was not covered by tests
if (dispositionFileId != dispositionAppraisal.DispositionFileId || dispositionAppraisal.DispositionAppraisalId != appraisalId || dispositionFileParent is null)
{
throw new BadRequestException("Invalid dispositionFileId.");

Check warning on line 248 in source/backend/api/Services/DispositionFileService.cs

View check run for this annotation

Codecov / codecov/patch

source/backend/api/Services/DispositionFileService.cs#L247-L248

Added lines #L247 - L248 were not covered by tests
Copy link
Collaborator

Choose a reason for hiding this comment

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

normally we do this at the repository level in my experience.

}

var updatedAppraisal = _dispositionFileRepository.UpdateDispositionFileAppraisal(appraisalId, dispositionAppraisal);
_dispositionFileRepository.CommitTransaction();

Check warning on line 252 in source/backend/api/Services/DispositionFileService.cs

View check run for this annotation

Codecov / codecov/patch

source/backend/api/Services/DispositionFileService.cs#L251-L252

Added lines #L251 - L252 were not covered by tests

return updatedAppraisal;
}

Check warning on line 255 in source/backend/api/Services/DispositionFileService.cs

View check run for this annotation

Codecov / codecov/patch

source/backend/api/Services/DispositionFileService.cs#L254-L255

Added lines #L254 - L255 were not covered by tests

public IEnumerable<PimsDispositionChecklistItem> GetChecklistItems(long id)
{
_logger.LogInformation("Getting disposition file checklist with DispositionFile id: {id}", id);
Expand Down
7 changes: 6 additions & 1 deletion source/backend/api/Services/IDispositionFileService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
using Pims.Dal.Entities;
using Pims.Dal.Entities.Models;
using Pims.Dal.Exceptions;
using Pims.Dal.Security;

namespace Pims.Api.Services
{
Expand Down Expand Up @@ -32,6 +31,12 @@ public interface IDispositionFileService

PimsDispositionSale GetDispositionFileSale(long dispositionFileId);

PimsDispositionAppraisal GetDispositionFileAppraisal(long dispositionFileId);

PimsDispositionAppraisal AddDispositionFileAppraisal(long dispositionFileId, PimsDispositionAppraisal dispositionAppraisal);

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

IEnumerable<PimsDispositionChecklistItem> GetChecklistItems(long id);

List<DispositionFileExportModel> GetDispositionFileExport(DispositionFilter filter);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
using System.Collections.Generic;
using System.Linq;
using Mapster;
using Pims.Api.Models.Base;
using Entity = Pims.Dal.Entities;

namespace Pims.Api.Models.Concepts.DispositionFile
Expand All @@ -11,19 +10,23 @@ public void Register(TypeAdapterConfig config)
{
config.NewConfig<Entity.PimsDispositionAppraisal, DispositionFileAppraisalModel>()
.Map(dest => dest.Id, src => src.DispositionAppraisalId)
.Map(dest => dest.DispositionFileId, src => src.DispositionFileId)
.Map(dest => dest.AppraisedAmount, src => src.AppraisedAmt)
.Map(dest => dest.AppraisalDate, src => src.AppraisalDt)
.Map(dest => dest.BcaValueAmount, src => src.BcaValueAmt)
.Map(dest => dest.BcaRollYear, src => src.BcaRollYear)
.Map(dest => dest.ListPriceAmount, src => src.ListPriceAmt);
.Map(dest => dest.ListPriceAmount, src => src.ListPriceAmt)
.Inherits<Entity.IBaseEntity, BaseConcurrentModel>();

config.NewConfig<DispositionFileAppraisalModel, Entity.PimsDispositionAppraisal>()
.Map(dest => dest.DispositionAppraisalId, src => src.Id)
.Map(dest => dest.DispositionFileId, src => src.DispositionFileId)
.Map(dest => dest.AppraisedAmt, src => src.AppraisedAmount)
.Map(dest => dest.AppraisalDt, src => src.AppraisalDate)
.Map(dest => dest.BcaValueAmt, src => src.BcaValueAmount)
.Map(dest => dest.BcaRollYear, src => src.BcaRollYear)
.Map(dest => dest.ListPriceAmt, src => src.ListPriceAmount);
.Map(dest => dest.ListPriceAmt, src => src.ListPriceAmount)
.Inherits<BaseConcurrentModel, Entity.IBaseEntity>();
}
}
}
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
using System;
using System.Collections.Generic;
using Pims.Api.Models.Base;
using Pims.Api.Models.Concepts.File;
using Pims.Api.Models.Concepts.DispositionFile;

/*
* Frontend model
* LINK @frontend/src\models\api\DispositionFile.ts:14
*/
namespace Pims.Api.Models.Concepts.DispositionFile
{
public class DispositionFileAppraisalModel : FileModel
public class DispositionFileAppraisalModel : BaseConcurrentModel
{
#region Properties
/// <summary>
/// get/set - The relationship id.
/// </summary>
public long? Id { get; set; }

/// <summary>
/// Parent Disposition File.
/// </summary>
public long DispositionFileId { get; set; }

/// <summary>
/// PIMS_DISPOSITION_APPRAISAL => get/set - The Disposition Apprasided Value amount.
Expand All @@ -38,7 +39,5 @@ public class DispositionFileAppraisalModel : FileModel
/// PIMS_DISPOSITION_APPRAISAL => get/set - BCA list price amount.
/// </summary>
public decimal? ListPriceAmount { get; set; }

#endregion
}
}
62 changes: 42 additions & 20 deletions source/backend/dal/Repositories/DispositionFileRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,48 @@
.Where(x => x.DispositionFileId == dispositionId).FirstOrDefault();
}

public PimsDispositionAppraisal GetDispositionFileAppraisal(long dispositionId)
{
return Context.PimsDispositionAppraisals.AsNoTracking()
.Where(x => x.DispositionFileId == dispositionId).FirstOrDefault();
}

Check warning on line 301 in source/backend/dal/Repositories/DispositionFileRepository.cs

View check run for this annotation

Codecov / codecov/patch

source/backend/dal/Repositories/DispositionFileRepository.cs#L298-L301

Added lines #L298 - L301 were not covered by tests

public PimsDispositionAppraisal AddDispositionFileAppraisal(PimsDispositionAppraisal dispositionAppraisal)
{
Context.PimsDispositionAppraisals.Add(dispositionAppraisal);

Check warning on line 305 in source/backend/dal/Repositories/DispositionFileRepository.cs

View check run for this annotation

Codecov / codecov/patch

source/backend/dal/Repositories/DispositionFileRepository.cs#L304-L305

Added lines #L304 - L305 were not covered by tests

return dispositionAppraisal;
}

Check warning on line 308 in source/backend/dal/Repositories/DispositionFileRepository.cs

View check run for this annotation

Codecov / codecov/patch

source/backend/dal/Repositories/DispositionFileRepository.cs#L307-L308

Added lines #L307 - L308 were not covered by tests

public PimsDispositionAppraisal UpdateDispositionFileAppraisal(long id, PimsDispositionAppraisal dispositionAppraisal)
{

Check warning on line 311 in source/backend/dal/Repositories/DispositionFileRepository.cs

View check run for this annotation

Codecov / codecov/patch

source/backend/dal/Repositories/DispositionFileRepository.cs#L311

Added line #L311 was not covered by tests
var existingAppraisal = Context.PimsDispositionAppraisals
.FirstOrDefault(x => x.DispositionAppraisalId.Equals(id)) ?? throw new KeyNotFoundException();

Check warning on line 313 in source/backend/dal/Repositories/DispositionFileRepository.cs

View check run for this annotation

Codecov / codecov/patch

source/backend/dal/Repositories/DispositionFileRepository.cs#L313

Added line #L313 was not covered by tests

Context.Entry(existingAppraisal).CurrentValues.SetValues(dispositionAppraisal);

Check warning on line 315 in source/backend/dal/Repositories/DispositionFileRepository.cs

View check run for this annotation

Codecov / codecov/patch

source/backend/dal/Repositories/DispositionFileRepository.cs#L315

Added line #L315 was not covered by tests

return existingAppraisal;
}

Check warning on line 318 in source/backend/dal/Repositories/DispositionFileRepository.cs

View check run for this annotation

Codecov / codecov/patch

source/backend/dal/Repositories/DispositionFileRepository.cs#L317-L318

Added lines #L317 - L318 were not covered by tests

/// <summary>
/// Get Disposition Files for Export.
/// </summary>
/// <param name="filter"></param>
/// <returns></returns>
public List<PimsDispositionFile> GetDispositionFileExportDeep(DispositionFilter filter)
{
// RECOMMENDED - use a log scope to group all potential SQL statements generated by EF for this method call
using var scope = Logger.QueryScope();

filter.ThrowIfNull(nameof(filter));
if (!filter.IsValid())
{
throw new ArgumentException("Argument must have a valid filter", nameof(filter));

Check warning on line 333 in source/backend/dal/Repositories/DispositionFileRepository.cs

View check run for this annotation

Codecov / codecov/patch

source/backend/dal/Repositories/DispositionFileRepository.cs#L332-L333

Added lines #L332 - L333 were not covered by tests
}

return GetCommonDispositionFileQueryDeep(filter).ToList();
}

/// <summary>
/// Generate a Common IQueryable for Disposition Files.
/// </summary>
Expand Down Expand Up @@ -395,26 +437,6 @@

return query;
}

/// <summary>
/// Get Disposition Files for Export.
/// </summary>
/// <param name="filter"></param>
/// <returns></returns>
public List<PimsDispositionFile> GetDispositionFileExportDeep(DispositionFilter filter)
{
// RECOMMENDED - use a log scope to group all potential SQL statements generated by EF for this method call
using var scope = Logger.QueryScope();

filter.ThrowIfNull(nameof(filter));
if (!filter.IsValid())
{
throw new ArgumentException("Argument must have a valid filter", nameof(filter));
}

return GetCommonDispositionFileQueryDeep(filter).ToList();
}

#endregion
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ public interface IDispositionFileRepository : IRepository

PimsDispositionSale GetDispositionFileSale(long dispositionId);

PimsDispositionAppraisal GetDispositionFileAppraisal(long dispositionId);

PimsDispositionAppraisal AddDispositionFileAppraisal(PimsDispositionAppraisal dispositionAppraisal);

PimsDispositionAppraisal UpdateDispositionFileAppraisal(long id, PimsDispositionAppraisal dispositionAppraisal);

long GetRowVersion(long id);

List<PimsDispositionFile> GetDispositionFileExportDeep(DispositionFilter filter);
Expand Down
13 changes: 13 additions & 0 deletions source/backend/entities/Partials/DispositionFileAppraisal.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
using System.ComponentModel.DataAnnotations.Schema;

namespace Pims.Dal.Entities
{
/// <summary>
/// PimsDispositionAppraisal class, provides an entity for the datamodel.
/// </summary>
public partial class PimsDispositionAppraisal : StandardIdentityBaseAppEntity<long>, IBaseAppEntity
{
[NotMapped]
public override long Internal_Id { get => this.DispositionAppraisalId; set => this.DispositionAppraisalId = value; }
}
}
Loading
Loading