Skip to content

Commit

Permalink
Fix handling of cancellation in CE move to folder commands
Browse files Browse the repository at this point in the history
They did not catch the corresponding refactoring exception since the user interaction was not contained in the try-catch-block.

This also adds a user notification in case one of the affected modules is stale.
  • Loading branch information
MDoerner committed Jun 7, 2020
1 parent f0cbd16 commit 213bf1c
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,24 +23,27 @@ public abstract class CodeExplorerMoveToFolderCommandBase : CodeExplorerCommandB
};

private readonly IParserStatusProvider _parserStatusProvider;
private readonly RubberduckParserState _state;

private readonly IRefactoringAction<MoveMultipleFoldersModel> _moveFolders;
private readonly IRefactoringAction<MoveMultipleToFolderModel> _moveToFolder;

private readonly IRefactoringFailureNotifier _failureNotifier;

public CodeExplorerMoveToFolderCommandBase(
protected CodeExplorerMoveToFolderCommandBase(
MoveMultipleFoldersRefactoringAction moveFolders,
MoveMultipleToFolderRefactoringAction moveToFolder,
MoveToFolderRefactoringFailedNotifier failureNotifier,
IParserStatusProvider parserStatusProvider,
IVbeEvents vbeEvents)
IVbeEvents vbeEvents,
RubberduckParserState state)
: base(vbeEvents)
{
_moveFolders = moveFolders;
_moveToFolder = moveToFolder;

_parserStatusProvider = parserStatusProvider;
_state = state;
_failureNotifier = failureNotifier;

AddToCanExecuteEvaluation(SpecialEvaluateCanExecute);
Expand All @@ -67,15 +70,13 @@ protected override void OnExecute(object parameter)
if (node is CodeExplorerComponentViewModel componentViewModel)
{
var model = ComponentModel(componentViewModel);
var modifiedModel = ModifiedComponentModel(model, parameter);
ExecuteRefactoringAction(modifiedModel, _moveToFolder, _failureNotifier);
ExecuteRefactoringAction(model, parameter, ValidateInitialComponentModel, ModifiedComponentModel, _moveToFolder, _failureNotifier);
}

if (node is CodeExplorerCustomFolderViewModel folderViewModel)
{
var model = FolderModel(folderViewModel);
var modifiedModel = ModifiedFolderModel(model, parameter);
ExecuteRefactoringAction(modifiedModel, _moveFolders, _failureNotifier);
ExecuteRefactoringAction(model, parameter, ValidateInitialFolderModel, ModifiedFolderModel, _moveFolders, _failureNotifier);
}
}

Expand Down Expand Up @@ -103,6 +104,17 @@ private static ICollection<ModuleDeclaration> ContainedModules(ICodeExplorerNode
.ToList();
}

private void ValidateInitialFolderModel(MoveMultipleFoldersModel model)
{
var firstStaleAffectedModules = model.ModulesBySourceFolder.Values
.SelectMany(modules => modules)
.FirstOrDefault(module => _state.IsNewOrModified(module.QualifiedModuleName));
if (firstStaleAffectedModules != null)
{
throw new AffectedModuleIsStaleException(firstStaleAffectedModules.QualifiedModuleName);
}
}

private MoveMultipleToFolderModel ComponentModel(CodeExplorerComponentViewModel componentViewModel)
{
if (!(componentViewModel.Declaration is ModuleDeclaration moduleDeclaration))
Expand All @@ -113,14 +125,32 @@ private MoveMultipleToFolderModel ComponentModel(CodeExplorerComponentViewModel
var targets = new List<ModuleDeclaration>{moduleDeclaration};
var targetFolder = moduleDeclaration.CustomFolder;
return new MoveMultipleToFolderModel(targets, targetFolder);
}
}

private void ValidateInitialComponentModel(MoveMultipleToFolderModel model)
{
var firstStaleAffectedModules = model.Targets
.FirstOrDefault(module => _state.IsNewOrModified(module.QualifiedModuleName));
if (firstStaleAffectedModules != null)
{
throw new AffectedModuleIsStaleException(firstStaleAffectedModules.QualifiedModuleName);
}
}

private static void ExecuteRefactoringAction<TModel>(TModel model, IRefactoringAction<TModel> refactoringAction, IRefactoringFailureNotifier failureNotifier)
private static void ExecuteRefactoringAction<TModel>(
TModel model,
object parameter,
Action<TModel> initialModelValidation,
Func<TModel,object,TModel> modelModification,
IRefactoringAction<TModel> refactoringAction,
IRefactoringFailureNotifier failureNotifier)
where TModel : class, IRefactoringModel
{
try
{
refactoringAction.Refactor(model);
initialModelValidation(model);
var modifiedModel = modelModification(model, parameter);
refactoringAction.Refactor(modifiedModel);
}
catch (RefactoringAbortedException)
{}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ public sealed class CodeExplorerMoveToFolderCommand : CodeExplorerMoveToFolderCo
RefactoringUserInteraction<IMoveMultipleToFolderPresenter, MoveMultipleToFolderModel> moveToFolderInteraction,
MoveToFolderRefactoringFailedNotifier failureNotifier,
IParserStatusProvider parserStatusProvider,
IVbeEvents vbeEvents)
: base(moveFolders, moveToFolder, failureNotifier, parserStatusProvider, vbeEvents)
IVbeEvents vbeEvents,
RubberduckParserState state)
: base(moveFolders, moveToFolder, failureNotifier, parserStatusProvider, vbeEvents, state)
{
_moveFoldersInteraction = moveFoldersInteraction;
_moveToFolderInteraction = moveToFolderInteraction;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Rubberduck.Navigation.CodeExplorer;
using Rubberduck.Parsing.Symbols;
using Rubberduck.Parsing.VBA;
using Rubberduck.Refactorings.Exceptions;
using Rubberduck.Refactorings.MoveFolder;
using Rubberduck.Refactorings.MoveToFolder;
using Rubberduck.Resources;
Expand All @@ -27,8 +28,9 @@ public sealed class CodeExplorerMoveToFolderDragAndDropCommand : CodeExplorerMov
IParserStatusProvider parserStatusProvider,
IVbeEvents vbeEvents,
IMessageBox messageBox,
IDeclarationFinderProvider declarationFinderProvider)
: base(moveFolders, moveToFolder, failureNotifier, parserStatusProvider, vbeEvents)
IDeclarationFinderProvider declarationFinderProvider,
RubberduckParserState state)
: base(moveFolders, moveToFolder, failureNotifier, parserStatusProvider, vbeEvents, state)
{
_declarationFinderProvider = declarationFinderProvider;
_messageBox = messageBox;
Expand Down Expand Up @@ -69,6 +71,11 @@ protected override MoveMultipleFoldersModel ModifiedFolderModel(MoveMultipleFold
{
model.TargetFolder = targetFolder;
}
else
{
throw new RefactoringAbortedException();
}

return model;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,6 @@ protected override string Message(RefactoringException exception)
DeclarationType.Module);
case NoTargetFolderException noTargetFolder:
return Resources.RubberduckUI.RefactoringFailure_NoTargetFolder;
case AffectedModuleIsStaleException affectedModuleIsStale:
return string.Format(
Resources.RubberduckUI.RefactoringFailure_AffectedModuleIsStale,
affectedModuleIsStale.StaleModule.ToString());
default:
return base.Message(exception);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,6 @@ protected override string Message(RefactoringException exception)
DeclarationType.Module);
case NoTargetFolderException noTargetFolder:
return Resources.RubberduckUI.RefactoringFailure_NoTargetFolder;
case AffectedModuleIsStaleException affectedModuleIsStale:
return string.Format(
Resources.RubberduckUI.RefactoringFailure_AffectedModuleIsStale,
affectedModuleIsStale.StaleModule.ToString());
default:
return base.Message(exception);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ protected virtual string Message(RefactoringException exception)
case SuspendParserFailureException suspendParserFailure:
Logger.Warn(suspendParserFailure);
return Resources.RubberduckUI.RefactoringFailure_SuspendParserFailure;
case AffectedModuleIsStaleException affectedModuleIsStale:
return string.Format(
Resources.RubberduckUI.RefactoringFailure_AffectedModuleIsStale,
affectedModuleIsStale.StaleModule.ToString());
default:
Logger.Error(exception);
return string.Empty;
Expand Down

0 comments on commit 213bf1c

Please sign in to comment.