Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions StabilityMatrix.Avalonia/ViewModels/CheckpointsPageViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -841,6 +841,70 @@ public async Task ImportFilesAsync(IEnumerable<string> files, DirectoryPath dest
.FirstOrDefault(x => x.Path == destinationFolder.FullPath);
}

public async Task MoveBetweenFolders(List<CheckpointFileViewModel>? sourceFiles, DirectoryPath destinationFolder)
{
if (sourceFiles != null && sourceFiles.Count() > 0)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a List you can use something like if (sourceFiles is { Count: > 0 }) instead of the LINQ count, but not a huge deal.

{
var sourceDirectory = Path.GetDirectoryName(sourceFiles[0].CheckpointFile.GetFullPath(settingsManager.ModelsDirectory));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Consider adding a null check for the result of Path.GetDirectoryName to handle cases where the path is a root directory or an invalid path. This will prevent potential NullReferenceException when accessing the CheckpointFile property.

            var sourceDirectory = sourceFiles[0].CheckpointFile != null ? Path.GetDirectoryName(sourceFiles[0].CheckpointFile.GetFullPath(settingsManager.ModelsDirectory)) : null;

foreach (CheckpointFileViewModel sourceFile in sourceFiles)
{
if (
destinationFolder.FullPath == settingsManager.ModelsDirectory
|| (sourceDirectory != null && sourceDirectory == destinationFolder.FullPath)
Comment on lines +851 to +853
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This condition checks if the destination folder is the same as the models directory or the source directory. The check destinationFolder.FullPath == settingsManager.ModelsDirectory seems redundant because the second part of the condition already covers the case where the source and destination are the same. Consider simplifying this to just check if the source and destination directories are equal.

Also, it would be good to add a null check for sourceDirectory before accessing FullPath to avoid potential NullReferenceException.

                if (
                    sourceDirectory == null || destinationFolder.FullPath == sourceDirectory
                )

)
{
notificationService.Show(
"Invalid Folder",
"Please select a different folder to import the files into.",
NotificationType.Error
);
return;
}

try
{
var sourcePath = new FilePath(sourceFile.CheckpointFile.GetFullPath(settingsManager.ModelsDirectory));
var fileNameWithoutExt = Path.GetFileNameWithoutExtension(sourcePath);
var sourceCmInfoPath = Path.Combine(sourcePath.Directory!, $"{fileNameWithoutExt}.cm-info.json");
var sourcePreviewPath = Path.Combine(sourcePath.Directory!, $"{fileNameWithoutExt}.preview.jpeg");
var destinationFilePath = Path.Combine(destinationFolder, sourcePath.Name);
var destinationCmInfoPath = Path.Combine(destinationFolder, $"{fileNameWithoutExt}.cm-info.json");
var destinationPreviewPath = Path.Combine(destinationFolder, $"{fileNameWithoutExt}.preview.jpeg");

// Move files
if (File.Exists(sourcePath))
await FileTransfers.MoveFileAsync(sourcePath, destinationFilePath);

if (File.Exists(sourceCmInfoPath))
await FileTransfers.MoveFileAsync(sourceCmInfoPath, destinationCmInfoPath);

if (File.Exists(sourcePreviewPath))
await FileTransfers.MoveFileAsync(sourcePreviewPath, destinationPreviewPath);

notificationService.Show(
"Model moved successfully",
$"Moved '{sourcePath.Name}' to '{Path.GetFileName(destinationFolder)}'"
);
}
catch (FileTransferExistsException)
{
notificationService.Show(
"Failed to move file",
"Destination file exists",
NotificationType.Error
);
Comment on lines +890 to +895
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The notification message in the catch block is generic. Consider including the file name in the notification to provide more context to the user about which file failed to move.

                    notificationService.Show(
                        "Failed to move file",
                        $"Failed to move '{sourcePath.Name}': Destination file exists",
                        NotificationType.Error
                    );

}
}

SelectedCategory = Categories
.SelectMany(c => c.Flatten())
.FirstOrDefault(x => x.Path == sourceDirectory);

await modelIndexService.RefreshIndex();
DelayedClearProgress(TimeSpan.FromSeconds(1.5));
}
}

public async Task MoveBetweenFolders(LocalModelFile sourceFile, DirectoryPath destinationFolder)
{
var sourceDirectory = Path.GetDirectoryName(sourceFile.GetFullPath(settingsManager.ModelsDirectory));
Expand Down
6 changes: 1 addition & 5 deletions StabilityMatrix.Avalonia/Views/CheckpointsPage.axaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -204,11 +204,7 @@ private async void OnDrop(object? sender, DragEventArgs e)
{
allSelectedCheckpoints.Add(vm);
}

foreach (var checkpoint in allSelectedCheckpoints)
{
await checkpointsVm.MoveBetweenFolders(checkpoint.CheckpointFile, category.Path);
}
await checkpointsVm.MoveBetweenFolders(allSelectedCheckpoints, category.Path);
}
else
{
Expand Down
Loading