mirror of
https://github.com/yattee/yattee.git
synced 2026-10-24 05:32:53 +00:00
Carry the login itself, and clean up after any failed read
Two things were conflated. Reloading the list bumped the generation, and the import treated that as its login having changed — so a routine refresh aborted a healthy import and told the user their account had moved. The listing and the login are separate facts: the login that listed a row is now carried with it, and the generation decides only what may be marked imported. The local playlist is also created before it is read, so anything stopping the read left it behind empty — a login that moved, but equally a server that did not answer. The import reports whether it completed, and an incomplete one takes its empty playlist with it. A playlist that genuinely had nothing to add keeps it.
This commit is contained in:
@@ -42,9 +42,10 @@ struct ImportPlaylistsView: View {
|
|||||||
// Merge warning state
|
// Merge warning state
|
||||||
@State private var showMergeWarning = false
|
@State private var showMergeWarning = false
|
||||||
@State private var pendingMergePlaylist: Playlist?
|
@State private var pendingMergePlaylist: Playlist?
|
||||||
/// The listing the pending merge was chosen from. The dialog waits on the
|
/// The listing and login the pending merge was chosen from. The dialog
|
||||||
/// user, so the list can be replaced before they answer it.
|
/// waits on the user, so both can be replaced before they answer it.
|
||||||
@State private var pendingMergeGeneration = 0
|
@State private var pendingMergeGeneration = 0
|
||||||
|
@State private var pendingMergeCredential = ""
|
||||||
@State private var existingLocalPlaylist: LocalPlaylist?
|
@State private var existingLocalPlaylist: LocalPlaylist?
|
||||||
|
|
||||||
// MARK: - Accessibility Identifiers
|
// MARK: - Accessibility Identifiers
|
||||||
@@ -108,8 +109,14 @@ struct ImportPlaylistsView: View {
|
|||||||
Button(String(localized: "import.playlists.mergeWarning.merge")) {
|
Button(String(localized: "import.playlists.mergeWarning.merge")) {
|
||||||
if let playlist = pendingMergePlaylist, let localPlaylist = existingLocalPlaylist {
|
if let playlist = pendingMergePlaylist, let localPlaylist = existingLocalPlaylist {
|
||||||
let generation = pendingMergeGeneration
|
let generation = pendingMergeGeneration
|
||||||
|
let credential = pendingMergeCredential
|
||||||
mergeImportTask = Task {
|
mergeImportTask = Task {
|
||||||
await performImport(playlist, into: localPlaylist, generation: generation)
|
await performImport(
|
||||||
|
playlist,
|
||||||
|
into: localPlaylist,
|
||||||
|
generation: generation,
|
||||||
|
credential: credential
|
||||||
|
)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -339,15 +346,16 @@ struct ImportPlaylistsView: View {
|
|||||||
private func addPlaylist(_ playlist: Playlist) async {
|
private func addPlaylist(_ playlist: Playlist) async {
|
||||||
guard let appEnvironment else { return }
|
guard let appEnvironment else { return }
|
||||||
|
|
||||||
// The listing this row came from, taken here rather than wherever the
|
// Both taken here rather than wherever the import eventually starts: a
|
||||||
// import eventually starts: a merge waits on the user, and by then the
|
// merge waits on the user, and by then the list may have been replaced
|
||||||
// list may have been replaced.
|
// and its credential re-recorded.
|
||||||
let generation = listGeneration
|
let generation = listGeneration
|
||||||
|
|
||||||
// Checked before anything is created: a playlist made for a login that
|
// Checked before anything is created: a playlist made for a login that
|
||||||
// has already gone would be left behind empty, because the import
|
// has already gone would be left behind empty, because the import
|
||||||
// aborts on its first write.
|
// aborts on its first write.
|
||||||
guard appEnvironment.credential(forInstance: instance) == loadedCredential else {
|
guard let credential = appEnvironment.credential(forInstance: instance),
|
||||||
|
credential == loadedCredential else {
|
||||||
loginChangedDuringImport = true
|
loginChangedDuringImport = true
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -358,6 +366,7 @@ struct ImportPlaylistsView: View {
|
|||||||
// Show merge warning
|
// Show merge warning
|
||||||
pendingMergePlaylist = playlist
|
pendingMergePlaylist = playlist
|
||||||
pendingMergeGeneration = generation
|
pendingMergeGeneration = generation
|
||||||
|
pendingMergeCredential = credential
|
||||||
existingLocalPlaylist = existing
|
existingLocalPlaylist = existing
|
||||||
importingPlaylistID = playlist.id.playlistID
|
importingPlaylistID = playlist.id.playlistID
|
||||||
showMergeWarning = true
|
showMergeWarning = true
|
||||||
@@ -366,34 +375,44 @@ struct ImportPlaylistsView: View {
|
|||||||
|
|
||||||
// Create new local playlist and import
|
// Create new local playlist and import
|
||||||
let localPlaylist = appEnvironment.dataManager.createPlaylist(title: playlist.title)
|
let localPlaylist = appEnvironment.dataManager.createPlaylist(title: playlist.title)
|
||||||
await performImport(playlist, into: localPlaylist, generation: generation)
|
let imported = await performImport(
|
||||||
|
playlist,
|
||||||
|
into: localPlaylist,
|
||||||
|
generation: generation,
|
||||||
|
credential: credential
|
||||||
|
)
|
||||||
|
|
||||||
// The guard above and the one inside are a suspension apart, so the
|
// It is created before it is read, so anything that stops the read —
|
||||||
// login can still go in between. Nothing was read, and an empty
|
// a login that moved, but equally a server that did not answer —
|
||||||
// playlist named after somebody else's is not worth keeping.
|
// leaves it behind with nothing in it. An import that genuinely had
|
||||||
if loginChangedDuringImport, localPlaylist.items?.isEmpty ?? true {
|
// nothing to add keeps its playlist; a failed one does not.
|
||||||
|
if !imported, localPlaylist.items?.isEmpty ?? true {
|
||||||
appEnvironment.dataManager.deletePlaylist(localPlaylist)
|
appEnvironment.dataManager.deletePlaylist(localPlaylist)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// - Parameter generation: the listing `playlist` was chosen from, taken at
|
/// - Parameters:
|
||||||
/// selection time. Reading it under a later one would fetch whatever that
|
/// - generation: the listing `playlist` was chosen from, taken at
|
||||||
/// ID happens to name to whoever replaced the login.
|
/// selection time. It decides only what may be marked imported — a
|
||||||
|
/// reload replaces the rows, not the login.
|
||||||
|
/// - credential: the login that listed `playlist`, taken at selection
|
||||||
|
/// time. Reading it under any other would fetch whatever that ID
|
||||||
|
/// happens to name to somebody else.
|
||||||
|
/// - Returns: whether the playlist was read and written in full.
|
||||||
|
@discardableResult
|
||||||
private func performImport(
|
private func performImport(
|
||||||
_ playlist: Playlist,
|
_ playlist: Playlist,
|
||||||
into localPlaylist: LocalPlaylist,
|
into localPlaylist: LocalPlaylist,
|
||||||
generation: Int
|
generation: Int,
|
||||||
) async {
|
credential: String
|
||||||
guard let appEnvironment else { return }
|
) async -> Bool {
|
||||||
|
guard let appEnvironment else { return false }
|
||||||
|
|
||||||
// The login can go, or be replaced, between choosing this playlist and
|
// The login can go, or be replaced, between choosing this playlist and
|
||||||
// reading it — the merge dialog in particular waits on the user.
|
// reading it — the merge dialog in particular waits on the user.
|
||||||
// Returning quietly would leave the row importing forever and let a
|
// Returning quietly would leave the row importing forever and let a
|
||||||
// batch carry on; reading it under a replacement would fetch whatever
|
// batch carry on.
|
||||||
// that ID means to somebody else.
|
guard appEnvironment.credential(forInstance: instance) == credential else {
|
||||||
guard generation == listGeneration,
|
|
||||||
let credential = appEnvironment.credential(forInstance: instance),
|
|
||||||
credential == loadedCredential else {
|
|
||||||
loginChangedDuringImport = true
|
loginChangedDuringImport = true
|
||||||
importingPlaylistID = nil
|
importingPlaylistID = nil
|
||||||
importProgress = nil
|
importProgress = nil
|
||||||
@@ -403,7 +422,7 @@ struct ImportPlaylistsView: View {
|
|||||||
String(localized: "import.playlists.failed.title"),
|
String(localized: "import.playlists.failed.title"),
|
||||||
subtitle: ImportError.loginChanged.localizedDescription
|
subtitle: ImportError.loginChanged.localizedDescription
|
||||||
)
|
)
|
||||||
return
|
return false
|
||||||
}
|
}
|
||||||
|
|
||||||
importingPlaylistID = playlist.id.playlistID
|
importingPlaylistID = playlist.id.playlistID
|
||||||
@@ -454,9 +473,7 @@ struct ImportPlaylistsView: View {
|
|||||||
// write rather than only at the start. What has already been
|
// write rather than only at the start. What has already been
|
||||||
// written stays — it was imported under the login that asked
|
// written stays — it was imported under the login that asked
|
||||||
// for it.
|
// for it.
|
||||||
guard generation == listGeneration,
|
guard appEnvironment.credential(forInstance: instance) == credential else {
|
||||||
appEnvironment.credential(forInstance: instance) == credential,
|
|
||||||
credential == loadedCredential else {
|
|
||||||
throw ImportError.loginChanged
|
throw ImportError.loginChanged
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -495,6 +512,8 @@ struct ImportPlaylistsView: View {
|
|||||||
appEnvironment.toastManager.showSuccess(String(localized: "import.playlists.added.title"))
|
appEnvironment.toastManager.showSuccess(String(localized: "import.playlists.added.title"))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
return true
|
||||||
} catch {
|
} catch {
|
||||||
await MainActor.run {
|
await MainActor.run {
|
||||||
if error as? ImportError == .loginChanged {
|
if error as? ImportError == .loginChanged {
|
||||||
@@ -507,6 +526,8 @@ struct ImportPlaylistsView: View {
|
|||||||
subtitle: error.localizedDescription
|
subtitle: error.localizedDescription
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
return false
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user