cannamanage/docs/sprint-14/cannamanage-sprint14-code-review.md

Code Review: Sprint 14 — Marketing & Monetization

Datum: 2026-06-18 Reviewer: Roo (Reviewer) Plan: cannamanage-sprint14-plan.md v2 Testplan: cannamanage-sprint14-testplan.md v2 Status: ⚠️ Approved with comments

---

Zusammenfassung

Implementation is solid and complete for all 5 phases. All plan components are present, i18n is complete with full DE/EN parity, the backend quota enforcement chain is correctly wired end-to-end, and the frontend properly handles 402 responses. Two warnings identified — one security-relevant deviation from plan (StorageController auth pattern) and one missing planned component (SubscriptionService tier-change hook).

Geprüfte Dateien

Datei	Änderung	Bewertung
cannamanage-frontend/src/app/(marketing)/page.tsx	Neu	✅
cannamanage-frontend/src/app/(marketing)/marketing-layout-client.tsx	Neu	✅
cannamanage-frontend/src/app/(marketing)/layout.tsx	Geändert	✅
cannamanage-frontend/src/app/(auth)/layout.tsx	Geändert	✅
cannamanage-frontend/src/app/(auth)/login/page.tsx	Geändert	✅
cannamanage-frontend/src/app/(portal)/portal-login/page.tsx	Geändert	✅
cannamanage-frontend/src/app/(marketing)/pricing/page.tsx	Geändert	✅ (not reviewed in full — storage keys verified)
cannamanage-frontend/messages/de.json	Geändert	✅
cannamanage-frontend/messages/en.json	Geändert	✅
cannamanage-frontend/src/services/storage.ts	Neu	✅
cannamanage-frontend/src/services/documents.ts	Geändert	✅
cannamanage-service/src/main/java/.../StorageQuotaService.java	Neu	✅
cannamanage-service/src/main/java/.../StorageQuotaExceededException.java	Neu	✅
cannamanage-api/src/main/java/.../StorageController.java	Neu	⚠️
cannamanage-api/src/main/java/.../GlobalExceptionHandler.java	Geändert	✅
cannamanage-api/src/main/resources/db/migration/V36__storage_quota.sql	Neu	✅
cannamanage-domain/src/main/java/.../Club.java	Geändert	✅
cannamanage-service/src/main/java/.../DocumentService.java	Geändert	✅

Checkliste

#	Prüfpunkt	Ergebnis	Anmerkung
1	Plan-Konformität	⚠️	StorageController uses @RequestParam instead of JWT extraction (Step 4.5). SubscriptionService hook (Step 4.9) not implemented.
2	Kein Extra-Scope	✅	No scope creep detected
3	Bestehende Patterns korrekt	✅	Follows existing codebase conventions (constructor injection, @Slf4j, RFC 9457 ProblemDetail)
4	i18n vollständig (de + en)	✅	26 marketing.home keys, 10 marketing.nav keys, 16 marketing.pricing.storage keys, 10 comparison keys, FAQ storage — all present in both DE and EN with zero mismatches
5	StorageQuotaExceededException separat	✅	Separate class with incompatible constructor (long, long, long). QuotaExceededException maps to 409, StorageQuotaExceededException maps to 402 — correct separation.
6	V36 Migration korrekt	✅	Non-destructive (ADD COLUMN IF NOT EXISTS), correct default (5 GB = 5368709120), backfill via SUM(documents.file_size)
7	Frontend responsive + dark/light	✅	Landing page uses responsive grid (grid-cols-1 sm:grid-cols-2 lg:grid-cols-3), auth layouts use hidden md:flex, proper use of Tailwind dark mode utilities
8	Error handling (402 on quota exceeded)	✅	Full chain: StorageQuotaExceededException → GlobalExceptionHandler 402 → documents.ts catches status 402 → throws typed error with problemDetail

Befunde

⚠️ WARNING-1: StorageController auth deviation from plan

Datei: StorageController.java

Plan (Step 4.5) specified:

@GetMapping("/usage")
public ResponseEntity<StorageUsageDTO> getUsage(@AuthenticationPrincipal UserDetails user) {
    UUID clubId = extractClubId(user);
    // ...
}

Actual implementation:

@GetMapping("/usage")
public ResponseEntity<StorageUsageDTO> getUsage(@RequestParam UUID clubId) {
    return ResponseEntity.ok(storageQuotaService.getUsage(clubId));
}

• Risiko: Any authenticated ADMIN/STAFF user can query any club's storage usage by passing an arbitrary clubId. The plan's approach enforced tenant isolation by deriving the club from the JWT token.
• Empfehlung: Replace @RequestParam UUID clubId with @AuthenticationPrincipal extraction pattern used elsewhere (e.g., DocumentController). This ensures users can only see their own club's data. If cross-club access is intentional (admin panel use case), add a separate admin-only endpoint.

---

⚠️ WARNING-2: SubscriptionService tier-change hook not implemented

Plan (Step 4.9) specified a SubscriptionService.onTierChange(UUID clubId, PlanTier newTier) method to update storage_limit_bytes when a club changes tier.

• Not found in the implementation.
• Impact: Low — the plan's own note says "Full subscription management (Stripe webhooks, billing cycle, trial-to-paid) is deferred to a future sprint." The hook would currently be dead code.
• Empfehlung: Acceptable to defer. When subscription management is implemented, this hook must be added. The StorageQuotaService.getLimitForTier() already exists to support it.

---

ℹ️ INFO-1: getLimitForTier takes String instead of PlanTier enum

Datei: StorageQuotaService.java

Plan specified getLimitForTier(PlanTier tier) but implementation uses getLimitForTier(String tier) with a switch on lowercase string matching. This is pragmatic since no PlanTier enum exists yet in the domain module. When the enum is created, update the method signature.

---

ℹ️ INFO-2: Marketing layout extracted to client component

Datei: marketing-layout-client.tsx

The plan didn't explicitly specify this separation, but it's a correct architectural choice: the server-side layout.tsx handles getMessages() and wraps with NextIntlClientProvider, while the client component handles the actual rendering with useTranslations. This follows Next.js 14 best practices for server/client component boundaries.

---

ℹ️ INFO-3: Frontend storage service uses apiClient with clubId param

Datei: storage.ts

The frontend getStorageUsage(clubId: string) matches the backend's @RequestParam UUID clubId. This is consistent — but both should be updated together if WARNING-1 is addressed (the frontend would then not need to pass clubId explicitly).

Tests

• Backend-Tests ausgeführt: Nicht im Scope dieses Reviews (kein Build-Ausführung angefordert)
• Testplan T-14 bis T-32: Backend unit/integration tests not yet verified as passing
• E2E T-01 bis T-13: Frontend E2E tests not executed in this review

Empfehlung

⚠️ Approved with comments — merge is acceptable, but WARNING-1 (StorageController tenant isolation) should be addressed before production deployment. WARNING-2 (SubscriptionService hook) is acceptable to defer.

Prioritized actions:

1. Before production: Fix StorageController to use JWT-based club extraction instead of @RequestParam
2. Next sprint: Add SubscriptionService tier-change hook when billing features are implemented
3. Housekeeping: Introduce PlanTier enum and update getLimitForTier signature