From 97e80d780d4d46afc88a7795b02b84b2cc9addac Mon Sep 17 00:00:00 2001 From: Sasha Heinen Date: Fri, 26 Apr 2019 08:41:04 -0700 Subject: [PATCH] fix for recurring web address launch from app (#932) * fix + tests * comment --- .../Presenter/ItemDetailPresenter.swift | 3 + .../ItemDetailPresenterSpec.swift | 57 +++++++++++-------- 2 files changed, 36 insertions(+), 24 deletions(-) diff --git a/lockbox-ios/Presenter/ItemDetailPresenter.swift b/lockbox-ios/Presenter/ItemDetailPresenter.swift index 46ae60208..9c1b2523b 100644 --- a/lockbox-ios/Presenter/ItemDetailPresenter.swift +++ b/lockbox-ios/Presenter/ItemDetailPresenter.swift @@ -47,6 +47,9 @@ class ItemDetailPresenter { target.itemDetailStore.itemDetailId .take(1) .flatMap { target.dataStore.get($0) } + // The first `take(1)` call does not push a completion after it emits, so a second one is necessary here + // to ensure that subsequent updates to the item do not result in subsequent dispacthed actions. + .take(1) .map { item -> [Action] in var actions: [Action] = [] if copyableFields.contains(value) { diff --git a/lockbox-iosTests/ItemDetailPresenterSpec.swift b/lockbox-iosTests/ItemDetailPresenterSpec.swift index 5c8d66618..e120e7f20 100644 --- a/lockbox-iosTests/ItemDetailPresenterSpec.swift +++ b/lockbox-iosTests/ItemDetailPresenterSpec.swift @@ -50,10 +50,10 @@ class ItemDetailPresenterSpec: QuickSpec { } class FakeDispatcher: Dispatcher { - var dispatchActionArgument: Action? + var dispatchActionArgument: [Action] = [] override func dispatch(action: Action) { - self.dispatchActionArgument = action + self.dispatchActionArgument.append(action) } } @@ -134,8 +134,8 @@ class ItemDetailPresenterSpec: QuickSpec { } it("starts the password display as hidden") { - expect(self.dispatcher.dispatchActionArgument).notTo(beNil()) - let action = self.dispatcher.dispatchActionArgument as! ItemDetailDisplayAction + expect(self.dispatcher.dispatchActionArgument).notTo(beEmpty()) + let action = self.dispatcher.dispatchActionArgument.first as! ItemDetailDisplayAction expect(action).to(equal(.togglePassword(displayed: false))) } @@ -152,8 +152,8 @@ class ItemDetailPresenterSpec: QuickSpec { } it("dispatches the password action with the value") { - expect(self.dispatcher.dispatchActionArgument).notTo(beNil()) - let action = self.dispatcher.dispatchActionArgument as! ItemDetailDisplayAction + expect(self.dispatcher.dispatchActionArgument).notTo(beEmpty()) + let action = self.dispatcher.dispatchActionArgument.last as! ItemDetailDisplayAction expect(action).to(equal(.togglePassword(displayed: passwordRevealSelected))) } } @@ -170,8 +170,8 @@ class ItemDetailPresenterSpec: QuickSpec { } it("routes to the item list") { - expect(self.dispatcher.dispatchActionArgument).notTo(beNil()) - let argument = self.dispatcher.dispatchActionArgument as! MainRouteAction + expect(self.dispatcher.dispatchActionArgument).notTo(beEmpty()) + let argument = self.dispatcher.dispatchActionArgument.last as! MainRouteAction expect(argument).to(equal(.list)) } } @@ -203,8 +203,8 @@ class ItemDetailPresenterSpec: QuickSpec { } it("dispatches the copy action") { - expect(self.dispatcher.dispatchActionArgument).notTo(beNil()) - let action = self.dispatcher.dispatchActionArgument as! CopyAction + expect(self.dispatcher.dispatchActionArgument).notTo(beEmpty()) + let action = self.dispatcher.dispatchActionArgument.last as! CopyAction expect(action).to(equal(CopyAction(text: username, field: .username, itemID: "", actionType: .tap))) } } @@ -216,8 +216,8 @@ class ItemDetailPresenterSpec: QuickSpec { } it("dispatches the copy action with no text") { - expect(self.dispatcher.dispatchActionArgument).notTo(beNil()) - let action = self.dispatcher.dispatchActionArgument as! CopyAction + expect(self.dispatcher.dispatchActionArgument).notTo(beEmpty()) + let action = self.dispatcher.dispatchActionArgument.last as! CopyAction expect(action).to(equal(CopyAction(text: "", field: .username, itemID: "", actionType: .tap))) } } @@ -250,8 +250,8 @@ class ItemDetailPresenterSpec: QuickSpec { } it("dispatches the copy action") { - expect(self.dispatcher.dispatchActionArgument).notTo(beNil()) - let action = self.dispatcher.dispatchActionArgument as! CopyAction + expect(self.dispatcher.dispatchActionArgument).notTo(beEmpty()) + let action = self.dispatcher.dispatchActionArgument.last as! CopyAction expect(action).to(equal(CopyAction(text: password, field: .password, itemID: "", actionType: .tap))) } } @@ -263,8 +263,8 @@ class ItemDetailPresenterSpec: QuickSpec { } it("dispatches the copy action with no text") { - expect(self.dispatcher.dispatchActionArgument).notTo(beNil()) - let action = self.dispatcher.dispatchActionArgument as! CopyAction + expect(self.dispatcher.dispatchActionArgument).notTo(beEmpty()) + let action = self.dispatcher.dispatchActionArgument.last as! CopyAction expect(action).to(equal(CopyAction(text: "", field: .password, itemID: "", actionType: .tap))) } } @@ -289,18 +289,27 @@ class ItemDetailPresenterSpec: QuickSpec { describe("getting the item") { let webAddress = "https://www.mozilla.org" + let item = LoginRecord(fromJSONDict: ["id": "sdfdfsfd", "hostname": webAddress, "username": "ffs", "password": "ilikecatz"]) beforeEach { - let item = LoginRecord(fromJSONDict: ["id": "sdfdfsfd", "hostname": webAddress, "username": "ffs", "password": "ilikecatz"]) - self.dataStore.onItemStub.onNext(item) } it("dispatches the externalLink action") { - expect(self.dispatcher.dispatchActionArgument).notTo(beNil()) - let action = self.dispatcher.dispatchActionArgument as! ExternalLinkAction + expect(self.dispatcher.dispatchActionArgument).notTo(beEmpty()) + let action = self.dispatcher.dispatchActionArgument.last as! ExternalLinkAction expect(action).to(equal(ExternalLinkAction(baseURLString: webAddress))) } + + describe("subsequent pushes of the same item") { + beforeEach { + self.dataStore.onItemStub.onNext(item) + } + + it("dispatches nothing new") { + expect(self.dispatcher.dispatchActionArgument.count).to(equal(2)) + } + } } } @@ -488,8 +497,8 @@ class ItemDetailPresenterSpec: QuickSpec { } it("dispatches the faq link action") { - expect(self.dispatcher.dispatchActionArgument).notTo(beNil()) - let argument = self.dispatcher.dispatchActionArgument as! ExternalWebsiteRouteAction + expect(self.dispatcher.dispatchActionArgument).notTo(beEmpty()) + let argument = self.dispatcher.dispatchActionArgument.last as! ExternalWebsiteRouteAction expect(argument).to(equal( ExternalWebsiteRouteAction( urlString: Constant.app.editExistingEntriesFAQ, @@ -524,8 +533,8 @@ class ItemDetailPresenterSpec: QuickSpec { } it("sends copy action") { - expect(self.dispatcher.dispatchActionArgument).notTo(beNil()) - let action = self.dispatcher.dispatchActionArgument as! CopyAction + expect(self.dispatcher.dispatchActionArgument).notTo(beEmpty()) + let action = self.dispatcher.dispatchActionArgument.last as! CopyAction expect(action).to(equal(CopyAction(text: "asdf", field: CopyField.username, itemID: "1234", actionType: CopyActionType.dnd))) } }