Grida Campaigns - Auto tag invitation claims#537
Conversation
- Updated the buildMailtoHref function to manually encode subject and body parameters using encodeURIComponent, ensuring proper encoding of spaces and special characters per RFC 6068. - This change enhances the reliability of email links generated for sharing content.
- Added a new column `ciam_invitee_on_claim_tag_names` to the campaign table to store tag names for automatic application to invitees upon claiming an invitation. - Created a trigger and associated function to handle the auto-tagging process when an invitation is claimed. - Updated the campaign settings UI to include a tagging section for managing invitee tags. - Added tests to verify the auto-tagging functionality works as expected.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
WalkthroughAdds campaign-level customer auto-tagging: new DB column, trigger and function to apply tags when an invitation is claimed, UI to configure per-campaign tag names, TypeScript types update, and pgTAP tests for the flow. Changes
Sequence DiagramsequenceDiagram
participant UI as Campaign Settings
participant DB as Database
participant Claim as Claim Operation
participant Trigger as Auto-Tag Trigger
participant TagFn as apply_customer_tags
UI->>DB: Update campaign with ciam_invitee_on_claim_tag_names
Note over DB: campaign.ciam_invitee_on_claim_tag_names stored
Claim->>DB: Update invitation (is_claimed: false → true)
DB->>Trigger: AFTER UPDATE trigger fires
Trigger->>DB: Fetch campaign.tag_names and project_id
alt tag_names not empty and customer present
Trigger->>TagFn: CALL apply_customer_tags(customer_uid, project_id, tag_names)
TagFn->>DB: Ensure tags exist in public.tag (create if missing)
TagFn->>DB: Insert into grida_ciam.customer_tag (ON CONFLICT DO NOTHING)
Note over DB: Customer associated with tags in project scope
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@supabase/migrations/20260208000000_campaign_ciam_auto_tags.sql`:
- Around line 18-48: Update the SECURITY DEFINER function
grida_west_referral.apply_customer_tags to harden privilege/scope: change the
SET search_path to only "pg_catalog, public" (remove grida_ciam) since all table
references are already fully qualified; before inserting into
grida_ciam.customer_tag validate the tenant boundary by checking that
p_customer_uid belongs to p_project_id (e.g., SELECT 1 FROM grida_ciam.customer
WHERE uid = p_customer_uid AND project_id = p_project_id INTO a boolean/raise
and RETURN if not found) so tags cannot be applied across projects; and tighten
grants after creation by explicitly revoking PUBLIC/anonymous execute rights and
granting EXECUTE only to the service_role (do a REVOKE ALL/EXECUTE FROM
PUBLIC/anon/authenticated and then GRANT EXECUTE ON FUNCTION
grida_west_referral.apply_customer_tags(...) TO service_role).
In `@supabase/tests/test_campaign_auto_tag.sql`:
- Around line 1-155: Add RLS/tenant-isolation pgTAP tests around the existing
happy-path: increase the plan count and add positive tests that the service_role
or an insider persona can claim (using grida_west_referral.claim) and that
grida_ciam.customer_tag and public.tag get written for the same project, and add
negative tests that a different-tenant persona (use SET LOCAL ROLE to a role
representing another project's user) cannot write tags for the target project
and that a non-member persona cannot trigger auto-tagging; specifically exercise
the write path through grida_west_referral.claim -> trigger ->
apply_customer_tags() and assert that cross-project writes to
grida_ciam.customer_tag and public.tag do not occur and that
apply_customer_tags() enforces customer-to-project membership before inserting
tags.
🧹 Nitpick comments (2)
editor/components/formfield-type-select/index.tsx (1)
37-59: Expose aclassNameprop on the trigger for composable styling.Right now the trigger styling is closed over, which makes it hard for consumers to restyle. Consider threading a
classNameprop and merging it withcn(...).♻️ Suggested change
export function TypeSelect({ value, onValueChange, options = all_supported_field_type_items, + className, }: { value?: FormInputType; onValueChange: (value: FormInputType) => void; options?: FormFieldTypeSelectOptionItem[]; + className?: string; }) { @@ - className="w-full justify-between" + className={cn("w-full justify-between", className)}As per coding guidelines: "Use composable styling with className and merge helpers (e.g. cn(...)) in /editor/components; avoid 'closed' styling that can't be overridden" and "Expose className on outer wrappers and important slots in components to allow consumers to restyle".
editor/app/(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/_components/settings.tsx (1)
723-782: Clean implementation of the tagging section.The component correctly bridges the
string[]form model toTagInput's{id, text}shape, handles both the function and array overloads ofsetTags, and conditionally enables autocomplete only when project tags exist. One minor note:Consider filtering out autocomplete options that are already selected, so the dropdown doesn't suggest tags the user has already added. This is a UX polish item and can be deferred.
💡 Optional: filter already-selected tags from autocomplete
render={({ field }) => { const tags = field.value ?? []; + const filtered = autocompleteOptions.filter( + (o) => !tags.includes(o.text) + ); return ( <FormItem> <FormLabel>Invitee tags (on claim)</FormLabel> <FormControl> <TagInput tags={tags.map((t) => ({ id: t, text: t }))} setTags={(newTags) => { const resolved = typeof newTags === "function" ? newTags(tags.map((t) => ({ id: t, text: t }))) : newTags; field.onChange(resolved.map((t) => t.text)); }} activeTagIndex={activeTagIndex} setActiveTagIndex={setActiveTagIndex} - enableAutocomplete={autocompleteOptions.length > 0} - autocompleteOptions={autocompleteOptions} + enableAutocomplete={filtered.length > 0} + autocompleteOptions={filtered} placeholder="Add tags" /> </FormControl>
- Updated the `apply_customer_tags` function to include a validation step ensuring that the customer belongs to the specified project before applying tags. - Adjusted the security settings to restrict function access to the service role only. - Enhanced test setup to accommodate multiple tenants and ensure proper tagging behavior across different projects.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@supabase/tests/test_campaign_auto_tag.sql`:
- Around line 248-260: Add a missing "Test 9" comment label to keep test
numbering consistent: locate the DO block that currently contains two ok()
assertions labeled "Test 8" and the subsequent SELECT that calls
test_set_auth('insider@grida.co') and SELECT
grida_west_referral.apply_customer_tags(...), then insert a comment line "--
Test 9: <brief description>" immediately before the SELECT
test_set_auth('insider@grida.co') (the test invoking apply_customer_tags) so the
sequence reads Test 8, Test 9, Test 10; no code changes to ok(), test_set_auth,
or apply_customer_tags are required.
🧹 Nitpick comments (1)
supabase/schemas/grida_west_referral.sql (1)
902-929: Consider locking down EXECUTE on the trigger function as well.
trg_auto_tag_invitee_on_claim()isSECURITY DEFINERbut has no explicitREVOKE/GRANT. The schema-level default (lines 18, 21) grantsALL ON ROUTINEStoanon, authenticated, service_role. While trigger functions aren't typically callable via PostgREST's RPC interface (they returntrigger, not a data type), a direct SQL caller could stillSELECT grida_west_referral.trg_auto_tag_invitee_on_claim()if they craft the right context. Adding the sameREVOKE ALL … FROM PUBLICpattern used forapply_customer_tagswould be consistent and defensive.Proposed diff
CREATE TRIGGER trg_auto_tag_invitee_on_claim AFTER UPDATE ON grida_west_referral.invitation FOR EACH ROW EXECUTE FUNCTION grida_west_referral.trg_auto_tag_invitee_on_claim(); + +REVOKE ALL ON FUNCTION grida_west_referral.trg_auto_tag_invitee_on_claim() FROM PUBLIC; +REVOKE EXECUTE ON FUNCTION grida_west_referral.trg_auto_tag_invitee_on_claim() FROM anon, authenticated;Based on learnings: "For SECURITY DEFINER functions, set a safe search_path with
SET search_path = pg_catalog, public, fully qualify table references, validate tenant boundary inside the function" and "Explicitly grant permissions only to required roles; avoid accidental PUBLIC access by usingREVOKE ALL FROM PUBLIC;followed by specific role grants".
| -- Test 10: Insider (authenticated) cannot execute apply_customer_tags | ||
| SELECT test_set_auth('insider@grida.co'); | ||
| SELECT throws_ok( | ||
| format( | ||
| 'SELECT grida_west_referral.apply_customer_tags(%L::uuid, %s::bigint, ARRAY[''test''])', | ||
| current_setting('test.local_invitee_uid'), | ||
| current_setting('test.local_project_id') | ||
| ), | ||
| '42501', -- insufficient_privilege | ||
| NULL, | ||
| 'authenticated user should be denied EXECUTE on apply_customer_tags' | ||
| ); | ||
| SELECT test_reset_auth(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, verify that 'insider@grida.co' exists in the seed data
rg "insider@grida.co" supabase/seed.sqlRepository: gridaco/grida
Length of output: 327
🏁 Script executed:
# Examine the test file around the mentioned lines to verify test numbering
cat -n supabase/tests/test_campaign_auto_tag.sql | sed -n '210,265p'Repository: gridaco/grida
Length of output: 2061
Add "Test 9" comment label for test numbering consistency.
Lines 214–241 (the DO block with two ok() assertions) are labeled "Test 8", but the next test at line 248 is labeled "Test 10". Add a "Test 9" comment above line 234 (or line 225) to maintain consistent numbering throughout the test file.
The seed data for insider@grida.co is present in supabase/seed.sql, so the helper will not raise an exception due to missing data.
🤖 Prompt for AI Agents
In `@supabase/tests/test_campaign_auto_tag.sql` around lines 248 - 260, Add a
missing "Test 9" comment label to keep test numbering consistent: locate the DO
block that currently contains two ok() assertions labeled "Test 8" and the
subsequent SELECT that calls test_set_auth('insider@grida.co') and SELECT
grida_west_referral.apply_customer_tags(...), then insert a comment line "--
Test 9: <brief description>" immediately before the SELECT
test_set_auth('insider@grida.co') (the test invoking apply_customer_tags) so the
sequence reads Test 8, Test 9, Test 10; no code changes to ok(), test_set_auth,
or apply_customer_tags are required.
1 participant
Summary by CodeRabbit
New Features
UI/UX Improvements
Tests