From ef713505f98441e80774aeecae5dc2a62f14c2e1 Mon Sep 17 00:00:00 2001 From: Yazeed Loonat Date: Wed, 13 Mar 2024 12:23:21 -0700 Subject: [PATCH] fix: limit requestedChangesUser in Listings response (#3921) * feat: connect up requsted changes user properly * fix: limit requestedChangesUser in Listings response #3889 * fix: update api service unit tests #3889 * fix: add details view unit test #3889 * fix: send id and name #3889 * fix: correct inport statement * fix: addressing comments #3889 * fix: cleanup swagger changes #3889 * fix: add missing return statement #3889 --------- Co-authored-by: Eric McGarry --- .../06_requested_user_updates/migration.sql | 2 + api/prisma/schema.prisma | 2 + api/src/dtos/listings/listing.dto.ts | 16 +- api/src/services/listing.service.ts | 12 +- api/src/utilities/requested-changes-user.ts | 16 ++ .../unit/services/listing.service.spec.ts | 138 +++++++++++++++++- shared-helpers/src/types/backend-swagger.ts | 3 +- .../listings-approval.spec.ts | 1 + .../sections/DetailNotes.tsx | 4 +- 9 files changed, 184 insertions(+), 10 deletions(-) create mode 100644 api/prisma/migrations/06_requested_user_updates/migration.sql create mode 100644 api/src/utilities/requested-changes-user.ts diff --git a/api/prisma/migrations/06_requested_user_updates/migration.sql b/api/prisma/migrations/06_requested_user_updates/migration.sql new file mode 100644 index 0000000000..e1d4bbebec --- /dev/null +++ b/api/prisma/migrations/06_requested_user_updates/migration.sql @@ -0,0 +1,2 @@ +-- AddForeignKey +ALTER TABLE "listings" ADD CONSTRAINT "listings_requested_changes_user_id_fkey" FOREIGN KEY ("requested_changes_user_id") REFERENCES "user_accounts"("id") ON DELETE NO ACTION ON UPDATE NO ACTION; diff --git a/api/prisma/schema.prisma b/api/prisma/schema.prisma index 7c59d76202..ba9261fcdc 100644 --- a/api/prisma/schema.prisma +++ b/api/prisma/schema.prisma @@ -567,6 +567,7 @@ model Listings { requestedChanges String? @map("requested_changes") requestedChangesDate DateTime? @default(dbgenerated("'1970-01-01 00:00:00-07'::timestamp with time zone")) @map("requested_changes_date") @db.Timestamptz(6) requestedChangesUserId String? @map("requested_changes_user_id") @db.Uuid + requestedChangesUser UserAccounts? @relation("requested_changes_user", fields: [requestedChangesUserId], references: [id], onDelete: NoAction, onUpdate: NoAction) @@index([jurisdictionId]) @@map("listings") @@ -797,6 +798,7 @@ model UserAccounts { jurisdictions Jurisdictions[] userPreferences UserPreferences? userRoles UserRoles? + requestedChangesListings Listings[] @relation("requested_changes_user") @@map("user_accounts") } diff --git a/api/src/dtos/listings/listing.dto.ts b/api/src/dtos/listings/listing.dto.ts index 307084d9d1..567e283bdc 100644 --- a/api/src/dtos/listings/listing.dto.ts +++ b/api/src/dtos/listings/listing.dto.ts @@ -34,6 +34,7 @@ import { UnitsSummary } from '../units/units-summary.dto'; import { IdDTO } from '../shared/id.dto'; import { listingUrlSlug } from '../../utilities/listing-url-slug'; import { User } from '../users/user.dto'; +import { requestedChangesUserMapper } from '../../utilities/requested-changes-user'; class Listing extends AbstractDTO { @Expose() @@ -546,9 +547,18 @@ class Listing extends AbstractDTO { @Expose() @ApiPropertyOptional() - @ValidateNested({ groups: [ValidationsGroupsEnum.default] }) - @Type(() => User) - requestedChangesUser?: User; + @IsString({ groups: [ValidationsGroupsEnum.default] }) + @Transform( + (obj: any) => { + return obj.obj.requestedChangesUser + ? requestedChangesUserMapper(obj.obj.requestedChangesUser as User) + : undefined; + }, + { + toClassOnly: true, + }, + ) + requestedChangesUser?: IdDTO; } export { Listing as default, Listing }; diff --git a/api/src/services/listing.service.ts b/api/src/services/listing.service.ts index e3575a744c..d30db6c7d6 100644 --- a/api/src/services/listing.service.ts +++ b/api/src/services/listing.service.ts @@ -107,6 +107,7 @@ views.full = { listingsApplicationPickUpAddress: true, listingsApplicationDropOffAddress: true, listingsApplicationMailingAddress: true, + requestedChangesUser: true, units: { include: { unitAmiChartOverrides: true, @@ -891,6 +892,7 @@ export class ListingService implements OnModuleInit { }, } : undefined, + requestedChangesUser: undefined, }, }); @@ -1358,11 +1360,15 @@ export class ListingService implements OnModuleInit { dto.status === ListingsStatusEnum.closed ? new Date() : storedListing.closedAt, - requestedChangesUserId: + requestedChangesUser: dto.status === ListingsStatusEnum.changesRequested && storedListing.status !== ListingsStatusEnum.changesRequested - ? requestingUser.id - : storedListing.requestedChangesUserId, + ? { + connect: { + id: requestingUser.id, + }, + } + : undefined, listingsResult: dto.listingsResult ? { create: { diff --git a/api/src/utilities/requested-changes-user.ts b/api/src/utilities/requested-changes-user.ts new file mode 100644 index 0000000000..4f359d86e3 --- /dev/null +++ b/api/src/utilities/requested-changes-user.ts @@ -0,0 +1,16 @@ +import { IdDTO } from '../dtos/shared/id.dto'; +import { User } from '../dtos/users/user.dto'; + +/* + This maps a user that has requested changes on a listing to a limited IdDTO + This is used by the partner site front end + */ +export function requestedChangesUserMapper(user: User): IdDTO { + return { + id: user?.id, + name: + user?.firstName && user?.lastName + ? user?.firstName + ' ' + user?.lastName + : undefined, + }; +} diff --git a/api/test/unit/services/listing.service.spec.ts b/api/test/unit/services/listing.service.spec.ts index 3c470cab39..76476db819 100644 --- a/api/test/unit/services/listing.service.spec.ts +++ b/api/test/unit/services/listing.service.spec.ts @@ -444,6 +444,7 @@ describe('Testing listing service', () => { include: { jurisdictions: true, listingsBuildingAddress: true, + requestedChangesUser: true, reservedCommunityTypes: true, listingImages: { include: { @@ -828,7 +829,7 @@ describe('Testing listing service', () => { }); }); - it('should handle no records returned when findOne() is called with base view', async () => { + it('should handle no records returned when findOne() is called with details view', async () => { prisma.listings.findUnique = jest.fn().mockResolvedValue(null); await expect( @@ -847,6 +848,7 @@ describe('Testing listing service', () => { include: { jurisdictions: true, listingsBuildingAddress: true, + requestedChangesUser: true, reservedCommunityTypes: true, listingImages: { include: { @@ -1526,6 +1528,136 @@ describe('Testing listing service', () => { }); }); + it('should get records from findOne() with details view found and units', async () => { + const date = new Date(); + + const mockedListing = mockListing(0, { numberToMake: 1, date }); + + prisma.listings.findUnique = jest.fn().mockResolvedValue(mockedListing); + + prisma.amiChart.findMany = jest.fn().mockResolvedValue([ + { + id: 'AMI0', + items: [], + name: '`AMI Name 0`', + }, + { + id: 'AMI1', + items: [], + name: '`AMI Name 1`', + }, + ]); + + const listing: Listing = await service.findOne( + 'listingId', + LanguagesEnum.en, + ListingViews.details, + ); + + expect(listing.id).toEqual('0'); + expect(listing.name).toEqual('listing 1'); + expect(listing.units).toEqual(mockedListing.units); + expect(listing.unitsSummarized.amiPercentages).toEqual(['0']); + expect(listing.unitsSummarized?.byAMI).toEqual([ + { + percent: '0', + byUnitType: [ + { + areaRange: { min: 0, max: 0 }, + minIncomeRange: { min: '$0', max: '$0' }, + occupancyRange: { min: 0, max: 0 }, + rentRange: { min: '$0', max: '$0' }, + rentAsPercentIncomeRange: { min: 0, max: 0 }, + floorRange: { min: 0, max: 0 }, + unitTypes: { + id: 'unitType 0', + createdAt: date, + updatedAt: date, + name: 'SRO', + numBedrooms: 0, + }, + totalAvailable: 1, + }, + ], + }, + ]); + expect(listing.unitsSummarized.unitTypes).toEqual([ + { + createdAt: date, + id: 'unitType 0', + name: 'SRO', + numBedrooms: 0, + updatedAt: date, + }, + ]); + + expect(prisma.listings.findUnique).toHaveBeenCalledWith({ + where: { + id: 'listingId', + }, + include: { + jurisdictions: true, + listingsBuildingAddress: true, + requestedChangesUser: true, + reservedCommunityTypes: true, + listingImages: { + include: { + assets: true, + }, + }, + listingMultiselectQuestions: { + include: { + multiselectQuestions: true, + }, + }, + listingFeatures: true, + listingUtilities: true, + applicationMethods: { + include: { + paperApplications: { + include: { + assets: true, + }, + }, + }, + }, + listingsBuildingSelectionCriteriaFile: true, + listingEvents: { + include: { + assets: true, + }, + }, + listingsResult: true, + listingsLeasingAgentAddress: true, + listingsApplicationPickUpAddress: true, + listingsApplicationDropOffAddress: true, + listingsApplicationMailingAddress: true, + units: { + include: { + unitAmiChartOverrides: true, + unitTypes: true, + unitRentTypes: true, + unitAccessibilityPriorityTypes: true, + amiChart: { + include: { + jurisdictions: true, + unitGroupAmiLevels: true, + }, + }, + }, + }, + }, + }); + + expect(prisma.amiChart.findMany).toHaveBeenCalledWith({ + where: { + id: { + in: mockedListing.units.map((unit) => unit.amiChart.id), + }, + }, + }); + }); + it('should return listings from findListingsWithMultiSelectQuestion()', async () => { prisma.listings.findMany = jest.fn().mockResolvedValue([ { @@ -1620,6 +1752,7 @@ describe('Testing listing service', () => { listingsBuildingSelectionCriteriaFile: true, listingsLeasingAgentAddress: true, listingsResult: true, + requestedChangesUser: true, reservedCommunityTypes: true, units: { include: { @@ -1718,6 +1851,7 @@ describe('Testing listing service', () => { listingsBuildingSelectionCriteriaFile: true, listingsLeasingAgentAddress: true, listingsResult: true, + requestedChangesUser: true, reservedCommunityTypes: true, units: { include: { @@ -2153,6 +2287,7 @@ describe('Testing listing service', () => { listingsApplicationMailingAddress: true, listingsLeasingAgentAddress: true, listingsResult: true, + requestedChangesUser: true, reservedCommunityTypes: true, units: { include: { @@ -2275,6 +2410,7 @@ describe('Testing listing service', () => { listingsLeasingAgentAddress: true, listingsResult: true, reservedCommunityTypes: true, + requestedChangesUser: true, units: { include: { amiChart: { diff --git a/shared-helpers/src/types/backend-swagger.ts b/shared-helpers/src/types/backend-swagger.ts index 47402d859f..a697d72111 100644 --- a/shared-helpers/src/types/backend-swagger.ts +++ b/shared-helpers/src/types/backend-swagger.ts @@ -3060,7 +3060,7 @@ export interface Listing { requestedChangesDate?: Date /** */ - requestedChangesUser?: User + requestedChangesUser?: IdDTO } export interface PaginationMeta { @@ -5223,6 +5223,7 @@ export enum EnumJurisdictionListingApprovalPermissions { "admin" = "admin", "jurisdictionAdmin" = "jurisdictionAdmin", } + export enum AfsView { "pending" = "pending", "pendingNameAndDoB" = "pendingNameAndDoB", diff --git a/sites/partners/cypress/e2e/listings-approval/listings-approval.spec.ts b/sites/partners/cypress/e2e/listings-approval/listings-approval.spec.ts index b45bb21549..7256cf238b 100644 --- a/sites/partners/cypress/e2e/listings-approval/listings-approval.spec.ts +++ b/sites/partners/cypress/e2e/listings-approval/listings-approval.spec.ts @@ -29,6 +29,7 @@ describe("Listings approval feature", () => { searchAndOpenListing(cy, uniqueListingName) cy.getByID("listing-status-changes-requested").should("be.visible") cy.getByID("requestedChanges").contains("Requested changes test summary") + cy.getByID("requestedChangesUser").contains("First Last") cy.getByID("listingEditButton").click() cy.getByTestId("nameField").should("be.visible").click().clear().type(uniqueListingNameEdited) cy.getByID("submitButton").contains("Submit").click() diff --git a/sites/partners/src/components/listings/PaperListingDetails/sections/DetailNotes.tsx b/sites/partners/src/components/listings/PaperListingDetails/sections/DetailNotes.tsx index dae63a6298..3d9dd0c1c9 100644 --- a/sites/partners/src/components/listings/PaperListingDetails/sections/DetailNotes.tsx +++ b/sites/partners/src/components/listings/PaperListingDetails/sections/DetailNotes.tsx @@ -29,9 +29,9 @@ const DetailListingNotes = () => { {getDetailFieldDate(listing.requestedChangesDate)} - {listing.requestedChangesUser && ( + {listing?.requestedChangesUser?.name && ( - {`${listing.requestedChangesUser?.firstName} ${listing.requestedChangesUser?.lastName}`} + {`${listing.requestedChangesUser.name}`} )}