diff --git a/backend/src/inventory/inventory.service.ts b/backend/src/inventory/inventory.service.ts index 86617173..d8f854ac 100644 --- a/backend/src/inventory/inventory.service.ts +++ b/backend/src/inventory/inventory.service.ts @@ -14,6 +14,26 @@ type InventoryQuery = { export class InventoryService { constructor(private prisma: PrismaService) {} + private throwInventoryItemNotFound(id: number): never { + throw new NotFoundException(`Inventory item with id ${id} not found`); + } + + private async findInventoryItemByIdOrThrow(id: number) { + const existing = await this.prisma.inventoryItem.findUnique({ where: { id } }); + if (!existing) { + this.throwInventoryItemNotFound(id); + } + return existing; + } + + private async ensureProductExists(productId: number) { + const product = await this.prisma.product.findUnique({ where: { id: productId } }); + if (!product) { + throw new NotFoundException('Product not found'); + } + return product; + } + async findAll(query?: InventoryQuery) { const where: Prisma.InventoryItemWhereInput = {}; const orderBy: Prisma.InventoryItemOrderByWithRelationInput[] = []; @@ -46,16 +66,7 @@ export class InventoryService { } async consume(id: number, data: ConsumeInventoryDto) { - const existing = await this.prisma.inventoryItem.findUnique({ - where: { id }, - include: { - product: true, - }, - }); - - if (!existing) { - throw new NotFoundException(`Inventory item with id ${id} not found`); - } + const existing = await this.findInventoryItemByIdOrThrow(id); const currentQuantity = Number(existing.quantity); const newQuantity = Math.max(0, currentQuantity - data.amountUsed); @@ -84,13 +95,7 @@ export class InventoryService { } async findConsumptionHistory(id: number) { - const existing = await this.prisma.inventoryItem.findUnique({ - where: { id }, - }); - - if (!existing) { - throw new NotFoundException(`Inventory item with id ${id} not found`); - } + await this.findInventoryItemByIdOrThrow(id); return this.prisma.inventoryConsumption.findMany({ where: { @@ -129,13 +134,7 @@ export class InventoryService { } async create(data: CreateInventoryDto) { - const product = await this.prisma.product.findUnique({ - where: { id: data.productId }, - }); - - if (!product) { - throw new NotFoundException('Product not found'); - } + await this.ensureProductExists(data.productId); return this.prisma.inventoryItem.create({ data: { @@ -161,22 +160,10 @@ export class InventoryService { } async update(id: number, data: UpdateInventoryDto) { - const existing = await this.prisma.inventoryItem.findUnique({ - where: { id }, - }); - - if (!existing) { - throw new NotFoundException(`Inventory item with id ${id} not found`); - } + await this.findInventoryItemByIdOrThrow(id); if (typeof data.productId === 'number') { - const product = await this.prisma.product.findUnique({ - where: { id: data.productId }, - }); - - if (!product) { - throw new NotFoundException('Product not found'); - } + await this.ensureProductExists(data.productId); } const updateData: Prisma.InventoryItemUpdateInput = {}; @@ -241,10 +228,7 @@ export class InventoryService { } async remove(id: number) { - const existing = await this.prisma.inventoryItem.findUnique({ where: { id } }); - if (!existing) { - throw new NotFoundException(`Inventory item with id ${id} not found`); - } + await this.findInventoryItemByIdOrThrow(id); return this.prisma.inventoryItem.delete({ where: { id } }); } } \ No newline at end of file diff --git a/backend/src/meal-plan/meal-plan.service.ts b/backend/src/meal-plan/meal-plan.service.ts index 44d78436..a8ed5a6c 100644 --- a/backend/src/meal-plan/meal-plan.service.ts +++ b/backend/src/meal-plan/meal-plan.service.ts @@ -68,11 +68,8 @@ export class MealPlanService { return this.prisma.mealPlanEntry.delete({ where: { id: entry.id } }); } - /** Samlad ingredienslista för ett datumintervall */ - async shoppingList(userId: number, from: string, to: string) { - const entries = await this.findByRange(userId, from, to); - - // Summera ingredienser per produkt+enhet (skalat per portionsantal) + /** Aggregerar ingredienser per produkt+enhet från matplansposter, skalat per portionsantal */ + private aggregateIngredients(entries: Awaited>) { const map = new Map(); for (const entry of entries) { const recipeServings = (entry.recipe as any).servings as number | null; @@ -80,8 +77,8 @@ export class MealPlanService { const scale = recipeServings && entryServings ? entryServings / recipeServings : 1; for (const ing of entry.recipe.ingredients) { const key = `${ing.product.id}-${ing.unit}`; - const existing = map.get(key); const qty = Number(ing.quantity) * scale; + const existing = map.get(key); if (existing) { existing.quantity += qty; } else { @@ -94,8 +91,13 @@ export class MealPlanService { } } } + return Array.from(map.values()); + } - return Array.from(map.values()).sort((a, b) => a.name.localeCompare(b.name, 'sv')); + /** Samlad ingredienslista för ett datumintervall */ + async shoppingList(userId: number, from: string, to: string) { + const entries = await this.findByRange(userId, from, to); + return this.aggregateIngredients(entries).sort((a, b) => a.name.localeCompare(b.name, 'sv')); } /** Jämför veckans ingrediensbehov mot inventariet */ @@ -109,32 +111,16 @@ export class MealPlanService { }); const pantryProductIds = new Set(pantryItems.map((p) => p.productId)); - // Aggregera ingredienser per produkt+enhet (skalat per portionsantal) - const map = new Map(); - for (const entry of entries) { - const recipeServings = (entry.recipe as any).servings as number | null; - const entryServings = (entry as any).servings as number | null; - const scale = recipeServings && entryServings ? entryServings / recipeServings : 1; - for (const ing of entry.recipe.ingredients) { - const key = `${ing.product.id}-${ing.unit}`; - const qty = Number(ing.quantity) * scale; - const existing = map.get(key); - if (existing) { - existing.required += qty; - } else { - map.set(key, { - productId: ing.product.id, - name: ing.product.canonicalName || ing.product.name, - required: qty, - unit: ing.unit, - }); - } - } - } + const aggregated = this.aggregateIngredients(entries).map((item) => ({ + productId: item.productId, + name: item.name, + required: item.quantity, + unit: item.unit, + })); // Kontrollera inventariet för varje ingrediens const result = await Promise.all( - Array.from(map.values()).map(async (item) => { + aggregated.map(async (item) => { // Pantry-varor anses alltid tillgängliga — visa inte i inköpslistan if (pantryProductIds.has(item.productId)) { return { diff --git a/backend/src/products/products.service.ts b/backend/src/products/products.service.ts index 4cb83384..f8879e3a 100644 --- a/backend/src/products/products.service.ts +++ b/backend/src/products/products.service.ts @@ -236,10 +236,7 @@ export class ProductsService { } async permanentDelete(id: number) { - const product = await this.prisma.product.findUnique({ where: { id } }); - if (!product) { - throw new NotFoundException(`Product with id ${id} not found`); - } + await this.findOne(id); // Ta bort beroenden först await this.prisma.productTag.deleteMany({ where: { productId: id } }); await this.prisma.userProduct.deleteMany({ where: { productId: id } }); @@ -262,30 +259,21 @@ export class ProductsService { }); } + private async findProductByIdOrThrow(id: number, label: string) { + const product = await this.prisma.product.findUnique({ where: { id } }); + if (!product) { + throw new NotFoundException(`${label} product with id ${id} not found`); + } + return product; + } + async merge(sourceProductId: number, targetProductId: number) { if (sourceProductId === targetProductId) { throw new Error('sourceProductId och targetProductId kan inte vara samma'); } - const source = await this.prisma.product.findUnique({ - where: { id: sourceProductId }, - }); - - if (!source) { - throw new NotFoundException( - `Source product with id ${sourceProductId} not found`, - ); - } - - const target = await this.prisma.product.findUnique({ - where: { id: targetProductId }, - }); - - if (!target) { - throw new NotFoundException( - `Target product with id ${targetProductId} not found`, - ); - } + const source = await this.findProductByIdOrThrow(sourceProductId, 'Source'); + const target = await this.findProductByIdOrThrow(targetProductId, 'Target'); return this.prisma.$transaction(async (tx) => { const movedInventoryCount = await tx.inventoryItem.updateMany({ @@ -318,12 +306,8 @@ export class ProductsService { const [source, target, sourceInventoryCount, targetInventoryCount] = await Promise.all([ - this.prisma.product.findUnique({ - where: { id: sourceProductId }, - }), - this.prisma.product.findUnique({ - where: { id: targetProductId }, - }), + this.findProductByIdOrThrow(sourceProductId, 'Source'), + this.findProductByIdOrThrow(targetProductId, 'Target'), this.prisma.inventoryItem.count({ where: { productId: sourceProductId }, }), @@ -332,18 +316,6 @@ export class ProductsService { }), ]); - if (!source) { - throw new NotFoundException( - `Source product with id ${sourceProductId} not found`, - ); - } - - if (!target) { - throw new NotFoundException( - `Target product with id ${targetProductId} not found`, - ); - } - return { source: { ...source, diff --git a/backend/src/receipt-alias/receipt-alias.service.ts b/backend/src/receipt-alias/receipt-alias.service.ts index f1a5e820..7613806e 100644 --- a/backend/src/receipt-alias/receipt-alias.service.ts +++ b/backend/src/receipt-alias/receipt-alias.service.ts @@ -1,4 +1,4 @@ -import { ForbiddenException, Injectable } from '@nestjs/common'; +import { ForbiddenException, Injectable, NotFoundException } from '@nestjs/common'; import { PrismaService } from '../prisma/prisma.service'; import { CreateReceiptAliasDto } from './dto/create-receipt-alias.dto'; @@ -25,59 +25,48 @@ export class ReceiptAliasService { async upsert(dto: CreateReceiptAliasDto, userId: number, role: string) { const normalized = dto.receiptName.toLowerCase().trim(); - const wantsGlobal = dto.isGlobal === true; + if (wantsGlobal && role !== 'admin') { throw new ForbiddenException('Endast admin kan skapa globala alias'); } - if (wantsGlobal) { - const existing = await this.prisma.receiptAlias.findFirst({ - where: { receiptName: normalized, isGlobal: true }, - }); - - if (existing) { - return this.prisma.receiptAlias.update({ - where: { id: existing.id }, - data: { productId: dto.productId }, - }); - } - - return this.prisma.receiptAlias.create({ - data: { - receiptName: normalized, - productId: dto.productId, - isGlobal: true, - ownerId: null, - }, - }); - } + return this.upsertAliasRecord( + normalized, + dto.productId, + wantsGlobal ? null : userId, + wantsGlobal, + ); + } + private async upsertAliasRecord( + receiptName: string, + productId: number, + ownerId: number | null, + isGlobal: boolean, + ) { const existing = await this.prisma.receiptAlias.findFirst({ - where: { receiptName: normalized, ownerId: userId, isGlobal: false }, + where: isGlobal + ? { receiptName, isGlobal: true } + : { receiptName, ownerId, isGlobal: false }, }); if (existing) { return this.prisma.receiptAlias.update({ where: { id: existing.id }, - data: { productId: dto.productId }, + data: { productId }, }); } return this.prisma.receiptAlias.create({ - data: { - receiptName: normalized, - productId: dto.productId, - ownerId: userId, - isGlobal: false, - }, + data: { receiptName, productId, ownerId, isGlobal }, }); } async remove(id: number, userId: number, role: string) { const alias = await this.prisma.receiptAlias.findUnique({ where: { id } }); if (!alias) { - return this.prisma.receiptAlias.delete({ where: { id } }); + throw new NotFoundException(`Aliaspost med id ${id} hittades inte`); } const canDelete = diff --git a/backend/src/recipes/recipes.service.ts b/backend/src/recipes/recipes.service.ts index 41a2b9c0..70bee5b7 100644 --- a/backend/src/recipes/recipes.service.ts +++ b/backend/src/recipes/recipes.service.ts @@ -31,6 +31,31 @@ export class RecipesService { constructor(private readonly prisma: PrismaService) {} + private throwRecipeNotFound(id: number): never { + throw new NotFoundException(`Recipe with id ${id} not found`); + } + + private async findRecipeByIdOrThrow(id: number) { + const recipe = await this.prisma.recipe.findUnique({ where: { id } }); + if (!recipe) { + this.throwRecipeNotFound(id); + } + return recipe; + } + + private assertRecipeEditableByUser(recipe: { ownerId: number | null }, userId: number, id: number) { + // Legacy behavior: ownerless recipes are editable to preserve existing semantics. + if (recipe.ownerId !== null && recipe.ownerId !== userId) { + this.throwRecipeNotFound(id); + } + } + + private assertRecipeOwnedByUser(recipe: { ownerId: number | null }, userId: number, id: number) { + if (recipe.ownerId !== userId) { + this.throwRecipeNotFound(id); + } + } + async getInventoryPreview(id: number, userId: number) { const recipe = await this.prisma.recipe.findFirst({ where: { @@ -218,19 +243,8 @@ export class RecipesService { } async update(id: number, updateRecipeDto: CreateRecipeDto, userId: number) { - // Verifiera att receptet finns och att användaren äger det - const existingRecipe = await this.prisma.recipe.findUnique({ - where: { id }, - }); - - if (!existingRecipe) { - throw new NotFoundException(`Recipe with id ${id} not found`); - } - - // Tillåt uppdatering om användaren är ägare ELLER om receptet är publikt utan ägare - if (existingRecipe.ownerId !== null && existingRecipe.ownerId !== userId) { - throw new NotFoundException(`Recipe with id ${id} not found`); - } + const existingRecipe = await this.findRecipeByIdOrThrow(id); + this.assertRecipeEditableByUser(existingRecipe, userId, id); // Ta bort gamla ingredienser await this.prisma.recipeIngredient.deleteMany({ @@ -269,17 +283,8 @@ export class RecipesService { } async remove(id: number, userId: number) { - const existingRecipe = await this.prisma.recipe.findUnique({ - where: { id }, - }); - - if (!existingRecipe) { - throw new NotFoundException(`Recipe with id ${id} not found`); - } - - if (existingRecipe.ownerId !== null && existingRecipe.ownerId !== userId) { - throw new NotFoundException(`Recipe with id ${id} not found`); - } + const existingRecipe = await this.findRecipeByIdOrThrow(id); + this.assertRecipeEditableByUser(existingRecipe, userId, id); await this.prisma.recipeIngredient.deleteMany({ where: { recipeId: id } }); await this.prisma.recipe.delete({ where: { id } }); @@ -295,13 +300,8 @@ export class RecipesService { } async updateImage(id: number, sourceUrl: string, userId: number) { - const existingRecipe = await this.prisma.recipe.findUnique({ where: { id } }); - if (!existingRecipe) { - throw new NotFoundException(`Recipe with id ${id} not found`); - } - if (existingRecipe.ownerId !== userId) { - throw new NotFoundException(`Recipe with id ${id} not found`); - } + const existingRecipe = await this.findRecipeByIdOrThrow(id); + this.assertRecipeOwnedByUser(existingRecipe, userId, id); const imageUrl = await downloadAndOptimizeImage(sourceUrl, IMAGE_DEST_DIR); @@ -317,10 +317,8 @@ export class RecipesService { } async setVisibility(id: number, userId: number, isPublic: boolean) { - const existingRecipe = await this.prisma.recipe.findUnique({ where: { id } }); - if (!existingRecipe || existingRecipe.ownerId !== userId) { - throw new NotFoundException(`Recipe with id ${id} not found`); - } + const existingRecipe = await this.findRecipeByIdOrThrow(id); + this.assertRecipeOwnedByUser(existingRecipe, userId, id); if (isPublic) { const owner = await this.prisma.user.findUnique({ @@ -344,13 +342,8 @@ export class RecipesService { } async shareWithUser(id: number, ownerId: number, username: string) { - const recipe = await this.prisma.recipe.findUnique({ - where: { id }, - select: { id: true, ownerId: true }, - }); - if (!recipe || recipe.ownerId !== ownerId) { - throw new NotFoundException(`Recipe with id ${id} not found`); - } + const recipe = await this.findRecipeByIdOrThrow(id); + this.assertRecipeOwnedByUser(recipe, ownerId, id); const owner = await this.prisma.user.findUnique({ where: { id: ownerId }, @@ -381,13 +374,8 @@ export class RecipesService { } async unshareWithUser(id: number, ownerId: number, username: string) { - const recipe = await this.prisma.recipe.findUnique({ - where: { id }, - select: { id: true, ownerId: true }, - }); - if (!recipe || recipe.ownerId !== ownerId) { - throw new NotFoundException(`Recipe with id ${id} not found`); - } + const recipe = await this.findRecipeByIdOrThrow(id); + this.assertRecipeOwnedByUser(recipe, ownerId, id); const targetUser = await this.prisma.user.findUnique({ where: { username }, diff --git a/review_backend.md b/review_backend.md new file mode 100644 index 00000000..c575269a --- /dev/null +++ b/review_backend.md @@ -0,0 +1,99 @@ +# Plan för systematisk backend-review och optimering + +## Mål +1. Minska komplexitet och duplicering. +2. Förbättra prestanda och stabilitet. +3. Göra koden enklare att underhålla och vidareutveckla. +4. Införa kvalitetsgrindar så förbättringar håller över tid. + +## Arbetssätt +1. Jobba i små, säkra iterationer per modul/domän. +2. Mät före och efter varje förändring. +3. Lås upp förbättringar med tester och CI-gates. +4. Prioritera förändringar med hög effekt och låg risk först. + +## Fas 1: Baslinje och kartläggning (1 vecka) +1. Inventera backend per modul: +- Endpoints, tjänster, databasaccess, externa integrationer. +2. Sätt baslinjemätningar: +- Responstider per kritisk endpoint (p50/p95), felgrad, DB-latens. +- Nuvarande testtäckning per modul. +3. Skapa hotspot-lista: +- Långa metoder, hög cyclomatic complexity, duplicerad logik, N+1-frågor. +4. Leverabel: +- Prioriterad backlog med topp 10 förbättringsområden. + +## Fas 2: Snabba vinster och kodhygien (1-2 veckor) +1. Standardisera felhantering: +- Enhetlig exception mapping och API-felmodell. +2. Rensa duplicerad kod: +- Flytta gemensam logik till tydliga utilities/domänservices. +3. Förbättra validering: +- Konsekvent DTO/valideringslager in och ut. +4. Inför striktare lint-regler: +- Max function length, complexity-tak, no-dead-code. +5. Leverabel: +- Minskad kodvolym i hotspots och jämnare kodstandard. + +## Fas 3: Arkitektur-förenkling (2-3 veckor) +1. Tydlig separering av lager: +- Controller = transport. +- Service = affärslogik. +- Repository/data layer = persistens. +2. Minska beroendekoppling: +- Ta bort korsberoenden mellan moduler. +3. Inför tydliga domängränser: +- En modul ska kunna förstås utan att läsa flera andra. +4. Leverabel: +- Enklare call-flöden och färre starkt kopplade beroenden. + +## Fas 4: Databas och prestanda (1-2 veckor) +1. Granska tunga queries: +- N+1, överhämtning, saknade index, ineffektiva joins. +2. Förbättra dataåtkomst: +- Standard för pagination/filtering. +- Selektiv hämtning av fält. +3. Caching där det är motiverat: +- Endast för dyra och frekventa läsningar. +4. Leverabel: +- Mätbar förbättring i p95 och minskad DB-belastning. + +## Fas 5: Teststrategi och regressionsskydd (1-2 veckor, löpande) +1. Lägg tester där risk och affärsvärde är högst: +- Kritiska use cases först. +2. Balans i testpyramiden: +- Fler enhetstester för domänlogik. +- Fokuserade integrations- och API-tester för flöden. +3. Kontrakttester för externa integrationer. +4. Leverabel: +- Högre täckning i kritiska moduler och färre regressionsbuggar. + +## Fas 6: Säkerhet och driftbarhet (parallellt) +1. Säkerhetsgranskning: +- Input-validering, auth/role-kontroller, secret-hantering. +2. Driftbarhet: +- Strukturerad loggning, korrelations-id, tydligare metrics. +3. Resiliens: +- Timeout/retry/circuit-breaker för externa beroenden. +4. Leverabel: +- Färre driftincidenter och enklare felsökning. + +## Kvalitetsgrindar i CI +1. Build och lint måste passera. +2. Tester måste passera. +3. Miniminivå för täckning i ändrade moduler. +4. Blockera PR vid ökad komplexitet över satt tröskel. +5. Enkel performance-smoke på kritiska endpoints. + +## Prioriteringsmodell för varje förbättring +1. Effekt: prestanda, stabilitet, underhållbarhet. +2. Risk: regressionsrisk och driftsrisk. +3. Insats: utvecklingstid. +4. Välj först: hög effekt + låg/medel risk + låg/medel insats. + +## Definition of Done +1. Kritiska endpoints har förbättrad p95. +2. Topp-hotspots är refaktorerade eller borttagna. +3. Kodduplicering reducerad i prioriterade moduler. +4. Testskydd finns för alla kritiska flöden. +5. CI-gates förhindrar att kvaliteten glider tillbaka.