feat: refactor inventory and recipe services for improved error handling and code reuse; add systematic backend review plan

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 } });
   }
 }

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<ReturnType<typeof this.findByRange>>) {
     const map = new Map<string, { productId: number; name: string; quantity: number; unit: string }>();
     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<string, { productId: number; name: string; required: number; unit: string }>();
-    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 {

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,

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) {
+    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, isGlobal: true },
+      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,
-          isGlobal: true,
-          ownerId: null,
-        },
-      });
-    }
-
-    const existing = await this.prisma.receiptAlias.findFirst({
-      where: { receiptName: normalized, ownerId: userId, isGlobal: false },
-    });
-
-    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,
-        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 =

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 },

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.