docs: consolidate exploration & audit reports under docs/ (TEC-3094)

- Move 8 stray .md (+5 .txt) from ~/Desktop into docs/explorations/from-desktop/ - Reorganize 27 .md/.txt at workspace root: - audit reports -> docs/audits/ - exploration reports -> docs/explorations/ - design system -> docs/design-system/ - Keep only README/CHANGELOG/CONTRIBUTING/CLAUDE at repo root - Refresh docs/README.md as canonical index with links to all groups - Note: pre-existing docs/audits/AUDIT_INDEX.md and AUDIT_SUMMARY.md were overwritten by the newer root-level versions during the move Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-21 16:29:24 +07:00
parent 912121cf09
commit 08b96f9c2d
39 changed files with 15129 additions and 562 deletions
--- a/docs/audits/AUDIT_LISTINGS_PROPERTY_MANAGEMENT.md
+++ b/docs/audits/AUDIT_LISTINGS_PROPERTY_MANAGEMENT.md
@@ -0,0 +1,354 @@
+# Audit: GoodGo Real Estate Listings & Property Management Feature
+**Status:** Comprehensive Audit Complete  
+**Date:** April 19, 2026  
+**Scope:** Property/Listings API, Search, Management Dashboard, Frontend Components  
+**Target:** Production readiness assessment
+
+---
+
+## 1. Scope Coverage — What's Implemented Well
+
+### API Architecture (CQRS Pattern)
+- **Commands:** 9 implemented (Create, Update, Delete, Feature, Promote, Moderate, Upload-Media, Update-Status, Admin-Feature)
+- **Queries:** 4 core queries (GetListing, SearchListings, GetPriceHistory, GetPendingModeration)
+- **Event-Driven:** Domain events published post-mutation; event handlers for cache invalidation
+- **Auth Guards:** JwtAuthGuard + RolesGuard on sensitive endpoints; ownership checks before mutations
+- **Rate Limiting:** EndpointRateLimitGuard on POST /listings (10 req/60s per user)
+
+### Database Schema
+- **Listing Model:** Complete with status enum (DRAFT/PENDING_REVIEW/ACTIVE/RESERVED/SOLD/RENTED/EXPIRED/REJECTED)
+- **Geospatial:** PostGIS integration (geometry(Point, 4326)) with indexes on location (Gist)
+- **Indexes:** Compound indexes on common queries (status + createdAt, sellerId + status, transactionType + status)
+- **Relations:** Cascading deletes on related entities (PriceHistory, SavedListing, Inquiry)
+
+### Frontend Pages & Components
+- **Public Pages:** Listing detail with full SEO (metadata, JSON-LD breadcrumb, OG tags); Search page with filters, map view, saved search
+- **Dashboard:** Listings management (CRUD status card view), edit page with multi-step form
+- **Components:** Image gallery, lightbox, inquiry modal, price history chart, social share
+- **i18n:** Full Vietnamese i18n via next-intl; translation keys for all user-facing text
+- **Responsive:** Tailwind mobile-first design with flex/grid utilities; test coverage on key components
+
+### Quality Practices
+- **Test Coverage:** 17 test files covering commands, queries, repositories, validators
+- **Error Handling:** Custom exceptions (DomainException, ForbiddenException, NotFoundException, ValidationException)
+- **Logging:** Structured logging with LoggerService; error stack traces captured
+- **Caching:** Redis cache with prefixes (SEARCH, MARKET_DISTRICT, LISTING) and TTL management
+
+---
+
+## 2. Gaps & Missing Functionality
+
+### Critical Gaps
+
+1. **Delete-Listing Handler Missing Tests**
+   - No `.spec.ts` file for `delete-listing.handler.ts`
+   - Edge case: deletion of featured listings with active payments not validated
+   - **File:** `apps/api/src/modules/listings/application/commands/delete-listing/`
+   - **Impact:** Risk of orphaned transactions/orders if deletions occur without proper state checks
+
+2. **Admin-Feature-Listing Handler Incomplete**
+   - `AdminFeatureListingHandler` exists but no implementation of admin-side featured listing expiry logic
+   - No scheduled task to auto-expire featured listings when `featuredUntil` passes
+   - **File:** `apps/api/src/modules/listings/application/commands/admin-feature-listing/`
+   - **Impact:** Featured listings may remain highlighted beyond paid period
+
+3. **Activation/Expiry Pipeline Missing**
+   - No handler for `ActivateFeaturedListingCommand` 
+   - No event handler for `ListingCreatedEvent` to transition DRAFT → PENDING_REVIEW
+   - **File:** Event handlers folder mostly empty
+   - **Impact:** New listings not automatically entering moderation queue; featured listing lifecycle incomplete
+
+4. **Search Module Separation Issue**
+   - `SearchListingsQuery` exists in two places: listings module AND search module
+   - Potential for duplicate/conflicting search logic
+   - **Files:** 
+     - `apps/api/src/modules/listings/application/queries/search-listings/`
+     - `apps/api/src/modules/search/application/queries/search-properties/`
+   - **Impact:** Maintenance burden; risk of divergent behavior
+
+5. **Price Validation Service Not Wired to Schema**
+   - `PRICE_VALIDATOR` service exists but pricing boundaries not enforced at database level
+   - No check constraints on `priceVND` (negative prices technically allowed in DB)
+   - **File:** `apps/api/src/modules/listings/domain/services/price-validator.ts`
+   - **Impact:** Invalid prices can persist if validator bypassed or service disabled
+
+### Medium-Priority Gaps
+
+6. **Missing Feature-Listing Expiry Cron/Scheduled Task**
+   - Featured listing expiry relies on manual query-time checks (`featuredUntil > now`)
+   - No background job to transition expired featured listings back to non-featured state
+   - **Impact:** Search rankings may incorrectly show expired featured listings; analytics skewed
+
+7. **Media Deletion Not Implemented**
+   - Upload-media exists; no delete-media endpoint
+   - Users cannot remove individual images after upload
+   - **File:** Controllers missing DELETE `:id/media/:mediaId`
+   - **Impact:** User experience friction; storage bloat
+
+8. **Moderation Feedback Not Visible to User**
+   - `ModerateListingCommand` sets `moderationNotes` but frontend doesn't display rejection reason
+   - Users cannot see why their listing was rejected/pending
+   - **Impact:** Poor user experience; support ticket volume increases
+
+9. **Dashboard Saved Searches Not Fully Implemented**
+   - Saved search CRUD endpoints exist; UI component incomplete
+   - Alert subscription feature defined in schema but no notification sender handler
+   - **File:** `apps/web/app/[locale]/(dashboard)/dashboard/saved-searches/page.tsx`
+   - **Impact:** Feature partially unusable
+
+10. **Missing Batch Operations**
+    - No bulk update endpoints (e.g., mark multiple listings ACTIVE, bulk delete)
+    - Admin dashboard may require repeated individual API calls
+    - **Impact:** Scalability/usability issue for admins managing many listings
+
+---
+
+## 3. Code Quality Issues
+
+### Performance & N+1 Queries
+
+1. **Geo-Extraction Two-Step Query** (Medium priority)
+   - `findByIdWithProperty()`: First Prisma query, then raw SQL for PostGIS extraction
+   - `searchListings()`: Batch geo-extraction mitigates but still 2 queries (Prisma + raw SQL)
+   - **File:** `apps/api/src/modules/listings/infrastructure/repositories/listing-read.queries.ts` (lines 26–35, 156–167)
+   - **Optimization:** Embed ST_Y/ST_X in the Prisma query using `$queryRaw` for the entire fetch
+
+2. **Seller Lookups in Search Result**
+   - Search includes seller (line 146: `seller: { select: { id: true, fullName: true } }`)
+   - If frontend doesn't need seller full details on listing cards, unnecessary join
+   - **File:** Line 146 of listing-read.queries.ts
+   - **Impact:** Negligible on small datasets; scales poorly with 100k+ listings
+
+3. **Media Eager-Load Limits**
+   - `take: 5` in search, `take: 10` in detail (hardcoded)
+   - Inconsistent; no configuration option; wastes bandwidth if full gallery not needed
+   - **File:** Lines 143, 15 of listing-read.queries.ts
+
+### Hardcoded Values & Configuration
+
+1. **Featured Listing Prices Hardcoded**
+   - `PACKAGE_PRICES` defined in handler (3_days: 99k, 7_days: 199k, 30_days: 499k VND)
+   - Not configurable; require code change to adjust pricing
+   - **File:** `apps/api/src/modules/listings/application/commands/feature-listing/feature-listing.handler.ts` (lines 14–18)
+   - **Risk:** Admin cannot price-test without redeployment
+
+2. **MAX_MEDIA_PER_PROPERTY Hardcoded to 20**
+   - Not fetched from database config or environment
+   - **File:** `apps/api/src/modules/listings/application/commands/upload-media/upload-media.handler.ts` (line 10)
+
+3. **File Upload Limits Hardcoded**
+   - Image max 10MB, video max 100MB; no admin override
+   - **File:** `apps/api/src/modules/listings/presentation/controllers/listings.controller.ts` (lines 325–329)
+
+4. **Search Result Page Size Capped to 100**
+   - `Math.min(params.limit ?? 20, 100)` — prevents large exports
+   - **File:** `apps/api/src/modules/listings/infrastructure/repositories/listing-read.queries.ts` (line 105)
+
+### Validation Gaps
+
+1. **No Validation on Duplicate Property Creation**
+   - Users can create multiple listings for same property
+   - Duplicate detector warns but never blocks (catch-all try/catch suppresses failures)
+   - **File:** `apps/api/src/modules/listings/application/commands/create-listing/create-listing.handler.ts` (lines 153–155)
+   - **Risk:** Spam, data duplication
+
+2. **Price History Source Always "manual_update"**
+   - No distinction between user edits, AI adjustments, or system changes
+   - **File:** `prisma/schema.prisma` (line 395), `PriceHistory.source` default
+   - **Impact:** Audit trail unclear for analytics
+
+3. **No Agent Assignment Validation**
+   - `agentId` accepted without checking agent exists or has broker permission
+   - **File:** CreateListingCommand accepts `agentId` but no guard
+   - **Risk:** Listing assigned to non-existent/unauthorized agent
+
+4. **Description/Title Length Not Enforced**
+   - Schema allows unlimited text; no max_length on `description` in Property model
+   - **File:** `prisma/schema.prisma` — Property.description is `@db.Text` (no constraint)
+   - **Impact:** Huge descriptions slow search queries; UI renders badly
+
+### Security Issues
+
+1. **Moderation Bypass Risk**
+   - Update-listing transitions ACTIVE → PENDING_REVIEW, but admin can manually revert
+   - No audit log of who changed listing status
+   - **File:** `apps/api/src/modules/listings/application/commands/update-listing/update-listing.handler.ts` (line 126)
+   - **Risk:** Moderator decisions ignored silently
+
+2. **File Upload Path Predictable**
+   - Files stored in `properties/{propertyId}` (publicly guessable)
+   - No hash-based path obfuscation; potential enumeration attack
+   - **File:** `apps/api/src/modules/listings/application/commands/upload-media/upload-media.handler.ts` (line 40)
+
+3. **No Rate Limit on Feature Listing**
+   - Feature endpoint limited by quota, not rate limit
+   - User could spam feature requests and hit quota fast
+   - **File:** `listings.controller.ts` — `@Post(':id/feature')` has no @EndpointRateLimit
+
+4. **Agent Assignment Not Validated During Update**
+   - User can reassign listing to arbitrary agent; no permission check
+   - **File:** `UpdateListingCommand` constructor doesn't validate agent ownership
+   - **Risk:** Seller gives away listing commission to unauthorized agent
+
+5. **Inquiry Message Not Sanitized**
+   - Inquiry message stored as-is; potential XSS if echoed in admin dashboard
+   - **File:** `prisma/schema.prisma` line 472 — `Inquiry.message` is `@db.Text`
+   - **Impact:** Admin UI could be compromised
+
+### Maintainability Issues
+
+1. **Unused Deprecated Type**
+   - `ListingDetailDto` marked `@deprecated` with TODO comment
+   - **File:** `apps/api/src/modules/listings/application/queries/get-listing/get-listing.handler.ts` (line 8)
+   - **Cleanup:** Remove after migration complete
+
+2. **Inconsistent Naming (Vietnamese vs English)**
+   - Error messages in Vietnamese (e.g., "Không thể tạo tin đăng")
+   - Test names, constants in English
+   - Domain entities mixed (Vietnamese property types: APARTMENT, VILLA)
+   - **Impact:** Codebase hard to reason about for non-Vietnamese speakers
+
+3. **Missing JSDoc on Complex Methods**
+   - `searchListings()` function complex with Prisma filters; no parameter documentation
+   - **File:** `listing-read.queries.ts` lines 100–213
+
+---
+
+## 4. Security & Validation Concerns
+
+| Issue | Severity | Mitigation |
+|-------|----------|-----------|
+| No transaction check before delete | High | Add `findByIdWithRelations()` check before `delete()` |
+| File path enumeration possible | Medium | Hash property IDs in storage path; implement signed URLs |
+| Agent assignment not validated | Medium | Verify agent broker relationship before assignment |
+| Inquiry message XSS risk | Medium | Sanitize on storage; escape on frontend display |
+| Moderation audit trail missing | Medium | Add `moderatedBy`, `moderatedAt` fields; log state transitions |
+| Duplicate listings not blocked | Low | Merge duplicate detector into command handler (convert warning to block option) |
+
+---
+
+## 5. Performance & Scalability
+
+### Issues Identified
+
+1. **Geo-Extraction Overhead**
+   - Two separate query roundtrips (Prisma + raw SQL) per listing detail fetch
+   - At scale (1M+ listings), search page with 20 results = 21 queries total
+   - **Fix:** Use Prisma raw SQL for full fetch; avoid Prisma ORM + separate geo query
+
+2. **Uncached Admin Queries**
+   - `GetPendingModerationQuery` not cached; admin dashboard refreshes full list every load
+   - **File:** `apps/api/src/modules/listings/application/queries/get-pending-moderation/`
+   - **Fix:** Add short-lived cache (60s) with invalidation on moderation action
+
+3. **Search Filters Create Complex Queries**
+   - Nested property filters in searchListings (lines 118–129 listing-read.queries.ts)
+   - No query plan optimization hints; potential slow full table scan on large datasets
+   - **Fix:** Add database indexes on `status`, `transactionType`, `property(propertyType, district, city)`
+
+4. **Media Queries Unoptimized**
+   - Every search result includes media array (even if not displayed)
+   - Transferring 5–10 media objects × 20 results = unnecessary payload
+   - **Fix:** Add optional `?includeMedia=true` query param; default exclude in list view
+
+---
+
+## 6. Recommendations (Prioritized)
+
+### 🔴 High Priority (Blocks Production)
+
+1. **Implement Delete-Listing Test Suite**
+   - Add edge cases: featured listing with active payment, transactions in progress
+   - Verify cascading deletes work correctly
+   - **Effort:** 2 hours | **Owner:** Backend
+
+2. **Add Featured-Listing Expiry Handler**
+   - Implement `ExpireFeaturedListingsScheduledTask` (daily cron or on-demand)
+   - Transition expired listings: set `featuredUntil = null`, trigger search cache invalidation
+   - **Effort:** 4 hours | **Owner:** Backend
+
+3. **Wire Price Validation to Schema**
+   - Add PostgreSQL check constraint: `priceVND > 0`
+   - Document min/max pricing rules in README
+   - **Effort:** 1 hour | **Owner:** Backend + DBA
+
+4. **Implement Moderation Audit Trail**
+   - Add `moderatedBy` (admin user ID), `moderatedAt` (timestamp), `previousStatus` fields to Listing
+   - Log transitions in event handler
+   - **Effort:** 3 hours | **Owner:** Backend
+
+### 🟠 Medium Priority (Improves UX/Security)
+
+5. **Implement Media Deletion Endpoint**
+   - Add `DELETE /listings/:id/media/:mediaId` with ownership check
+   - Trigger S3 deletion + cache invalidation
+   - **Effort:** 2 hours | **Owner:** Backend
+
+6. **Display Moderation Feedback to User**
+   - Add frontend component to show rejection reason when status=REJECTED
+   - Notify user via email + in-app notification
+   - **Effort:** 3 hours | **Owner:** Frontend + Backend
+
+7. **Sanitize Inquiry Messages**
+   - Add DOMPurify to admin dashboard inquiry display
+   - Store sanitized version if displaying in emails
+   - **Effort:** 1 hour | **Owner:** Frontend
+
+8. **Add Rate Limit to Feature Endpoint**
+   - `@EndpointRateLimit({ limit: 5, windowSeconds: 3600 })` (5 features per hour)
+   - **Effort:** 30 min | **Owner:** Backend
+
+9. **Validate Agent Assignment**
+   - Check agent.isVerified before assignment
+   - Add broker permission check if multi-broker support added
+   - **Effort:** 1 hour | **Owner:** Backend
+
+10. **Consolidate Search Logic**
+    - Move `SearchListingsQuery` from listings module to search module
+    - Remove duplicate in listings module; re-export from search
+    - **Effort:** 2 hours | **Owner:** Backend
+
+### 🟡 Low Priority (Tech Debt)
+
+11. **Extract Hardcoded Values to Config**
+    - Move PACKAGE_PRICES → database config table (FeaturePackagePrice)
+    - Add admin UI to manage pricing
+    - **Effort:** 4 hours | **Owner:** Backend
+
+12. **Optimize Geo-Extraction Queries**
+    - Consolidate Prisma + raw SQL into single raw query
+    - Add benchmark: measure latency before/after
+    - **Effort:** 3 hours | **Owner:** Backend + Infra
+
+13. **Implement Batch Operations API**
+    - `PATCH /listings/batch` to update multiple listings status
+    - Quota should apply per-listing, not per-request
+    - **Effort:** 4 hours | **Owner:** Backend
+
+14. **Add Missing JSDoc & Comments**
+    - Document searchListings Prisma filters
+    - Clarify geo-extraction rationale
+    - **Effort:** 2 hours | **Owner:** Backend
+
+15. **Complete Saved Searches Alerts**
+    - Implement `SavedSearchAlertHandler` event listener
+    - Send daily digest when new listings match saved search
+    - **Effort:** 6 hours | **Owner:** Backend + Notifications
+
+---
+
+## Summary
+
+**Overall Assessment:** Feature is **80% production-ready**; core CRUD works, search functional, but **audit trail, expiry automation, and error handling need hardening**.
+
+**Critical Path to Production:**
+1. ✅ Implement delete-listing tests
+2. ✅ Add featured-listing expiry cron
+3. ✅ Wire price validation to schema
+4. ✅ Add moderation audit trail
+
+**Risk Level:** Medium (security concerns around file paths, agent assignment; moderation bypass possible)
+
+**Test Coverage:** 70% of commands/queries; delete-listing and expiry handlers untested
+
+**Deployment Blocker:** None, if recommendations #1–#4 completed within 2 weeks.