-
Notifications
You must be signed in to change notification settings - Fork 1
accounts system, backups, pttm update, dependency updates #411
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…/bits-ui-2.8.11 Bump bits-ui from 2.8.0 to 2.8.11
…-metal update pttm
add some analytics events
WalkthroughThis update introduces an account system with authentication via Clerk, cloud backup management using Convex, and analytics event tracking with PostHog. It adds new backend handlers, Svelte components, UI elements for backup operations, and updates workflow files and dependencies. The backup feature allows users to create, restore, and manage backups both locally and in the cloud. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SvelteApp
participant Clerk
participant Convex
participant PostHog
User->>SvelteApp: Loads /backups page
SvelteApp->>Clerk: Get session token (JWT)
Clerk-->>SvelteApp: Returns JWT
SvelteApp->>Convex: Query backups with JWT
Convex-->>SvelteApp: Returns backup list
User->>SvelteApp: Clicks "Create Backup"
SvelteApp->>createBackup: Serialize storage/cookies
createBackup-->>SvelteApp: Returns backup data
SvelteApp->>Convex: Mutation create (JWT, name, data)
Convex-->>SvelteApp: Confirms creation
User->>SvelteApp: Clicks "Restore" on backup
SvelteApp->>restoreBackup: Decode and restore storage/cookies
restoreBackup-->>SvelteApp: Restores state
SvelteApp->>User: Reloads page
User->>SvelteApp: Clicks "Delete" on backup
SvelteApp->>Convex: Mutation remove (JWT, backup ID)
Convex-->>SvelteApp: Confirms deletion
SvelteApp->>PostHog: Track user actions (create, restore, delete, etc.)
Possibly related PRs
📜 Recent review detailsConfiguration used: .coderabbit.yaml ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (7)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 20
🧹 Nitpick comments (6)
src/lib/createBackup.ts (2)
37-37: Remove debug console.log statement.The console.log statement should be removed from production code or made conditional for debugging purposes.
- console.log(data); + // console.log(data); // Debug: uncomment for development
1-45: Consider adding browser compatibility checks and security improvements.The function should handle edge cases and provide better security filtering:
export default function createBackup() { // Check for required APIs if (typeof localStorage === 'undefined' || typeof sessionStorage === 'undefined') { throw new Error('Storage APIs not available in this environment'); } // More comprehensive filtering for sensitive data const isSensitiveKey = (key: string) => { const sensitivePatterns = ['__', 'clerk', 'ph', 'auth', 'token', 'session']; const sensitiveKeys = ['lastSeenSurveyDate', 'password', 'secret']; return sensitivePatterns.some(pattern => key.toLowerCase().startsWith(pattern)) || sensitiveKeys.some(sensitiveKey => key.toLowerCase().includes(sensitiveKey)); }; // Rest of the function with enhanced filtering... }src/convex/schema.ts (1)
1-15: LGTM: Well-structured schema with suggestions for enhancementThe schema definition is clean and follows Convex conventions properly. Both tables are appropriately designed for their use cases.
Consider these optional enhancements:
- Add timestamp fields for audit trails (
createdAt,updatedAt)- Consider indexing strategies for the
userfield for performance- Verify if
gmaeidnaming is consistent with your domain model (game vs gmae)For improved data modeling, consider:
export default defineSchema({ comments: defineTable({ body: v.string(), gmaeid: v.string(), - user: v.string() + user: v.string(), + createdAt: v.number(), + updatedAt: v.optional(v.number()) - }), + }).index("by_user", ["user"]).index("by_gmae", ["gmaeid"]), backup: defineTable({ name: v.string(), data: v.string(), - user: v.string() + user: v.string(), + createdAt: v.number(), + updatedAt: v.optional(v.number()) - }) + }).index("by_user", ["user"]) });src/lib/components/sidebar-auth.svelte (1)
115-122: Consider improving the UX for domain restrictionsThe current implementation requires users to click through multiple UI elements before discovering the domain limitation. Consider showing this notice more prominently or automatically redirecting users.
src/routes/backups/backup.svelte (2)
42-42: Fix time format for better clarityThe current format uses
hhwhich shows hours with leading zeros (e.g., "01:30 PM"). For 12-hour format, usehinstead, or consider using 24-hour format for international users.Apply one of these options:
-{dayjs(backup.creationTime).format('hh:mm a - DD/MM/YY')} +{dayjs(backup.creationTime).format('h:mm A - DD/MM/YY')}Or for 24-hour format:
-{dayjs(backup.creationTime).format('hh:mm a - DD/MM/YY')} +{dayjs(backup.creationTime).format('HH:mm - DD/MM/YY')}
106-106: Enhance error logging with backup contextInclude backup details in the error log to aid debugging.
-console.error('Failed to delete backup:', error); +console.error('Failed to delete backup:', { backupId: backup.id, backupName: backup.name, error });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlsrc/convex/_generated/api.d.tsis excluded by!**/_generated/**src/convex/_generated/api.jsis excluded by!**/_generated/**src/convex/_generated/dataModel.d.tsis excluded by!**/_generated/**src/convex/_generated/server.d.tsis excluded by!**/_generated/**src/convex/_generated/server.jsis excluded by!**/_generated/**
📒 Files selected for processing (19)
.github/workflows/beta_deploy.yml(1 hunks).github/workflows/build.yml(1 hunks).github/workflows/deploy.yml(1 hunks).prettierignore(1 hunks)convex.json(1 hunks)package.json(2 hunks)src/convex/backups.ts(1 hunks)src/convex/schema.ts(1 hunks)src/lib/components/app-sidebar.svelte(4 hunks)src/lib/components/play.svelte(7 hunks)src/lib/components/settings.svelte(1 hunks)src/lib/components/sidebar-auth.svelte(1 hunks)src/lib/createBackup.ts(1 hunks)src/lib/restoreBackup.ts(1 hunks)src/routes/+layout.svelte(3 hunks)src/routes/+page.svelte(1 hunks)src/routes/backups/+page.svelte(1 hunks)src/routes/backups/backup.svelte(1 hunks)src/routes/identify.svelte(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.*
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🪛 Gitleaks (8.27.2)
.github/workflows/beta_deploy.yml
32-32: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (12)
src/routes/+page.svelte (1)
7-8: LGTM! Icon import migration looks correct.The import path changes from
lucide-svelte/icons/to@lucide/svelte/icons/align with the package consolidation mentioned in the summary where@lucide/sveltewas moved to dependencies andlucide-sveltewas removed.convex.json (1)
1-3: LGTM! Standard Convex configuration.The configuration correctly points to the Convex functions directory and follows the standard Convex project structure.
.github/workflows/build.yml (1)
25-27: LGTM: Environment variables properly configured for buildThe addition of
PUBLIC_CLERK_PUBLISHABLE_KEYandPUBLIC_CONVEX_URLenvironment variables is appropriate. These are public-facing configuration values that are safe to expose in the build environment..prettierignore (1)
5-7: LGTM: Appropriate exclusions for Convex integrationThe additions correctly exclude generated Convex files and configuration from Prettier formatting, which is a best practice for auto-generated code and configuration files.
.github/workflows/beta_deploy.yml (1)
31-33: LGTM: Environment variables properly configured (static analysis false positive)The environment variables are correctly added. The static analysis warning about the
PUBLIC_CLERK_PUBLISHABLE_KEYis a false positive - Clerk publishable keys are designed to be exposed publicly and are safe to include in client-side builds and CI environments.src/lib/components/play.svelte (3)
12-18: LGTM: Icon import path migrationThe migration from
lucide-svelte/icons/to@lucide/svelte/icons/is correctly implemented across all icon imports.
24-25: LGTM: Analytics dependencies properly importedPostHog and onMount imports are correctly added for the analytics integration.
130-130: LGTM: Comprehensive analytics tracking for user interactionsThe analytics events are well-placed to capture all major user interactions (reload, fullscreen, new tab, share, favorite) with appropriate metadata including the game ID. This provides good insight into user behavior patterns.
Also applies to: 143-143, 153-153, 163-163, 185-185
src/lib/components/app-sidebar.svelte (3)
20-35: LGTM! Clean integration of authentication and analytics.The icon imports have been properly updated to use
@lucide/svelte, and the new authentication-related imports are well organized. The analytics integration follows good practices.Also applies to: 54-56
155-160: LGTM! Backups navigation properly gated behind experimental features.The new "Backups" navigation item is correctly implemented and appropriately marked as experimental.
206-206: LGTM! Analytics events properly implemented.The PostHog event tracking for search and sidebar toggle actions follows good analytics practices by capturing relevant context.
Also applies to: 384-384
src/lib/components/sidebar-auth.svelte (1)
69-73: LGTM!The logout handler properly cleans up the session, resets analytics, and handles UI state.
Deploying edutools-testing with
|
| Latest commit: |
c3c14f8
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://cd68c95f.edutools-testing.pages.dev |
| Branch Preview URL: | https://219-feature-bug-report-witho.edutools-testing.pages.dev |
|
wtf it works on my machine... |
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores
Documentation