feat: frontend revamps and connection establishment; enabling org owners to setup DevR AI to their respected platforms #141
Conversation
WalkthroughAdds an integrations feature end-to-end: new FastAPI router and service with Supabase-backed CRUD/status endpoints, DB migration and RLS, auth dependency for Bearer tokens, frontend API client and UI modals, plus CORS and frontend env/dependency updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Frontend as BotIntegration / Modal
participant Api as apiClient (Axios)
participant Auth as Supabase Auth
participant Backend as FastAPI /v1/integrations
participant Service as IntegrationService
participant DB as Supabase (organization_integrations)
Note over Frontend,Api: Load status on mount
User->>Frontend: Open Integrations page
Frontend->>Api: GET /v1/integrations/status/{platform} (Bearer)
Api->>Auth: Read session, validate token
Api->>Backend: Forward request with Authorization
Backend->>Service: get_integration_status(user_id, platform)
Service->>DB: SELECT WHERE user_id & platform
DB-->>Service: Row or none
Service-->>Backend: IntegrationStatusResponse
Backend-->>Api: 200 OK
Api-->>Frontend: Status payload
rect rgba(200,255,200,0.15)
Note right of Frontend: Create / Update flow
User->>Frontend: Click Connect / Manage
Frontend->>Frontend: Open IntegrationModal
User->>Frontend: Submit form
Frontend->>Api: POST or PUT /v1/integrations (Bearer, payload)
Api->>Backend: Authenticated request
Backend->>Service: create_integration / update_integration
Service->>DB: INSERT/UPDATE row
DB-->>Service: Row data
Service-->>Backend: IntegrationResponse
Backend-->>Api: 200/201 OK
Api-->>Frontend: Response
Frontend->>Api: GET status (refresh)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (17)
backend/main.py (1)
107-119: Consider moving CORS origins to configuration.The CORS middleware uses hardcoded localhost origins. For production deployments and flexibility, consider loading allowed origins from environment variables or the settings module.
Example approach:
In
backend/app/core/config.py, add:allowed_origins: list[str] = Field( default_factory=lambda: ["http://localhost:5173", "http://localhost:3000"] )Then update the middleware configuration:
api.add_middleware( CORSMiddleware, - allow_origins=[ - "http://localhost:5173", # Vite default dev server - "http://localhost:3000", # Alternative dev server - "http://127.0.0.1:5173", - "http://127.0.0.1:3000", - ], + allow_origins=settings.allowed_origins, allow_credentials=True, allow_methods=["*"], allow_headers=["*"], )frontend/src/components/integration/ComingSoonModal.tsx (1)
1-89: LGTM! Well-structured modal component.The component follows good React patterns:
- Early return for performance when modal is closed
- Proper accessibility with backdrop click-to-close
- Smooth animations with framer-motion
- Clear visual hierarchy and user experience
Optional:
AnimatePresenceon line 21 is not strictly required here since the component returnsnullbefore rendering when closed. Consider removing it or adding an exit animation by restructuring the conditional rendering:<AnimatePresence> {isOpen && ( <div className="fixed inset-0 z-50 flex items-center justify-center p-4"> {/* Modal content */} </div> )} </AnimatePresence>This would enable proper exit animations when the modal closes.
frontend/src/lib/api.ts (2)
65-76: Use AxiosHeaders-safe header assignment for AuthorizationDirectly writing config.headers.Authorization can be brittle in Axios 1.x. Prefer
.setwhen available, or defensively assign a plain object.Apply one of these patterns:
- if (session?.access_token) { - config.headers.Authorization = `Bearer ${session.access_token}`; - } + if (session?.access_token) { + if (config.headers && 'set' in config.headers) { + // AxiosHeaders instance + (config.headers as any).set('Authorization', `Bearer ${session.access_token}`); + } else { + // Plain object + config.headers = { + ...(config.headers || {}), + Authorization: `Bearer ${session.access_token}`, + }; + } + }
57-63: Add a client timeout to avoid hanging requestsSet a sane timeout (e.g., 10s) to protect the UI from stalled network calls.
this.client = axios.create({ baseURL: API_BASE_URL, headers: { 'Content-Type': 'application/json', }, + timeout: 10000, });frontend/src/components/integration/BotIntegration.tsx (2)
27-28: Remove unused isLoading state and spinner (modal handles loading)Local isLoading is never set; the button’s spinner/disabled states never engage. Simplify by removing this state and related UI.
Consider:
- Delete
const [isLoading, setIsLoading] = useState(false);- Remove
disabled={isLoading}and the spinner branch; always show the Connect/Manage label (modal shows progress).Also applies to: 127-147
35-45: Normalize platform once to reduce repeated toLowerCase castsCompute a single
normalizedPlatformand reuse it to avoid repeated casts.- const loadIntegrationStatus = async () => { + const loadIntegrationStatus = async () => { + const normalizedPlatform = platform.toLowerCase() as Platform; try { - const status = await apiClient.getIntegrationStatus(platform.toLowerCase() as Platform); + const status = await apiClient.getIntegrationStatus(normalizedPlatform); setIsConnected(status.is_connected); if (status.is_connected) { const integrations = await apiClient.getIntegrations(); - const platformIntegration = integrations.find( - i => i.platform === platform.toLowerCase() - ); + const platformIntegration = integrations.find(i => i.platform === normalizedPlatform); setIntegration(platformIntegration || null); } } catch (error) { console.error('Error loading integration status:', error); } }; ... - await apiClient.createIntegration({ - platform: platform.toLowerCase() as Platform, + await apiClient.createIntegration({ + platform: normalizedPlatform,Also applies to: 56-71
frontend/src/components/integration/IntegrationModal.tsx (2)
83-96: Exit animations won’t play due to earlyreturn nullReturning null when
!isOpenunmounts the modal immediately. RenderAnimatePresencealways and conditionally render the modal inside it.- if (!isOpen) return null; - - return ( - <AnimatePresence> - <div className="fixed inset-0 z-50 flex items-center justify-center p-4"> + return ( + <AnimatePresence> + {isOpen && ( + <div className="fixed inset-0 z-50 flex items-center justify-center p-4"> {/* Backdrop */} <motion.div initial={{ opacity: 0 }} animate={{ opacity: 1 }} exit={{ opacity: 0 }} onClick={onClose} className="absolute inset-0 bg-black/60 backdrop-blur-sm" /> {/* Modal */} <motion.div initial={{ opacity: 0, scale: 0.95, y: 20 }} animate={{ opacity: 1, scale: 1, y: 0 }} exit={{ opacity: 0, scale: 0.95, y: 20 }} className="relative w-full max-w-md bg-gray-900 rounded-xl border border-gray-800 shadow-2xl" > ... - </div> + </div> + )} </AnimatePresence> );Also applies to: 97-105, 120-121, 221-225
32-39: Reinitialize form state on open/existingData changesForm state persists across openings; update it when the modal opens or
existingDatachanges so users see fresh values after updates.+ React.useEffect(() => { + if (!isOpen) return; + setFormData({ + organization_name: existingData?.organization_name || '', + organization_link: existingData?.config?.organization_link || '', + config: existingData?.config || {}, + }); + setErrors({}); + }, [isOpen, existingData]);Also applies to: 24-31
backend/app/api/v1/integrations.py (1)
24-31: Avoid blindexcept Exception; use exception chaining (from e)Keep the error context and reduce blanket catches where possible. At minimum, chain the original exception.
Example change (repeat similarly across handlers):
- except Exception as e: - raise HTTPException( - status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - detail=f"Failed to create integration: {str(e)}" - ) + except Exception as e: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=f"Failed to create integration: {str(e)}" + ) from eAlso applies to: 39-43, 55-58, 80-83, 98-101, 115-118
backend/app/services/integration_service.py (3)
47-49: Use timezone-aware UTC timestampsStore consistent UTC times to avoid TZ drift.
- "created_at": datetime.now().isoformat(), - "updated_at": datetime.now().isoformat() + "created_at": datetime.utcnow().isoformat() + "Z", + "updated_at": datetime.utcnow().isoformat() + "Z" @@ - update_data = {"updated_at": datetime.now().isoformat()} + update_data = {"updated_at": datetime.utcnow().isoformat() + "Z"}Also applies to: 132-133
16-21: Introduce a domainNotFoundErrorand raise it instead of genericExceptionThis allows the API layer to map missing resources to HTTP 404 cleanly.
class IntegrationService: @@ - if not result.data: - raise Exception("Integration not found or update failed") + if not result.data: + raise NotFoundError("Integration not found") @@ - return IntegrationResponse(**result.data[0]) + return IntegrationResponse(**result.data[0]) @@ - return True + return TrueAdd the exception near the top:
+class NotFoundError(Exception): + passAlso applies to: 157-158, 166-166, 178-179
66-69: Log exceptions with tracebacksUse
logger.exception(...)insideexceptblocks to capture stack traces.- except Exception as e: - logger.error(f"Error creating integration: {str(e)}") + except Exception as e: + logger.exception(f"Error creating integration: {e}") raise(Apply similarly to other
exceptblocks in this file.)Also applies to: 80-83, 98-101, 121-123, 164-166, 181-182, 207-209, 222-223
backend/database/01_create_integration_tables.sql (5)
16-19: Index review: drop redundancy and prefer a partial boolean index.
- idx_org_integrations_user_id is redundant: the UNIQUE(user_id, platform[, organization_name]) creates a btree with user_id as the leading column; it already serves user_id filters.
- idx_org_integrations_is_active on a boolean is low‑selectivity; prefer a partial index on the hot value if commonly queried (e.g., TRUE).
-CREATE INDEX IF NOT EXISTS idx_org_integrations_user_id ON organization_integrations(user_id); CREATE INDEX IF NOT EXISTS idx_org_integrations_platform ON organization_integrations(platform); -CREATE INDEX IF NOT EXISTS idx_org_integrations_is_active ON organization_integrations(is_active); +-- Optional: only if most queries filter active rows +-- CREATE INDEX IF NOT EXISTS idx_org_integrations_active_true +-- ON organization_integrations (is_active) +-- WHERE is_active;
8-8: Make config non‑nullable to simplify queries and constraints.Avoid tri‑state semantics (NULL vs '{}' vs object).
- config JSONB DEFAULT '{}', -- Stores org link, discord_guild_id, etc. + config JSONB NOT NULL DEFAULT '{}'::jsonb, -- Stores org link, discord_guild_id, etc.
3-3: Ensure gen_random_uuid() is available.If your environment doesn’t pre‑enable pgcrypto, guard it in migrations.
+-- Ensure pgcrypto is available for gen_random_uuid() +CREATE EXTENSION IF NOT EXISTS pgcrypto;
5-6: Optional: prefer a dedicated enum for platform.A PostgreSQL ENUM (or domain) gives clearer typing and reuse vs raw VARCHAR + CHECK; it also centralizes allowed values for other tables.
6-6: Optional: enforce non‑empty organization_name.Prevent empty/whitespace names at the DB layer.
ALTER TABLE organization_integrations ADD CONSTRAINT organization_name_non_empty_chk CHECK (btrim(organization_name) <> '');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
backend/app/api/router.py(2 hunks)backend/app/api/v1/integrations.py(1 hunks)backend/app/core/dependencies.py(1 hunks)backend/app/models/database/supabase.py(1 hunks)backend/app/models/integration.py(1 hunks)backend/app/services/integration_service.py(1 hunks)backend/database/01_create_integration_tables.sql(1 hunks)backend/main.py(4 hunks)frontend/.env example(1 hunks)frontend/src/components/integration/BotIntegration.tsx(1 hunks)frontend/src/components/integration/BotIntegrationPage.tsx(2 hunks)frontend/src/components/integration/ComingSoonModal.tsx(1 hunks)frontend/src/components/integration/IntegrationModal.tsx(1 hunks)frontend/src/lib/api.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
frontend/src/components/integration/BotIntegration.tsx (2)
frontend/src/lib/api.ts (3)
Integration(19-28)apiClient(176-176)Platform(12-12)frontend/src/components/integration/IntegrationModal.tsx (1)
IntegrationFormData(18-22)
frontend/src/lib/api.ts (1)
frontend/src/lib/supabaseClient.ts (1)
supabase(12-12)
backend/app/api/v1/integrations.py (3)
backend/app/models/integration.py (3)
IntegrationResponse(25-35)IntegrationListResponse(38-41)IntegrationStatusResponse(44-49)backend/app/core/dependencies.py (1)
get_current_user(18-69)backend/app/services/integration_service.py (1)
get_integrations(70-82)
backend/app/core/dependencies.py (2)
backend/app/database/supabase/client.py (1)
get_supabase_client(9-13)backend/main.py (1)
DevRAIApplication(27-88)
backend/app/services/integration_service.py (2)
backend/app/database/supabase/client.py (1)
get_supabase_client(9-13)backend/app/models/integration.py (2)
IntegrationResponse(25-35)IntegrationStatusResponse(44-49)
🪛 Ruff (0.14.0)
backend/app/api/v1/integrations.py
19-19: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
25-25: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
26-26: Do not catch blind exception: Exception
(BLE001)
27-30: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
29-29: Use explicit conversion flag
Replace with conversion flag
(RUF010)
34-34: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
39-39: Do not catch blind exception: Exception
(BLE001)
40-43: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
42-42: Use explicit conversion flag
Replace with conversion flag
(RUF010)
49-49: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
54-54: Do not catch blind exception: Exception
(BLE001)
55-58: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
57-57: Use explicit conversion flag
Replace with conversion flag
(RUF010)
64-64: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
71-74: Abstract raise to an inner function
(TRY301)
76-76: Consider moving this statement to an else block
(TRY300)
79-79: Do not catch blind exception: Exception
(BLE001)
80-83: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
82-82: Use explicit conversion flag
Replace with conversion flag
(RUF010)
90-90: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
97-97: Do not catch blind exception: Exception
(BLE001)
98-101: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
100-100: Use explicit conversion flag
Replace with conversion flag
(RUF010)
107-107: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
114-114: Do not catch blind exception: Exception
(BLE001)
115-118: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
117-117: Use explicit conversion flag
Replace with conversion flag
(RUF010)
backend/app/core/dependencies.py
53-57: Abstract raise to an inner function
(TRY301)
63-63: Do not catch blind exception: Exception
(BLE001)
64-64: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
64-64: Use explicit conversion flag
Replace with conversion flag
(RUF010)
65-69: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
backend/app/services/integration_service.py
37-37: Abstract raise to an inner function
(TRY301)
37-37: Avoid specifying long messages outside the exception class
(TRY003)
60-60: Abstract raise to an inner function
(TRY301)
60-60: Create your own exception
(TRY002)
60-60: Avoid specifying long messages outside the exception class
(TRY003)
67-67: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
67-67: Use explicit conversion flag
Replace with conversion flag
(RUF010)
81-81: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
81-81: Use explicit conversion flag
Replace with conversion flag
(RUF010)
99-99: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
99-99: Use explicit conversion flag
Replace with conversion flag
(RUF010)
121-121: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
121-121: Use explicit conversion flag
Replace with conversion flag
(RUF010)
157-157: Abstract raise to an inner function
(TRY301)
157-157: Create your own exception
(TRY002)
157-157: Avoid specifying long messages outside the exception class
(TRY003)
164-164: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
164-164: Use explicit conversion flag
Replace with conversion flag
(RUF010)
178-178: Consider moving this statement to an else block
(TRY300)
181-181: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
181-181: Use explicit conversion flag
Replace with conversion flag
(RUF010)
207-207: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
207-207: Use explicit conversion flag
Replace with conversion flag
(RUF010)
222-222: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
222-222: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🔇 Additional comments (5)
frontend/src/components/integration/BotIntegrationPage.tsx (1)
109-134: LGTM! Appropriate use of coming soon flag.The
comingSoon={true}prop correctly marks Slack and Discourse as upcoming integrations, aligning with the new modal-driven workflow introduced in this PR.backend/app/api/router.py (1)
20-24: LGTM! Standard router mounting.The integrations router is correctly mounted at
/v1/integrations, following the existing pattern for auth and health routers.backend/app/models/database/supabase.py (1)
191-212: LGTM! Well-structured database model.The
OrganizationIntegrationmodel appropriately represents integration metadata with clear field types and documentation. The explicit note about not storing credentials is a good practice.backend/app/models/integration.py (1)
1-49: LGTM! Well-designed API models.The integration models follow best practices:
Literaltype constrains platform values to valid options- Response models appropriately exclude sensitive credentials
- Update request model supports partial updates with optional fields
- Clear separation between request and response models
backend/database/01_create_integration_tables.sql (1)
12-14: Clarify unique constraint scope
The currentUNIQUE(user_id, platform)prevents a user from registering more than oneorganization_nameper platform. To allow multiple orgs (e.g. multiple GitHub orgs), update to:- UNIQUE(user_id, platform) + UNIQUE(user_id, platform, organization_name)If the intent is truly one integration per platform, remove the extra column or enforce its consistency. Confirm the desired business rule.
backend/app/core/dependencies.py
Outdated
| except Exception as e: | ||
| logger.error(f"Authentication error: {str(e)}") | ||
| raise HTTPException( | ||
| status_code=status.HTTP_401_UNAUTHORIZED, | ||
| detail="Authentication failed", | ||
| headers={"WWW-Authenticate": "Bearer"}, | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Improve exception handling in authentication flow.
The current exception handling has several areas for improvement:
- Catching bare
Exceptionon line 63 is too broad - Line 64 should use
logging.exceptioninstead oflogging.errorto automatically include stack traces - The raised exception should be chained with
fromto preserve the original error context
Apply this diff to improve error handling:
except HTTPException:
raise
- except Exception as e:
- logger.error(f"Authentication error: {str(e)}")
+ except Exception as e:
+ logger.exception("Authentication error")
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail="Authentication failed",
headers={"WWW-Authenticate": "Bearer"},
- )
+ ) from eBased on learnings and static analysis hints.
📝 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.
| except Exception as e: | |
| logger.error(f"Authentication error: {str(e)}") | |
| raise HTTPException( | |
| status_code=status.HTTP_401_UNAUTHORIZED, | |
| detail="Authentication failed", | |
| headers={"WWW-Authenticate": "Bearer"}, | |
| ) | |
| except HTTPException: | |
| raise | |
| except Exception as e: | |
| logger.exception("Authentication error") | |
| raise HTTPException( | |
| status_code=status.HTTP_401_UNAUTHORIZED, | |
| detail="Authentication failed", | |
| headers={"WWW-Authenticate": "Bearer"}, | |
| ) from e |
🧰 Tools
🪛 Ruff (0.14.0)
63-63: Do not catch blind exception: Exception
(BLE001)
64-64: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
64-64: Use explicit conversion flag
Replace with conversion flag
(RUF010)
65-69: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🤖 Prompt for AI Agents
In backend/app/core/dependencies.py around lines 63-69, replace the broad
"except Exception" with a narrower exception tuple that reflects authentication
failures (e.g., JWTError and ValueError or your project's specific auth
exceptions) so you don't catch unrelated errors, change logger.error(...) to
logger.exception(...) so the stack trace is logged automatically, and re-raise
the HTTPException using "raise HTTPException(... ) from e" to preserve exception
chaining; also add any necessary imports for the specific exception classes you
catch.
| <input | ||
| type="url" | ||
| value={formData.organization_link || ''} | ||
| onChange={(e) => handleChange('organization_link', e.target.value)} | ||
| placeholder={ | ||
| platform === 'GitHub' | ||
| ? 'https://github.com/your-org' | ||
| : platform === 'Discord' | ||
| ? 'Discord Server ID' | ||
| : 'Organization link' | ||
| } | ||
| className="w-full px-4 py-2 bg-gray-800 border border-gray-700 rounded-lg text-white placeholder-gray-500 focus:outline-none focus:border-green-500 transition-colors" | ||
| /> | ||
| <p className="mt-1 text-xs text-gray-500"> | ||
| {platform === 'GitHub' && 'Your GitHub organization URL'} | ||
| {platform === 'Discord' && 'Your Discord server/guild ID (optional)'} | ||
| </p> | ||
| </div> |
There was a problem hiding this comment.
❓ Verification inconclusive
Discord field uses type="url" but expects a server ID — form will reject numeric input
HTML5 URL validation will block submission when a non-URL ID is entered for Discord. Use text for Discord (URL for GitHub).
- <input
- type="url"
+ <input
+ type={platform.toLowerCase() === 'github' ? 'url' : 'text'}
value={formData.organization_link || ''}
onChange={(e) => handleChange('organization_link', e.target.value)}
placeholder={
platform === 'GitHub'
? 'https://github.com/your-org'
: platform === 'Discord'
- ? 'Discord Server ID'
+ ? 'Discord Server ID'
: 'Organization link'
}
className="w-full px-4 py-2 bg-gray-800 border border-gray-700 rounded-lg text-white placeholder-gray-500 focus:outline-none focus:border-green-500 transition-colors"
/>Also, the comment at Line 20 says “Discord invite link” but the UI/placeholder says “Discord Server ID”. Please confirm which one is intended so we can align validation and help text.
Use text input for Discord IDs and align help text
- Set
type={platform.toLowerCase() === 'github' ? 'url' : 'text'}so Discord server IDs aren’t blocked by URL validation. - Ensure UI text and comments consistently reference either “Discord invite link” or “Discord server ID.”
- <input
- type="url"
+ <input
+ type={platform.toLowerCase() === 'github' ? 'url' : 'text'}
value={formData.organization_link || ''}
onChange={(e) => handleChange('organization_link', e.target.value)}📝 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.
| <input | |
| type="url" | |
| value={formData.organization_link || ''} | |
| onChange={(e) => handleChange('organization_link', e.target.value)} | |
| placeholder={ | |
| platform === 'GitHub' | |
| ? 'https://github.com/your-org' | |
| : platform === 'Discord' | |
| ? 'Discord Server ID' | |
| : 'Organization link' | |
| } | |
| className="w-full px-4 py-2 bg-gray-800 border border-gray-700 rounded-lg text-white placeholder-gray-500 focus:outline-none focus:border-green-500 transition-colors" | |
| /> | |
| <p className="mt-1 text-xs text-gray-500"> | |
| {platform === 'GitHub' && 'Your GitHub organization URL'} | |
| {platform === 'Discord' && 'Your Discord server/guild ID (optional)'} | |
| </p> | |
| </div> | |
| <input | |
| type={platform.toLowerCase() === 'github' ? 'url' : 'text'} | |
| value={formData.organization_link || ''} | |
| onChange={(e) => handleChange('organization_link', e.target.value)} | |
| placeholder={ | |
| platform === 'GitHub' | |
| ? 'https://github.com/your-org' | |
| : platform === 'Discord' | |
| ? 'Discord Server ID' | |
| : 'Organization link' | |
| } | |
| className="w-full px-4 py-2 bg-gray-800 border border-gray-700 rounded-lg text-white placeholder-gray-500 focus:outline-none focus:border-green-500 transition-colors" | |
| /> | |
| <p className="mt-1 text-xs text-gray-500"> | |
| {platform === 'GitHub' && 'Your GitHub organization URL'} | |
| {platform === 'Discord' && 'Your Discord server/guild ID (optional)'} | |
| </p> |
🤖 Prompt for AI Agents
In frontend/src/components/integration/IntegrationModal.tsx around lines 149 to
166, the input uses type="url" for all platforms which blocks Discord server IDs
and the help text is inconsistent; change the input type to be conditional (use
'url' only when platform.toLowerCase() === 'github', otherwise 'text' for
Discord and others) and update the placeholder/help text to consistently
reference either "Discord server ID" or "Discord invite link" (pick one) so the
UI text, placeholder and help paragraph match the chosen terminology.
There was a problem hiding this comment.
Actionable comments posted: 13
♻️ Duplicate comments (1)
frontend/src/components/integration/IntegrationModal.tsx (1)
20-20: Align comment with UI terminology.The comment references "Discord invite link" but the UI consistently uses "Discord Server ID" (lines 157, 164). Update the comment to match.
- organization_link?: string; // GitHub org URL or Discord invite link + organization_link?: string; // GitHub org URL or Discord server ID
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
backend/app/api/v1/integrations.py(1 hunks)backend/app/core/dependencies.py(1 hunks)backend/app/services/integration_service.py(1 hunks)backend/database/01_create_integration_tables.sql(1 hunks)frontend/.env example(1 hunks)frontend/package.json(1 hunks)frontend/src/components/integration/BotIntegration.tsx(1 hunks)frontend/src/components/integration/IntegrationModal.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/database/01_create_integration_tables.sql
- frontend/.env example
🧰 Additional context used
🧬 Code graph analysis (4)
backend/app/core/dependencies.py (2)
backend/app/database/supabase/client.py (1)
get_supabase_client(9-13)backend/main.py (1)
DevRAIApplication(27-88)
backend/app/services/integration_service.py (3)
backend/app/database/supabase/client.py (1)
get_supabase_client(9-13)backend/app/models/integration.py (2)
IntegrationResponse(25-35)IntegrationStatusResponse(44-49)backend/app/api/v1/integrations.py (5)
create_integration(17-30)get_integration(62-83)update_integration(87-103)delete_integration(106-121)get_integration_status(47-58)
frontend/src/components/integration/BotIntegration.tsx (2)
frontend/src/lib/api.ts (3)
Integration(19-28)apiClient(176-176)Platform(12-12)frontend/src/components/integration/IntegrationModal.tsx (1)
IntegrationFormData(18-22)
backend/app/api/v1/integrations.py (3)
backend/app/models/integration.py (3)
IntegrationResponse(25-35)IntegrationListResponse(38-41)IntegrationStatusResponse(44-49)backend/app/core/dependencies.py (1)
get_current_user(18-69)backend/app/services/integration_service.py (5)
create_integration(23-69)get_integrations(71-83)get_integration(85-101)update_integration(125-168)delete_integration(170-185)
🪛 Ruff (0.14.0)
backend/app/core/dependencies.py
53-57: Abstract raise to an inner function
(TRY301)
backend/app/services/integration_service.py
38-38: Abstract raise to an inner function
(TRY301)
38-38: Avoid specifying long messages outside the exception class
(TRY003)
61-61: Abstract raise to an inner function
(TRY301)
61-61: Create your own exception
(TRY002)
61-61: Avoid specifying long messages outside the exception class
(TRY003)
68-68: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
68-68: Use explicit conversion flag
Replace with conversion flag
(RUF010)
82-82: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
82-82: Use explicit conversion flag
Replace with conversion flag
(RUF010)
100-100: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
100-100: Use explicit conversion flag
Replace with conversion flag
(RUF010)
122-122: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
122-122: Use explicit conversion flag
Replace with conversion flag
(RUF010)
146-146: Abstract raise to an inner function
(TRY301)
146-146: Avoid specifying long messages outside the exception class
(TRY003)
160-160: Abstract raise to an inner function
(TRY301)
160-160: Create your own exception
(TRY002)
160-160: Avoid specifying long messages outside the exception class
(TRY003)
167-167: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
167-167: Use explicit conversion flag
Replace with conversion flag
(RUF010)
181-181: Consider moving this statement to an else block
(TRY300)
184-184: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
184-184: Use explicit conversion flag
Replace with conversion flag
(RUF010)
210-210: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
210-210: Use explicit conversion flag
Replace with conversion flag
(RUF010)
225-225: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
225-225: Use explicit conversion flag
Replace with conversion flag
(RUF010)
backend/app/api/v1/integrations.py
19-19: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
25-25: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
26-26: Do not catch blind exception: Exception
(BLE001)
27-30: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
29-29: Use explicit conversion flag
Replace with conversion flag
(RUF010)
34-34: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
39-39: Do not catch blind exception: Exception
(BLE001)
40-43: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
42-42: Use explicit conversion flag
Replace with conversion flag
(RUF010)
49-49: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
54-54: Do not catch blind exception: Exception
(BLE001)
55-58: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
57-57: Use explicit conversion flag
Replace with conversion flag
(RUF010)
64-64: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
71-74: Abstract raise to an inner function
(TRY301)
76-76: Consider moving this statement to an else block
(TRY300)
79-79: Do not catch blind exception: Exception
(BLE001)
80-83: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
82-82: Use explicit conversion flag
Replace with conversion flag
(RUF010)
90-90: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
102-102: Use explicit conversion flag
Replace with conversion flag
(RUF010)
108-108: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
120-120: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🔇 Additional comments (14)
frontend/package.json (1)
14-14: No action required; axios 1.12.2 is published.Verification confirms that axios 1.12.2 is available on npm and can be installed without issues.
backend/app/core/dependencies.py (2)
13-15: LGTM!The synchronous implementation correctly retrieves the application instance from FastAPI's app state, which is the appropriate pattern for this use case.
18-69: LGTM! Previous review concerns addressed.The authentication flow is well-implemented:
- Authorization header validation is thorough (presence, format, Bearer prefix)
- The synchronous call to
supabase.auth.get_user(token)on line 50 is correct (previous review concern resolved)- Exception handling uses
logger.exceptionand proper chaining withfrom e(previous review concern resolved)- User validation and UUID extraction are appropriate
backend/app/api/v1/integrations.py (2)
86-103: LGTM! Previous review concerns addressed.The endpoint correctly:
- Maps
NotFoundErrorto HTTP 404 with exception chaining (line 96)- Preserves context for generic exceptions with
from e(line 103)This addresses the past review comment about proper 404 handling and exception chaining.
105-121: LGTM! Previous review concerns addressed.The endpoint correctly:
- Maps
NotFoundErrorto HTTP 404 with exception chaining (line 114)- Preserves context for generic exceptions with
from e(line 121)This addresses the past review comment about proper 404 handling and exception chaining.
backend/app/services/integration_service.py (1)
144-151: LGTM! Previous review concern addressed.The implementation correctly:
- Checks if
existingisNonebefore accessing its properties (line 145-146)- Raises
NotFoundErrorwhen integration doesn't exist (line 146)- Safely merges the
organization_linkinto config using a defensive copy (lines 149-151)This addresses the past review comment about potential
AttributeErrorand proper 404 handling.frontend/src/components/integration/IntegrationModal.tsx (4)
40-84: LGTM!Validation, submission flow, and error handling are well-structured with appropriate user feedback.
85-118: LGTM!Modal structure and animations follow established patterns with proper accessibility (backdrop click-to-close and explicit close button).
120-166: LGTM!Conditional input type (line 150) correctly handles GitHub URLs and Discord IDs, addressing the past review concern. Form structure and validation feedback are clear.
197-221: LGTM!Action buttons properly handle loading states and display appropriate labels based on whether updating or creating an integration.
frontend/src/components/integration/BotIntegration.tsx (4)
1-25: LGTM!New imports and props appropriately support the modal-driven integration workflow with optional parent callbacks.
26-52: LGTM!The explicit
elsebranch (lines 46-48) correctly clears stale integration data when disconnected, addressing the past review concern.
54-84: LGTM!Modal submission flow correctly handles both create and update paths, refreshes integration status, and notifies parent components via callback.
86-173: LGTM!Card UI with animations, conditional modal rendering, and proper data passing to
IntegrationModalcreate a polished integration management experience.
smokeyScraper
force-pushed
the
frontend_revamp
branch
from
6288960 to
cedaad3
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
frontend/src/components/integration/BotIntegration.tsx (1)
35-52: Consider notifying users when status loading fails.The error is logged to console but users receive no visual feedback when integration status fails to load. This could leave them uncertain about the connection state.
Consider adding a toast notification for load failures:
const loadIntegrationStatus = async () => { try { const status = await apiClient.getIntegrationStatus(platform.toLowerCase() as Platform); setIsConnected(status.is_connected); if (status.is_connected) { const integrations = await apiClient.getIntegrations(); const platformIntegration = integrations.find( i => i.platform === platform.toLowerCase() ); setIntegration(platformIntegration || null); } else { setIntegration(null); } } catch (error) { console.error('Error loading integration status:', error); + toast.error('Failed to load integration status. Please refresh the page.'); } };frontend/src/components/integration/IntegrationModal.tsx (1)
32-38: Consider resetting form state when modal closes.Currently, form data persists when the modal is closed and reopened. This could show stale data if the user opens the modal for a different integration or after an external update.
Add a cleanup effect:
+ useEffect(() => { + if (!isOpen) { + setFormData({ + organization_name: existingData?.organization_name || '', + organization_link: existingData?.config?.organization_link || '', + config: existingData?.config || {}, + }); + setErrors({}); + setIsLoading(false); + } + }, [isOpen, existingData]);backend/app/api/v1/integrations.py (1)
51-69: Inconsistent 404 handling pattern.This endpoint checks for
Noneand raises 404 within the try block, whileupdate_integrationanddelete_integrationcatchNotFoundErrorfrom the service layer. This inconsistency makes error handling harder to reason about.Consider moving the None check into the service layer to make it consistent with other endpoints:
In
backend/app/services/integration_service.py, modifyget_integration:async def get_integration(self, user_id: UUID, integration_id: UUID) -> IntegrationResponse: """Get a specific integration.""" try: result = await self.supabase.table("organization_integrations")\ .select("*")\ .eq("id", str(integration_id))\ .eq("user_id", str(user_id))\ .execute() if not result.data: raise NotFoundError("Integration not found") return IntegrationResponse(**result.data[0]) except Exception as e: logger.exception("Error getting integration") raiseThen simplify the endpoint:
@router.get("/{integration_id}", response_model=IntegrationResponse) async def get_integration( integration_id: UUID, user_id: UUID = Depends(get_current_user) ): """Get a specific integration.""" try: - integration = await integration_service.get_integration(user_id, integration_id) - - if not integration: - raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, - detail="Integration not found" - ) - - return integration + return await integration_service.get_integration(user_id, integration_id) + except NotFoundError as e: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=str(e)) from e except Exception as e: raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail=str(e)) from ebackend/app/services/integration_service.py (1)
48-49: Consider using timezone-aware datetime for consistency.The code uses
datetime.now()which returns a naive datetime (no timezone). Since PostgreSQL'sTIMESTAMPTZstores timezone-aware timestamps, this could lead to inconsistencies or interpretation issues.Use timezone-aware datetime:
+from datetime import datetime, timezone + # ... - "created_at": datetime.now().isoformat(), - "updated_at": datetime.now().isoformat() + "created_at": datetime.now(timezone.utc).isoformat(), + "updated_at": datetime.now(timezone.utc).isoformat()Apply the same change to line 133:
- update_data = {"updated_at": datetime.now().isoformat()} + update_data = {"updated_at": datetime.now(timezone.utc).isoformat()}backend/database/01_create_integration_tables.sql (1)
5-5: Consider future extensibility of platform enum.The platform CHECK constraint uses a hardcoded list of values. Adding new platforms requires a schema migration (ALTER TABLE ... DROP CONSTRAINT / ADD CONSTRAINT), which can be disruptive in production.
For better extensibility, consider using a separate reference table:
-- Create platforms reference table CREATE TABLE IF NOT EXISTS platforms ( name VARCHAR(50) PRIMARY KEY, display_name VARCHAR(100) NOT NULL, is_active BOOLEAN NOT NULL DEFAULT true, created_at TIMESTAMPTZ NOT NULL DEFAULT NOW() ); -- Insert initial platforms INSERT INTO platforms (name, display_name) VALUES ('github', 'GitHub'), ('discord', 'Discord'), ('slack', 'Slack'), ('discourse', 'Discourse'); -- Update organization_integrations to reference the table CREATE TABLE IF NOT EXISTS organization_integrations ( -- ... other columns ... platform VARCHAR(50) NOT NULL REFERENCES platforms(name), -- ... rest of schema ... );This allows adding new platforms via simple INSERT statements without schema migrations. However, if the platform list is truly fixed, the current CHECK constraint is simpler and more performant.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
backend/app/api/v1/integrations.py(1 hunks)backend/app/core/dependencies.py(1 hunks)backend/app/services/integration_service.py(1 hunks)backend/database/01_create_integration_tables.sql(1 hunks)frontend/.env example(1 hunks)frontend/package.json(1 hunks)frontend/src/components/integration/BotIntegration.tsx(1 hunks)frontend/src/components/integration/IntegrationModal.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/package.json
🧰 Additional context used
🧬 Code graph analysis (4)
frontend/src/components/integration/BotIntegration.tsx (2)
frontend/src/lib/api.ts (3)
Integration(19-28)apiClient(176-176)Platform(12-12)frontend/src/components/integration/IntegrationModal.tsx (1)
IntegrationFormData(18-22)
backend/app/core/dependencies.py (2)
backend/app/database/supabase/client.py (1)
get_supabase_client(9-13)backend/main.py (1)
DevRAIApplication(27-88)
backend/app/services/integration_service.py (3)
backend/app/database/supabase/client.py (1)
get_supabase_client(9-13)backend/app/models/integration.py (2)
IntegrationResponse(25-35)IntegrationStatusResponse(44-49)backend/app/api/v1/integrations.py (5)
create_integration(17-27)get_integration(52-68)update_integration(71-87)delete_integration(90-105)get_integration_status(41-49)
backend/app/api/v1/integrations.py (3)
backend/app/models/integration.py (3)
IntegrationResponse(25-35)IntegrationListResponse(38-41)IntegrationStatusResponse(44-49)backend/app/core/dependencies.py (1)
get_current_user(18-69)backend/app/services/integration_service.py (5)
create_integration(23-69)get_integrations(71-83)get_integration(85-101)update_integration(125-168)delete_integration(170-185)
🪛 Ruff (0.14.0)
backend/app/core/dependencies.py
53-57: Abstract raise to an inner function
(TRY301)
backend/app/services/integration_service.py
38-38: Abstract raise to an inner function
(TRY301)
38-38: Avoid specifying long messages outside the exception class
(TRY003)
61-61: Abstract raise to an inner function
(TRY301)
61-61: Create your own exception
(TRY002)
61-61: Avoid specifying long messages outside the exception class
(TRY003)
67-67: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
81-81: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
99-99: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
121-121: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
146-146: Abstract raise to an inner function
(TRY301)
146-146: Avoid specifying long messages outside the exception class
(TRY003)
160-160: Abstract raise to an inner function
(TRY301)
160-160: Create your own exception
(TRY002)
160-160: Avoid specifying long messages outside the exception class
(TRY003)
166-166: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
181-181: Consider moving this statement to an else block
(TRY300)
183-183: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
209-209: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
224-224: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
backend/app/api/v1/integrations.py
19-19: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
25-25: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
31-31: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
43-43: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
54-54: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
61-64: Abstract raise to an inner function
(TRY301)
66-66: Consider moving this statement to an else block
(TRY300)
74-74: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
86-86: Use explicit conversion flag
Replace with conversion flag
(RUF010)
92-92: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
104-104: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🔇 Additional comments (10)
backend/app/core/dependencies.py (1)
1-69: LGTM! Authentication flow is well-structured.The JWT authentication implementation properly validates the Bearer token format, extracts user information via Supabase, and handles errors with appropriate HTTP status codes. The use of
logger.exceptionensures stack traces are captured for debugging.frontend/.env example (1)
1-5: LGTM! Environment variables properly configured.The environment variables are well-structured with clear placeholder values. The addition of
VITE_BACKEND_URLandVITE_DISCORD_BOT_INVITE_URLsupports the new integration features.frontend/src/components/integration/BotIntegration.tsx (1)
58-84: LGTM! Integration submission handles both create and update flows.The function properly differentiates between creating new integrations and updating existing ones, refreshes the status after submission, and notifies parent components via the callback.
frontend/src/components/integration/IntegrationModal.tsx (2)
149-166: LGTM! Input type properly discriminates between platforms.The conditional
typeattribute correctly uses URL validation for GitHub (expecting URLs) and text input for Discord (expecting server IDs), preventing the browser from blocking valid Discord IDs.
168-192: LGTM! Discord bot invite guidance properly handles missing configuration.The component gracefully handles the case where
VITE_DISCORD_BOT_INVITE_URLis not configured by displaying a clear warning instead of rendering a broken link.backend/app/api/v1/integrations.py (2)
16-28: LGTM! Create endpoint properly handles validation errors.The endpoint correctly distinguishes between client errors (400 for ValueError) and server errors (500 for other exceptions), with proper exception chaining to preserve debugging context.
70-88: LGTM! Update endpoint properly handles NotFoundError.The endpoint correctly catches
NotFoundErrorand maps it to HTTP 404, with proper exception chaining to preserve the original context.backend/app/services/integration_service.py (2)
23-69: LGTM! Create integration properly prevents duplicates.The method correctly checks for existing integrations before creating new ones, stores organization metadata with proper config handling, and includes comprehensive error logging.
144-151: LGTM! Update properly validates existence and merges config.The update flow now correctly fetches the existing integration first, raises
NotFoundErrorif missing, and carefully merges the organization_link into the config without losing other config fields.backend/database/01_create_integration_tables.sql (1)
36-60: LGTM! RLS policies properly enforce user-level isolation.The policies correctly restrict all operations (SELECT, INSERT, UPDATE, DELETE) to the authenticated user's own integrations. The UPDATE policy now includes both USING and WITH CHECK clauses, preventing privilege escalation via user_id changes.
|
could you please review and merge this @chandansgowda ? |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/app/models/database/supabase.py (1)
190-211: Consider adding platform validation and improving config type safety.The
OrganizationIntegrationmodel looks well-structured overall. However, consider these improvements:
- Platform field validation: The
platformfield accepts any string, but the docstring specifies valid values (github, discord, slack, discourse). Use an enum orLiteraltype to enforce this at the model level:+from typing import Optional, List, Literal + +PlatformType = Literal["github", "discord", "slack", "discourse"] + class OrganizationIntegration(BaseModel): """ Represents a registered organization (just metadata, no credentials). Attributes: id (UUID): Unique identifier for the integration. user_id (UUID): User/Owner who registered this organization. platform (str): Platform name (github, discord, slack, discourse). organization_name (str): Name of the organization. is_active (bool): Whether the integration is active. created_at (datetime): Timestamp when registered. updated_at (datetime): Timestamp when last updated. config (dict): Platform-specific data (org link, guild_id, etc.). """ id: UUID user_id: UUID - platform: str + platform: PlatformType organization_name: str is_active: bool = True created_at: datetime updated_at: datetime config: Optional[dict] = None
- Config field type safety (optional): The generic
Optional[dict]forconfigprovides flexibility but loses type safety. If platform-specific structures are consistent, consider using a union of typed dicts or Pydantic models for better validation.
2 participants
Interactions
Summary by CodeRabbit
New Features
Improvements
Chores