Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change Email Unverified Email state UX tweak #632

Merged
merged 4 commits into from Mar 20, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -31,8 +31,8 @@ final class ChangeEmailViewControllerTests: TestCase {
}

func testChangeEmailScreen_unverifiedEmail() {
let userEnvelope = UserEnvelope(me: UserEmailFields.template |> \.isEmailVerified .~ false)
let service = MockService(changeEmailResponse: userEnvelope)
let userEmailFields = UserEmailFields.template |> \.isEmailVerified .~ false
let service = MockService(fetchGraphUserEmailFieldsResponse: userEmailFields)
combos(Language.allLanguages, Device.allCases).forEach { language, device in
withEnvironment(apiService: service, currentUser: User.template, language: language) {
let controller = ChangeEmailViewController.instantiate()
Expand All @@ -48,10 +48,10 @@ final class ChangeEmailViewControllerTests: TestCase {
func testChangeEmailScreen_unverifiedEmail_isCreator() {
let creator = User.template
|> \.stats.createdProjectsCount .~ 3
let userEnvelope = UserEnvelope(me: UserEmailFields.template |> \.isEmailVerified .~ false)
let userEmailFields = UserEmailFields.template |> \.isEmailVerified .~ false

combos(Language.allLanguages, Device.allCases).forEach { language, device in
withEnvironment(apiService: MockService(changeEmailResponse: userEnvelope),
withEnvironment(apiService: MockService(fetchGraphUserEmailFieldsResponse: userEmailFields),
currentUser: creator,
language: language) {
let controller = ChangeEmailViewController.instantiate()
Expand All @@ -65,10 +65,8 @@ final class ChangeEmailViewControllerTests: TestCase {
}

func testChangeEmailScreen_undeliverableEmail() {
let userEnvelope = UserEnvelope(me: UserEmailFields.template
|> \.isEmailVerified .~ false
|> \.isDeliverable .~ false)
let service = MockService(changeEmailResponse: userEnvelope)
let userEmailFields = UserEmailFields.template |> \.isEmailVerified .~ false |> \.isDeliverable .~ false
let service = MockService(fetchGraphUserEmailFieldsResponse: userEmailFields)
combos(Language.allLanguages, Device.allCases).forEach { language, device in
withEnvironment(apiService: service, currentUser: User.template, language: language) {
let controller = ChangeEmailViewController.instantiate()
Expand Down
10 changes: 6 additions & 4 deletions KsApi/MockService.swift
Expand Up @@ -64,7 +64,7 @@ internal struct MockService: ServiceType {
fileprivate let fetchDraftResponse: UpdateDraft?
fileprivate let fetchDraftError: ErrorEnvelope?

fileprivate let fetchGraphUserEmailResponse: UserEmailFields?
fileprivate let fetchGraphUserEmailFieldsResponse: UserEmailFields?

fileprivate let fetchGraphCreditCardsResponse: UserEnvelope<GraphUserCreditCard>?
fileprivate let fetchGraphCreditCardsError: GraphError?
Expand Down Expand Up @@ -217,7 +217,7 @@ internal struct MockService: ServiceType {
exportDataError: ErrorEnvelope? = nil,
fetchDraftResponse: UpdateDraft? = nil,
fetchDraftError: ErrorEnvelope? = nil,
fetchGraphUserEmailResponse: UserEmailFields? = nil,
fetchGraphUserEmailFieldsResponse: UserEmailFields? = nil,
fetchGraphUserAccountFieldsResponse: UserEnvelope<UserAccountFields>? = nil,
fetchGraphUserAccountFieldsError: GraphError? = nil,
addAttachmentResponse: UpdateDraft.Image? = nil,
Expand Down Expand Up @@ -324,7 +324,7 @@ internal struct MockService: ServiceType {
?? UserEnvelope(me: UserAccountFields.template)
self.fetchGraphUserAccountFieldsError = fetchGraphUserAccountFieldsError

self.fetchGraphUserEmailResponse = fetchGraphUserEmailResponse
self.fetchGraphUserEmailFieldsResponse = fetchGraphUserEmailFieldsResponse

self.fetchCheckoutResponse = fetchCheckoutResponse
self.fetchCheckoutError = fetchCheckoutError
Expand Down Expand Up @@ -668,7 +668,9 @@ internal struct MockService: ServiceType {

internal func fetchGraphUserEmailFields(query: NonEmptySet<Query>)
-> SignalProducer<UserEnvelope<UserEmailFields>, GraphError> {
return SignalProducer(value: changeEmailResponse ?? UserEnvelope<UserEmailFields>(me: .template))
let response = self.fetchGraphUserEmailFieldsResponse ?? .template

return SignalProducer(value: UserEnvelope(me: response))
}

internal func fetchGraphUserAccountFields(query: NonEmptySet<Query>)
Expand Down
7 changes: 2 additions & 5 deletions Library/ViewModels/ChangeEmailViewModel.swift
Expand Up @@ -63,10 +63,7 @@ ChangeEmailViewModelOutputs {
self.newEmailProperty <~ clearValues
self.passwordProperty <~ clearValues

let userEmailEvent = Signal.merge(
self.viewDidLoadProperty.signal,
changeEmailEvent.values().ignoreValues()
)
let userEmailEvent = self.viewDidLoadProperty.signal
.switchMap { _ in
AppEnvironment.current
.apiService
Expand Down Expand Up @@ -101,7 +98,7 @@ ChangeEmailViewModelOutputs {
.map { $0 && $1 }

self.resendVerificationEmailViewIsHidden = Signal.merge(viewDidLoadProperty.signal.mapConst(true),
emailVerifiedAndDeliverable)
emailVerifiedAndDeliverable).skipRepeats()

self.unverifiedEmailLabelHidden = Signal
.combineLatest(isEmailVerified, isEmailDeliverable)
Expand Down
39 changes: 28 additions & 11 deletions Library/ViewModels/ChangeEmailViewModelTests.swift
Expand Up @@ -60,16 +60,33 @@ final class ChangeEmailViewModelTests: TestCase {
self.vm.outputs.verificationEmailButtonTitle.observe(self.verificationEmailButtonTitle.observer)
}

func testDidChangeEmailEmits_OnSuccess() {
func testChangeEmail_OnSuccess() {
let updatedEmail = UserEmailFields.template |> \.email .~ "apple@kickstarter.com"
let response = UserEnvelope<UserEmailFields>(me: updatedEmail)
let mockService = MockService(changeEmailResponse: response)

self.vm.inputs.emailFieldTextDidChange(text: "ksr@kickstarter.com")
self.vm.inputs.passwordFieldTextDidChange(text: "123456")
withEnvironment(apiService: mockService) {
self.vm.inputs.viewDidLoad()

self.vm.inputs.saveButtonTapped()
self.scheduler.advance()

self.scheduler.advance()
self.resendVerificationEmailViewIsHidden.assertValues([true])
self.unverifiedEmailLabelHidden.assertValues([true])
self.emailText.assertValues(["ksr@kickstarter.com"])

self.didChangeEmail.assertDidEmitValue()
self.vm.inputs.emailFieldTextDidChange(text: "apple@kickstarter.com")
self.vm.inputs.passwordFieldTextDidChange(text: "123456")

self.vm.inputs.saveButtonTapped()

self.scheduler.advance()

self.didChangeEmail.assertDidEmitValue()
self.emailText.assertValues(["ksr@kickstarter.com", "apple@kickstarter.com"])
self.resendVerificationEmailViewIsHidden.assertValues([true],
"Resend verification email button does not show")
self.unverifiedEmailLabelHidden.assertValues([true])
}
}

func testDidFailToChangeEmailEmits_OnFailure() {
Expand Down Expand Up @@ -209,14 +226,14 @@ final class ChangeEmailViewModelTests: TestCase {
self.scheduler.advance()

self.resendVerificationEmailViewIsHidden
.assertValues([true, true], "Email is deliverable and verified")
.assertValues([true], "Email is deliverable and verified")
}

func testResendVerificationViewIsNotHidden_IfEmailIsNotVerified() {
let userEmailFields = UserEmailFields.template
|> \.isEmailVerified .~ false

let mockService = MockService(changeEmailResponse: UserEnvelope(me: userEmailFields))
let mockService = MockService(fetchGraphUserEmailFieldsResponse: userEmailFields)

withEnvironment(apiService: mockService) {
self.vm.inputs.viewDidLoad()
Expand All @@ -231,7 +248,7 @@ final class ChangeEmailViewModelTests: TestCase {
let userEmailFields = UserEmailFields.template
|> \.isDeliverable .~ false

let mockService = MockService(changeEmailResponse: UserEnvelope(me: userEmailFields))
let mockService = MockService(fetchGraphUserEmailFieldsResponse: userEmailFields)

withEnvironment(apiService: mockService) {
self.vm.inputs.viewDidLoad()
Expand All @@ -255,7 +272,7 @@ final class ChangeEmailViewModelTests: TestCase {
let userEmailFields = UserEmailFields.template
|> \.isDeliverable .~ false

withEnvironment(apiService: MockService(changeEmailResponse: UserEnvelope(me: userEmailFields))) {
withEnvironment(apiService: MockService(fetchGraphUserEmailFieldsResponse: userEmailFields)) {
self.vm.inputs.viewDidLoad()

self.scheduler.advance()
Expand All @@ -277,7 +294,7 @@ final class ChangeEmailViewModelTests: TestCase {
let userEmailFields = UserEmailFields.template
|> \.isEmailVerified .~ false

withEnvironment(apiService: MockService(changeEmailResponse: UserEnvelope(me: userEmailFields))) {
withEnvironment(apiService: MockService(fetchGraphUserEmailFieldsResponse: userEmailFields)) {
self.vm.inputs.viewDidLoad()

self.scheduler.advance()
Expand Down