From d56718a0f7b91fcd0a81e77dfc7765e8ca5c0dae Mon Sep 17 00:00:00 2001 From: prooflesben Date: Fri, 10 Apr 2026 13:10:17 -0400 Subject: [PATCH 1/2] edit cost backend now tells the frontend that a cost with this name already exist --- backend/src/cost/cashflow-cost.service.ts | 190 ++++++++++++++-------- 1 file changed, 126 insertions(+), 64 deletions(-) diff --git a/backend/src/cost/cashflow-cost.service.ts b/backend/src/cost/cashflow-cost.service.ts index c988bb19..aacda893 100644 --- a/backend/src/cost/cashflow-cost.service.ts +++ b/backend/src/cost/cashflow-cost.service.ts @@ -5,10 +5,10 @@ import { InternalServerErrorException, Logger, NotFoundException, -} from '@nestjs/common'; -import * as AWS from 'aws-sdk'; -import { CashflowCost } from '../../../middle-layer/types/CashflowCost'; -import { CostType } from '../../../middle-layer/types/CostType'; +} from "@nestjs/common"; +import * as AWS from "aws-sdk"; +import { CashflowCost } from "../../../middle-layer/types/CashflowCost"; +import { CostType } from "../../../middle-layer/types/CostType"; interface UpdateCostBody { amount?: number; @@ -25,46 +25,50 @@ export class CostService { private validateCostType(type: string) { if (!Object.values(CostType).includes(type as CostType) || type === null) { throw new BadRequestException( - `type must be one of: ${Object.values(CostType).join(', ')}`, + `type must be one of: ${Object.values(CostType).join(", ")}`, ); } } private validateAmount(amount: number) { if (!Number.isFinite(amount) || amount <= 0 || amount === null) { - throw new BadRequestException('amount must be a finite positive number'); + throw new BadRequestException("amount must be a finite positive number"); } - } private validateName(name: string) { if (name === null || name.trim().length === 0) { - throw new BadRequestException('name must be a non-empty string'); + throw new BadRequestException("name must be a non-empty string"); } } private validateDate(date: string) { if (date === null) { - throw new BadRequestException('date is required and must be a non-null string'); + throw new BadRequestException( + "date is required and must be a non-null string", + ); } - const iso8601Regex = /^\d{4}-\d{2}-\d{2}(T\d{2}:\d{2}:\d{2}(.\d{3})?(Z|[+-]\d{2}:\d{2})?)?$/; + const iso8601Regex = + /^\d{4}-\d{2}-\d{2}(T\d{2}:\d{2}:\d{2}(.\d{3})?(Z|[+-]\d{2}:\d{2})?)?$/; if (!iso8601Regex.test(date)) { - throw new BadRequestException('date must be a valid ISO 8601 format string (e.g., "2026-03-22" or "2026-03-22T16:09:52Z")'); + throw new BadRequestException( + 'date must be a valid ISO 8601 format string (e.g., "2026-03-22" or "2026-03-22T16:09:52Z")', + ); } const parsedDate = new Date(date); if (isNaN(parsedDate.getTime())) { - throw new BadRequestException('date must be a valid ISO 8601 date'); + throw new BadRequestException("date must be a valid ISO 8601 date"); } } // get all costs for cash flow async getAllCosts(): Promise { - const tableName = process.env.CASHFLOW_COST_TABLE_NAME || ''; - this.logger.log('Retrieving all costs'); + const tableName = process.env.CASHFLOW_COST_TABLE_NAME || ""; + this.logger.log("Retrieving all costs"); if (!tableName) { - this.logger.error('CASHFLOW_COST_TABLE_NAME is not defined'); - throw new InternalServerErrorException('Server configuration error'); + this.logger.error("CASHFLOW_COST_TABLE_NAME is not defined"); + throw new InternalServerErrorException("Server configuration error"); } try { @@ -76,21 +80,21 @@ export class CostService { return (result.Items ?? []) as CashflowCost[]; } catch (error) { - this.logger.error('Failed to retrieve costs', error as Error); - throw new InternalServerErrorException('Failed to retrieve costs'); + this.logger.error("Failed to retrieve costs", error as Error); + throw new InternalServerErrorException("Failed to retrieve costs"); } } // gets a specific cost by its name, the key async getCostByName(costName: string): Promise { - const tableName = process.env.CASHFLOW_COST_TABLE_NAME || ''; + const tableName = process.env.CASHFLOW_COST_TABLE_NAME || ""; this.validateName(costName); const normalizedName = costName.trim(); this.logger.log(`Retrieving cost with name ${normalizedName}`); if (!tableName) { - this.logger.error('CASHFLOW_COST_TABLE_NAME is not defined'); - throw new InternalServerErrorException('Server configuration error'); + this.logger.error("CASHFLOW_COST_TABLE_NAME is not defined"); + throw new InternalServerErrorException("Server configuration error"); } try { @@ -103,7 +107,9 @@ export class CostService { if (!result.Item) { this.logger.error(`Cost with name ${normalizedName} not found`); - throw new NotFoundException(`Cost with name ${normalizedName} not found`); + throw new NotFoundException( + `Cost with name ${normalizedName} not found`, + ); } return result.Item as CashflowCost; @@ -112,25 +118,30 @@ export class CostService { throw error; } - this.logger.error(`Failed to retrieve cost ${normalizedName}`, error as Error); - throw new InternalServerErrorException(`Failed to retrieve cost ${normalizedName}`); + this.logger.error( + `Failed to retrieve cost ${normalizedName}`, + error as Error, + ); + throw new InternalServerErrorException( + `Failed to retrieve cost ${normalizedName}`, + ); } } async createCost(cost: CashflowCost): Promise { - const tableName = process.env.CASHFLOW_COST_TABLE_NAME || ''; + const tableName = process.env.CASHFLOW_COST_TABLE_NAME || ""; this.validateAmount(cost.amount); this.validateCostType(cost.type); this.validateName(cost.name); const normalizedName = cost.name.trim(); if (!tableName) { - this.logger.error('CASHFLOW_COST_TABLE_NAME is not defined'); - throw new InternalServerErrorException('Server configuration error'); + this.logger.error("CASHFLOW_COST_TABLE_NAME is not defined"); + throw new InternalServerErrorException("Server configuration error"); } this.logger.log(`Creating cost with name ${normalizedName}`); - + try { await this.dynamoDb .put({ @@ -139,9 +150,9 @@ export class CostService { ...cost, name: normalizedName, }, - ConditionExpression: 'attribute_not_exists(#name)', + ConditionExpression: "attribute_not_exists(#name)", ExpressionAttributeNames: { - '#name': 'name', + "#name": "name", }, }) .promise(); @@ -153,27 +164,32 @@ export class CostService { } catch (error) { const awsError = error as { code?: string }; - if (awsError.code === 'ConditionalCheckFailedException') { - throw new ConflictException(`Cost with name ${normalizedName} already exists`); + if (awsError.code === "ConditionalCheckFailedException") { + throw new ConflictException( + `Cost with name ${normalizedName} already exists`, + ); } if (error instanceof BadRequestException) { throw error; } - this.logger.error('Failed to create cost', error as Error); - throw new InternalServerErrorException('Failed to create cost'); + this.logger.error("Failed to create cost", error as Error); + throw new InternalServerErrorException("Failed to create cost"); } } - async updateCost(costName: string, updates: CashflowCost): Promise { - const tableName = process.env.CASHFLOW_COST_TABLE_NAME || ''; + async updateCost( + costName: string, + updates: CashflowCost, + ): Promise { + const tableName = process.env.CASHFLOW_COST_TABLE_NAME || ""; this.validateName(costName); const normalizedName = costName.trim(); if (!tableName) { - this.logger.error('CASHFLOW_COST_TABLE_NAME is not defined'); - throw new InternalServerErrorException('Server configuration error'); + this.logger.error("CASHFLOW_COST_TABLE_NAME is not defined"); + throw new InternalServerErrorException("Server configuration error"); } this.validateAmount(updates.amount); @@ -212,14 +228,18 @@ export class CostService { datesAreEqual; if (isUnchanged) { - this.logger.log(`No changes detected for cost ${normalizedName}; skipping update`); + this.logger.log( + `No changes detected for cost ${normalizedName}; skipping update`, + ); return existingCost; } const shouldRename = updates.name.trim() !== normalizedName; if (shouldRename) { - this.logger.log(`Renaming cost ${normalizedName} to ${updates.name.trim()}`); + this.logger.log( + `Renaming cost ${normalizedName} to ${updates.name.trim()}`, + ); try { await this.dynamoDb @@ -229,9 +249,9 @@ export class CostService { Put: { TableName: tableName, Item: updates, - ConditionExpression: 'attribute_not_exists(#name)', + ConditionExpression: "attribute_not_exists(#name)", ExpressionAttributeNames: { - '#name': 'name', + "#name": "name", }, }, }, @@ -239,9 +259,9 @@ export class CostService { Delete: { TableName: tableName, Key: { name: normalizedName }, - ConditionExpression: 'attribute_exists(#name)', + ConditionExpression: "attribute_exists(#name)", ExpressionAttributeNames: { - '#name': 'name', + "#name": "name", }, }, }, @@ -249,16 +269,42 @@ export class CostService { }) .promise(); - this.logger.log(`Successfully renamed cost ${normalizedName} to ${updates.name.trim()}`); + this.logger.log( + `Successfully renamed cost ${normalizedName} to ${updates.name.trim()}`, + ); return updates; } catch (error) { if (error instanceof NotFoundException) { throw error; } - const awsError = error as { code?: string }; - if (awsError.code === 'ConditionalCheckFailedException') { - throw new ConflictException(`Cost with name ${updates.name.trim()} already exists`); + const awsError = error as { + code?: string; + CancellationReasons?: { Code: string }[]; + }; + + if (awsError.code === "TransactionCanceledException") { + const reasons = awsError.CancellationReasons ?? []; + + // Put failed — new name already exists (index 0) + if (reasons[0]?.Code === "ConditionalCheckFailed") { + this.logger.error( + `Rename conflict: cost with name "${updates.name.trim()}" already exists, cannot rename from "${normalizedName}"` + ); + throw new ConflictException( + `Cost with name ${updates.name.trim()} already exists`, + ); + } + + // Delete failed — original item no longer exists (index 1) + if (reasons[1]?.Code === "ConditionalCheckFailed") { + this.logger.error( + `Rename failed: original cost "${normalizedName}" no longer exists in the table` + ); + throw new NotFoundException( + `Cost with name ${normalizedName} not found`, + ); + } } this.logger.error( @@ -271,16 +317,18 @@ export class CostService { } } - this.logger.log(`Replacing cost ${normalizedName} with payload: ${JSON.stringify(updates)}`); + this.logger.log( + `Replacing cost ${normalizedName} with payload: ${JSON.stringify(updates)}`, + ); try { await this.dynamoDb .put({ TableName: tableName, Item: updates, - ConditionExpression: 'attribute_exists(#name)', + ConditionExpression: "attribute_exists(#name)", ExpressionAttributeNames: { - '#name': 'name', + "#name": "name", }, }) .promise(); @@ -288,23 +336,30 @@ export class CostService { return updates; } catch (error) { const awsError = error as { code?: string }; - if (awsError.code === 'ConditionalCheckFailedException') { - throw new NotFoundException(`Cost with name ${normalizedName} not found`); + if (awsError.code === "ConditionalCheckFailedException") { + throw new NotFoundException( + `Cost with name ${normalizedName} not found`, + ); } - this.logger.error(`Failed to update cost ${normalizedName}`, error as Error); - throw new InternalServerErrorException(`Failed to update cost ${normalizedName}`); + this.logger.error( + `Failed to update cost ${normalizedName}`, + error as Error, + ); + throw new InternalServerErrorException( + `Failed to update cost ${normalizedName}`, + ); } } async deleteCost(costName: string): Promise { - const tableName = process.env.CASHFLOW_COST_TABLE_NAME || ''; + const tableName = process.env.CASHFLOW_COST_TABLE_NAME || ""; this.validateName(costName); const normalizedName = costName.trim(); if (!tableName) { - this.logger.error('CASHFLOW_COST_TABLE_NAME is not defined'); - throw new InternalServerErrorException('Server configuration error'); + this.logger.error("CASHFLOW_COST_TABLE_NAME is not defined"); + throw new InternalServerErrorException("Server configuration error"); } this.logger.log(`Deleting cost ${normalizedName}`); @@ -314,9 +369,9 @@ export class CostService { .delete({ TableName: tableName, Key: { name: normalizedName }, - ConditionExpression: 'attribute_exists(#name)', + ConditionExpression: "attribute_exists(#name)", ExpressionAttributeNames: { - '#name': 'name', + "#name": "name", }, }) .promise(); @@ -324,12 +379,19 @@ export class CostService { return `Cost ${normalizedName} deleted successfully`; } catch (error) { const awsError = error as { code?: string }; - if (awsError.code === 'ConditionalCheckFailedException') { - throw new NotFoundException(`Cost with name ${normalizedName} not found`); + if (awsError.code === "ConditionalCheckFailedException") { + throw new NotFoundException( + `Cost with name ${normalizedName} not found`, + ); } - this.logger.error(`Failed to delete cost ${normalizedName}`, error as Error); - throw new InternalServerErrorException(`Failed to delete cost ${normalizedName}`); + this.logger.error( + `Failed to delete cost ${normalizedName}`, + error as Error, + ); + throw new InternalServerErrorException( + `Failed to delete cost ${normalizedName}`, + ); } } } From b9600e70893421d1eddf882bda6db92626cf1610 Mon Sep 17 00:00:00 2001 From: prooflesben Date: Fri, 10 Apr 2026 13:29:22 -0400 Subject: [PATCH 2/2] fixed test --- .../__test__/cashflow-cost.service.spec.ts | 34 +++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/backend/src/cost/__test__/cashflow-cost.service.spec.ts b/backend/src/cost/__test__/cashflow-cost.service.spec.ts index 8e9b33a5..ecd92146 100644 --- a/backend/src/cost/__test__/cashflow-cost.service.spec.ts +++ b/backend/src/cost/__test__/cashflow-cost.service.spec.ts @@ -547,12 +547,14 @@ describe('CostService', () => { ).rejects.toThrow('Cost with name Food not found'); }); - it('throws ConflictException when rename target already exists', async () => { + // --- Updated: was mocking ConditionalCheckFailedException directly which is wrong for transactWrite --- + it('throws ConflictException when rename target already exists (Put condition fails)', async () => { mockGetPromise.mockResolvedValue({ Item: { name: 'Food', amount: 200, type: CostType.MealsFood }, }); mockTransactWritePromise.mockRejectedValue({ - code: 'ConditionalCheckFailedException', + code: 'TransactionCanceledException', + CancellationReasons: [{ Code: 'ConditionalCheckFailed' }, { Code: 'None' }], }); await expect( @@ -573,6 +575,34 @@ describe('CostService', () => { ).rejects.toThrow('Cost with name Meals already exists'); }); + // --- New: covers the Delete condition failing mid-transaction --- + it('throws NotFoundException when rename source disappears mid-transaction (Delete condition fails)', async () => { + mockGetPromise.mockResolvedValue({ + Item: { name: 'Food', amount: 200, type: CostType.MealsFood }, + }); + mockTransactWritePromise.mockRejectedValue({ + code: 'TransactionCanceledException', + CancellationReasons: [{ Code: 'None' }, { Code: 'ConditionalCheckFailed' }], + }); + + await expect( + service.updateCost('Food', { + name: 'Meals', + amount: 300, + type: CostType.MealsFood, + date: '2026-03-22' as TDateISO, + }), + ).rejects.toThrow(NotFoundException); + await expect( + service.updateCost('Food', { + name: 'Meals', + amount: 300, + type: CostType.MealsFood, + date: '2026-03-22' as TDateISO, + }), + ).rejects.toThrow('Cost with name Food not found'); + }); + it('throws InternalServerErrorException on rename transaction error', async () => { mockGetPromise.mockResolvedValue({ Item: { name: 'Food', amount: 200, type: CostType.MealsFood },