Skip to content

Conversation

@Professor-Chen
Copy link
Contributor

📝 Description | 描述

English:
This PR fixes multiple bugs that prevented users from creating and deleting AI traders. The issues included database schema mismatches, incorrect field name mappings, and missing React context providers.

中文:
此 PR 修复了阻止用户创建和删除 AI 交易员的多个 bug。问题包括数据库 schema 不匹配、字段名映射错误以及缺失的 React Context Provider。


🎯 Type of Change | 变更类型

  • 🐛 Bug fix | 修复 Bug

🔗 Related Issues | 相关 Issue

  • Fixes trader creation returning 500 error
  • Fixes delete trader button not working

📋 Changes Made | 具体变更

English:

Backend Fixes (Go)

  1. Fix time.Time scanning error - SQLite stores datetime as TEXT, now parsing manually instead of relying on driver auto-conversion
  2. Fix foreign key mismatch - traders table referenced exchanges(id) but exchanges uses composite primary key (id, user_id). Removed problematic FK constraints
  3. Add missing backtestManager field to Server struct
  4. Add missing Shutdown method to Server struct
  5. Fix NewFuturesTrader call - pass userId parameter
  6. Fix UpdateExchange call - pass all required parameters
  7. Add migrateTradersTable() - migration function to fix existing databases
  8. Fix balance extraction field name mismatch - Binance API returns availableBalance (camelCase) but code looked for available_balance (snake_case)

Frontend Fixes (React)

  1. Add missing ConfirmDialogProvider - The delete confirmation dialog required this provider to be wrapped around the App component

中文:

后端修复 (Go)

  1. 修复 time.Time 扫描错误 - SQLite 将 datetime 存储为 TEXT,现在手动解析而非依赖驱动自动转换
  2. 修复外键不匹配 - traders 表引用 exchanges(id),但 exchanges 使用复合主键 (id, user_id),已移除问题外键约束
  3. 添加缺失的 backtestManager 字段到 Server 结构体
  4. 添加缺失的 Shutdown 方法到 Server 结构体
  5. 修复 NewFuturesTrader 调用 - 传递 userId 参数
  6. 修复 UpdateExchange 调用 - 传递所有必需参数
  7. 添加 migrateTradersTable() - 迁移函数以修复现有数据库
  8. 修复余额提取字段名不匹配 - Binance API 返回 availableBalance (驼峰),但代码查找 available_balance (下划线)

前端修复 (React)

  1. 添加缺失的 ConfirmDialogProvider - 删除确认对话框需要此 Provider 包裹 App 组件

🧪 Testing | 测试

  • Tested locally | 本地测试通过
  • Verified no existing functionality broke | 确认没有破坏现有功能

Test Results:

  • ✅ Create trader with auto balance fetch: Working
  • ✅ Delete trader with confirmation dialog: Working
  • ✅ Balance correctly extracted from Binance API (17.74 USDT)

✅ Checklist | 检查清单

Code Quality | 代码质量

  • Code follows project style | 代码遵循项目风格
  • Self-review completed | 已完成代码自查
  • Comments added for complex logic | 已添加必要注释

Documentation | 文档

  • Updated relevant documentation | 已更新相关文档

Git

  • Commits follow conventional format | 提交遵循 Conventional Commits 格式
  • Rebased on latest dev branch | 已 rebase 到最新 dev 分支
  • No merge conflicts | 无合并冲突

📚 Additional Notes | 补充说明

English:
These bugs were present in the original codebase and affected new user onboarding. The fixes are backward compatible and include database migration for existing installations.

中文:
这些 bug 存在于原始代码库中,影响新用户上手体验。修复向后兼容,并包含现有安装的数据库迁移。


By submitting this PR, I confirm | 提交此 PR,我确认:

  • I have read the Contributing Guidelines | 已阅读贡献指南
  • I agree to the Code of Conduct | 同意行为准则
  • My contribution is licensed under AGPL-3.0 | 贡献遵循 AGPL-3.0 许可证

NOFX Trader added 3 commits November 29, 2025 23:57
Bug fixes:
1. Fix time.Time scanning error - SQLite stores datetime as TEXT, now parsing manually
2. Fix foreign key mismatch - traders table referenced exchanges(id) but exchanges uses composite primary key (id, user_id)
3. Add missing backtestManager field to Server struct
4. Add missing Shutdown method to Server struct
5. Fix NewFuturesTrader call - pass userId parameter
6. Fix UpdateExchange call - pass all required parameters
7. Add migrateTradersTable() to fix existing databases

These issues prevented creating new traders with 500 errors.
Binance API returns 'availableBalance' (camelCase) but code was looking for
'available_balance' (snake_case). Now supports both formats.

Also added 'totalWalletBalance' as fallback for total balance extraction.
The delete trader button required ConfirmDialogProvider to be wrapped
around the App component for the confirmation dialog to work.
@github-actions
Copy link

🤖 Advisory Check Results

These are advisory checks to help improve code quality. They won't block your PR from being merged.

📋 PR Information

Title Format: ✅ Good - Follows Conventional Commits
PR Size: 🟡 Medium (689 lines: +407 -282)

🔧 Backend Checks

Go Formatting: ⚠️ Needs formatting

Files needing formatting
api/server.go
api/utils_test.go
config/database.go
mcp/client.go
mcp/config.go
mcp/deepseek_client.go
mcp/options.go
mcp/qwen_client.go
mcp/request.go
mcp/request_builder.go

Go Vet: ✅ Good
Tests: ✅ Passed

Fix locally:

go fmt ./...      # Format code
go vet ./...      # Check for issues
go test ./...     # Run tests

⚛️ Frontend Checks

Build & Type Check: ✅ Success

Fix locally:

cd web
npm run build  # Test build (includes type checking)

📖 Resources

Questions? Feel free to ask in the comments! 🙏


These checks are advisory and won't block your PR from being merged. This comment is automatically generated from pr-checks-run.yml.

@tinkle-community
Copy link
Collaborator

DM me on Telegram: https://t.me/CcPrimary

@tinkle-community tinkle-community merged commit 7c43c88 into NoFxAiOS:dev Nov 30, 2025
15 of 24 checks passed
tinkle-community pushed a commit that referenced this pull request Dec 7, 2025
…1140)

* fix: resolve multiple bugs preventing trader creation
Bug fixes:
1. Fix time.Time scanning error - SQLite stores datetime as TEXT, now parsing manually
2. Fix foreign key mismatch - traders table referenced exchanges(id) but exchanges uses composite primary key (id, user_id)
3. Add missing backtestManager field to Server struct
4. Add missing Shutdown method to Server struct
5. Fix NewFuturesTrader call - pass userId parameter
6. Fix UpdateExchange call - pass all required parameters
7. Add migrateTradersTable() to fix existing databases
These issues prevented creating new traders with 500 errors.
* fix(api): fix balance extraction field name mismatch
Binance API returns 'availableBalance' (camelCase) but code was looking for
'available_balance' (snake_case). Now supports both formats.
Also added 'totalWalletBalance' as fallback for total balance extraction.
* fix(frontend): add missing ConfirmDialogProvider to App
The delete trader button required ConfirmDialogProvider to be wrapped
around the App component for the confirmation dialog to work.
---------
Co-authored-by: NOFX Trader <[email protected]>
miantj pushed a commit to miantj/nofx that referenced this pull request Dec 7, 2025
…oFxAiOS#1140)

* fix: resolve multiple bugs preventing trader creation

Bug fixes:
1. Fix time.Time scanning error - SQLite stores datetime as TEXT, now parsing manually
2. Fix foreign key mismatch - traders table referenced exchanges(id) but exchanges uses composite primary key (id, user_id)
3. Add missing backtestManager field to Server struct
4. Add missing Shutdown method to Server struct
5. Fix NewFuturesTrader call - pass userId parameter
6. Fix UpdateExchange call - pass all required parameters
7. Add migrateTradersTable() to fix existing databases

These issues prevented creating new traders with 500 errors.

* fix(api): fix balance extraction field name mismatch

Binance API returns 'availableBalance' (camelCase) but code was looking for
'available_balance' (snake_case). Now supports both formats.

Also added 'totalWalletBalance' as fallback for total balance extraction.

* fix(frontend): add missing ConfirmDialogProvider to App

The delete trader button required ConfirmDialogProvider to be wrapped
around the App component for the confirmation dialog to work.

---------

Co-authored-by: NOFX Trader <[email protected]>
miantj pushed a commit to miantj/nofx that referenced this pull request Dec 7, 2025
…oFxAiOS#1140)

* fix: resolve multiple bugs preventing trader creation

Bug fixes:
1. Fix time.Time scanning error - SQLite stores datetime as TEXT, now parsing manually
2. Fix foreign key mismatch - traders table referenced exchanges(id) but exchanges uses composite primary key (id, user_id)
3. Add missing backtestManager field to Server struct
4. Add missing Shutdown method to Server struct
5. Fix NewFuturesTrader call - pass userId parameter
6. Fix UpdateExchange call - pass all required parameters
7. Add migrateTradersTable() to fix existing databases

These issues prevented creating new traders with 500 errors.

* fix(api): fix balance extraction field name mismatch

Binance API returns 'availableBalance' (camelCase) but code was looking for
'available_balance' (snake_case). Now supports both formats.

Also added 'totalWalletBalance' as fallback for total balance extraction.

* fix(frontend): add missing ConfirmDialogProvider to App

The delete trader button required ConfirmDialogProvider to be wrapped
around the App component for the confirmation dialog to work.

---------

Co-authored-by: NOFX Trader <[email protected]>
miantj pushed a commit to miantj/nofx that referenced this pull request Dec 8, 2025
…oFxAiOS#1140)

* fix: resolve multiple bugs preventing trader creation

Bug fixes:
1. Fix time.Time scanning error - SQLite stores datetime as TEXT, now parsing manually
2. Fix foreign key mismatch - traders table referenced exchanges(id) but exchanges uses composite primary key (id, user_id)
3. Add missing backtestManager field to Server struct
4. Add missing Shutdown method to Server struct
5. Fix NewFuturesTrader call - pass userId parameter
6. Fix UpdateExchange call - pass all required parameters
7. Add migrateTradersTable() to fix existing databases

These issues prevented creating new traders with 500 errors.

* fix(api): fix balance extraction field name mismatch

Binance API returns 'availableBalance' (camelCase) but code was looking for
'available_balance' (snake_case). Now supports both formats.

Also added 'totalWalletBalance' as fallback for total balance extraction.

* fix(frontend): add missing ConfirmDialogProvider to App

The delete trader button required ConfirmDialogProvider to be wrapped
around the App component for the confirmation dialog to work.

---------

Co-authored-by: NOFX Trader <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants