Skip to content

Commit

Permalink
[MBL-59] Fix personal data export. (#1836)
Browse files Browse the repository at this point in the history
* [MBL-59] Fix personal data export.

Update `ExportDataEnvelope` `State` types to align with the types defined by the server. Update related logic to be more readable. Also update some test cases to default to `none` instead of `expired`, now that `none` is the default state for users that have never requested their data before.

* [MBL-59] Handle unknown states

Update the ExportDataEnvelope `state` enum to rely on an underlying `stateString`. This allows a default `unknown` value to be used if the state from the server is missing or doesn't match a value in the enum.

* rename stateString to rawState
  • Loading branch information
ifosli committed Aug 2, 2023
1 parent 7958ce3 commit 6266d87
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 22 deletions.
Expand Up @@ -116,8 +116,8 @@ internal final class SettingsPrivacyRequestDataCell: UITableViewCell, ValueCell
self.requestedDataStatusAndDateLabel.rac.text = self.viewModel.outputs.requestedDataExpirationDate
self.chevron.rac.hidden = self.viewModel.outputs.dataExpirationAndChevronHidden
self.requestDataLabel.rac.hidden = self.viewModel.outputs.requestDataTextHidden
self.preparingDataLabel.rac.hidden = self.viewModel.outputs.showPreparingDataAndCheckBackLaterText
self.checkBackLaterLabel.rac.hidden = self.viewModel.outputs.showPreparingDataAndCheckBackLaterText
self.preparingDataLabel.rac.hidden = self.viewModel.outputs.preparingDataAndCheckBackLaterTextHidden
self.checkBackLaterLabel.rac.hidden = self.viewModel.outputs.preparingDataAndCheckBackLaterTextHidden
self.requestedDataStatusAndDateLabel.rac.hidden = self.viewModel.outputs.dataExpirationAndChevronHidden
}

Expand Down
31 changes: 28 additions & 3 deletions KsApi/models/ExportDataEnvelope.swift
Expand Up @@ -2,21 +2,46 @@

public struct ExportDataEnvelope {
public let expiresAt: String?
public let state: State
public let dataUrl: String?
public var state: State {
guard let rawState, let state = State(rawValue: rawState) else {
return .unknown
}
return state
}

private let rawState: String?

public init(expiresAt: String?, state: State, dataUrl: String?) {
self.expiresAt = expiresAt
self.dataUrl = dataUrl
self.rawState = state.rawValue
}

public init(expiresAt: String?, rawState: String?, dataUrl: String?) {
self.expiresAt = expiresAt
self.dataUrl = dataUrl
self.rawState = rawState
}

public enum State: String, Decodable {
case none
case created
case queued
case processing
case assembling
case assembled
case uploading
case completed
case failed
case expired
case unknown // Default value if server response can't be parsed.
}
}

extension ExportDataEnvelope: Decodable {
enum CodingKeys: String, CodingKey {
case expiresAt = "expires_at"
case state
case rawState = "state"
case dataUrl = "data_url"
}
}
5 changes: 5 additions & 0 deletions KsApi/models/lenses/ExportDataEnvelopeLenses.swift
Expand Up @@ -14,5 +14,10 @@ extension ExportDataEnvelope {
view: { $0.dataUrl },
set: { ExportDataEnvelope(expiresAt: $1.expiresAt, state: $1.state, dataUrl: $0) }
)

public static let rawState = Lens<ExportDataEnvelope, String?>(
view: { $0.state.rawValue },
set: { ExportDataEnvelope(expiresAt: $1.expiresAt, rawState: $0, dataUrl: $1.dataUrl) }
)
}
}
23 changes: 13 additions & 10 deletions Library/ViewModels/SettingsRequestDataCellViewModel.swift
Expand Up @@ -19,7 +19,7 @@ public protocol SettingsRequestDataCellViewModelOutputs {
var requestDataLoadingIndicator: Signal<Bool, Never> { get }
var requestDataText: Signal<String, Never> { get }
var requestDataTextHidden: Signal<Bool, Never> { get }
var showPreparingDataAndCheckBackLaterText: Signal<Bool, Never> { get }
var preparingDataAndCheckBackLaterTextHidden: Signal<Bool, Never> { get }
var showRequestDataPrompt: Signal<String, Never> { get }
var unableToRequestDataError: Signal<String, Never> { get }
}
Expand Down Expand Up @@ -73,7 +73,7 @@ public final class SettingsRequestDataCellViewModel: SettingsRequestDataCellView

self.requestDataLoadingIndicator = Signal.merge(
self.configureWithUserProperty.signal.mapConst(false),
exportEnvelope.map { $0.state == .processing },
exportEnvelope.map { [.assembling, .assembled, .uploading].contains($0.state) },
self.startRequestDataTappedProperty.signal.mapConst(true)
)

Expand All @@ -82,7 +82,7 @@ public final class SettingsRequestDataCellViewModel: SettingsRequestDataCellView
self.requestDataText = Signal.merge(
initialText,
exportEnvelope
.map { $0.state == .expired || $0.expiresAt == nil || $0.dataUrl == nil
.map { canRequestData($0)
? Strings.Request_my_personal_data() : Strings.Download_your_personal_data()
}
)
Expand All @@ -96,21 +96,23 @@ public final class SettingsRequestDataCellViewModel: SettingsRequestDataCellView
self.dataExpirationAndChevronHidden = Signal.merge(
self.awakeFromNibProperty.signal.mapConst(true),
exportEnvelope
.map { $0.state == .expired || $0.expiresAt == nil || $0.dataUrl == nil }
.map(canRequestData)
)

self.goToSafari = exportEnvelope
.filter { $0.state != .expired || $0.expiresAt != nil }
.filter { $0.state == .completed || $0.expiresAt != nil }
.map { $0.dataUrl ?? "" }
.takeWhen(self.exportDataTappedProperty.signal)

self.showPreparingDataAndCheckBackLaterText = Signal.merge(
self.preparingDataAndCheckBackLaterTextHidden = Signal.merge(
self.configureWithUserProperty.signal.mapConst(true),
exportEnvelope.map { $0.state != .processing },
exportEnvelope.map {
![.assembling, .assembled, .uploading].contains($0.state)
},
self.startRequestDataTappedProperty.signal.mapConst(false)
)

self.requestDataTextHidden = self.showPreparingDataAndCheckBackLaterText.signal.map { !$0 }
self.requestDataTextHidden = self.preparingDataAndCheckBackLaterTextHidden.signal.map { !$0 }
}

fileprivate let awakeFromNibProperty = MutableProperty(())
Expand Down Expand Up @@ -140,7 +142,7 @@ public final class SettingsRequestDataCellViewModel: SettingsRequestDataCellView
public let requestDataLoadingIndicator: Signal<Bool, Never>
public let requestDataText: Signal<String, Never>
public let requestDataTextHidden: Signal<Bool, Never>
public let showPreparingDataAndCheckBackLaterText: Signal<Bool, Never>
public let preparingDataAndCheckBackLaterTextHidden: Signal<Bool, Never>
public let showRequestDataPrompt: Signal<String, Never>
public let unableToRequestDataError: Signal<String, Never>

Expand All @@ -149,7 +151,8 @@ public final class SettingsRequestDataCellViewModel: SettingsRequestDataCellView
}

private func canRequestData(_ envelope: ExportDataEnvelope) -> Bool {
return envelope.dataUrl == nil || envelope.state == .expired || envelope.expiresAt == nil
return envelope.dataUrl == nil || envelope.expiresAt == nil
|| [.expired, .failed, .none, .unknown].contains(envelope.state)
}

private func dateFormatter(for dateString: String?, state: ExportDataEnvelope.State) -> String {
Expand Down
27 changes: 20 additions & 7 deletions Library/ViewModels/SettingsRequestDataCellViewModelTests.swift
Expand Up @@ -15,7 +15,7 @@ internal final class SettingsRequestDataCellViewModelTests: TestCase {
internal let requestDataLoadingIndicator = TestObserver<Bool, Never>()
internal let requestDataText = TestObserver<String, Never>()
internal let requestDataTextHidden = TestObserver<Bool, Never>()
internal let showPreparingDataAndCheckBackLaterText = TestObserver<Bool, Never>()
internal let preparingDataAndCheckBackLaterTextHidden = TestObserver<Bool, Never>()
internal let showRequestDataPrompt = TestObserver<String, Never>()
internal let unableToRequestDataError = TestObserver<String, Never>()

Expand All @@ -28,8 +28,8 @@ internal final class SettingsRequestDataCellViewModelTests: TestCase {
self.vm.outputs.requestDataText.observe(self.requestDataText.observer)
self.vm.outputs.requestDataTextHidden.observe(self.requestDataTextHidden.observer)
self.vm.outputs.dataExpirationAndChevronHidden.observe(self.dataExpirationAndChevronHidden.observer)
self.vm.outputs.showPreparingDataAndCheckBackLaterText
.observe(self.showPreparingDataAndCheckBackLaterText.observer)
self.vm.outputs.preparingDataAndCheckBackLaterTextHidden
.observe(self.preparingDataAndCheckBackLaterTextHidden.observer)
self.vm.outputs.showRequestDataPrompt.observe(self.showRequestDataPrompt.observer)
self.vm.outputs.unableToRequestDataError.observe(self.unableToRequestDataError.observer)
}
Expand All @@ -49,7 +49,7 @@ internal final class SettingsRequestDataCellViewModelTests: TestCase {
func testRequestDataButtonIsEnabled() {
let user = User.template
let export = .template
|> ExportDataEnvelope.lens.state .~ .expired
|> ExportDataEnvelope.lens.state .~ .none
|> ExportDataEnvelope.lens.expiresAt .~ nil
|> ExportDataEnvelope.lens.dataUrl .~ nil

Expand All @@ -61,6 +61,19 @@ internal final class SettingsRequestDataCellViewModelTests: TestCase {
}
}

func testRequestDataButtonIsEnabledUnknownState() {
let user = User.template
let export = .template
|> ExportDataEnvelope.lens.rawState .~ "invalid-state"
|> ExportDataEnvelope.lens.expiresAt .~ nil
|> ExportDataEnvelope.lens.dataUrl .~ nil

withEnvironment(apiService: MockService(fetchExportStateResponse: export)) {
self.vm.inputs.configureWith(user: user)
self.requestDataButtonEnabled.assertValues([true])
}
}

func testDataExpirationDate() {
let user = User.template

Expand Down Expand Up @@ -137,7 +150,7 @@ internal final class SettingsRequestDataCellViewModelTests: TestCase {
let user = User.template
let userEnvelope = UserEnvelope(me: GraphUser.template)
let export = .template
|> ExportDataEnvelope.lens.state .~ .expired
|> ExportDataEnvelope.lens.state .~ .none
|> ExportDataEnvelope.lens.expiresAt .~ nil
|> ExportDataEnvelope.lens.dataUrl .~ nil

Expand Down Expand Up @@ -167,13 +180,13 @@ internal final class SettingsRequestDataCellViewModelTests: TestCase {

self.requestDataLoadingIndicator.assertValues([false])
self.requestDataTextHidden.assertValues([false])
self.showPreparingDataAndCheckBackLaterText.assertValues([true])
self.preparingDataAndCheckBackLaterTextHidden.assertValues([true])

self.vm.inputs.startRequestDataTapped()

self.requestDataLoadingIndicator.assertValues([false, true])
self.requestDataTextHidden.assertValues([false, true])
self.showPreparingDataAndCheckBackLaterText.assertValues([true, false])
self.preparingDataAndCheckBackLaterTextHidden.assertValues([true, false])
}
}

Expand Down

0 comments on commit 6266d87

Please sign in to comment.