Perform migrate + import in one step (#882)

- Drop the .importing / .imported steps
- Animate rows re-sorting during process
- Rephrase some strings better
- Test fake migration with launch argument
This commit is contained in:
Davide 2024-11-16 21:16:25 +01:00 committed by GitHub
parent 83eb02aa9d
commit 9e5beff23a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
14 changed files with 77 additions and 85 deletions

View File

@ -111,6 +111,11 @@
value = "1" value = "1"
isEnabled = "NO"> isEnabled = "NO">
</EnvironmentVariable> </EnvironmentVariable>
<EnvironmentVariable
key = "PP_FAKE_MIGRATION"
value = "1"
isEnabled = "YES">
</EnvironmentVariable>
</EnvironmentVariables> </EnvironmentVariables>
<StoreKitConfigurationFileReference <StoreKitConfigurationFileReference
identifier = "../../Passepartout/Passepartout.storekit"> identifier = "../../Passepartout/Passepartout.storekit">

View File

@ -43,25 +43,19 @@ private extension MigrateButton {
return Strings.Views.Migrate.Items.migrate return Strings.Views.Migrate.Items.migrate
case .migrating, .migrated: case .migrating, .migrated:
return Strings.Views.Migrate.Items.import
case .importing, .imported:
return Strings.Global.done return Strings.Global.done
} }
} }
var isEnabled: Bool { var isEnabled: Bool {
switch step { switch step {
case .initial, .fetching, .migrating, .importing: case .initial, .fetching, .migrating:
return false return false
case .fetched(let profiles): case .fetched(let profiles):
return !profiles.isEmpty return !profiles.isEmpty
case .migrated(let profiles): case .migrated:
return !profiles.isEmpty
case .imported:
return true return true
} }
} }

View File

@ -141,7 +141,7 @@ private extension MigrateContentView.ListView {
} }
} }
Spacer() Spacer()
Button(isEditing ? Strings.Global.delete : Strings.Global.edit, role: isEditing ? .destructive : nil) { Button(title, role: role) {
if isEditing { if isEditing {
if !selection.isEmpty { if !selection.isEmpty {
onDelete() onDelete()
@ -157,6 +157,14 @@ private extension MigrateContentView.ListView {
} }
.frame(height: 30) .frame(height: 30)
} }
var title: String {
isEditing ? Strings.Views.Migrate.Items.discard : Strings.Global.edit
}
var role: ButtonRole? {
isEditing ? .destructive : nil
}
} }
} }
@ -269,7 +277,7 @@ private extension MigrateContentView.ListView {
case .pending: case .pending:
ProgressView() ProgressView()
case .migrated, .imported: case .done:
ThemeImage(.marked) ThemeImage(.marked)
case .failed: case .failed:

View File

@ -82,7 +82,7 @@ private extension MigrateContentView.TableView {
) )
.environmentObject(theme) // TODO: #873, Table loses environment .environmentObject(theme) // TODO: #873, Table loses environment
} }
.width(20) .width(30)
TableColumn("") { profile in TableColumn("") { profile in
Button { Button {
onDelete([profile]) onDelete([profile])
@ -147,7 +147,7 @@ private extension MigrateContentView.TableView {
case .pending: case .pending:
ThemeImage(.progress) ThemeImage(.progress)
case .migrated, .imported: case .done:
ThemeImage(.marked) ThemeImage(.marked)
case .failed: case .failed:

View File

@ -70,7 +70,13 @@ struct MigrateContentView<PerformButton>: View where PerformButton: View {
extension Optional where Wrapped == MigrationStatus { extension Optional where Wrapped == MigrationStatus {
var style: some ShapeStyle { var style: some ShapeStyle {
self != .excluded ? .primary : .secondary switch self {
case .excluded, .failed:
return .secondary
default:
return .primary
}
} }
} }
@ -101,9 +107,8 @@ extension Dictionary where Key == UUID, Value == MigrationStatus {
initialStatuses: [ initialStatuses: [
PrivatePreviews.profiles[0].id: .excluded, PrivatePreviews.profiles[0].id: .excluded,
PrivatePreviews.profiles[1].id: .pending, PrivatePreviews.profiles[1].id: .pending,
PrivatePreviews.profiles[2].id: .migrated, PrivatePreviews.profiles[2].id: .done,
PrivatePreviews.profiles[3].id: .imported, PrivatePreviews.profiles[3].id: .failed
PrivatePreviews.profiles[4].id: .failed
] ]
) )
.withMockEnvironment() .withMockEnvironment()

View File

@ -46,30 +46,14 @@ extension MigrateView {
} }
extension MigrateView.Model { extension MigrateView.Model {
// XXX: filtering out the excluded rows may crash on macOS, because ThemeImage is
// momentarily removed from the hierarchy and loses access to the Theme
// .environmentObject(). this is certainly a SwiftUI bug
//
// https://github.com/passepartoutvpn/passepartout/pull/867#issuecomment-2476293204
//
var visibleProfiles: [MigratableProfile] { var visibleProfiles: [MigratableProfile] {
profiles profiles
// .filter {
// switch step {
// case .initial, .fetching, .fetched:
// return true
//
// case .migrating, .migrated, .importing, .imported:
// return statuses[$0.id] != .excluded
// }
// }
.sorted { .sorted {
switch step { switch step {
case .initial, .fetching, .fetched: case .initial, .fetching, .fetched:
return $0.name.lowercased() < $1.name.lowercased() return $0.name.lowercased() < $1.name.lowercased()
case .migrating, .migrated, .importing, .imported: case .migrating, .migrated:
return (statuses[$0.id].rank, $0.name.lowercased()) < (statuses[$1.id].rank, $1.name.lowercased()) return (statuses[$0.id].rank, $0.name.lowercased()) < (statuses[$1.id].rank, $1.name.lowercased())
} }
} }
@ -78,9 +62,15 @@ extension MigrateView.Model {
private extension Optional where Wrapped == MigrationStatus { private extension Optional where Wrapped == MigrationStatus {
var rank: Int { var rank: Int {
if self == .excluded { switch self {
return .max case .failed:
return 1
case .excluded:
return 2
default:
return .min
} }
return .min
} }
} }

View File

@ -79,10 +79,10 @@ struct MigrateView: View {
isEmpty: model.profiles.isEmpty, isEmpty: model.profiles.isEmpty,
emptyMessage: Strings.Views.Migrate.noProfiles emptyMessage: Strings.Views.Migrate.noProfiles
) )
.themeAnimation(on: model.step, category: .profiles) .themeAnimation(on: model, category: .profiles)
.themeConfirmation( .themeConfirmation(
isPresented: $isDeleting, isPresented: $isDeleting,
title: Strings.Views.Migrate.Alerts.Delete.title, title: Strings.Views.Migrate.Items.discard,
message: messageForDeletion, message: messageForDeletion,
isDestructive: true, isDestructive: true,
action: confirmPendingDeletion action: confirmPendingDeletion
@ -130,10 +130,7 @@ private extension MigrateView {
case .fetched(let profiles): case .fetched(let profiles):
await migrate(profiles) await migrate(profiles)
case .migrated(let profiles): case .migrated:
await save(profiles)
case .imported:
dismiss() dismiss()
default: default:
@ -180,10 +177,20 @@ private extension MigrateView {
do { do {
pp_log(.App.migration, .notice, "Migrate \(profiles.count) profiles...") pp_log(.App.migration, .notice, "Migrate \(profiles.count) profiles...")
let profiles = try await migrationManager.migratedProfiles(profiles) { let profiles = try await migrationManager.migratedProfiles(profiles) {
guard $1 != .done else {
return
}
model.statuses[$0] = $1 model.statuses[$0] = $1
} }
model.excludeFailed() pp_log(.App.migration, .notice, "Mapped \(profiles.count) profiles to the new format, saving...")
model.step = .migrated(profiles) await migrationManager.importProfiles(profiles, into: profileManager) {
model.statuses[$0] = $1
}
let migrated = profiles.filter {
model.statuses[$0.id] == .done
}
pp_log(.App.migration, .notice, "Migrated \(migrated.count) profiles")
model.step = .migrated(migrated)
} catch { } catch {
pp_log(.App.migration, .error, "Unable to migrate profiles: \(error)") pp_log(.App.migration, .error, "Unable to migrate profiles: \(error)")
errorHandler.handle(error, title: title) errorHandler.handle(error, title: title)
@ -191,28 +198,6 @@ private extension MigrateView {
} }
} }
func save(_ allProfiles: [Profile]) async {
guard case .migrated = model.step else {
assertionFailure("Must call migrate() and succeed, why is button enabled?")
return
}
let profiles = allProfiles.filter {
model.statuses[$0.id] != .excluded
}
guard !profiles.isEmpty else {
assertionFailure("Nothing to import, why is button enabled?")
return
}
model.step = .importing
pp_log(.App.migration, .notice, "Import \(profiles.count) migrated profiles...")
await migrationManager.importProfiles(profiles, into: profileManager) {
model.statuses[$0] = $1
}
model.step = .imported
}
func confirmPendingDeletion() { func confirmPendingDeletion() {
guard let profilesPendingDeletion else { guard let profilesPendingDeletion else {
isEditing = false isEditing = false

View File

@ -38,10 +38,6 @@ enum MigrateViewStep: Equatable {
case migrated([Profile]) case migrated([Profile])
case importing
case imported
var canSelect: Bool { var canSelect: Bool {
guard case .fetched = self else { guard case .fetched = self else {
return false return false

View File

@ -82,7 +82,7 @@ extension MigrationManager {
await onUpdate(migratable.id, .failed) await onUpdate(migratable.id, .failed)
return nil return nil
} }
await onUpdate(migratable.id, .migrated) await onUpdate(migratable.id, .done)
return profile return profile
} catch { } catch {
await onUpdate(migratable.id, .failed) await onUpdate(migratable.id, .failed)
@ -115,7 +115,7 @@ extension MigrationManager {
do { do {
try await self.simulateBehavior() try await self.simulateBehavior()
try await self.simulateSaveProfile(profile, manager: manager) try await self.simulateSaveProfile(profile, manager: manager)
await onUpdate(profile.id, .imported) await onUpdate(profile.id, .done)
} catch { } catch {
await onUpdate(profile.id, .failed) await onUpdate(profile.id, .failed)
} }

View File

@ -30,9 +30,7 @@ public enum MigrationStatus: Equatable {
case pending case pending
case migrated case done
case imported
case failed case failed
} }

View File

@ -734,20 +734,18 @@ public enum Strings {
public static func message(_ p1: Any) -> String { public static func message(_ p1: Any) -> String {
return Strings.tr("Localizable", "views.migrate.alerts.delete.message", String(describing: p1), fallback: "Do you want to discard these profiles? You will not be able to recover them later.\n\n%@") return Strings.tr("Localizable", "views.migrate.alerts.delete.message", String(describing: p1), fallback: "Do you want to discard these profiles? You will not be able to recover them later.\n\n%@")
} }
/// Discard
public static let title = Strings.tr("Localizable", "views.migrate.alerts.delete.title", fallback: "Discard")
} }
} }
public enum Items { public enum Items {
/// Import /// Discard
public static let `import` = Strings.tr("Localizable", "views.migrate.items.import", fallback: "Import") public static let discard = Strings.tr("Localizable", "views.migrate.items.discard", fallback: "Discard")
/// Proceed /// Proceed
public static let migrate = Strings.tr("Localizable", "views.migrate.items.migrate", fallback: "Proceed") public static let migrate = Strings.tr("Localizable", "views.migrate.items.migrate", fallback: "Proceed")
} }
public enum Sections { public enum Sections {
public enum Main { public enum Main {
/// Select below the profiles from old versions of Passepartout that you want to import. Profiles will disappear from this list once imported successfully. /// Select below the profiles from old versions of Passepartout that you want to import. They will disappear from this list once imported successfully.
public static let header = Strings.tr("Localizable", "views.migrate.sections.main.header", fallback: "Select below the profiles from old versions of Passepartout that you want to import. Profiles will disappear from this list once imported successfully.") public static let header = Strings.tr("Localizable", "views.migrate.sections.main.header", fallback: "Select below the profiles from old versions of Passepartout that you want to import. They will disappear from this list once imported successfully.")
} }
} }
} }

View File

@ -161,10 +161,9 @@
"views.migrate.title" = "Migrate"; "views.migrate.title" = "Migrate";
"views.migrate.no_profiles" = "Nothing to migrate"; "views.migrate.no_profiles" = "Nothing to migrate";
"views.migrate.items.discard" = "Discard";
"views.migrate.items.migrate" = "Proceed"; "views.migrate.items.migrate" = "Proceed";
"views.migrate.items.import" = "Import"; "views.migrate.sections.main.header" = "Select below the profiles from old versions of Passepartout that you want to import. They will disappear from this list once imported successfully.";
"views.migrate.sections.main.header" = "Select below the profiles from old versions of Passepartout that you want to import. Profiles will disappear from this list once imported successfully.";
"views.migrate.alerts.delete.title" = "Discard";
"views.migrate.alerts.delete.message" = "Do you want to discard these profiles? You will not be able to recover them later.\n\n%@"; "views.migrate.alerts.delete.message" = "Do you want to discard these profiles? You will not be able to recover them later.\n\n%@";
"views.donate.title" = "Make a donation"; "views.donate.title" = "Make a donation";

View File

@ -101,7 +101,17 @@ extension AppContext {
profilesContainerName: Constants.shared.containers.legacyV2, profilesContainerName: Constants.shared.containers.legacyV2,
cloudKitIdentifier: BundleConfiguration.mainString(for: .legacyV2CloudKitId) cloudKitIdentifier: BundleConfiguration.mainString(for: .legacyV2CloudKitId)
) )
let migrationManager = MigrationManager(profileStrategy: profileStrategy) let migrationSimulation: MigrationManager.Simulation?
if Configuration.Environment.isFakeMigration {
migrationSimulation = MigrationManager.Simulation(
fakeProfiles: true,
maxMigrationTime: 3.0,
randomFailures: true
)
} else {
migrationSimulation = nil
}
let migrationManager = MigrationManager(profileStrategy: profileStrategy, simulation: migrationSimulation)
return AppContext( return AppContext(
iapManager: .sharedForApp, iapManager: .sharedForApp,

View File

@ -106,6 +106,10 @@ extension Configuration.Environment {
static var isFakeIAP: Bool { static var isFakeIAP: Bool {
ProcessInfo.processInfo.environment["PP_FAKE_IAP"] == "1" ProcessInfo.processInfo.environment["PP_FAKE_IAP"] == "1"
} }
static var isFakeMigration: Bool {
ProcessInfo.processInfo.environment["PP_FAKE_MIGRATION"] == "1"
}
} }
// MARK: ProfileManager // MARK: ProfileManager