Some checks failed
CI / Lint → Typecheck → Test → Build (22) (push) Failing after 7s
CI / E2E Tests (push) Has been skipped
CodeQL Analysis / CodeQL (javascript-typescript) (push) Failing after 10s
Deploy / Build API Image (push) Failing after 23s
E2E Tests / Playwright E2E (push) Failing after 7s
Security Scanning / Dependency Audit (pnpm) (push) Failing after 3s
Security Scanning / Trivy Scan — API Image (push) Failing after 43s
Security Scanning / Trivy Scan — Web Image (push) Failing after 28s
Security Scanning / Trivy Scan — AI Services Image (push) Failing after 28s
Deploy / Build Web Image (push) Failing after 10s
Deploy / Build AI Services Image (push) Failing after 9s
Security Scanning / Trivy Filesystem Scan (push) Failing after 38s
Deploy / Deploy to Staging (push) Has been skipped
Deploy / Smoke Test Staging (push) Has been skipped
Deploy / Deploy to Production (push) Has been skipped
Deploy / Smoke Test Production (push) Has been skipped
Security Scanning / Security Gate (push) Failing after 1s
Deploy / Rollback Staging (push) Has been skipped
Deploy / Rollback Production (push) Has been skipped
Batch-committing concurrent work-in-progress so it isn't lost: Listings — bulk update + duplicate detection --------------------------------------------- - New command BulkUpdateListings + handler + tests under application/commands/bulk-update-listings/. - New DTO presentation/dto/bulk-update-listings.dto.ts. - Controller wires the bulk endpoint; update DTO extended. - Property duplicate detector hardened: normalized-address pipeline (new migration 20260420020000_add_property_address_normalized), repository + service updates, tests refreshed. - Listing entity gains ownership-transferred event (new event file). - Integration specs for price constraints (20260420000000_add_price_check_constraints) and duplicates. - E2E: e2e/api/listings-duplicates.spec.ts. Admin — moderation audit log ---------------------------- - New Prisma table (migration 20260420010000_add_moderation_audit_log) + Prisma repo + interface + DI wiring. - Listener `moderation-audit.listener.ts` + unit spec. - Query GetModerationAuditLogs + handler + controller `admin-moderation-audit.controller.ts` + DTO. Supporting ---------- - shared/infrastructure/cache.service.ts tweak. - AUDIT_LISTINGS_PROPERTY_MANAGEMENT.md — in-repo audit notes. - Various test + module wiring updates to keep the tree green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
355 lines
17 KiB
Markdown
355 lines
17 KiB
Markdown
# 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.
|