feat: Implement get_latest_memojis RPC and Edge Function endpoint

[iAmVishal16]

No description provided.

[claude]

Code Review: Memoji Pagination Feature

I've reviewed PR #16 implementing the get_latest_memojis RPC function and Edge Function endpoint. Here's my detailed feedback:

✅ Strengths

1. Good parameter validation - The endpoint validates limit (1-100) and offset (≥0) parameters properly
2. Consistent error handling - Error responses follow the existing pattern with proper status codes
3. Test coverage - End-to-end test included with assertions for response structure
4. Security - Auth check is present and SECURITY DEFINER function has appropriate grants

---

🔴 Critical Issues

1. SECURITY: Missing RLS Bypass in SECURITY DEFINER Function (supabase/migrations/20251220095459_add_get_latest_memojis_function.sql:16, supabase/schemas/031_memoji_functions.sql:39)

Issue: The get_latest_memojis function is marked SECURITY DEFINER and queries storage.objects directly, but it doesn't filter results by user ownership. This means any authenticated user can see ALL memojis from ALL users.

Current code:

SELECT o.id, o.name, o.created_at, o.metadata
FROM storage.objects o
WHERE o.bucket_id = 'memoji-images'
ORDER BY o.created_at DESC

Risk: Privacy violation - users can see other users' memojis.

Recommendation: Add user filtering to respect data ownership:

WHERE o.bucket_id = 'memoji-images'
  AND o.owner = auth.uid()  -- Only return current user's memojis

Or if this is intended to be a global feed, add documentation explaining this behavior and ensure it aligns with your privacy policy.

---

2. Missing Rate Limiting (supabase/functions/ingredicheck/memoji.ts:363)

Issue: The POST /ingredicheck/memoji endpoint has rate limiting (lines 242-251), but the new GET /ingredicheck/memojis/latest endpoint has none.

Risk: Abuse via automated scraping or DoS attacks.

Recommendation: Apply similar rate limiting:

const rate = checkRateLimit(clientIP ?? "unknown");
if (!rate.allowed) {
    ctx.response.status = 429;
    ctx.response.body = { error: { message: "Rate limit exceeded." } };
    return;
}

---

⚠️ Important Issues

3. Schema Duplication

The same function is defined in both:

• supabase/migrations/20251220095459_add_get_latest_memojis_function.sql
• supabase/schemas/031_memoji_functions.sql

Recommendation: Per your README pattern, migrations should be generated from schemas. Consider whether the migration file should be regenerated or if the schema file needs updating.

---

4. Incomplete Test Coverage (supabase/tests/EndToEndTests/memoji.test.ts:82)

Issues:

• Metadata assertion is commented out (line 79)
• No test for pagination (offset parameter)
• No test for limit parameter edge cases
• No test for unauthorized access (missing auth token)
• Test creates test data but doesn't clean it up

Recommendations:

// Add tests for:
// - Pagination: verify offset skips correct number of items
// - Limit edge cases: limit=1, limit=100, limit>100 (should fail)
// - Invalid params: negative offset, non-numeric values
// - Unauthorized: request without auth token should return 401
// - Cleanup: delete test files after test completion

---

5. Missing Input Sanitization

While TypeScript Number() is used for parsing, there's no check for:

• Non-integer values (e.g., limit=10.5)
• Very large integers that could cause performance issues

Recommendation:

const limit = Math.floor(Number(ctx.request.url.searchParams.get("limit") ?? "10"));
if (!Number.isInteger(limit) || limit < 1 || limit > 100) { ... }

---

💡 Suggestions

6. Performance Considerations

• Consider adding an index on storage.objects(bucket_id, created_at DESC) for faster queries
• The function always queries metadata which can be large. Consider making it optional via a parameter.

7. API Design

• Consider returning total count for pagination UI: { memojis: [...], total: 42, limit: 10, offset: 0 }
• Consider using cursor-based pagination instead of offset for better performance with large datasets

8. Code Quality

• Extract magic numbers to constants:

const DEFAULT_LIMIT = 10;
const MAX_LIMIT = 100;
const MIN_LIMIT = 1;
const DEFAULT_OFFSET = 0;

9. Test Quality (supabase/tests/EndToEndTests/memoji.test.ts:26)

• Custom assertion functions reinvent the wheel. Consider using a proper test framework like Deno's built-in assertions:

import { assertEquals, assertExists } from "https://deno.land/std/assert/mod.ts";

---

📋 Summary

Must Fix Before Merge:

1. ✋ Fix the privacy issue in the RPC function (filter by user or document global access)
2. ✋ Add rate limiting to the GET endpoint

Should Fix:
3. Resolve schema/migration duplication
4. Expand test coverage

Nice to Have:
5. Input sanitization improvements
6. API design enhancements
7. Code quality improvements

---

Great work on implementing pagination! The code structure is clean and follows existing patterns well. The main concern is the security issue around data access. Once that's addressed, this will be a solid addition to the codebase.

🤖 Generated with Claude Code