feat: add app import preview flow#239
feat: add app import preview flow#239super-niuma-001 wants to merge 1 commit intodotnetcore:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds an “app import” workflow that lets users upload an exported apps JSON file, preview validation results, and then import apps/configs in dependency order.
Changes:
- Added API endpoints to preview and import exported app JSON files, including validation (duplicates/missing parents/cycles) and topological ordering.
- Added UI import modal flow (upload → preview table → import) and supporting request/types/i18n.
- Added API tests covering preview validation and import ordering/config creation.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| test/ApiSiteTests/TestAppController.cs | Adds tests for preview validation errors and import ordering/config creation. |
| src/AgileConfig.Server.UI/react-ui-antd/src/pages/Apps/service.ts | Adds client API calls for preview-import (multipart upload) and import (JSON body). |
| src/AgileConfig.Server.UI/react-ui-antd/src/pages/Apps/index.tsx | Adds “Import” button and toggles the import modal. |
| src/AgileConfig.Server.UI/react-ui-antd/src/pages/Apps/data.d.ts | Defines types for import file schema and preview response payload. |
| src/AgileConfig.Server.UI/react-ui-antd/src/pages/Apps/comps/AppImport.tsx | Implements the upload/preview/import modal UX. |
| src/AgileConfig.Server.UI/react-ui-antd/src/locales/zh-CN/pages.ts | Adds zh-CN strings for import flow UI. |
| src/AgileConfig.Server.UI/react-ui-antd/src/locales/en-US/pages.ts | Adds en-US strings for import flow UI. |
| src/AgileConfig.Server.Apisite/Models/AppExportVM.cs | Adds import request model and preview view models. |
| src/AgileConfig.Server.Apisite/Controllers/AppController.cs | Implements PreviewImport/Import endpoints, validation, and topological sort. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| preview.Errors.Add($"App '{item.App.Id}' references missing parent '{parentId}'. Parent must already exist or be included in the import file."); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
BuildImportPreviewAsync can add validation errors for missing/duplicate AppIds but still proceeds to TryTopologicalSort. ToDictionary(x => x.App.Id, ...) will throw when an AppId is null/whitespace or duplicated, turning a user error into a 500. Mandatory fix: short-circuit before topo sort when preview.Errors.Any(), and/or pass only items with non-empty unique AppIds into TryTopologicalSort so preview errors are returned as JSON instead of throwing.
| if (preview.Errors.Any()) return preview; |
| private static Dictionary<string, int> TryTopologicalSort(List<AppExportItemVM> appItems, HashSet<string> importedAppIds, List<string> errors) | ||
| { | ||
| var dependencyMap = appItems.ToDictionary( | ||
| x => x.App.Id, | ||
| x => (x.App.InheritancedApps ?? new List<string>()) | ||
| .Where(parentId => !string.IsNullOrWhiteSpace(parentId) && importedAppIds.Contains(parentId)) | ||
| .Distinct(StringComparer.OrdinalIgnoreCase) | ||
| .ToList(), | ||
| StringComparer.OrdinalIgnoreCase); |
There was a problem hiding this comment.
BuildImportPreviewAsync can add validation errors for missing/duplicate AppIds but still proceeds to TryTopologicalSort. ToDictionary(x => x.App.Id, ...) will throw when an AppId is null/whitespace or duplicated, turning a user error into a 500. Mandatory fix: short-circuit before topo sort when preview.Errors.Any(), and/or pass only items with non-empty unique AppIds into TryTopologicalSort so preview errors are returned as JSON instead of throwing.
| var inheritanceApps = BuildInheritanceLinks(importItem.App.InheritancedApps, app.Id); | ||
| await _appService.AddAsync(app, inheritanceApps); |
There was a problem hiding this comment.
The import endpoint always returns success = true even if _appService.AddAsync(...) or _configService.AddAsync(...) fails (these return bool). This can silently produce partial imports and inconsistent state. Mandatory fix: check return values and fail fast (return success=false with an error message), and consider wrapping the whole import in a transaction/unit-of-work so apps/configs are imported atomically.
| OnlineStatus = OnlineStatus.WaitPublish, | ||
| EditStatus = EditStatus.Add | ||
| }; | ||
| await _configService.AddAsync(config, envConfigs.Key); |
There was a problem hiding this comment.
The import endpoint always returns success = true even if _appService.AddAsync(...) or _configService.AddAsync(...) fails (these return bool). This can silently produce partial imports and inconsistent state. Mandatory fix: check return values and fail fast (return success=false with an error message), and consider wrapping the whole import in a transaction/unit-of-work so apps/configs are imported atomically.
| return Json(new | ||
| { | ||
| success = true, | ||
| data = preview | ||
| }); |
There was a problem hiding this comment.
The import endpoint always returns success = true even if _appService.AddAsync(...) or _configService.AddAsync(...) fails (these return bool). This can silently produce partial imports and inconsistent state. Mandatory fix: check return values and fail fast (return success=false with an error message), and consider wrapping the whole import in a transaction/unit-of-work so apps/configs are imported atomically.
| private async Task<AppExportFileVM> ReadImportFileAsync(IFormFile file) | ||
| { | ||
| if (file == null || file.Length == 0) throw new ArgumentException("file"); | ||
|
|
||
| using var stream = file.OpenReadStream(); | ||
| using var reader = new System.IO.StreamReader(stream, Encoding.UTF8); | ||
| var content = await reader.ReadToEndAsync(); | ||
| var importFile = JsonConvert.DeserializeObject<AppExportFileVM>(content); | ||
| if (importFile == null) throw new ArgumentException("file"); |
There was a problem hiding this comment.
ReadImportFileAsync reads the entire uploaded file into memory with no upper bound and throws ArgumentException("file") on bad input, which will surface as 500s unless handled elsewhere. Mandatory fix: enforce a maximum upload size (reject large files early using file.Length and/or request limits) and return a structured 400-style JSON error for invalid/empty/invalid-JSON uploads instead of throwing generic exceptions.
| return request<{ success: boolean; data: AppImportPreviewResult; message?: string }>('app/PreviewImport', { | ||
| method: 'POST', | ||
| data: formData, | ||
| requestType: 'form', |
There was a problem hiding this comment.
You are sending a FormData payload but set requestType: 'form' (typically URL-encoded). With @umijs/request, multipart uploads generally require requestType: 'formData' (or omitting requestType so it can infer from FormData). As-is, the file may not be transmitted correctly and the API will receive file=null.
| requestType: 'form', | |
| requestType: 'formData', |
| Group = importItem.App.Group, | ||
| Secret = importItem.App.Secret, | ||
| Enabled = importItem.App.Enabled, | ||
| Type = importItem.App.Inheritanced ? AppType.Inheritance : AppType.PRIVATE, |
There was a problem hiding this comment.
Import derives App.Type solely from Inheritanced, ignoring the exported type field included in the import schema (and likely in AppExportAppVM). This can change app types on import (e.g., any non-inheritance type becomes PRIVATE). Mandatory fix: map the exported Type value to App.Type (with validation/fallback if the value is unknown) instead of hardcoding it based on Inheritanced.
Closes #235
Summary
Testing