-
Notifications
You must be signed in to change notification settings - Fork 0
Integrate external services for data management #6
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
base: main
Are you sure you want to change the base?
Integrate external services for data management #6
Conversation
Co-authored-by: info <info@reeseastor.com>
|
Cursor Agent can help with this pull request. Just |
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.
Pull Request Overview
Adds optional external service integrations (Supabase Storage, Notion, Google Drive) to the application with configuration via environment variables and UI checkboxes for per-action opt-in functionality.
- New integration configuration module with service client initialization and availability checking
- Extended file upload and credit memo APIs to support optional external service uploads/exports
- Added UI checkboxes and client-side integration availability loading
- Database schema updates to store external service references
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| server.js | Adds integration imports, database columns, migration logic, and extended APIs |
| public/script.js | Adds integration availability loading and checkbox handling |
| public/index.html | Adds UI checkboxes for Supabase, Drive, and Notion integrations |
| package.json | Updates dependencies to include integration libraries |
| config/integrations.js | New module for service client initialization and helper functions |
| README.md | Adds integration setup documentation |
| .env.example | Adds environment variable template |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if (googlePrivateKey && googlePrivateKey.includes('\\n')) { | ||
| googlePrivateKey = googlePrivateKey.replace(/\\n/g, '\n'); |
Copilot
AI
Sep 29, 2025
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.
The private key processing logic should handle all common escape sequences, not just \\n. Consider also handling \\r\\n and other whitespace characters that might be escaped in environment variables.
| if (googlePrivateKey && googlePrivateKey.includes('\\n')) { | |
| googlePrivateKey = googlePrivateKey.replace(/\\n/g, '\n'); | |
| if (googlePrivateKey) { | |
| // Unescape all common escape sequences (e.g., \n, \r, \t, etc.) | |
| googlePrivateKey = JSON.parse('"' + googlePrivateKey.replace(/\\/g, '\\\\').replace(/"/g, '\\"') + '"'); |
| fileId, | ||
| requestBody: { role: 'reader', type: 'anyone' }, | ||
| }); | ||
| } catch (e) {} |
Copilot
AI
Sep 29, 2025
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.
The empty catch block silently ignores permission setting failures. Consider logging the error or at least adding a comment explaining why this failure is acceptable.
| } catch (e) {} | |
| } catch (e) { | |
| console.error('Failed to set Google Drive file permissions for fileId:', fileId, e); | |
| } |
| (async () => { | ||
| const updates = {}; | ||
| const originalName = (req.file.originalname || 'file').replace(/\s+/g, '_'); | ||
| const destinationPath = `company_${company_id || 'unknown'}/${Date.now()}_${originalName}`; |
Copilot
AI
Sep 29, 2025
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.
The filename sanitization only replaces spaces with underscores but doesn't handle other potentially dangerous characters. Consider using a more comprehensive sanitization approach that removes or escapes characters like ../, special symbols, or unicode characters that could cause path traversal or filesystem issues.
| // Utility to add a column if it does not exist (for existing DBs) | ||
| function ensureColumnExists(tableName, columnName, columnType) { | ||
| return new Promise((resolve, reject) => { |
Copilot
AI
Sep 29, 2025
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.
Direct string interpolation in SQL queries creates a potential SQL injection vulnerability. Use parameterized queries or validate tableName against a whitelist of allowed table names.
| // Utility to add a column if it does not exist (for existing DBs) | |
| function ensureColumnExists(tableName, columnName, columnType) { | |
| return new Promise((resolve, reject) => { | |
| // Utility to add a column if it does not exist (for existing DBs) | |
| // Whitelist of allowed table names for schema changes | |
| const ALLOWED_TABLES = [ | |
| 'novels', | |
| 'chapters', | |
| 'story_beats', | |
| 'financial_data', | |
| 'credit_memos' | |
| ]; | |
| function ensureColumnExists(tableName, columnName, columnType) { | |
| return new Promise((resolve, reject) => { | |
| if (!ALLOWED_TABLES.includes(tableName)) { | |
| return reject(new Error(`Invalid table name: ${tableName}`)); | |
| } |
| if (err) return reject(err); | ||
| const exists = rows.some(r => r.name === columnName); | ||
| if (exists) return resolve(); | ||
| db.run(`ALTER TABLE ${tableName} ADD COLUMN ${columnName} ${columnType}`, (alterErr) => { |
Copilot
AI
Sep 29, 2025
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.
Direct string interpolation in SQL ALTER TABLE statement is unsafe. Validate tableName, columnName, and columnType parameters against whitelists or use a safer approach for schema migrations.
| const setClauses = Object.keys(updates).map(k => `${k} = ?`).join(', '); | ||
| const params = [...Object.values(updates), recordId]; | ||
| db.run(`UPDATE financial_data SET ${setClauses} WHERE id = ?`, params, (updErr) => { | ||
| if (updErr) { | ||
| console.warn('Failed to update financial_data with external links:', updErr.message); | ||
| } | ||
| res.json({ id: recordId, message: 'Financial document uploaded and processed', ...updates }); | ||
| }); | ||
| } else { | ||
| res.json({ id: recordId, message: 'Financial document uploaded and processed' }); | ||
| } |
Copilot
AI
Sep 29, 2025
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.
Column names from Object.keys(updates) are directly interpolated into SQL without validation. Validate column names against a whitelist to prevent SQL injection through malicious property names.
| "license": "MIT", | ||
| "dependencies": { | ||
| "@notionhq/client": "^5.1.0", | ||
| "@supabase/storage-js": "^2.12.2", |
Copilot
AI
Sep 29, 2025
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.
The @supabase/storage-js package is redundant since @supabase/supabase-js already includes storage functionality. Remove this dependency to avoid potential version conflicts and reduce bundle size.
| "@supabase/storage-js": "^2.12.2", |
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Integrate Supabase, Google Drive, and Notion to enable optional file uploads and credit memo exports, configured via environment variables and controlled by new UI checkboxes.
Note
Adds optional Supabase Storage, Notion export, and Google Drive upload integrations with UI checkboxes, new API endpoints, and DB fields configured via environment variables.
config/integrations.jsfor Supabase, Notion, and Google Drive clients plus helpers (uploadFileToSupabase,uploadFileToGoogleDrive,exportCreditMemoToNotion).POST /api/upload-financialacceptsuploadToSupabase/uploadToDrive;POST /api/credit-memosacceptsexportToNotion; newGET /api/integrationsreturns availability flags.dotenv; ensuresuploads/exists.financial_datawithsupabase_path,supabase_public_url,gdrive_file_id,gdrive_web_view_link.credit_memoswithnotion_page_id,notion_url.ensureColumnExists).public/index.htmlfor Supabase/Drive on upload and Notion on memo.public/script.js: sends flags, parses metrics, and disables checkboxes based onGET /api/integrations..env.examplewith required vars;README.mdadds setup for Supabase, Notion, Google Drive and runtime notes.@supabase/supabase-js,@notionhq/client,googleapis, anddotenv(updatespackage.json/lock).Written by Cursor Bugbot for commit 865d622. This will update automatically on new commits. Configure here.