[PULSE-58] Data retention settings in Site Settings #33
Reference in New Issue
Block a user
No description provided.
Delete Branch "staging"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Work Item
PULSE-58
Summary
Changes
Test Plan
Greptile Summary
This PR implements plan-based data retention settings for sites. Users can now select retention periods from 1 month to 3 years depending on their subscription tier (free: 6mo, solo: 12mo, team: 24mo, business: 36mo). The implementation includes robust error handling for subscription loading failures, automatic clamping to valid plan options when subscriptions load, and updates to the privacy snippet generation. All previously raised issues regarding default values, error handling, and value clamping have been addressed in subsequent commits.
Confidence Score: 4/5
Important Files Changed
Last reviewed commit:
d1d82f56 files reviewed, 2 comments
Edit Code Review Agent Settings | Greptile
Silent error swallowing - if
getSubscription()fails, user gets free tier options even if they're actually on a paid planThis could prevent paid users from accessing their full retention options if the billing API is temporarily unavailable
Prompt To Fix With AI
Race condition: if
subscriptionloads after user interacts with Select, changing from null to loaded state will re-render with new options, butformData.data_retention_monthsmight be outside the new plan's allowed valuesIf user has 24 months set but subscription loads showing they're on free tier (max 6), the Select will show "24 months" in UI but that option won't exist in the dropdown
Prompt To Fix With AI
Issue: If subscription loads after user interaction, formData.data_retention_months can exceed the plan’s allowed values, so the Select shows a value that isn’t in the options.
Fix: Added a useEffect that runs when subscription loads. If data_retention_months is not in the plan’s options, it clamps the value to the plan max via setFormData.
Why: Avoids an invalid Select state where the displayed value doesn’t match any option.
Issue: When getSubscription() fails, we fall back to free-tier options without surfacing the error, so paid users may incorrectly see limited options.
Fix: Call toast.error with the auth error message when getSubscription fails, while still using free-tier options as a fallback so the UI remains usable.
Why: Paid users are aware when billing info fails to load and can retry, without blocking them from setting retention.
6 files reviewed, 3 comments
Edit Code Review Agent Settings | Greptile
Clamping only enforced client-side, no backend validation visible
data_retention_monthsis clamped in the UI when subscription loads, but there's no indication of backend validation. If a user saves before subscription loads, or if they manipulate the request, they could set retention beyond their plan limits. The backend should enforce plan-based retention caps.Prompt To Fix With AI
Default of 12 months exceeds free plan's 6-month limit
When
data.data_retention_monthsis null/undefined (e.g., existing sites before this feature), formData defaults to 12 months. Free tier users are capped at 6 months, so the clamping effect on line 115-117 will immediately change this to 6 when subscription loads. Default should align with the minimum common denominator (6 months) or the backend should set appropriate defaults.Prompt To Fix With AI
Error toast shown but still silently defaults to free tier options
When
getSubscription()fails, the error toast appears butsubscriptionstays null, so line 869 callsgetRetentionOptionsForPlan(null)which returns free tier (6mo max). A paid user experiencing a temporary API issue would see only free options without realizing their actual plan tier is unavailable.Prompt To Fix With AI
Issue: When getSubscription() fails, the toast appears but we still default to free tier options, so paid users may think the limited options are correct.
Fix: Added subscriptionLoadFailed state. On failure, show a persistent inline banner: "Plan limits could not be loaded. Options shown may be limited." plus a Retry button.
Why: Makes it clear that plan info failed to load and gives users a way to retry instead of relying only on a transient toast.
Issue: Reviewer questioned whether the backend enforces plan-based retention caps.
Fix: No code change. The backend already validates in UpdateSiteHandler (internal/api/sites.go): it fetches org billing, derives MaxRetentionMonthsForPlan, and returns 400 when data_retention_months exceeds that value. Client-side clamping is only for UX; the server enforces the limit.
Why: The backend was implemented with validation from the start; this is a clarification rather than a missing check.
Issue: Default of 12 months exceeds the free plan limit of 6 months, causing a visible clamp when subscription loads.
Fix: Changed the default from 12 to 6 when data.data_retention_months is null/undefined, and in the initial formData state.
Why: Uses a safe minimum across all plans so free users avoid the 12→6 jump and the default matches the lowest plan cap.
6 files reviewed, 2 comments
Edit Code Review Agent Settings | Greptile
Clamping can produce invalid Select values
If a site's saved
data_retention_monthsis a value not in the plan's option set (e.g.,2, which isn't in [1, 3, 6, ...]), this code setsMath.min(2, 6) = 2— still not a valid option. TheSelectcomponent would then have a value with no matching dropdown entry, which may render incorrectly or show a blank selection.Consider snapping to the nearest valid option instead of just clamping to the max:
Prompt To Fix With AI
@@ -22,6 +23,7 @@ export function generatePrivacySnippet(site: Site): string {const screen = site.collect_screen_resolution ?? trueMismatched default retention value
The fallback here is
?? 12(12 months), but the settings page (page.tsx:146) defaults to?? 6(6 months). For existing sites that haven't saved adata_retention_monthsvalue yet, the privacy snippet will claim "Raw event data is automatically deleted after 1 year" while the settings page displays and saves "6 months."These should use the same default to avoid misleading the user's privacy policy text.
Prompt To Fix With AI
Issue: Clamping with Math.min(current, max) can yield values not in the options (e.g. 2 with [1,3,6]), so the Select has no matching option and may render incorrectly.
Fix: Choose the largest valid option ≤ current value; if none, use max.
bestFit = values.filter(v => v <= prev.data_retention_months).pop() ?? maxVal.Why: Ensures the Select always receives a value that exists in its options.
@@ -22,6 +23,7 @@ export function generatePrivacySnippet(site: Site): string {const screen = site.collect_screen_resolution ?? trueIssue: The privacy snippet defaulted to 12 months while the settings page defaults to 6, so unsaved sites could show "1 year" in the snippet but "6 months" in the UI.
Fix: Use
?? 6to match the settings page default.Why: Keeps defaults aligned so the generated privacy text matches what the user sees in settings.