diff --git a/apps/backend/src/donationItems/donationItems.service.spec.ts b/apps/backend/src/donationItems/donationItems.service.spec.ts index 847041741..ae58c23c0 100644 --- a/apps/backend/src/donationItems/donationItems.service.spec.ts +++ b/apps/backend/src/donationItems/donationItems.service.spec.ts @@ -228,71 +228,17 @@ describe('DonationItemsService', () => { expect(rice.detailsConfirmed).toEqual(true); }); - it('creates items with optional fields omitted', async () => { + it('sets detailsConfirmed to true since ozPerItem and estimatedValue are required', async () => { const donation = await getSeedDonation(); const transactionManager = testDataSource.createEntityManager(); - const minimalItems: CreateDonationItemDto[] = [ - { - itemName: 'Plain Item', - quantity: 3, - foodType: FoodType.DRIED_BEANS, - foodRescue: true, - }, - ]; - - const result = await service.createMultiple( - donation, - minimalItems, - transactionManager, - ); - - expect(result).toHaveLength(1); - expect(result[0].itemId).toBeDefined(); - expect(result[0].ozPerItem).toBeNull(); - expect(result[0].estimatedValue).toBeNull(); - expect(result[0].detailsConfirmed).toEqual(false); - }); - - it('sets detailsConfirmed to true only when both ozPerItem and estimatedValue are provided', async () => { - const donation = await getSeedDonation(); - const transactionManager = testDataSource.createEntityManager(); - - const mixedItems: CreateDonationItemDto[] = [ - { - itemName: 'Both Fields', - quantity: 4, - ozPerItem: 12, - estimatedValue: 3.5, - foodType: FoodType.DRIED_BEANS, - foodRescue: false, - }, - { - itemName: 'Missing Estimated Value', - quantity: 2, - ozPerItem: 8, - foodType: FoodType.DRIED_BEANS, - foodRescue: false, - }, - { - itemName: 'Missing Oz Per Item', - quantity: 6, - estimatedValue: 1.99, - foodType: FoodType.DRIED_BEANS, - foodRescue: false, - }, - ]; - const result = await service.createMultiple( donation, - mixedItems, + validItems, transactionManager, ); - const byName = Object.fromEntries(result.map((i) => [i.itemName, i])); - expect(byName['Both Fields'].detailsConfirmed).toEqual(true); - expect(byName['Missing Estimated Value'].detailsConfirmed).toEqual(false); - expect(byName['Missing Oz Per Item'].detailsConfirmed).toEqual(false); + expect(result.every((item) => item.detailsConfirmed)).toBe(true); }); it('rolls back all items when one fails within a transaction', async () => { @@ -308,6 +254,8 @@ describe('DonationItemsService', () => { { itemName: 'a'.repeat(1000), quantity: 5, + ozPerItem: 10, + estimatedValue: 2.5, foodType: FoodType.DRIED_BEANS, foodRescue: false, }, @@ -496,50 +444,6 @@ describe('DonationItemsService', () => { expect(item?.ozPerItem).toBeNull(); }); - it('returns false and does not confirm when only some fields are provided', async () => { - const donationId = await insertMatchedDonation(); - const itemId = await insertDonationItem(donationId, 10, 5); - - const result = await testDataSource.transaction((tm) => - service.updateItemDetails(donationId, [{ itemId, ozPerItem: 8.5 }], tm), - ); - - expect(result).toBe(false); - const item = await testDataSource - .getRepository(DonationItem) - .findOneBy({ itemId }); - expect(Number(item?.ozPerItem)).toBe(8.5); - expect(item?.estimatedValue).toBeNull(); - expect(item?.detailsConfirmed).toBe(false); - }); - - it('confirms item on a second call that supplies the remaining fields', async () => { - const donationId = await insertMatchedDonation(); - const itemId = await insertDonationItem(donationId, 10, 5); - - const firstResult = await testDataSource.transaction((tm) => - service.updateItemDetails(donationId, [{ itemId, ozPerItem: 8.5 }], tm), - ); - expect(firstResult).toBe(false); - - const secondResult = await testDataSource.transaction((tm) => - service.updateItemDetails( - donationId, - [{ itemId, estimatedValue: 12.0, foodRescue: true }], - tm, - ), - ); - expect(secondResult).toBe(true); - - const item = await testDataSource - .getRepository(DonationItem) - .findOneBy({ itemId }); - expect(Number(item?.ozPerItem)).toBe(8.5); - expect(Number(item?.estimatedValue)).toBe(12.0); - expect(item?.foodRescue).toBe(true); - expect(item?.detailsConfirmed).toBe(true); - }); - it('allows updating an already-confirmed item without throwing', async () => { const donationId = await insertMatchedDonation(); const itemId = await insertDonationItem(donationId, 10, 5); @@ -551,7 +455,18 @@ describe('DonationItemsService', () => { ); const result = await testDataSource.transaction((tm) => - service.updateItemDetails(donationId, [{ itemId, ozPerItem: 9.0 }], tm), + service.updateItemDetails( + donationId, + [ + { + itemId, + ozPerItem: 9.0, + estimatedValue: 10.0, + foodRescue: true, + }, + ], + tm, + ), ); expect(result).toBe(true); diff --git a/apps/backend/src/donationItems/donationItems.service.ts b/apps/backend/src/donationItems/donationItems.service.ts index 916210174..350a33db5 100644 --- a/apps/backend/src/donationItems/donationItems.service.ts +++ b/apps/backend/src/donationItems/donationItems.service.ts @@ -104,26 +104,15 @@ export class DonationItemsService { ); } - const updateData: Partial = {}; - if (dto.ozPerItem !== undefined) updateData.ozPerItem = dto.ozPerItem; - if (dto.estimatedValue !== undefined) - updateData.estimatedValue = dto.estimatedValue; - if (dto.foodRescue !== undefined) updateData.foodRescue = dto.foodRescue; - - // If included in DTO, keep it, otherwise use whatever is in the DB (could be null) - const resultingOzPerItem = - updateData.ozPerItem !== undefined - ? updateData.ozPerItem - : item.ozPerItem; - const resultingEstimatedValue = - updateData.estimatedValue !== undefined - ? updateData.estimatedValue - : item.estimatedValue; - - if (resultingOzPerItem != null && resultingEstimatedValue != null) { - updateData.detailsConfirmed = true; - confirmedDetailsForAnItem = true; - } + // ozPerItem, estimatedValue, and foodRescue are required on the DTO, so an + // update always supplies the full set of details and confirms the item. + const updateData: Partial = { + ozPerItem: dto.ozPerItem, + estimatedValue: dto.estimatedValue, + foodRescue: dto.foodRescue, + detailsConfirmed: true, + }; + confirmedDetailsForAnItem = true; await donationItemTransactionRepo.update(dto.itemId, updateData); } @@ -148,7 +137,7 @@ export class DonationItemsService { estimatedValue: item.estimatedValue, foodType: item.foodType, foodRescue: item.foodRescue, - detailsConfirmed: item.ozPerItem != null && item.estimatedValue != null, + detailsConfirmed: true, }), ); return transactionRepo.save(donationItems); diff --git a/apps/backend/src/donationItems/dtos/create-donation-items.dto.ts b/apps/backend/src/donationItems/dtos/create-donation-items.dto.ts index 3380ef7a6..6059732cd 100644 --- a/apps/backend/src/donationItems/dtos/create-donation-items.dto.ts +++ b/apps/backend/src/donationItems/dtos/create-donation-items.dto.ts @@ -5,7 +5,6 @@ import { IsEnum, IsNotEmpty, Length, - IsOptional, IsInt, IsBoolean, } from 'class-validator'; @@ -26,16 +25,14 @@ export class CreateDonationItemDto { { message: 'ozPerItem must have at most 2 decimal places' }, ) @Min(0.01) - @IsOptional() - ozPerItem?: number; + ozPerItem!: number; @IsNumber( { maxDecimalPlaces: 2 }, { message: 'estimatedValue must have at most 2 decimal places' }, ) @Min(0.01) - @IsOptional() - estimatedValue?: number; + estimatedValue!: number; @IsEnum(FoodType) foodType!: FoodType; diff --git a/apps/backend/src/donationItems/dtos/update-donation-item-details.dto.ts b/apps/backend/src/donationItems/dtos/update-donation-item-details.dto.ts index 7c2366761..83812ea93 100644 --- a/apps/backend/src/donationItems/dtos/update-donation-item-details.dto.ts +++ b/apps/backend/src/donationItems/dtos/update-donation-item-details.dto.ts @@ -1,27 +1,24 @@ -import { IsNumber, Min, IsBoolean, IsInt, IsOptional } from 'class-validator'; +import { IsNumber, Min, IsBoolean, IsInt } from 'class-validator'; export class UpdateDonationItemDetailsDto { @IsInt() @Min(1) itemId!: number; - @IsOptional() @IsNumber( { maxDecimalPlaces: 2 }, { message: 'Oz per item must have at most 2 decimal places' }, ) @Min(0.01, { message: 'Oz per item must be at least 0.01' }) - ozPerItem?: number; + ozPerItem!: number; - @IsOptional() @IsNumber( { maxDecimalPlaces: 2 }, { message: 'Estimated value must have at most 2 decimal places' }, ) @Min(0.01, { message: 'Estimated value must be at least 0.01' }) - estimatedValue?: number; + estimatedValue!: number; - @IsOptional() @IsBoolean() - foodRescue?: boolean; + foodRescue!: boolean; } diff --git a/apps/backend/src/donations/donations.controller.ts b/apps/backend/src/donations/donations.controller.ts index ab493edd5..b5315dbde 100644 --- a/apps/backend/src/donations/donations.controller.ts +++ b/apps/backend/src/donations/donations.controller.ts @@ -65,8 +65,8 @@ export class DonationsController { properties: { itemName: { type: 'string', example: 'Canned Beans' }, quantity: { type: 'integer', example: 1 }, - ozPerItem: { type: 'number', example: 0.01, nullable: true }, - estimatedValue: { type: 'number', example: 0.01, nullable: true }, + ozPerItem: { type: 'number', example: 0.01 }, + estimatedValue: { type: 'number', example: 0.01 }, foodType: { type: 'enum', enum: Object.values(FoodType), diff --git a/apps/backend/src/donations/donations.service.spec.ts b/apps/backend/src/donations/donations.service.spec.ts index 6fa0bcda3..5ce2843ca 100644 --- a/apps/backend/src/donations/donations.service.spec.ts +++ b/apps/backend/src/donations/donations.service.spec.ts @@ -1229,6 +1229,8 @@ describe('DonationService', () => { quantity: 5, foodType: FoodType.DAIRY_FREE_ALTERNATIVES, foodRescue: false, + ozPerItem: 3.4, + estimatedValue: 3.4, }, ], }), @@ -1355,21 +1357,6 @@ describe('DonationService', () => { expect(spy).toHaveBeenCalled(); }); - - it('does not call checkAndFulfillDonation when no items are fully confirmed', async () => { - const donationId = await insertMatchedDonation(); - const itemId = await insertDonationItem(donationId, 10, 5); - - const spy = jest.spyOn(service, 'checkAndFulfillDonation'); - - await service.updateDonationItemDetails(donationId, [ - { itemId, ozPerItem: 5.0 }, - ]); - - const dbDonation = await service.findOne(donationId); - expect(dbDonation.status).toBe(DonationStatus.MATCHED); - expect(spy).not.toHaveBeenCalled(); - }); }); describe('checkAndFulfillDonation', () => { diff --git a/apps/backend/src/users/users.service.spec.ts b/apps/backend/src/users/users.service.spec.ts index c81a6664b..c7f53ae6c 100644 --- a/apps/backend/src/users/users.service.spec.ts +++ b/apps/backend/src/users/users.service.spec.ts @@ -358,6 +358,8 @@ describe('UsersService', () => { itemName: 'Test Item', quantity: 10, foodType: FoodType.GRANOLA, + ozPerItem: 3.4, + estimatedValue: 3.4, foodRescue: false, }, ], diff --git a/apps/frontend/src/components/forms/fmCompleteRequiredActionsModal.tsx b/apps/frontend/src/components/forms/fmCompleteRequiredActionsModal.tsx index 20e76d0ff..11ef3d573 100644 --- a/apps/frontend/src/components/forms/fmCompleteRequiredActionsModal.tsx +++ b/apps/frontend/src/components/forms/fmCompleteRequiredActionsModal.tsx @@ -233,14 +233,14 @@ const FmCompleteRequiredActionsModal: React.FC< }) .map((item) => { const formData = itemFormData[item.itemId]; + // Submit is gated on ozPerItem and estimatedValue being filled for every + // item, so all required fields are guaranteed present here. const dto: UpdateDonationItemDetailsDto = { itemId: item.itemId, + ozPerItem: parseFloat(formData.ozPerItem), + estimatedValue: parseFloat(formData.estimatedValue), foodRescue: formData.foodRescue, }; - if (formData.ozPerItem !== '') - dto.ozPerItem = parseFloat(formData.ozPerItem); - if (formData.estimatedValue !== '') - dto.estimatedValue = parseFloat(formData.estimatedValue); return dto; }); diff --git a/apps/frontend/src/components/forms/newDonationFormModal.tsx b/apps/frontend/src/components/forms/newDonationFormModal.tsx index d9bf495a8..266719406 100644 --- a/apps/frontend/src/components/forms/newDonationFormModal.tsx +++ b/apps/frontend/src/components/forms/newDonationFormModal.tsx @@ -85,10 +85,16 @@ const getFirstValidationError = ( if (!isValidPositiveInt(row.numItems)) { return `Quantity${rowLabel} must be a positive whole number.`; } - if (row.ozPerItem !== '' && !isValidDecimal(row.ozPerItem)) { + if (row.ozPerItem === '') { + return `Oz. per item${rowLabel} is required.`; + } + if (!isValidDecimal(row.ozPerItem)) { return `Oz. per item${rowLabel} must be a positive number with at most 2 decimal places.`; } - if (row.valuePerItem !== '' && !isValidDecimal(row.valuePerItem)) { + if (row.valuePerItem === '') { + return `Donation value${rowLabel} is required.`; + } + if (!isValidDecimal(row.valuePerItem)) { return `Donation value${rowLabel} must be a positive number with at most 2 decimal places.`; } } @@ -220,10 +226,8 @@ const NewDonationFormModal: React.FC = ({ items: rows.map((row) => ({ itemName: row.foodItem, quantity: parseInt(row.numItems), - ozPerItem: row.ozPerItem ? parseFloat(row.ozPerItem) : undefined, - estimatedValue: row.valuePerItem - ? parseFloat(row.valuePerItem) - : undefined, + ozPerItem: parseFloat(row.ozPerItem), + estimatedValue: parseFloat(row.valuePerItem), foodType: row.foodType as FoodType, foodRescue: row.foodRescue, })), @@ -297,10 +301,14 @@ const NewDonationFormModal: React.FC = ({ - + Please fill out the following information to record donation details. + + Please do not include shipping/delivery costs in Food Donation + Value. + = ({ Oz. per item + + * + - Donation Value + Donation Value + (Fair Market Value of Food Only) + + * + = ({ lineHeight="tight" > Food Rescue + + * + diff --git a/apps/frontend/src/components/forms/resubmitDonationModal.tsx b/apps/frontend/src/components/forms/resubmitDonationModal.tsx index afa697297..a4fa437f7 100644 --- a/apps/frontend/src/components/forms/resubmitDonationModal.tsx +++ b/apps/frontend/src/components/forms/resubmitDonationModal.tsx @@ -116,12 +116,8 @@ const ResubmitDonationModal: React.FC = ({ items: items.map((item) => ({ itemName: item.itemName, quantity: item.quantity, - ozPerItem: - item.ozPerItem != null ? Number(item.ozPerItem) : undefined, - estimatedValue: - item.estimatedValue != null - ? Number(item.estimatedValue) - : undefined, + ozPerItem: Number(item.ozPerItem), + estimatedValue: Number(item.estimatedValue), foodType: item.foodType, foodRescue: item.foodRescue, })), diff --git a/apps/frontend/src/types/types.ts b/apps/frontend/src/types/types.ts index 8addbea64..48fb8f574 100644 --- a/apps/frontend/src/types/types.ts +++ b/apps/frontend/src/types/types.ts @@ -481,8 +481,8 @@ export interface CreateDonationDto { export interface CreateDonationItemDto { itemName: string; quantity: number; - ozPerItem?: number; - estimatedValue?: number; + ozPerItem: number; + estimatedValue: number; foodType: FoodType; foodRescue: boolean; } @@ -606,7 +606,7 @@ export interface BulkUpdateTrackingCostDto { export interface UpdateDonationItemDetailsDto { itemId: number; - ozPerItem?: number; - estimatedValue?: number; - foodRescue?: boolean; + ozPerItem: number; + estimatedValue: number; + foodRescue: boolean; }