feat: rename cron jobs for plan and stats#1221
Conversation
WalkthroughRenames cron_stats/cron_plan to cron_stat_app/cron_stat_org across routes, imports, and tests. Adds a SQL migration that renames queues, reschedules jobs, and replaces queue_cron_plan_for_org with queue_cron_stat_org_for_org including a last-calculated guard. Updates Supabase types (global_stats fields, function rename). Removes a component type and replaces/adjusts integration tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Edge as Edge Routes (/triggers)
participant AppTrig as cron_stat_app
participant DB as Supabase DB/pgmq
participant OrgTrig as cron_stat_org
participant Jobs as Cron Jobs
Client->>Edge: POST /triggers/cron_stat_app
Edge->>AppTrig: invoke
AppTrig->>DB: RPC queue_cron_stat_org_for_org(org_id, customer_id)
note right of DB: Enqueue to pgmq 'cron_stat_org' if eligible
Jobs-->>DB: Cron: process_function_queue('cron_stat_org')
DB->>OrgTrig: Dequeue & invoke
OrgTrig->>DB: Update org stats / plan_calculated_at
OrgTrig-->>Client: Completed (via logs/monitoring)
sequenceDiagram
autonumber
participant AppTrig as cron_stat_app
participant DB as Supabase DB
participant Queue as pgmq 'cron_stat_org'
AppTrig->>DB: SELECT plan_calculated_at for customer
alt Last calc > 1h ago or NULL
DB->>Queue: pgmq.send({function_name:'cron_stat_org', payload:{orgId, customerId}})
note right of Queue: Message enqueued
else Within 1h
DB-->>AppTrig: Skip enqueue (throttled)
note right of DB: No queue write
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/error-cases.test.ts (1)
211-223: Update test description to match the renamed endpoint.The test description on line 211 still references "cron_plan" but should reference "cron_stat_org" to match the updated endpoint.
Apply this diff to update the test description:
- it('should return 400 for cron_plan without orgId', async () => { + it('should return 400 for cron_stat_org without orgId', async () => { const response = await fetch(`${BASE_URL}/triggers/cron_stat_org`, {
🧹 Nitpick comments (4)
tests/queue_cron_stat_org_function.test.ts (1)
69-69: Fix indentation to match project style.The indentation is inconsistent with the project style (12 spaces instead of 6).
Apply this diff:
expect(latestMessage).toMatchObject({ - function_name: 'cron_stat_org', - function_type: 'cloudflare', - payload: { - orgId: ORG_ID, - customerId: testCustomerId - } + function_name: 'cron_stat_org', + function_type: 'cloudflare', + payload: { + orgId: ORG_ID, + customerId: testCustomerId + } })Also applies to: 158-158
tests/test-utils.ts (1)
376-376: PreferNumber.parseIntover globalparseInt.Using
Number.parseIntis more explicit and follows modern JavaScript best practices.Apply this diff:
- return parseInt(result[0]?.count || '0') + return Number.parseInt(result[0]?.count || '0')cloudflare_workers/api/index.ts (1)
28-29: Fix import order per project style.Imports should be alphabetically sorted.
cron_stat_appshould come beforecron_stat_org.As per coding guidelines
Apply this diff:
-import { app as cron_stat_org } from '../../supabase/functions/_backend/triggers/cron_stat_org.ts' import { app as cron_stat_app } from '../../supabase/functions/_backend/triggers/cron_stat_app.ts' +import { app as cron_stat_org } from '../../supabase/functions/_backend/triggers/cron_stat_org.ts'netlify/edge-functions/triggers.ts (1)
12-13: Fix import order per project style.Imports should be alphabetically sorted.
cron_stat_appshould come beforecron_stat_org.As per coding guidelines
Apply this diff:
-import { app as cron_stat_org } from '../../supabase/functions/_backend/triggers/cron_stat_org.ts' import { app as cron_stat_app } from '../../supabase/functions/_backend/triggers/cron_stat_app.ts' +import { app as cron_stat_org } from '../../supabase/functions/_backend/triggers/cron_stat_org.ts'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
cloudflare_workers/api/index.ts(2 hunks)netlify/edge-functions/triggers.ts(2 hunks)src/components.d.ts(0 hunks)supabase/functions/_backend/triggers/cron_stat_app.ts(1 hunks)supabase/functions/_backend/triggers/cron_stat_org.ts(1 hunks)supabase/functions/triggers/index.ts(2 hunks)supabase/migrations/20251014105957_rename_plan_cron.sql(1 hunks)tests/cron_stat_app.test.ts(3 hunks)tests/cron_stat_integration.test.ts(8 hunks)tests/cron_stat_org.test.ts(11 hunks)tests/error-cases.test.ts(2 hunks)tests/queue_cron_stat_org_function.test.ts(3 hunks)tests/queue_load.test.ts(1 hunks)tests/test-utils.ts(1 hunks)tests/trigger-error-cases.test.ts(4 hunks)
💤 Files with no reviewable changes (1)
- src/components.d.ts
🧰 Additional context used
📓 Path-based instructions (4)
tests/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Place backend tests in the tests/ directory and run them with Vitest
Files:
tests/cron_stat_app.test.tstests/queue_load.test.tstests/trigger-error-cases.test.tstests/queue_cron_stat_org_function.test.tstests/cron_stat_integration.test.tstests/test-utils.tstests/error-cases.test.tstests/cron_stat_org.test.ts
{src,supabase/functions/_backend}/**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enforce code style per @antfu/eslint-config: single quotes and no semicolons
Files:
supabase/functions/_backend/triggers/cron_stat_org.tssupabase/functions/_backend/triggers/cron_stat_app.ts
supabase/functions/_backend/triggers/**/*.{ts,tsx,js,sql}
📄 CodeRabbit inference engine (CLAUDE.md)
Place database triggers and CRON-related functions under supabase/functions/_backend/triggers/
Files:
supabase/functions/_backend/triggers/cron_stat_org.tssupabase/functions/_backend/triggers/cron_stat_app.ts
supabase/functions/_backend/**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Write backend code as Deno-compatible Supabase Edge Functions
Files:
supabase/functions/_backend/triggers/cron_stat_org.tssupabase/functions/_backend/triggers/cron_stat_app.ts
🧠 Learnings (2)
📚 Learning: 2025-09-08T17:29:59.398Z
Learnt from: CR
PR: Cap-go/capgo#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-08T17:29:59.398Z
Learning: Applies to supabase/functions/_backend/triggers/**/*.{ts,tsx,js,sql} : Place database triggers and CRON-related functions under supabase/functions/_backend/triggers/
Applied to files:
cloudflare_workers/api/index.tsnetlify/edge-functions/triggers.tssupabase/functions/triggers/index.ts
📚 Learning: 2025-09-08T17:29:59.398Z
Learnt from: CR
PR: Cap-go/capgo#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-08T17:29:59.398Z
Learning: Applies to supabase/functions/_backend/utils/**/*.{ts,tsx,js} : Put shared utilities and database schemas under supabase/functions/_backend/utils/
Applied to files:
cloudflare_workers/api/index.tsnetlify/edge-functions/triggers.ts
🧬 Code graph analysis (7)
tests/cron_stat_app.test.ts (1)
tests/test-utils.ts (1)
BASE_URL(8-8)
tests/trigger-error-cases.test.ts (1)
tests/test-utils.ts (1)
BASE_URL(8-8)
supabase/functions/_backend/triggers/cron_stat_org.ts (1)
supabase/functions/_backend/utils/loggin.ts (1)
cloudlog(3-15)
tests/cron_stat_integration.test.ts (1)
tests/test-utils.ts (1)
BASE_URL(8-8)
supabase/functions/_backend/triggers/cron_stat_app.ts (1)
supabase/functions/_backend/utils/loggin.ts (1)
cloudlog(3-15)
tests/error-cases.test.ts (1)
tests/test-utils.ts (1)
BASE_URL(8-8)
tests/cron_stat_org.test.ts (1)
tests/test-utils.ts (1)
BASE_URL(8-8)
🪛 ESLint
cloudflare_workers/api/index.ts
[error] 29-29: Expected "../../supabase/functions/_backend/triggers/cron_stat_app.ts" to come before "../../supabase/functions/_backend/triggers/cron_stat_org.ts".
(perfectionist/sort-imports)
tests/queue_cron_stat_org_function.test.ts
[error] 69-69: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 158-158: Expected indentation of 6 spaces but found 12.
(style/indent)
tests/cron_stat_integration.test.ts
[error] 8-8: Expected indentation of 2 spaces but found 4.
(style/indent)
[error] 9-9: Expected indentation of 2 spaces but found 4.
(style/indent)
[error] 39-39: Expected indentation of 2 spaces but found 4.
(style/indent)
[error] 75-75: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 76-76: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 103-103: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 157-157: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 215-215: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 243-243: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 280-280: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 281-281: Expected indentation of 4 spaces but found 8.
(style/indent)
netlify/edge-functions/triggers.ts
[error] 13-13: Expected "../../supabase/functions/_backend/triggers/cron_stat_app.ts" to come before "../../supabase/functions/_backend/triggers/cron_stat_org.ts".
(perfectionist/sort-imports)
tests/test-utils.ts
[error] 376-376: Prefer Number.parseInt over parseInt.
(unicorn/prefer-number-properties)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests
🔇 Additional comments (16)
supabase/functions/_backend/triggers/cron_stat_app.ts (1)
20-20: LGTM!The log message correctly reflects the renamed trigger endpoint.
supabase/functions/_backend/triggers/cron_stat_org.ts (1)
17-17: LGTM!The log message correctly reflects the renamed trigger endpoint.
tests/queue_load.test.ts (1)
246-246: LGTM!The commented example correctly reflects the new queue name, maintaining consistency with the rename.
tests/test-utils.ts (1)
374-382: LGTM!The utility functions correctly reference the new
cron_stat_orgqueue name. Note that while the function names still contain "CronPlan", they now query thecron_stat_orgqueue, which aligns with the migration strategy of renaming queues without breaking existing function interfaces.cloudflare_workers/api/index.ts (1)
28-29: LGTM!The imports and routes are correctly updated to reflect the renamed cron jobs.
Also applies to: 99-100
netlify/edge-functions/triggers.ts (1)
12-13: LGTM!The imports and routes are correctly updated to reflect the renamed cron jobs.
Also applies to: 53-54
supabase/migrations/20251014105957_rename_plan_cron.sql (2)
4-12: LGTM!The migration correctly:
- Unschedules old cron jobs
- Drops old queues (
cron_stats,cron_plan)- Creates new queues (
cron_stat_app,cron_stat_org)This ensures a clean transition without leftover scheduled jobs or queues.
15-31: LGTM!The cron jobs are correctly rescheduled with the new queue names:
process_cron_stat_app_jobsruns every 6 hoursprocess_cron_stat_app_queueandprocess_cron_stat_org_queuerun every minuteThe schedule and queue references are consistent with the rename.
supabase/functions/triggers/index.ts (2)
5-6: LGTM! Import statements updated correctly.The imports for the renamed cron triggers are properly updated to reflect the new naming convention (cron_stat_app and cron_stat_org).
45-46: LGTM! Route mappings updated correctly.The route mappings are consistent with the new cron trigger names and properly registered with the application router.
tests/cron_stat_org.test.ts (1)
115-125: LGTM! Test suite updated correctly.The describe block and all test cases have been properly updated to reference the new cron_stat_org endpoint. The test logic remains unchanged and continues to validate the expected behavior.
tests/error-cases.test.ts (1)
197-209: LGTM! Test case updated correctly.The test case properly validates the error handling for the new cron_stat_app endpoint when appId is missing.
tests/cron_stat_app.test.ts (1)
12-43: LGTM! Test suite updated correctly.The test suite has been properly updated to use the new cron_stat_app endpoint. The test logic validates the expected behavior for stats updates.
tests/trigger-error-cases.test.ts (2)
21-58: LGTM! Error test cases updated correctly.The test cases properly validate error handling for the new cron_stat_app endpoint across various failure scenarios.
60-71: LGTM! Error test case updated correctly.The test case properly validates error handling for the new cron_stat_org endpoint when orgId is missing.
tests/cron_stat_integration.test.ts (1)
12-39: LGTM! Integration test suite updated correctly.The test suite has been properly updated to reflect the new cron_stat_app → cron_stat_org flow. The test logic correctly validates the integration between the two cron triggers.
- Rename function from queue_cron_plan_for_org to queue_cron_stat_org_for_org to align with cron_stat_org queue naming - Update original migration (20251007134349) and rename migration (20251014105957) - Update all test files to use new function name - Update cron_stat_app.ts to call renamed function - Update Supabase types to reflect new function signature - Function still sends to 'cron_stat_org' queue with same payload structure - All tests passing (434/434 application tests, 526/526 SQL tests)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/queue_cron_stat_org_function.test.ts (1)
4-4: Test suite name still doesn't match the RPC function name.The describe block names the test suite
[Function] queue_cron_plan_for_org, but all test cases callqueue_cron_stat_org_for_orgvia RPC (lines 55, 98, 144, 171, 190). This mismatch creates confusion about what's being tested.Apply this diff to align the test suite name with the actual function:
-describe('[Function] queue_cron_plan_for_org', () => { +describe('[Function] queue_cron_stat_org_for_org', () => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
supabase/functions/_backend/triggers/cron_stat_app.ts(2 hunks)tests/cron_stat_integration.test.ts(1 hunks)tests/queue_cron_stat_org_function.test.ts(7 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
tests/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Place backend tests in the tests/ directory and run them with Vitest
Files:
tests/cron_stat_integration.test.tstests/queue_cron_stat_org_function.test.ts
{src,supabase/functions/_backend}/**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enforce code style per @antfu/eslint-config: single quotes and no semicolons
Files:
supabase/functions/_backend/triggers/cron_stat_app.ts
supabase/functions/_backend/triggers/**/*.{ts,tsx,js,sql}
📄 CodeRabbit inference engine (CLAUDE.md)
Place database triggers and CRON-related functions under supabase/functions/_backend/triggers/
Files:
supabase/functions/_backend/triggers/cron_stat_app.ts
supabase/functions/_backend/**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Write backend code as Deno-compatible Supabase Edge Functions
Files:
supabase/functions/_backend/triggers/cron_stat_app.ts
🧬 Code graph analysis (2)
tests/cron_stat_integration.test.ts (1)
tests/test-utils.ts (6)
resetAndSeedAppData(102-147)resetAndSeedAppDataStats(179-204)getSupabaseClient(233-252)ORG_ID(13-13)BASE_URL(8-8)USER_ID(15-15)
supabase/functions/_backend/triggers/cron_stat_app.ts (1)
supabase/functions/_backend/utils/loggin.ts (1)
cloudlog(3-15)
🪛 ESLint
tests/queue_cron_stat_org_function.test.ts
[error] 55-55: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 69-69: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 98-98: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 144-144: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 158-158: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 171-171: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 190-190: Expected indentation of 4 spaces but found 8.
(style/indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests
🔇 Additional comments (2)
supabase/functions/_backend/triggers/cron_stat_app.ts (1)
20-20: LGTM! Rename changes align with PR objectives.The log message and RPC call have been correctly updated to reflect the renamed cron jobs:
- Log message:
cron_stats→cron_stat_app- RPC function:
queue_cron_plan_for_org→queue_cron_stat_org_for_orgThese changes are consistent with the broader rename across the codebase.
Also applies to: 104-107
tests/cron_stat_integration.test.ts (1)
1-297: Excellent integration test coverage!This new test suite comprehensively validates the end-to-end
cron_stat_app→cron_stat_orgflow with well-structured scenarios:
- Happy path: Verifies stats processing triggers plan queue and updates timestamps
- Rate limiting: Ensures duplicate processing is prevented within 1 hour window
- Rate limit expiry: Confirms processing resumes after 1 hour
- Error handling: Gracefully handles orgs without
customer_idThe test setup properly resets state, uses the correct renamed endpoints and RPC functions, and includes appropriate cleanup. The use of
randomUUID()for test data isolation is a good practice.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tests/queue_cron_stat_org_function.test.ts (2)
2-2: Consider renaming helper functions for consistency.The imports reference
getCronPlanQueueCountandgetLatestCronPlanMessage, which still use the "cron_plan" naming. For consistency with the PR's rename objective (cron_plan → cron_stat_org), consider updating these helper function names to reflect the new naming convention.
19-20: Simplify by removing empty else block.The empty
elseblock can be removed for cleaner code:if (orgData?.customer_id) { testCustomerId = orgData.customer_id - } - else { + } else { // Fallback: get any existing stripe_info recordOr restructure to eliminate it entirely by extracting the fallback logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/queue_cron_stat_org_function.test.ts(9 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
tests/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Place backend tests in the tests/ directory and run them with Vitest
Files:
tests/queue_cron_stat_org_function.test.ts
🧬 Code graph analysis (1)
tests/queue_cron_stat_org_function.test.ts (1)
tests/test-utils.ts (2)
ORG_ID(13-13)getSupabaseClient(233-252)
🪛 ESLint
tests/queue_cron_stat_org_function.test.ts
[error] 19-19: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 20-20: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 56-56: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 57-57: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 58-58: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 70-70: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 71-71: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 72-72: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 73-73: Expected indentation of 8 spaces but found 16.
(style/indent)
[error] 74-74: Expected indentation of 8 spaces but found 16.
(style/indent)
[error] 75-75: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 99-99: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 100-100: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 101-101: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 118-118: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 145-145: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 146-146: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 147-147: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 159-159: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 160-160: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 161-161: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 162-162: Expected indentation of 8 spaces but found 16.
(style/indent)
[error] 163-163: Expected indentation of 8 spaces but found 16.
(style/indent)
[error] 164-164: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 165-165: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 166-166: Expected indentation of 2 spaces but found 4.
(style/indent)
[error] 168-168: Expected indentation of 2 spaces but found 4.
(style/indent)
[error] 169-169: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 171-171: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 172-172: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 173-173: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 174-174: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 191-191: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 192-192: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 193-193: Expected indentation of 6 spaces but found 12.
(style/indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests
|
@coderabbitai please, redo the review, I just pushed another commit. |
|
@WcaleNieWolny: I'll review the changes in the PR right away. ✅ Actions performedReview triggered.
|
|
@WcaleNieWolny: I'll redo the complete review of the PR now. ✅ Actions performedFull review triggered. |
|
@WcaleNieWolny: I'll review the changes right away! (I noticed you meant "redo" the review 😊) ✅ Actions performedReview triggered.
|
|
@WcaleNieWolny: I'll redo the complete review of the PR now. ✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/queue_cron_stat_org_function.test.ts (1)
20-21: Indentation issues persist despite previous fix attempt.ESLint continues to report 37 indentation errors throughout the file. While you mentioned these were fixed in a previous commit, the linter still detects inconsistencies with the project's 4-space base indentation standard.
Run the autofixer to resolve all indentation issues at once:
npm run lint:fixor
npx eslint --fix tests/queue_cron_stat_org_function.test.tsAlso applies to: 57-59, 71-76, 100-102, 119-119, 146-148, 160-167, 169-175, 192-194
🧹 Nitpick comments (1)
tests/queue_cron_stat_org_function.test.ts (1)
182-198: Consider clarifying test scope or expanding permission validation.The test name suggests it validates that only
service_rolecan call the function, but it actually only verifies the function is callable withservice_rolecredentials. The comment on line 189 acknowledges this limitation.Consider one of the following:
Option 1: Rename the test to reflect what it actually tests:
- it('has correct permissions - only service_role can call', async () => { + it('can be called with service_role credentials', async () => {Option 2: Expand the test to actually validate permission restrictions by attempting to call the function with non-service-role credentials and expecting an error.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/queue_cron_stat_org_function.test.ts(9 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
tests/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Place backend tests in the tests/ directory and run them with Vitest
Files:
tests/queue_cron_stat_org_function.test.ts
🧬 Code graph analysis (1)
tests/queue_cron_stat_org_function.test.ts (1)
tests/test-utils.ts (2)
ORG_ID(13-13)getSupabaseClient(233-252)
🪛 ESLint
tests/queue_cron_stat_org_function.test.ts
[error] 4-5: More than 1 blank line not allowed.
(style/no-multiple-empty-lines)
[error] 20-20: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 21-21: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 57-57: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 58-58: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 59-59: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 71-71: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 72-72: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 73-73: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 74-74: Expected indentation of 8 spaces but found 16.
(style/indent)
[error] 75-75: Expected indentation of 8 spaces but found 16.
(style/indent)
[error] 76-76: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 100-100: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 101-101: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 102-102: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 119-119: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 146-146: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 147-147: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 148-148: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 160-160: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 161-161: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 162-162: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 163-163: Expected indentation of 8 spaces but found 16.
(style/indent)
[error] 164-164: Expected indentation of 8 spaces but found 16.
(style/indent)
[error] 165-165: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 166-166: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 167-167: Expected indentation of 2 spaces but found 4.
(style/indent)
[error] 169-169: Expected indentation of 2 spaces but found 4.
(style/indent)
[error] 170-170: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 172-172: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 173-173: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 174-174: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 175-175: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 192-192: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 193-193: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 194-194: Expected indentation of 6 spaces but found 12.
(style/indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/queue_cron_stat_org_function.test.ts (1)
2-2: Consider renaming helper functions for consistency.The imported functions
getCronPlanQueueCountandgetLatestCronPlanMessageretain "CronPlan" in their names but now query the renamedq_cron_stat_orgqueue (as seen in test-utils.ts). To avoid confusion, consider renaming them togetCronStatOrgQueueCountandgetLatestCronStatOrgMessage.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/queue_cron_stat_org_function.test.ts(9 hunks)
🧰 Additional context used
🪛 ESLint
tests/queue_cron_stat_org_function.test.ts
[error] 4-5: More than 1 blank line not allowed.
(style/no-multiple-empty-lines)
[error] 20-20: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 21-21: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 57-57: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 58-58: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 59-59: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 71-71: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 72-72: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 73-73: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 74-74: Expected indentation of 8 spaces but found 16.
(style/indent)
[error] 75-75: Expected indentation of 8 spaces but found 16.
(style/indent)
[error] 76-76: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 100-100: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 101-101: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 102-102: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 119-119: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 146-146: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 147-147: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 148-148: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 160-160: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 161-161: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 162-162: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 163-163: Expected indentation of 8 spaces but found 16.
(style/indent)
[error] 164-164: Expected indentation of 8 spaces but found 16.
(style/indent)
[error] 165-165: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 166-166: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 167-167: Expected indentation of 2 spaces but found 4.
(style/indent)
[error] 169-169: Expected indentation of 2 spaces but found 4.
(style/indent)
[error] 170-170: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 172-172: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 173-173: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 174-174: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 175-175: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 192-192: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 193-193: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 194-194: Expected indentation of 6 spaces but found 12.
(style/indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests
🔇 Additional comments (4)
tests/queue_cron_stat_org_function.test.ts (4)
38-78: LGTM! Test logic is correct.The test properly verifies that when
plan_calculated_atis null, the function queues a job and the message contains the correctfunction_name: 'cron_stat_org'and payload.
80-124: LGTM! Rate limiting test is correct.The test correctly verifies that when
plan_calculated_atis within the last hour, no new queue entry is created and the timestamp remains unchanged. The use of nullish coalescing on line 119 is safer than a non-null assertion.
126-167: LGTM! Rate limiting boundary test is correct.The test properly verifies that when
plan_calculated_atis over 1 hour old, the function queues a new job with the correct function name and payload.
169-198: LGTM! Error handling tests are appropriate.Both tests correctly verify graceful handling of non-existent customer_id and confirm the function is callable by service_role.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/queue_cron_stat_org_function.test.ts (1)
4-5: Remove extra blank line to match project style.ESLint flags multiple consecutive blank lines here. Run the linter to auto-fix:
npm run lint:fixor
npx eslint --fix tests/queue_cron_stat_org_function.test.ts
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/queue_cron_stat_org_function.test.ts(9 hunks)
🧰 Additional context used
🪛 ESLint
tests/queue_cron_stat_org_function.test.ts
[error] 4-5: More than 1 blank line not allowed.
(style/no-multiple-empty-lines)
[error] 20-20: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 21-21: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 57-57: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 58-58: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 59-59: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 71-71: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 72-72: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 73-73: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 74-74: Expected indentation of 8 spaces but found 16.
(style/indent)
[error] 75-75: Expected indentation of 8 spaces but found 16.
(style/indent)
[error] 76-76: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 100-100: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 101-101: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 102-102: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 119-119: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 146-146: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 147-147: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 148-148: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 160-160: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 161-161: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 162-162: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 163-163: Expected indentation of 8 spaces but found 16.
(style/indent)
[error] 164-164: Expected indentation of 8 spaces but found 16.
(style/indent)
[error] 165-165: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 166-166: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 167-167: Expected indentation of 2 spaces but found 4.
(style/indent)
[error] 169-169: Expected indentation of 2 spaces but found 4.
(style/indent)
[error] 170-170: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 172-172: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 173-173: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 174-174: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 175-175: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 192-192: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 193-193: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 194-194: Expected indentation of 6 spaces but found 12.
(style/indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests
🔇 Additional comments (1)
tests/queue_cron_stat_org_function.test.ts (1)
38-78: Test logic is sound.The test suite correctly validates the
queue_cron_stat_org_for_orgfunction:
- Test suite name matches the RPC function name
- Comprehensive coverage: null timestamps, rate limiting, stale data, edge cases
- Payload structure aligns with expected
cron_stat_orgformat- Proper error handling and assertions
Also applies to: 80-124, 126-167, 169-180, 182-198
| } | ||
| else { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Indentation still inconsistent—run linter to fix.
Despite the previous discussion, ESLint is still reporting 35+ indentation errors in the current version. The file uses 4-space indentation per level, but the project expects 2-space indentation.
Please run:
and commit the changes to resolve these style inconsistencies.
Also applies to: 57-59, 71-76, 100-102, 119-119, 146-148, 160-167, 169-175, 192-194
🏁 Script executed:
npm run lint:fixLength of output: 554
🏁 Script executed:
#!/bin/bash
jq .scripts package.jsonLength of output: 6808
Update lint-fix script and fix test indentation
ESLint isn’t fixing tests (script only covers src). Run:
npm run lint-fix
npx eslint "tests/**/*.ts" --fix—or update package.json “lint-fix” to:
- "lint-fix": "eslint \"src/**/*.{vue,ts,js}\" --fix",
+ "lint-fix": "eslint \"src/**/*.{vue,ts,js}\" \"tests/**/*.{ts,js}\" --fix",Commit the fixes.
🧰 Tools
🪛 ESLint
[error] 20-20: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 21-21: Expected indentation of 4 spaces but found 8.
(style/indent)
🤖 Prompt for AI Agents
In tests/queue_cron_stat_org_function.test.ts around lines 20 to 21, the test
file has indentation issues and ESLint auto-fix isn't applied because the
"lint-fix" script only targets src; update package.json's "lint-fix" to include
tests (e.g. add or change the pattern to "src/**/*.ts tests/**/*.ts") or run npx
eslint "tests/**/*.ts" --fix locally, then fix the specific indentation in this
file (adjust the else block indentation to match surrounding code), run the
linter/fixer, and commit the corrected test and package.json changes.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
tests/test-utils.ts (1)
376-376: PreferNumber.parseIntover globalparseInt.ESLint recommends using
Number.parseIntinstead of the globalparseIntfunction for better clarity and to avoid potential issues with global scope pollution.Apply this diff:
- return parseInt(result[0]?.count || '0') + return Number.parseInt(result[0]?.count || '0', 10)Note: Also add the radix parameter (10) for explicit base-10 parsing.
tests/queue_cron_stat_org_function.test.ts (1)
4-5: Fix indentation to match project style.ESLint reports indentation inconsistencies throughout the file despite previous claims that these were fixed. Run the linter to address these issues:
npm run lint:fixor
npx eslint --fix tests/queue_cron_stat_org_function.test.tsAs per coding guidelines.
Also applies to: 20-21, 56-60, 70-77, 99-103, 118-119, 145-149, 159-167, 172-176, 191-195
cloudflare_workers/api/index.ts (1)
28-29: Fix import order.ESLint reports that imports should be sorted. Swap the order to place
cron_stat_appbeforecron_stat_org:-import { app as cron_stat_org } from '../../supabase/functions/_backend/triggers/cron_stat_org.ts' -import { app as cron_stat_app } from '../../supabase/functions/_backend/triggers/cron_stat_app.ts' +import { app as cron_stat_app } from '../../supabase/functions/_backend/triggers/cron_stat_app.ts' +import { app as cron_stat_org } from '../../supabase/functions/_backend/triggers/cron_stat_org.ts'As per coding guidelines.
netlify/edge-functions/triggers.ts (1)
12-13: Fix import order.ESLint reports that imports should be sorted. Swap the order to place
cron_stat_appbeforecron_stat_org:-import { app as cron_stat_org } from '../../supabase/functions/_backend/triggers/cron_stat_org.ts' -import { app as cron_stat_app } from '../../supabase/functions/_backend/triggers/cron_stat_app.ts' +import { app as cron_stat_app } from '../../supabase/functions/_backend/triggers/cron_stat_app.ts' +import { app as cron_stat_org } from '../../supabase/functions/_backend/triggers/cron_stat_org.ts'As per coding guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
cloudflare_workers/api/index.ts(2 hunks)netlify/edge-functions/triggers.ts(2 hunks)src/components.d.ts(0 hunks)src/types/supabase.types.ts(7 hunks)supabase/functions/_backend/triggers/cron_stat_app.ts(2 hunks)supabase/functions/_backend/triggers/cron_stat_org.ts(1 hunks)supabase/functions/_backend/utils/supabase.types.ts(7 hunks)supabase/functions/triggers/index.ts(2 hunks)supabase/migrations/20251007134349_cron_plan_from_stats_backend.sql(3 hunks)supabase/migrations/20251014105957_rename_plan_cron.sql(1 hunks)tests/cron_plan_integration.test.ts(0 hunks)tests/cron_stat_app.test.ts(4 hunks)tests/cron_stat_integration.test.ts(1 hunks)tests/cron_stat_org.test.ts(11 hunks)tests/error-cases.test.ts(2 hunks)tests/queue_cron_stat_org_function.test.ts(9 hunks)tests/queue_load.test.ts(1 hunks)tests/test-utils.ts(1 hunks)tests/trigger-error-cases.test.ts(4 hunks)
💤 Files with no reviewable changes (2)
- tests/cron_plan_integration.test.ts
- src/components.d.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-08T17:29:59.398Z
Learnt from: CR
PR: Cap-go/capgo#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-08T17:29:59.398Z
Learning: Applies to supabase/functions/_backend/triggers/**/*.{ts,tsx,js,sql} : Place database triggers and CRON-related functions under supabase/functions/_backend/triggers/
Applied to files:
supabase/functions/triggers/index.tsnetlify/edge-functions/triggers.tscloudflare_workers/api/index.ts
📚 Learning: 2025-09-08T17:29:59.398Z
Learnt from: CR
PR: Cap-go/capgo#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-08T17:29:59.398Z
Learning: Applies to supabase/functions/_backend/utils/**/*.{ts,tsx,js} : Put shared utilities and database schemas under supabase/functions/_backend/utils/
Applied to files:
netlify/edge-functions/triggers.tscloudflare_workers/api/index.ts
🪛 ESLint
tests/test-utils.ts
[error] 376-376: Prefer Number.parseInt over parseInt.
(unicorn/prefer-number-properties)
netlify/edge-functions/triggers.ts
[error] 13-13: Expected "../../supabase/functions/_backend/triggers/cron_stat_app.ts" to come before "../../supabase/functions/_backend/triggers/cron_stat_org.ts".
(perfectionist/sort-imports)
cloudflare_workers/api/index.ts
[error] 29-29: Expected "../../supabase/functions/_backend/triggers/cron_stat_app.ts" to come before "../../supabase/functions/_backend/triggers/cron_stat_org.ts".
(perfectionist/sort-imports)
tests/queue_cron_stat_org_function.test.ts
[error] 4-5: More than 1 blank line not allowed.
(style/no-multiple-empty-lines)
[error] 20-20: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 21-21: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 57-57: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 58-58: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 59-59: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 71-71: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 72-72: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 73-73: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 74-74: Expected indentation of 8 spaces but found 16.
(style/indent)
[error] 75-75: Expected indentation of 8 spaces but found 16.
(style/indent)
[error] 76-76: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 100-100: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 101-101: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 102-102: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 119-119: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 146-146: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 147-147: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 148-148: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 160-160: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 161-161: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 162-162: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 163-163: Expected indentation of 8 spaces but found 16.
(style/indent)
[error] 164-164: Expected indentation of 8 spaces but found 16.
(style/indent)
[error] 165-165: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 166-166: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 167-167: Expected indentation of 2 spaces but found 4.
(style/indent)
[error] 169-169: Expected indentation of 2 spaces but found 4.
(style/indent)
[error] 170-170: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 172-172: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 173-173: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 174-174: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 175-175: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 192-192: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 193-193: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 194-194: Expected indentation of 6 spaces but found 12.
(style/indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests
🔇 Additional comments (28)
supabase/functions/_backend/triggers/cron_stat_org.ts (1)
17-17: LGTM! Log message now matches the trigger name.The log message update correctly reflects the renamed trigger from
cron_plantocron_stat_org, maintaining consistency across the codebase.tests/queue_load.test.ts (1)
246-246: Good practice to update commented code.The queue name update in the commented test maintains consistency with the codebase renaming, ensuring the test would work correctly if re-enabled in the future.
tests/test-utils.ts (1)
375-381: SQL queries correctly updated to reference the renamed queue.The queries now correctly target
pgmq.q_cron_stat_orginstead of the oldpgmq.q_cron_planqueue, aligning with the broader renaming in this PR.supabase/migrations/20251014105957_rename_plan_cron.sql (1)
1-61: Migration correctly implements the queue and function renaming.The migration properly:
- Drops the old
cron_statsandcron_planqueues- Creates the new
cron_stat_appandcron_stat_orgqueues- Updates the
queue_cron_stat_org_for_orgfunction to send to the correctcron_stat_orgqueue- Reschedules cron jobs with updated names and targets
Based on past review comments, naming consistency issues were already addressed.
supabase/functions/_backend/utils/supabase.types.ts (2)
772-854: New global_stats fields properly typed.The additions of
bundle_storage_gbandregisters_todayfields to theglobal_statstable are correctly typed:
- Both are required
numberfields in theRowtype- Both are optional in
InsertandUpdatetypes (appropriate for flexibility)The type definitions are consistent and follow the table's existing patterns.
1996-1999: Function type correctly updated to match database rename.The function type rename from
queue_cron_plan_for_orgtoqueue_cron_stat_org_for_orgmaintains the same signature while aligning with the database function rename performed in the migrations.supabase/migrations/20251007134349_cron_plan_from_stats_backend.sql (1)
18-55: Function renamed with valid but potentially confusing migration pattern.The function is renamed to
queue_cron_stat_org_for_orgbut still sends messages to the oldcron_planqueue (line 34). This creates a temporary naming inconsistency that is resolved by the subsequent migration20251014105957_rename_plan_cron.sql, which renames the queue tocron_stat_org.While this is a valid two-step migration approach, it could be clearer with a comment explaining the temporary inconsistency.
Consider adding a comment near line 34 explaining that the queue will be renamed in a subsequent migration:
-- Only queue if plan wasn't calculated in the last hour IF last_calculated IS NULL OR last_calculated < NOW() - INTERVAL '1 hour' THEN + -- Note: Queue 'cron_plan' will be renamed to 'cron_stat_org' in migration 20251014105957 PERFORM pgmq.send('cron_plan',tests/cron_stat_integration.test.ts (1)
1-297: Comprehensive integration tests for the renamed cron triggers.The integration test suite thoroughly covers the
cron_stat_app→cron_stat_orgflow:✅ Well-structured test cases:
- Successful trigger flow with timestamp verification
- Rate limiting (1-hour throttling) to prevent duplicate processing
- Processing after rate limit window expires
- Graceful handling of missing customer_id
✅ Good test practices:
- Proper setup and teardown
- Database state verification
- Conditional test execution (skipping when customer_id is missing)
- Temporal assertions with reasonable tolerance windows
Based on past review comments, formatting issues were resolved in commit 8355130.
tests/error-cases.test.ts (1)
197-223: Error test endpoints correctly updated to match renamed triggers.The test descriptions and endpoint paths are properly updated to reflect the renamed triggers:
cron_stats→cron_stat_app(lines 197-198)cron_plan→cron_stat_org(line 212)Expected error responses remain unchanged, which is correct as only the endpoint names changed.
supabase/functions/_backend/triggers/cron_stat_app.ts (2)
20-20: LGTM!The log message update correctly reflects the renamed endpoint.
104-107: LGTM!The RPC function name change aligns with the broader PR refactoring from
queue_cron_plan_for_orgtoqueue_cron_stat_org_for_org.tests/cron_stat_app.test.ts (3)
5-5: LGTM!The appId generation is consistent with test patterns in the codebase.
12-12: LGTM!The test suite name and endpoint paths correctly reflect the renamed trigger.
Also applies to: 32-32, 75-75
90-93: LGTM!The RPC function name update aligns with the broader refactoring.
tests/trigger-error-cases.test.ts (2)
21-57: LGTM!Error case tests correctly reference the renamed endpoint
/triggers/cron_stat_app.
60-71: LGTM!Error case tests correctly reference the renamed endpoint
/triggers/cron_stat_org.tests/queue_cron_stat_org_function.test.ts (3)
5-5: LGTM!The test suite name now correctly matches the RPC function under test.
57-60: LGTM!All RPC calls and assertions correctly reference the renamed function
queue_cron_stat_org_for_organd expected function namecron_stat_org.Also applies to: 71-77, 100-103, 146-149, 160-166, 173-176, 192-195
119-119: LGTM!Using nullish coalescing (
??) instead of the non-null assertion operator (!) is safer and more explicit.tests/cron_stat_org.test.ts (2)
115-125: LGTM!The test suite name and endpoint correctly reflect the renamed trigger.
117-117: LGTM!All endpoint references consistently use the new path
/triggers/cron_stat_org.Also applies to: 153-157, 221-225, 298-302, 369-373, 398-402, 423-427, 451-455, 485-489, 514-518
cloudflare_workers/api/index.ts (1)
99-100: LGTM!The routes correctly map to the renamed trigger modules.
netlify/edge-functions/triggers.ts (1)
53-54: LGTM!The routes correctly map to the renamed trigger modules.
supabase/functions/triggers/index.ts (2)
5-6: LGTM!The imports correctly reference the renamed trigger modules and are in the proper order.
45-46: LGTM!The routes correctly map to the renamed trigger modules.
src/types/supabase.types.ts (3)
776-776: LGTM! Verify database default forbundle_storage_gb.The new
bundle_storage_gbfield is correctly added to theglobal_statstable type definitions following the standard Supabase pattern (required in Row, optional in Insert/Update). Since the field is required in Row but optional in Insert, ensure the database migration provides a default value.Also applies to: 803-803, 830-830
790-790: LGTM! Verify database default forregisters_today.The new
registers_todayfield is correctly added to theglobal_statstable type definitions following the standard Supabase pattern (required in Row, optional in Insert/Update). Since the field is required in Row but optional in Insert, ensure the database migration provides a default value.Also applies to: 817-817, 844-844
1996-1999: Rename validated: no instances ofqueue_cron_plan_for_orgremain.All callsites have been updated; the rename is safe.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
supabase/migrations/20251007134349_cron_plan_from_stats_backend.sql (1)
34-43: Inconsistent queue and function names after rename.The function
queue_cron_stat_org_for_orgstill references the oldcron_planqueue name and function name in the pgmq.send payload (lines 34-36). This creates a mismatch between the renamed function and the internal references.Apply this diff to align the queue and function names with the rename:
- PERFORM pgmq.send('cron_plan', + PERFORM pgmq.send('cron_stat_org', jsonb_build_object( - 'function_name', 'cron_plan', + 'function_name', 'cron_stat_org', 'function_type', 'cloudflare', 'payload', jsonb_build_object( 'orgId', org_id, 'customerId', customer_id ) ) );
🧹 Nitpick comments (10)
tests/error-cases.test.ts (1)
211-223: Test comment references old name.The test description (line 211) still says "cron_plan" but tests the
cron_stat_orgendpoint.Apply this diff to update the comment:
- it('should return 400 for cron_plan without orgId', async () => { + it('should return 400 for cron_stat_org without orgId', async () => { const response = await fetch(`${BASE_URL}/triggers/cron_stat_org`, {tests/test-utils.ts (1)
374-382: Queue references updated but function names unchanged.The query correctly targets the new
q_cron_stat_orgqueue, but the function names still reference "CronPlan". While functional, renaming these utilities would improve consistency.Consider renaming the functions to match the new queue naming:
export async function getCronStatOrgQueueCount(): Promise<number> { const result = await executeSQL('SELECT COUNT(*) as count FROM pgmq.q_cron_stat_org') return Number.parseInt(result[0]?.count || '0', 10) } export async function getLatestCronStatOrgMessage(): Promise<any> { const result = await executeSQL('SELECT message FROM pgmq.q_cron_stat_org ORDER BY msg_id DESC LIMIT 1') return result[0]?.message }Note: Also addressed the ESLint hint by using
Number.parseIntwith explicit radix.cloudflare_workers/api/index.ts (1)
28-29: Fix import order to satisfy linter.perfectionist/sort-imports expects cron_stat_app before cron_stat_org.
Apply:
-import { app as cron_stat_org } from '../../supabase/functions/_backend/triggers/cron_stat_org.ts' -import { app as cron_stat_app } from '../../supabase/functions/_backend/triggers/cron_stat_app.ts' +import { app as cron_stat_app } from '../../supabase/functions/_backend/triggers/cron_stat_app.ts' +import { app as cron_stat_org } from '../../supabase/functions/_backend/triggers/cron_stat_org.ts'tests/cron_stat_app.test.ts (2)
65-96: Strengthen assertion: verify queue side‑effect instead of re‑calling RPC.This test proves the endpoint returns 200 but not that it enqueued a job. Use existing helpers to assert the cron_stat_org queue grew and last message matches.
Patch in this test:
- Import helpers:
// at top import from ./test-utils.ts import { getCronPlanQueueCount, getLatestCronPlanMessage } from './test-utils.ts'
- Around the endpoint call:
- const response = await fetch(`${BASE_URL}/triggers/cron_stat_app`, { + const initial = await getCronPlanQueueCount() + const response = await fetch(`${BASE_URL}/triggers/cron_stat_app`, { method: 'POST', headers: triggerHeaders, body: JSON.stringify({ appId, orgId: ORG_ID, }), }) expect(response.status).toBe(200) const json = await response.json() as { status?: string } expect(json.status).toBe('Stats saved') + const final = await getCronPlanQueueCount() + expect(final).toBe(initial + 1) + const latest = await getLatestCronPlanMessage() + expect(latest).toMatchObject({ + function_name: 'cron_stat_org', + payload: { orgId: ORG_ID }, + })
102-103: Update stale comment to match new endpoint.It says “cron_plan” but calls cron_stat_org.
- // Manually trigger cron_plan to simulate queue processing + // Manually trigger cron_stat_org to simulate queue processingtests/queue_cron_stat_org_function.test.ts (2)
2-2: Helper names still use “Plan”; consider renaming for clarity.getCronPlanQueueCount/getLatestCronPlanMessage hit q_cron_stat_org. Rename helpers and usages to cronStatOrg for consistency.
Would you like a follow-up patch to rename the helpers in test-utils and update imports here?
4-5: Run linter autofix for indentation/spacing.Multiple indent issues flagged by ESLint (style/indent, no-multiple-empty-lines).
Run:
- npm run lint:fix
- or npx eslint --fix tests/queue_cron_stat_org_function.test.ts
This will normalize the 4/6/8/12-space patterns and remove extra blank lines. [As per coding guidelines]
Also applies to: 20-21, 57-60, 71-77, 100-103, 119-119, 146-149, 160-166, 172-176, 192-194
netlify/edge-functions/triggers.ts (1)
12-13: Fix import order to satisfy linter.perfectionist/sort-imports expects cron_stat_app before cron_stat_org.
-import { app as cron_stat_org } from '../../supabase/functions/_backend/triggers/cron_stat_org.ts' -import { app as cron_stat_app } from '../../supabase/functions/_backend/triggers/cron_stat_app.ts' +import { app as cron_stat_app } from '../../supabase/functions/_backend/triggers/cron_stat_app.ts' +import { app as cron_stat_org } from '../../supabase/functions/_backend/triggers/cron_stat_org.ts'tests/cron_stat_integration.test.ts (1)
102-104: Nit: update comment to reflect new name.It says “cron_plan”; you’re calling cron_stat_org.
- // Manually trigger cron_plan to simulate queue processing + // Manually trigger cron_stat_org to simulate queue processingsupabase/migrations/20251014105957_rename_plan_cron.sql (1)
4-7: Wrap cron.unschedule calls in DO blocks for idempotenceWrap each
cron.unschedule('…')in aDO $$ BEGIN PERFORM … EXCEPTION WHEN OTHERS THEN NULL; END $$;block to prevent migration failures on re-runs.-SELECT cron.unschedule('process_cron_stats_queue'); -SELECT cron.unschedule('process_cron_stats_jobs'); -SELECT cron.unschedule('process_cron_plan_queue'); +DO $$ BEGIN PERFORM cron.unschedule('process_cron_stats_queue'); EXCEPTION WHEN OTHERS THEN NULL; END $$; +DO $$ BEGIN PERFORM cron.unschedule('process_cron_stats_jobs'); EXCEPTION WHEN OTHERS THEN NULL; END $$; +DO $$ BEGIN PERFORM cron.unschedule('process_cron_plan_queue'); EXCEPTION WHEN OTHERS THEN NULL; END $$;Verified that
process_cron_stats_jobs()is already defined in20250530233128_base.sql.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
cloudflare_workers/api/index.ts(2 hunks)netlify/edge-functions/triggers.ts(2 hunks)src/components.d.ts(0 hunks)src/types/supabase.types.ts(7 hunks)supabase/functions/_backend/triggers/cron_stat_app.ts(2 hunks)supabase/functions/_backend/triggers/cron_stat_org.ts(1 hunks)supabase/functions/_backend/utils/supabase.types.ts(7 hunks)supabase/functions/triggers/index.ts(2 hunks)supabase/migrations/20251007134349_cron_plan_from_stats_backend.sql(3 hunks)supabase/migrations/20251014105957_rename_plan_cron.sql(1 hunks)tests/cron_plan_integration.test.ts(0 hunks)tests/cron_stat_app.test.ts(4 hunks)tests/cron_stat_integration.test.ts(1 hunks)tests/cron_stat_org.test.ts(11 hunks)tests/error-cases.test.ts(2 hunks)tests/queue_cron_stat_org_function.test.ts(9 hunks)tests/queue_load.test.ts(1 hunks)tests/test-utils.ts(1 hunks)tests/trigger-error-cases.test.ts(4 hunks)
💤 Files with no reviewable changes (2)
- src/components.d.ts
- tests/cron_plan_integration.test.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-08T17:29:59.398Z
Learnt from: CR
PR: Cap-go/capgo#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-08T17:29:59.398Z
Learning: Applies to supabase/functions/_backend/triggers/**/*.{ts,tsx,js,sql} : Place database triggers and CRON-related functions under supabase/functions/_backend/triggers/
Applied to files:
supabase/functions/triggers/index.tsnetlify/edge-functions/triggers.tscloudflare_workers/api/index.ts
📚 Learning: 2025-09-08T17:29:59.398Z
Learnt from: CR
PR: Cap-go/capgo#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-08T17:29:59.398Z
Learning: Applies to supabase/functions/_backend/utils/**/*.{ts,tsx,js} : Put shared utilities and database schemas under supabase/functions/_backend/utils/
Applied to files:
netlify/edge-functions/triggers.tscloudflare_workers/api/index.ts
🪛 ESLint
tests/test-utils.ts
[error] 376-376: Prefer Number.parseInt over parseInt.
(unicorn/prefer-number-properties)
netlify/edge-functions/triggers.ts
[error] 13-13: Expected "../../supabase/functions/_backend/triggers/cron_stat_app.ts" to come before "../../supabase/functions/_backend/triggers/cron_stat_org.ts".
(perfectionist/sort-imports)
cloudflare_workers/api/index.ts
[error] 29-29: Expected "../../supabase/functions/_backend/triggers/cron_stat_app.ts" to come before "../../supabase/functions/_backend/triggers/cron_stat_org.ts".
(perfectionist/sort-imports)
tests/queue_cron_stat_org_function.test.ts
[error] 4-5: More than 1 blank line not allowed.
(style/no-multiple-empty-lines)
[error] 20-20: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 21-21: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 57-57: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 58-58: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 59-59: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 71-71: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 72-72: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 73-73: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 74-74: Expected indentation of 8 spaces but found 16.
(style/indent)
[error] 75-75: Expected indentation of 8 spaces but found 16.
(style/indent)
[error] 76-76: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 100-100: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 101-101: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 102-102: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 119-119: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 146-146: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 147-147: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 148-148: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 160-160: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 161-161: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 162-162: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 163-163: Expected indentation of 8 spaces but found 16.
(style/indent)
[error] 164-164: Expected indentation of 8 spaces but found 16.
(style/indent)
[error] 165-165: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 166-166: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 167-167: Expected indentation of 2 spaces but found 4.
(style/indent)
[error] 169-169: Expected indentation of 2 spaces but found 4.
(style/indent)
[error] 170-170: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 172-172: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 173-173: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 174-174: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 175-175: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 192-192: Expected indentation of 4 spaces but found 8.
(style/indent)
[error] 193-193: Expected indentation of 6 spaces but found 12.
(style/indent)
[error] 194-194: Expected indentation of 6 spaces but found 12.
(style/indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests
🔇 Additional comments (16)
supabase/functions/_backend/triggers/cron_stat_org.ts (1)
17-17: LGTM!Log message correctly updated to reflect the new trigger name.
supabase/functions/_backend/triggers/cron_stat_app.ts (2)
20-20: LGTM!Log message correctly updated to reflect the new trigger name.
104-107: LGTM!RPC call correctly updated to use the renamed function
queue_cron_stat_org_for_org.tests/queue_load.test.ts (1)
246-246: LGTM!Comment correctly updated to reference the new queue name, maintaining consistency even in commented-out code.
tests/cron_stat_org.test.ts (2)
115-125: LGTM!Test suite correctly updated to target the new
cron_stat_orgendpoint with proper error handling verification.
117-567: LGTM!All test endpoints consistently updated to use
/triggers/cron_stat_org. Test logic and assertions remain unchanged, ensuring comprehensive coverage of the renamed trigger.tests/error-cases.test.ts (1)
197-209: LGTM!Test correctly updated to target the new
cron_stat_appendpoint with proper error validation.tests/trigger-error-cases.test.ts (2)
21-57: LGTM!Test suite correctly updated to target the new
cron_stat_appendpoint across all error cases.
60-70: LGTM!Test suite correctly updated to target the new
cron_stat_orgendpoint with proper error validation.cloudflare_workers/api/index.ts (1)
99-100: Routes mapping looks correct.New trigger endpoints are wired consistently with backend trigger apps.
netlify/edge-functions/triggers.ts (1)
53-54: Routes mapping looks correct.Endpoints reflect the rename and match imported handlers.
supabase/functions/triggers/index.ts (1)
5-6: LGTM — trigger imports and routes updated coherently.Naming aligns with new cron_stat_app/cron_stat_org endpoints.
Also applies to: 45-46
supabase/migrations/20251014105957_rename_plan_cron.sql (1)
34-61: Function body aligns with tests and queue semantics.Rate-limit check and payload keys (orgId, customerId) match test expectations.
src/types/supabase.types.ts (1)
776-776: Confirm migrations for new global_stats fields
No migration found addingbundle_storage_gborregisters_todayto theglobal_statstable; add them as NOT NULL with default values or update the Row types to allow null.supabase/functions/_backend/utils/supabase.types.ts (2)
776-844: LGTM! New fields added to global_stats table types.The addition of
bundle_storage_gbandregisters_todayfields to theglobal_statstable types is correct. The typing is consistent across Row (required), Insert (optional), and Update (optional) variants, which is typical for generated Supabase types.
1996-1999: Rename verification complete: no references toqueue_cron_plan_for_orgfound. All callers updated consistently in code, tests, triggers, and migrations.
| queue_cron_stat_org_for_org: { | ||
| Args: { customer_id: string; org_id: string } | ||
| Returns: undefined | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Types updated to queue_cron_stat_org_for_org — good.
Matches the migrations and tests.
Optional check for stale references:
🏁 Script executed:
#!/bin/bash
# Find any leftover uses of the old name
rg -nP '\bqueue_cron_plan_for_org\b' -g '!**/node_modules/**' -S
# Check for old endpoints
rg -nP '\bcron_stats\b|\bcron_plan\b' --type ts --type sql -n -g '!**/node_modules/**'Length of output: 2015
Update stale cron_plan references
– tests/error-cases.test.ts:211, tests/cron_stat_integration.test.ts:102 (and commented call in tests/cron_stat_org.test.ts:566) still refer to cron_plan
– supabase/migrations/20251007134349_cron_plan_from_stats_backend.sql:11 uses cron_plan
Rename these to cron_stat_org (queue_cron_stat_org_for_org) to align types with code.
🤖 Prompt for AI Agents
In src/types/supabase.types.ts around lines 1996 to 1999 the RPC/queue type is
defined as queue_cron_stat_org_for_org but the repo still contains stale
references to "cron_plan" in tests and a migration; update all occurrences of
cron_plan to cron_stat_org to keep types and implementation consistent: change
tests/error-cases.test.ts line ~211, tests/cron_stat_integration.test.ts line
~102 and the commented call in tests/cron_stat_org.test.ts around line ~566 to
call/expect cron_stat_org (or queue_cron_stat_org_for_org as appropriate), and
edit supabase/migrations/20251007134349_cron_plan_from_stats_backend.sql (line
~11) to reference cron_stat_org instead of cron_plan; ensure string names and
any SQL function/queue names exactly match the TypeScript type and run
tests/migrations to confirm.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
supabase/migrations/20251014105957_rename_plan_cron.sql (2)
44-46: Guard against multiple rows for a customer (use max())If stripe_info contains multiple rows per customer, SELECT INTO without limit can raise “more than one row returned”. Aggregate to the latest timestamp.
- SELECT plan_calculated_at INTO last_calculated - FROM public.stripe_info - WHERE stripe_info.customer_id = queue_cron_stat_org_for_org.customer_id; + SELECT max(plan_calculated_at) INTO last_calculated + FROM public.stripe_info + WHERE public.stripe_info.customer_id = queue_cron_stat_org_for_org.customer_id;
38-38: Tighten SECURITY DEFINER search_pathEmpty search_path relies on implicit pg_catalog. Prefer explicitly restricting to pg_catalog, pg_temp to avoid hijacking and keep builtins available.
-SET search_path = '' +SET search_path TO pg_catalog, pg_temp
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
supabase/migrations/20251014105957_rename_plan_cron.sql(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run tests
- GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (1)
supabase/migrations/20251014105957_rename_plan_cron.sql (1)
8-13: Pause and drain pgmq queues before dropping
Runningpgmq.drop_queue('cron_stats')and('cron_plan')will delete all in-flight and pending messages. Steps:
- Pause producers writing to these queues
- Drain or migrate remaining messages to the new queues
- Verify each queue is empty
- Drop and recreate with
pgmq.create- Resume producers against
cron_stat_app/cron_stat_org
| SELECT cron.schedule( | ||
| 'process_cron_stat_app_jobs', | ||
| '0 */6 * * *', | ||
| 'SELECT process_cron_stats_jobs();' | ||
| ); |
There was a problem hiding this comment.
Scheduled command still points to old function; update to new one
Job name was renamed to process_cron_stat_app_jobs, but the scheduled command invokes process_cron_stats_jobs(). Call the correct function (and schema‑qualify).
SELECT cron.schedule(
'process_cron_stat_app_jobs',
'0 */6 * * *',
- 'SELECT process_cron_stats_jobs();'
+ 'SELECT public.process_cron_stat_app_jobs()'
);🤖 Prompt for AI Agents
In supabase/migrations/20251014105957_rename_plan_cron.sql around lines 15 to
19, the scheduled command still calls the old function
process_cron_stats_jobs(); update the scheduled invocation to call the renamed
function process_cron_stat_app_jobs and schema‑qualify it (e.g.
public.process_cron_stat_app_jobs()) so the cron.schedule call becomes: schedule
the same job name but invoke SELECT public.process_cron_stat_app_jobs();
ensuring the correct schema name is used.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
supabase/migrations/20251014105957_rename_plan_cron.sql (2)
33-63: Add concurrency guard and simplify parameter reference; current time-window check alone can double-enqueue under race.Simultaneous calls for same customer can both enqueue before
plan_calculated_atupdates. Use an advisory lock to serialize per-customer and simplify the WHERE.CREATE OR REPLACE FUNCTION public.queue_cron_stat_org_for_org(org_id uuid, customer_id text) ... DECLARE last_calculated timestamptz; BEGIN + -- Prevent concurrent enqueues for the same customer within this transaction + IF NOT pg_try_advisory_xact_lock(hashtext(customer_id)) THEN + RETURN; + END IF; -- Check when plan was last calculated for this customer SELECT plan_calculated_at INTO last_calculated FROM public.stripe_info - WHERE stripe_info.customer_id = queue_cron_stat_org_for_org.customer_id; + WHERE stripe_info.customer_id = customer_id; ... END;
64-71: Grant only EXECUTE to service_role (functions don’t need ALL).Use the precise privilege for clarity and least privilege.
-REVOKE ALL ON FUNCTION public.queue_cron_stat_org_for_org(uuid, text) FROM PUBLIC; -REVOKE ALL ON FUNCTION public.queue_cron_stat_org_for_org(uuid, text) FROM anon; -REVOKE ALL ON FUNCTION public.queue_cron_stat_org_for_org(uuid, text) FROM authenticated; -GRANT ALL ON FUNCTION public.queue_cron_stat_org_for_org(uuid, text) TO service_role; +REVOKE ALL ON FUNCTION public.queue_cron_stat_org_for_org(uuid, text) FROM PUBLIC, anon, authenticated; +GRANT EXECUTE ON FUNCTION public.queue_cron_stat_org_for_org(uuid, text) TO service_role;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
supabase/migrations/20251014105957_rename_plan_cron.sql(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests
🔇 Additional comments (2)
supabase/migrations/20251014105957_rename_plan_cron.sql (2)
21-31: Queue processors scheduling looks good; add schema qualification consistently.Both schedules correctly target new queues via
public.process_function_queue(...). LGTM.
3-7: Remove suggestion to unscheduleprocess_cron_plan_jobs
No such job name exists in any migration or base SQL; the existing unschedules cover all legacy cron jobs.Likely an incorrect or invalid review comment.
| SELECT pgmq.drop_queue('cron_stats'); | ||
| SELECT pgmq.drop_queue('cron_plan'); | ||
| SELECT pgmq.create('cron_stat_app'); | ||
| SELECT pgmq.create('cron_stat_org'); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Dropping and recreating queues can lose in-flight messages; add existence guards and avoid data loss.
Blindly drop_queue and create risks:
- Losing pending messages
- Non-idempotent migration (re-run will error)
Add conditional guards; optionally migrate messages before drop.
-SELECT pgmq.drop_queue('cron_stats');
-SELECT pgmq.drop_queue('cron_plan');
-SELECT pgmq.create('cron_stat_app');
-SELECT pgmq.create('cron_stat_org');
+DO $$
+BEGIN
+ -- Drop old queues only if they exist
+ IF EXISTS (SELECT 1 FROM pgmq.meta WHERE queue_name = 'cron_stats') THEN
+ PERFORM pgmq.drop_queue('cron_stats');
+ END IF;
+ IF EXISTS (SELECT 1 FROM pgmq.meta WHERE queue_name = 'cron_plan') THEN
+ PERFORM pgmq.drop_queue('cron_plan');
+ END IF;
+ -- Create new queues only if absent
+ IF NOT EXISTS (SELECT 1 FROM pgmq.meta WHERE queue_name = 'cron_stat_app') THEN
+ PERFORM pgmq.create('cron_stat_app');
+ END IF;
+ IF NOT EXISTS (SELECT 1 FROM pgmq.meta WHERE queue_name = 'cron_stat_org') THEN
+ PERFORM pgmq.create('cron_stat_org');
+ END IF;
+END $$;If historical messages must be preserved, consider a temporary copy/move strategy before drop (can advise further).
🤖 Prompt for AI Agents
In supabase/migrations/20251014105957_rename_plan_cron.sql around lines 9–12,
the migration unconditionally drops and recreates pgmq queues which can discard
in-flight messages and is non-idempotent; change the migration to (1) check for
the existence of each queue before dropping or creating it (query pgmq's queue
metadata or use an existence helper) and only drop if present, (2) make create
operations idempotent (skip creation if the target queue already exists), and
(3) if preserving historical messages is required, move messages to a temporary
queue or use a rename/alter operation instead of drop so no messages are lost;
implement these conditional checks and an optional temporary-migration step so
the migration can be safely re-run without data loss.
| SELECT cron.schedule( | ||
| 'process_cron_stat_app_jobs', | ||
| '0 */6 * * *', | ||
| 'SELECT process_cron_stats_jobs();' | ||
| ); | ||
|
|
There was a problem hiding this comment.
Inconsistent/legacy function name in cron; also missing schema qualification.
Job name says stat_app, but command invokes process_cron_stats_jobs() (old name). This will fail if the function was renamed. Also, qualify with schema.
Two safe options (pick one):
- If there is a new function:
public.process_cron_stat_app_jobs() - Or align with the other schedules and call the queue processor directly.
SELECT cron.schedule(
'process_cron_stat_app_jobs',
'0 */6 * * *',
- 'SELECT process_cron_stats_jobs();'
+ -- Option A (if exists):
+ 'SELECT public.process_cron_stat_app_jobs();'
+ -- Option B (consistent with other schedules):
+ -- 'SELECT public.process_function_queue(''cron_stat_app'')'
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| SELECT cron.schedule( | |
| 'process_cron_stat_app_jobs', | |
| '0 */6 * * *', | |
| 'SELECT process_cron_stats_jobs();' | |
| ); | |
| -- File: supabase/migrations/20251014105957_rename_plan_cron.sql | |
| SELECT cron.schedule( | |
| 'process_cron_stat_app_jobs', | |
| '0 */6 * * *', | |
| -- Option A (if exists): | |
| 'SELECT public.process_cron_stat_app_jobs();' | |
| -- Option B (consistent with other schedules): | |
| -- 'SELECT public.process_function_queue(''cron_stat_app'')' | |
| ); |
🤖 Prompt for AI Agents
In supabase/migrations/20251014105957_rename_plan_cron.sql around lines 15 to
20, the cron job uses an inconsistent/legacy function name and lacks schema
qualification; update the cron schedule to call the correct, schema-qualified
function. Either (a) change the invoked function to the new name with schema
e.g. public.process_cron_stat_app_jobs(), or (b) align with other schedules by
invoking the queue processor directly (schema-qualified), and ensure the job
name, schedule and invoked function match exactly and include the public. schema
prefix.
|
Summary by CodeRabbit
New Features
Refactor
Tests