From f3f0d27a3fc7f3742a4360f7e6a8d53a55df0d5b Mon Sep 17 00:00:00 2001 From: michael starke Date: Thu, 16 Jun 2016 19:20:49 +0200 Subject: [PATCH] Resolved issues with undo/redo not working properly on create/remove entry --- MacPass/MPDocument.m | 15 ++++++ MacPass/MPEntryViewController.m | 12 ----- MacPass/MPNodeDelegate.h | 14 ++++++ MacPass/MPNodeDelegate.m | 17 +++++++ MacPassTests/MPTestNodeDelegate.m | 79 +++++++++++++++++++++++++++++++ 5 files changed, 125 insertions(+), 12 deletions(-) create mode 100644 MacPass/MPNodeDelegate.h create mode 100644 MacPass/MPNodeDelegate.m create mode 100644 MacPassTests/MPTestNodeDelegate.m diff --git a/MacPass/MPDocument.m b/MacPass/MPDocument.m index 657dde63..641996be 100644 --- a/MacPass/MPDocument.m +++ b/MacPass/MPDocument.m @@ -462,6 +462,9 @@ NSString *const MPDocumentGroupKey = @"MPDocumentGroupKey return nil; // no new Groups in trash } KPKEntry *newEntry = [self.tree createEntry:parent]; + /* setting properties on entries is undoable, but we do not want to record this so disable on creation */ + BOOL wasUndoEnabeld = self.undoManager.isUndoRegistrationEnabled; + [self.undoManager disableUndoRegistration]; newEntry.title = NSLocalizedString(@"DEFAULT_ENTRY_TITLE", @"Title for a newly created entry"); if([self.tree.metaData.defaultUserName length] > 0) { newEntry.username = self.tree.metaData.defaultUserName; @@ -470,6 +473,10 @@ NSString *const MPDocumentGroupKey = @"MPDocumentGroupKey if(defaultPassword) { newEntry.password = defaultPassword; } + /* re-enable undo/redo if we did turn it off */ + if(wasUndoEnabeld) { + [self.undoManager enableUndoRegistration]; + } [newEntry addToGroup:parent]; [newEntry.undoManager setActionName:NSLocalizedString(@"ADD_ENTRY", "")]; NSDictionary *userInfo = @{ MPDocumentEntryKey: newEntry }; @@ -485,8 +492,15 @@ NSString *const MPDocumentGroupKey = @"MPDocumentGroupKey return nil; // no new Groups in trash } KPKGroup *newGroup = [self.tree createGroup:parent]; + /* setting properties on entries is undoable, but we do not want to record this so disable on creation */ + BOOL wasUndoEnabeld = self.undoManager.isUndoRegistrationEnabled; + [self.undoManager disableUndoRegistration]; newGroup.title = NSLocalizedString(@"DEFAULT_GROUP_NAME", @"Title for a newly created group"); newGroup.iconId = MPIconFolder; + /* re-enable undo/redo if we did turn it off */ + if(wasUndoEnabeld) { + [self.undoManager enableUndoRegistration]; + } [newGroup addToGroup:parent]; [newGroup.undoManager setActionName:NSLocalizedString(@"ADD_GROUP", "")]; NSDictionary *userInfo = @{ MPDocumentGroupKey : newGroup }; @@ -571,6 +585,7 @@ NSString *const MPDocumentGroupKey = @"MPDocumentGroupKey for(KPKGroup *group in node.asGroup.childGroups) { [node.undoManager removeAllActionsWithTarget:group]; } + //[self.undoManager setActionIsDiscardable:YES]; [node remove]; } } diff --git a/MacPass/MPEntryViewController.m b/MacPass/MPEntryViewController.m index 9b8184ae..a231aad5 100644 --- a/MacPass/MPEntryViewController.m +++ b/MacPass/MPEntryViewController.m @@ -434,13 +434,6 @@ NSString *const _MPTableSecurCellView = @"PasswordCell"; - (void)_didExitSearch:(NSNotification *)notification { [[self.entryTable tableColumnWithIdentifier:MPEntryTableParentColumnIdentifier] setHidden:YES]; - // MPDocument *document = [[self windowController] document]; - // document.selectedItem = document.selectedGroup; - // // TODO: really necessary? - // if( nil == document.selectedItem && nil == document.selectedGroup ) { - // [self.entryArrayController unbind:NSContentArrayBinding]; - // [self.entryArrayController setContent:nil]; - // } [self _updateContextBar]; } @@ -461,15 +454,10 @@ NSString *const _MPTableSecurCellView = @"PasswordCell"; - (void)_didEnterHistory:(NSNotification *)notification { [self _showContextBar]; - /* TODO: Show modification date column if not present? */ - MPDocument *document = [[self windowController] document]; - //[self.entryArrayController bind:NSContentArrayBinding toObject:document.selectedEntry withKeyPath:NSStringFromSelector(@selector(history)) options:nil]; } - (void)_didExitHistory:(NSNotification *)notification { [self _hideContextBar]; - MPDocument *document = [[self windowController] document]; - //document.selectedItem = document.selectedEntry; } diff --git a/MacPass/MPNodeDelegate.h b/MacPass/MPNodeDelegate.h new file mode 100644 index 00000000..cdf91e01 --- /dev/null +++ b/MacPass/MPNodeDelegate.h @@ -0,0 +1,14 @@ +// +// MPNodeDelegate.h +// MacPass +// +// Created by Michael Starke on 13/06/16. +// Copyright © 2016 HicknHack Software GmbH. All rights reserved. +// + +#import +#import + +@interface MPNodeDelegate : NSObject + +@end diff --git a/MacPass/MPNodeDelegate.m b/MacPass/MPNodeDelegate.m new file mode 100644 index 00000000..0bb141fe --- /dev/null +++ b/MacPass/MPNodeDelegate.m @@ -0,0 +1,17 @@ +// +// MPNodeDelegate.m +// MacPass +// +// Created by Michael Starke on 13/06/16. +// Copyright © 2016 HicknHack Software GmbH. All rights reserved. +// + +#import "MPNodeDelegate.h" + +@implementation MPNodeDelegate + +- (void)willModifyNode:(KPKNode *)node { + +} + +@end diff --git a/MacPassTests/MPTestNodeDelegate.m b/MacPassTests/MPTestNodeDelegate.m new file mode 100644 index 00000000..30dfabc7 --- /dev/null +++ b/MacPassTests/MPTestNodeDelegate.m @@ -0,0 +1,79 @@ +// +// MPTestNodeDelegate.m +// MacPass +// +// Created by Michael Starke on 13/06/16. +// Copyright © 2016 HicknHack Software GmbH. All rights reserved. +// + +#import +#import + +@interface MPDummyDelegate : NSObject + +@property (strong) NSMutableSet *uuids; + +@end + +@implementation MPDummyDelegate + +- (instancetype)init { + self = [super init]; + if(self) { + self.uuids = [[NSMutableSet alloc] init]; + } + return self; +} + +- (void)willModifyNode:(KPKNode *)node { + if(node.asGroup || ! node.asEntry) { + NSLog(@"Node is no entry, no need to do anything!"); + return; + } + KPKEntry *entry = node.asEntry; + if(![self.uuids containsObject:entry.uuid]) { + [self.uuids addObject:entry.uuid]; + NSLog(@"First mutation for %@ detected. Pushin history", entry); + [entry pushHistory]; + } +} + +@end + +@interface MPTestNodeDelegate : XCTestCase + +@property (strong) KPKEntry *entry; +@property (strong) MPDummyDelegate *delegate; + +@end + +@implementation MPTestNodeDelegate + +- (void)setUp { + [super setUp]; + self.entry = [[KPKEntry alloc] init]; + self.entry.title = @"Entry Title"; + self.entry.url = @"http://www.internet.com"; + self.entry.password = @"1234"; + self.entry.username = @"Entry Username"; + self.entry.autotype.defaultKeystrokeSequence = @"{TAB 3}"; + + self.delegate = [[MPDummyDelegate alloc] init]; + + self.entry.delegate = self.delegate; +} + +- (void)tearDown { + [super tearDown]; +} + +- (void)testModificationDetection { + XCTAssertTrue(self.entry.history.count == 0, @"No History entry is present on newly created entry!"); + self.entry.password = @"New Password"; + XCTAssertEqualObjects(self.entry.password, @"New Password", @"Password is set on entry!"); + XCTAssertTrue(self.entry.history.count == 1, @"Changin the password creates a history entry!"); + KPKEntry *historyEntry = self.entry.history.firstObject; + XCTAssertEqualObjects(historyEntry.password, @"1234", @"Password on history entry did not change!"); +} + +@end