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-7618 : Re-add project/product to Disposition Files (Create, Update and View File Summary) #3777

Merged
merged 14 commits into from
Feb 8, 2024
1 change: 1 addition & 0 deletions source/backend/api/Services/DispositionFileService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ public PimsDispositionSale UpdateDispositionFileSale(PimsDispositionSale disposi
_user.ThrowIfNotAuthorized(Permissions.DispositionEdit);

var updatedSale = _dispositionFileRepository.UpdateDispositionFileSale(dispositionSale);

_dispositionFileRepository.CommitTransaction();

return updatedSale;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ public void Register(TypeAdapterConfig config)
.Map(dest => dest.AssignedDate, src => src.AssignedDt)
.Map(dest => dest.InitiatingDocumentDate, src => src.InitiatingDocumentDt)
.Map(dest => dest.CompletionDate, src => src.CompletedDt)
.Map(dest => dest.ProjectId, src => src.ProjectId)
.Map(dest => dest.Project, src => src.Project)
.Map(dest => dest.ProductId, src => src.ProductId)
.Map(dest => dest.Product, src => src.Product)
.Map(dest => dest.FundingTypeCode, src => src.DispositionFundingTypeCodeNavigation)
.Map(dest => dest.FileStatusTypeCode, src => src.DispositionFileStatusTypeCodeNavigation)
.Map(dest => dest.DispositionStatusTypeCode, src => src.DispositionStatusTypeCodeNavigation)
Expand All @@ -43,6 +47,8 @@ public void Register(TypeAdapterConfig config)
.Map(dest => dest.AssignedDt, src => src.AssignedDate)
.Map(dest => dest.InitiatingDocumentDt, src => src.InitiatingDocumentDate)
.Map(dest => dest.CompletedDt, src => src.CompletionDate)
.Map(dest => dest.ProjectId, src => src.ProjectId)
.Map(dest => dest.ProductId, src => src.ProductId)
.Map(dest => dest.DispositionFundingTypeCode, src => src.FundingTypeCode.Id)
.Map(dest => dest.DispositionFileStatusTypeCode, src => src.FileStatusTypeCode.Id)
.Map(dest => dest.DspPhysFileStatusTypeCode, src => src.PhysicalFileStatusTypeCode.Id)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
using System.Collections.Generic;
using Pims.Api.Models.Base;
using Pims.Api.Models.Concepts.File;
using Pims.Api.Models.Concepts.Product;
using Pims.Api.Models.Concepts.Project;

/*
* Frontend model
Expand Down Expand Up @@ -53,6 +55,26 @@ public class DispositionFileModel : FileWithChecklistModel
/// </summary>
public CodeTypeModel<string> PhysicalFileStatusTypeCode { get; set; }

/// <summary>
/// get/set - The project's id.
/// </summary>
public long? ProjectId { get; set; }

/// <summary>
/// get/set - The acquisition project.
Copy link
Collaborator

Choose a reason for hiding this comment

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

acquisition -> disposition

/// </summary>
public ProjectModel Project { get; set; }

/// <summary>
/// get/set - The product's id.
/// </summary>
public long? ProductId { get; set; }

/// <summary>
/// get/set - The product.
/// </summary>
public ProductModel Product { get; set; }

/// <summary>
/// get/set - The funding type.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
using System.Collections.Generic;
using System.Linq;
using Mapster;
using Entity = Pims.Dal.Entities;

Expand All @@ -23,9 +21,11 @@ public void Register(TypeAdapterConfig config)
.Map(dest => dest.TotalCostAmount, src => src.TotalCostAmt)
.Map(dest => dest.SppAmount, src => src.SppAmt)
.Map(dest => dest.RemediationAmount, src => src.RemediationAmt)
.Map(dest => dest.DispositionPurchasers, src => src.PimsDispositionPurchasers);
// .Map(dest => dest.DispositionPurchaserAgent, src => src.PimsDspPurchAgents.FirstOrDefault()) TODO: Fix mapping
// .Map(dest => dest.DispositionPurchaserSolicitor, src => src.PimsDspPurchSolicitors.FirstOrDefault());
.Map(dest => dest.DispositionPurchasers, src => src.PimsDispositionPurchasers)
.Map(dest => dest.PurchaserAgentId, src => src.DspPurchAgentId)
.Map(dest => dest.DispositionPurchaserAgent, src => src.DspPurchAgent)
.Map(dest => dest.PurchaserSolicitorId, src => src.DspPurchSolicitorId)
.Map(dest => dest.DispositionPurchaserSolicitor, src => src.DspPurchSolicitor);

config.NewConfig<DispositionFileSaleModel, Entity.PimsDispositionSale>()
.Map(dest => dest.DispositionSaleId, src => src.Id)
Expand All @@ -41,9 +41,11 @@ public void Register(TypeAdapterConfig config)
.Map(dest => dest.TotalCostAmt, src => src.TotalCostAmount)
.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.DispositionPurchaserAgent == null ? null : new List<DispositionSalePurchaserAgentModel> { src.DispositionPurchaserAgent }) TODO: Fix mapping
// .Map(dest => dest.PimsDspPurchSolicitors, src => src.DispositionPurchaserSolicitor == null ? null : new List<DispositionSalePurchaserSolicitorModel> { src.DispositionPurchaserSolicitor });
.Map(dest => dest.PimsDispositionPurchasers, src => src.DispositionPurchasers)
.Map(dest => dest.DspPurchAgentId, src => src.PurchaserAgentId)
.Map(dest => dest.DspPurchAgent, src => src.DispositionPurchaserAgent)
.Map(dest => dest.DspPurchSolicitorId, src => src.PurchaserSolicitorId)
.Map(dest => dest.DspPurchSolicitor, src => src.DispositionPurchaserSolicitor);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,16 @@ public class DispositionFileSaleModel : BaseConcurrentModel
/// </summary>
public decimal? RemediationAmount { get; set; }

/// <summary>
/// Purchaser Agent FK.
/// </summary>
public long? PurchaserAgentId { get; set; }

/// <summary>
/// Purchaser Solicitor FK.
/// </summary>
public long? PurchaserSolicitorId { get; set; }

/// <summary>
/// get/set - A list of disposition Sale Purchaser(s).
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ public void Register(TypeAdapterConfig config)
{
config.NewConfig<Entity.PimsDspPurchAgent, DispositionSalePurchaserAgentModel>()
.Map(dest => dest.Id, src => src.DspPurchAgentId)
// .Map(dest => dest.DispositionSaleId, src => src.DispositionSaleId) TODO: Fix mapping
.Map(dest => dest.Person, src => src.Person)
.Map(dest => dest.PersonId, src => src.PersonId)
.Map(dest => dest.Organization, src => src.Organization)
Expand All @@ -21,7 +20,6 @@ public void Register(TypeAdapterConfig config)

config.NewConfig<DispositionSalePurchaserAgentModel, Entity.PimsDspPurchAgent>()
.Map(dest => dest.DspPurchAgentId, src => src.Id)
// .Map(dest => dest.DispositionSaleId, src => src.DispositionSaleId)
.Map(dest => dest.PersonId, src => src.PersonId)
.Map(dest => dest.OrganizationId, src => src.OrganizationId)
.Map(dest => dest.PrimaryContactId, src => src.PrimaryContactId)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,6 @@ public class DispositionSalePurchaserAgentModel : BaseConcurrentModel
/// </summary>
public long Id { get; set; }

/// <summary>
/// Parent Disposition Sale.
/// </summary>
public long DispositionSaleId { get; set; }

/// <summary>
/// get/set - The Id of the person(s) associated with a disposition Sale as Purchaser Agent.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ public void Register(TypeAdapterConfig config)
{
config.NewConfig<Entity.PimsDspPurchSolicitor, DispositionSalePurchaserSolicitorModel>()
.Map(dest => dest.Id, src => src.DspPurchSolicitorId)
// .Map(dest => dest.DispositionSaleId, src => src.DispositionSaleId) TODO: fix mapping
.Map(dest => dest.Person, src => src.Person)
.Map(dest => dest.PersonId, src => src.PersonId)
.Map(dest => dest.Organization, src => src.Organization)
Expand All @@ -21,7 +20,6 @@ public void Register(TypeAdapterConfig config)

config.NewConfig<DispositionSalePurchaserSolicitorModel, Entity.PimsDspPurchSolicitor>()
.Map(dest => dest.DspPurchSolicitorId, src => src.Id)
// .Map(dest => dest.DispositionSaleId, src => src.DispositionSaleId)
.Map(dest => dest.PersonId, src => src.PersonId)
.Map(dest => dest.OrganizationId, src => src.OrganizationId)
.Map(dest => dest.PrimaryContactId, src => src.PrimaryContactId)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,6 @@ public class DispositionSalePurchaserSolicitorModel : BaseConcurrentModel
/// </summary>
public long Id { get; set; }

/// <summary>
/// Parent Disposition Sale.
/// </summary>
public long DispositionSaleId { get; set; }

/// <summary>
/// get/set - The Id of the person(s) associated with a disposition Sale as Purchaser.
/// </summary>
Expand Down
1 change: 1 addition & 0 deletions source/backend/dal/Repositories/BaseRepository.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Security.Claims;
using Microsoft.EntityFrameworkCore.Storage;
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 unused to me (remove)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed

using Microsoft.Extensions.Logging;

namespace Pims.Dal.Repositories
Expand Down
40 changes: 38 additions & 2 deletions source/backend/dal/Repositories/DispositionFileRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@

return this.Context.PimsDispositionFiles.AsNoTracking()
.Include(d => d.DispositionFileStatusTypeCodeNavigation)
.Include(d => d.Project)
.Include(d => d.Product)
.Include(d => d.DispositionFundingTypeCodeNavigation)
.Include(d => d.DispositionInitiatingDocTypeCodeNavigation)
.Include(d => d.DispositionStatusTypeCodeNavigation)
Expand Down Expand Up @@ -344,12 +346,46 @@
public PimsDispositionSale UpdateDispositionFileSale(PimsDispositionSale dispositionSale)
{
var existingSale = Context.PimsDispositionSales
.Include(x => x.DspPurchAgent)
.Include(x => x.DspPurchSolicitor)
.FirstOrDefault(x => x.DispositionSaleId.Equals(dispositionSale.DispositionSaleId)) ?? throw new KeyNotFoundException();

if (existingSale.DspPurchAgent != null && dispositionSale.DspPurchAgent == null)
{
Context.Remove(existingSale.DspPurchAgent);
dispositionSale.DspPurchAgentId = null;
}

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

View check run for this annotation

Codecov / codecov/patch

source/backend/dal/Repositories/DispositionFileRepository.cs#L354-L357

Added lines #L354 - L357 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.

Test coverage on these conditionals is a bit spotty, can you add some tests for this?

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

else if (existingSale.DspPurchAgent != null && dispositionSale.DspPurchAgentId.HasValue && dispositionSale.DspPurchAgent != null)
{
Context.Entry(existingSale.DspPurchAgent).CurrentValues.SetValues(dispositionSale.DspPurchAgent);
}

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

View check run for this annotation

Codecov / codecov/patch

source/backend/dal/Repositories/DispositionFileRepository.cs#L359-L361

Added lines #L359 - L361 were not covered by tests
else if (existingSale.DspPurchAgent == null && dispositionSale.DspPurchAgent != null)
{
Context.PimsDspPurchAgents.Add(dispositionSale.DspPurchAgent);
Context.SaveChanges();

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

View check run for this annotation

Codecov / codecov/patch

source/backend/dal/Repositories/DispositionFileRepository.cs#L363-L365

Added lines #L363 - L365 were not covered by tests

dispositionSale.DspPurchAgentId = dispositionSale.DspPurchAgent.DspPurchAgentId;
}

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

View check run for this annotation

Codecov / codecov/patch

source/backend/dal/Repositories/DispositionFileRepository.cs#L367-L368

Added lines #L367 - L368 were not covered by tests

if (existingSale.DspPurchSolicitor != null && dispositionSale.DspPurchSolicitor == null)
{
Context.Remove(existingSale.DspPurchSolicitor);
dispositionSale.DspPurchSolicitorId = null;
}

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

View check run for this annotation

Codecov / codecov/patch

source/backend/dal/Repositories/DispositionFileRepository.cs#L371-L374

Added lines #L371 - L374 were not covered by tests
else if (existingSale.DspPurchSolicitor != null && dispositionSale.DspPurchSolicitorId.HasValue && dispositionSale.DspPurchSolicitor != null)
{
Context.Entry(existingSale.DspPurchSolicitor).CurrentValues.SetValues(dispositionSale.DspPurchSolicitor);
}

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

View check run for this annotation

Codecov / codecov/patch

source/backend/dal/Repositories/DispositionFileRepository.cs#L376-L378

Added lines #L376 - L378 were not covered by tests
else if (existingSale.DspPurchSolicitor == null && dispositionSale.DspPurchSolicitor != null)
{
Context.PimsDspPurchSolicitors.Add(dispositionSale.DspPurchSolicitor);
Context.SaveChanges();

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

View check run for this annotation

Codecov / codecov/patch

source/backend/dal/Repositories/DispositionFileRepository.cs#L380-L382

Added lines #L380 - L382 were not covered by tests

dispositionSale.DspPurchSolicitorId = dispositionSale.DspPurchSolicitor.DspPurchSolicitorId;
}

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

View check run for this annotation

Codecov / codecov/patch

source/backend/dal/Repositories/DispositionFileRepository.cs#L384-L385

Added lines #L384 - L385 were not covered by tests

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()); TODO: Fix agent update
//Context.UpdateChild<PimsDispositionSale, long, PimsDspPurchSolicitor, long>(p => p.PimsDspPurchSolicitors, dispositionSale.Internal_Id, dispositionSale.PimsDspPurchSolicitors.ToArray());

return existingSale;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@ namespace Pims.Dal.Entities
public partial class PimsDspPurchSolicitor : StandardIdentityBaseAppEntity<long>, IBaseAppEntity
{
[NotMapped]
public override long Internal_Id { get => this.DspPurchSolicitorId; set => this.DspPurchSolicitorId = value; }
public override long Internal_Id { get => DspPurchSolicitorId; set => DspPurchSolicitorId = value; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ describe('Disposition List View', () => {
});

it('matches snapshot', async () => {
const results = mockPagedResults([mockDispositionFileResponse()]);
const results = mockPagedResults([
mockDispositionFileResponse() as unknown as Api_DispositionFile,
]);
getDispositionFilesPagedApiFn.mockResolvedValue(results);
const { asFragment } = setup();
await waitForElementToBeRemoved(screen.getByTitle('table-loading'));
Expand All @@ -88,7 +90,9 @@ describe('Disposition List View', () => {
await waitForElementToBeRemoved(screen.getByTitle('table-loading'));
expect(await screen.queryByText(/test disposition/i)).toBeNull();

results = mockPagedResults([mockDispositionFileResponse(1, 'test disposition')]);
results = mockPagedResults([
mockDispositionFileResponse(1, 'test disposition') as unknown as Api_DispositionFile,
]);
getDispositionFilesPagedApiFn.mockResolvedValue(results);

const input = getByName('fileNameOrNumberOrReference');
Expand All @@ -108,7 +112,7 @@ describe('Disposition List View', () => {
it('searches by pid', async () => {
let results = mockPagedResults([
{
...mockDispositionFileResponse(),
...(mockDispositionFileResponse() as unknown as Api_DispositionFile),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm having trouble understanding the need for this, if the entire point of this mock is to create a disposition file, why are you needing to cast this to an Api_DispositionFile via an unknown?

Copy link
Collaborator Author

@eddherrera eddherrera Feb 7, 2024

Choose a reason for hiding this comment

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

I am casting from the new generated models ApiGen_Concepts_XXX I am limiting the use of the new models scoped to what I am currently working and not trying to spill over everywhere. The story to do the removal of old models would just need to remove the casting.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah ok. I see. That is fine for now then.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, can you log a follow-up task for is74 that would cover correcting this once we merge 5.1

That way we don't forget.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Task created.

fileProperties: [
{
id: 12,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ import {
jest.mock('@react-keycloak/web');

const setSort = jest.fn();
const mockResults: Api_DispositionFile[] = [mockDispositionFileResponse(1, 'test disposition')];
const mockResults: Api_DispositionFile[] = [
mockDispositionFileResponse(1, 'test disposition') as unknown as Api_DispositionFile,
];

describe('Disposition search results table', () => {
const getTableRows = () => document.querySelectorAll('.table .tbody .tr-wrapper');
Expand Down Expand Up @@ -77,7 +79,7 @@ describe('Disposition search results table', () => {
setup({
results: [
DispositionSearchResultModel.fromApi({
...mockDispositionFileResponse(),
...(mockDispositionFileResponse() as unknown as Api_DispositionFile),
fileProperties: [
{
id: 100,
Expand All @@ -104,14 +106,14 @@ describe('Disposition search results table', () => {

const addresses = screen.getAllByText('1234 mock Street', { exact: false });
expect(addresses).toHaveLength(2);
expect(screen.getByText('[+1 more...]')).toBeVisible();
expect(screen.getAllByText('[+1 more...]')).toHaveLength(2);
});

it('displays a team member organization', () => {
setup({
results: [
DispositionSearchResultModel.fromApi({
...mockDispositionFileResponse(),
...(mockDispositionFileResponse() as unknown as Api_DispositionFile),
dispositionTeam: [
{
id: 1,
Expand Down Expand Up @@ -147,7 +149,7 @@ describe('Disposition search results table', () => {
setup({
results: [
DispositionSearchResultModel.fromApi({
...mockDispositionFileResponse(),
...(mockDispositionFileResponse() as unknown as Api_DispositionFile),
dispositionTeam: [
{
id: 1,
Expand Down Expand Up @@ -178,7 +180,7 @@ describe('Disposition search results table', () => {
setup({
results: [
DispositionSearchResultModel.fromApi({
...mockDispositionFileResponse(),
...(mockDispositionFileResponse() as unknown as Api_DispositionFile),
dispositionTeam: [
{
id: 1,
Expand Down
Loading
Loading