fix: resolve staging deploy failures in migration and frontend build#70
Conversation
- Migration d88681a7b3d0: drop ADK tables conditionally — they won't exist on a fresh DB, causing ProgrammingError on first deploy - widget-settings: double-cast res.data through unknown to satisfy TS strict check when extracting numeric limit fields from BusinessProfile Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
🤖 Hi @stenwire, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the database migration script to safely drop tables only if they exist and adds a 'tier' column to the 'plans' table. On the frontend, it refactors how business profile limits are retrieved. The reviewer noted that the double type assertion used during this refactoring is redundant and bypasses TypeScript's type safety, suggesting direct property access instead.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const d = res.data as unknown as Record<string, number | undefined>; | ||
| setLimits({ | ||
| allocated_messages_per_session: (res.data as Record<string, unknown>).allocated_messages_per_session, | ||
| allocated_daily_sessions: (res.data as Record<string, unknown>).allocated_daily_sessions, | ||
| allocated_whitelisted_domains: (res.data as Record<string, unknown>).allocated_whitelisted_domains, | ||
| allocated_messages_per_session: d.allocated_messages_per_session, | ||
| allocated_daily_sessions: d.allocated_daily_sessions, | ||
| allocated_whitelisted_domains: d.allocated_whitelisted_domains, | ||
| }); |
There was a problem hiding this comment.
The double type assertion (as unknown as Record<string, number | undefined>) is redundant and bypasses TypeScript's type safety. Since getBusinessProfile() returns ApiResponse<BusinessProfile> and BusinessProfile already defines allocated_messages_per_session, allocated_daily_sessions, and allocated_whitelisted_domains as optional numbers, you can access these properties directly on res.data without any type assertions.
setLimits({
allocated_messages_per_session: res.data.allocated_messages_per_session,
allocated_daily_sessions: res.data.allocated_daily_sessions,
allocated_whitelisted_domains: res.data.allocated_whitelisted_domains,
});
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
📋 Review Summary
This Pull Request addresses staging deploy failures related to a database migration and a frontend build issue. The changes are well-implemented and improve the robustness of the migration script and the readability of the frontend code.
🔍 General Feedback
- The changes demonstrate careful consideration for error handling in database migrations.
- The frontend code refactoring enhances code clarity by reducing redundant type assertions.
| op.drop_table('sessions') | ||
| op.drop_table('user_states') | ||
| conn = op.get_bind() | ||
| inspector = sa.inspect(conn) |
There was a problem hiding this comment.
🟢 This is a good improvement to make the migration more robust by checking if tables exist before attempting to drop them. This prevents potential errors in environments where these tables might not be present.
| const res = await getBusinessProfile(); | ||
| if (res.status === 'success' && res.data) { | ||
| setBusiness(res.data); | ||
| const d = res.data as unknown as Record<string, number | undefined>; |
There was a problem hiding this comment.
🟢 Introducing a temporary variable d to hold the casted res.data improves readability and reduces repetitive type assertions.
There was a problem hiding this comment.
📋 Security Analysis Summary
I have analyzed the changes in this pull request and have not identified any new security vulnerabilities. The changes appear to be safe.
🔍 General Feedback
- The database migration script is straightforward and uses Alembic as expected.
- The frontend code uses React and properly handles data binding, which helps to prevent XSS vulnerabilities. The use of
datasetfor the widget ID is a good practice. - Keep up the great work!
1 participant
No description provided.