mirror of
https://github.com/yattee/yattee.git
synced 2026-11-03 10:27:19 +00:00
Let an untouched Save keep the name it was already showing
This commit is contained in:
@@ -230,8 +230,16 @@ final class AccountsManager {
|
|||||||
pendingMarkerUploads.insert(instanceID)
|
pendingMarkerUploads.insert(instanceID)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Stores the account's user-set label, or clears it with `nil`.
|
||||||
|
///
|
||||||
|
/// A save that changes nothing is dropped rather than written. The rename
|
||||||
|
/// field is prefilled, so "Save" without an edit is the ordinary way out of
|
||||||
|
/// the alert — and every write here stamps `updatedAt` and queues an
|
||||||
|
/// upload, which under last-write-wins would let that idle tap overwrite a
|
||||||
|
/// peer's genuine rename that has not reached this device yet.
|
||||||
func updateDisplayName(_ displayName: String?, for accountID: UUID) {
|
func updateDisplayName(_ displayName: String?, for accountID: UUID) {
|
||||||
guard let index = accounts.firstIndex(where: { $0.id == accountID }) else { return }
|
guard let index = accounts.firstIndex(where: { $0.id == accountID }) else { return }
|
||||||
|
guard accounts[index].displayName != displayName else { return }
|
||||||
accounts[index].displayName = displayName
|
accounts[index].displayName = displayName
|
||||||
accounts[index].updatedAt = Date()
|
accounts[index].updatedAt = Date()
|
||||||
pendingUploads.insert(accountID)
|
pendingUploads.insert(accountID)
|
||||||
|
|||||||
@@ -239,14 +239,32 @@ final class DataContextValidator {
|
|||||||
/// What a rename field's contents mean once the user is done typing.
|
/// What a rename field's contents mean once the user is done typing.
|
||||||
///
|
///
|
||||||
/// `nil` clears the user-set label and hands the account back to
|
/// `nil` clears the user-set label and hands the account back to
|
||||||
/// `defaultDisplayName`. An empty field says that outright — and so does
|
/// `defaultDisplayName`; anything else is stored as typed.
|
||||||
/// typing exactly the name the account already falls back to. The field
|
///
|
||||||
/// starts prefilled with the label the row is showing, so saving it
|
/// The question the field cannot answer on its own is what an *untouched*
|
||||||
/// untouched must not turn a *device-local* fallback into a synced label
|
/// save means. The field starts prefilled with the label the row shows, and
|
||||||
/// that every other device is then made to display.
|
/// on an account with no user-set label that label is device-local — the
|
||||||
static func renamedDisplayName(_ input: String, fallingBackTo defaultName: String) -> String? {
|
/// login identifier only exists where the credential does. Saving it
|
||||||
|
/// unedited must not turn one device's fallback into a synced label every
|
||||||
|
/// other device is then made to display, so that case clears instead.
|
||||||
|
///
|
||||||
|
/// The test is editedness, never equality with the default: on a second
|
||||||
|
/// device the string that device happens to fall back to is a perfectly
|
||||||
|
/// ordinary name to *choose*, and typing it has to store and sync like any
|
||||||
|
/// other. An unedited save on an account that already carries a label
|
||||||
|
/// re-states that label, which the manager then drops as a no-op — the name
|
||||||
|
/// stays, rather than being silently cleared by a tap that changed nothing.
|
||||||
|
static func renamedDisplayName(
|
||||||
|
_ input: String,
|
||||||
|
prefilledWith prefill: String,
|
||||||
|
hasUserSetLabel: Bool
|
||||||
|
) -> String? {
|
||||||
let trimmed = input.trimmingCharacters(in: .whitespacesAndNewlines)
|
let trimmed = input.trimmingCharacters(in: .whitespacesAndNewlines)
|
||||||
guard !trimmed.isEmpty, trimmed != defaultName else { return nil }
|
guard !trimmed.isEmpty else { return nil }
|
||||||
|
|
||||||
|
let wasEdited = trimmed != prefill.trimmingCharacters(in: .whitespacesAndNewlines)
|
||||||
|
guard wasEdited || hasUserSetLabel else { return nil }
|
||||||
|
|
||||||
return trimmed
|
return trimmed
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -22,6 +22,51 @@ struct AccountRenameRequest: Identifiable {
|
|||||||
|
|
||||||
/// What the field is prefilled with: the label the row is showing now.
|
/// What the field is prefilled with: the label the row is showing now.
|
||||||
let initialText: String
|
let initialText: String
|
||||||
|
|
||||||
|
/// Whether that prefill is a name the user chose, as opposed to the label
|
||||||
|
/// the account falls back to on this device. It is what tells an untouched
|
||||||
|
/// save to keep the name from one that must not publish a device-local
|
||||||
|
/// fallback — see `DataContextValidator.renamedDisplayName`.
|
||||||
|
let hasUserSetLabel: Bool
|
||||||
|
}
|
||||||
|
|
||||||
|
extension AccountRenameRequest {
|
||||||
|
/// The request for an account that still resolves through a source.
|
||||||
|
///
|
||||||
|
/// The single place the alert's strings are derived, so both surfaces that
|
||||||
|
/// offer Rename ask the validator the same two questions: what the row is
|
||||||
|
/// showing, and what clearing the field goes back to.
|
||||||
|
@MainActor
|
||||||
|
static func forAccount(_ account: Account, resolvedBy validator: DataContextValidator) -> Self {
|
||||||
|
Self(
|
||||||
|
id: account.id,
|
||||||
|
defaultName: validator.defaultDisplayName(for: account),
|
||||||
|
initialText: validator.displayName(for: account),
|
||||||
|
hasUserSetLabel: account.hasUserSetDisplayName
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The request for an account with no source to resolve through.
|
||||||
|
///
|
||||||
|
/// The fallback is the caller's to name: the validator's last resort for an
|
||||||
|
/// account whose instance is gone is the words "Source missing", which is a
|
||||||
|
/// diagnosis and not a name to offer the field.
|
||||||
|
static func forAccount(_ account: Account, fallingBackTo defaultName: String) -> Self {
|
||||||
|
Self(
|
||||||
|
id: account.id,
|
||||||
|
defaultName: defaultName,
|
||||||
|
initialText: account.displayName ?? "",
|
||||||
|
hasUserSetLabel: account.hasUserSetDisplayName
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
extension Account {
|
||||||
|
/// Whether the account carries a label the user chose. An empty string is
|
||||||
|
/// no more a name than `nil` is, and both arrive here from old records.
|
||||||
|
var hasUserSetDisplayName: Bool {
|
||||||
|
!(displayName ?? "").isEmpty
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The rename alert, shared by every screen that lists accounts.
|
/// The rename alert, shared by every screen that lists accounts.
|
||||||
@@ -40,8 +85,14 @@ private struct AccountRenameAlertModifier: ViewModifier {
|
|||||||
// Keyed on the ID rather than on the request: the prefill has to be
|
// Keyed on the ID rather than on the request: the prefill has to be
|
||||||
// in place before the alert's field is built, and re-entering the
|
// in place before the alert's field is built, and re-entering the
|
||||||
// same row is the only time the text should be reset.
|
// same row is the only time the text should be reset.
|
||||||
|
//
|
||||||
|
// Seeded only when a request appears, never blanked when one goes
|
||||||
|
// away. Dismissal sets the request to nil while the alert's buttons
|
||||||
|
// are still live, and Save reads this text when it runs — blanking
|
||||||
|
// here would let the two race, with an emptied field committing as
|
||||||
|
// "clear the name".
|
||||||
.onChange(of: request?.id) { _, _ in
|
.onChange(of: request?.id) { _, _ in
|
||||||
text = request?.initialText ?? ""
|
if let request { text = request.initialText }
|
||||||
}
|
}
|
||||||
.alert(
|
.alert(
|
||||||
String(localized: "accounts.rename.title"),
|
String(localized: "accounts.rename.title"),
|
||||||
@@ -56,7 +107,11 @@ private struct AccountRenameAlertModifier: ViewModifier {
|
|||||||
Button(String(localized: "common.save")) {
|
Button(String(localized: "common.save")) {
|
||||||
onCommit(
|
onCommit(
|
||||||
pending.id,
|
pending.id,
|
||||||
DataContextValidator.renamedDisplayName(text, fallingBackTo: pending.defaultName)
|
DataContextValidator.renamedDisplayName(
|
||||||
|
text,
|
||||||
|
prefilledWith: pending.initialText,
|
||||||
|
hasUserSetLabel: pending.hasUserSetLabel
|
||||||
|
)
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -349,18 +349,10 @@ struct AccountsListView: View {
|
|||||||
/// prefill a field with.
|
/// prefill a field with.
|
||||||
private func makeRenameRequest(for account: Account, instance: Instance?) -> AccountRenameRequest {
|
private func makeRenameRequest(for account: Account, instance: Instance?) -> AccountRenameRequest {
|
||||||
guard instance != nil, let validator = appEnvironment?.dataContextValidator else {
|
guard instance != nil, let validator = appEnvironment?.dataContextValidator else {
|
||||||
return AccountRenameRequest(
|
return .forAccount(account, fallingBackTo: String(localized: "accounts.orphan.unnamed"))
|
||||||
id: account.id,
|
|
||||||
defaultName: String(localized: "accounts.orphan.unnamed"),
|
|
||||||
initialText: account.displayName ?? ""
|
|
||||||
)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
return AccountRenameRequest(
|
return .forAccount(account, resolvedBy: validator)
|
||||||
id: account.id,
|
|
||||||
defaultName: validator.defaultDisplayName(for: account),
|
|
||||||
initialText: validator.displayName(for: account)
|
|
||||||
)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private func status(for account: Account) -> AccountStatus {
|
private func status(for account: Account) -> AccountStatus {
|
||||||
|
|||||||
@@ -195,13 +195,16 @@ struct InstanceAccountsSection: View {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// What the rename field starts on, and what clearing it goes back to.
|
/// What the rename field starts on, and what clearing it goes back to.
|
||||||
|
///
|
||||||
|
/// The section always has its instance, so the only reason to name the
|
||||||
|
/// fallback here is a missing environment, where the row's own label is
|
||||||
|
/// already the instance's.
|
||||||
private func makeRenameRequest(for account: Account) -> AccountRenameRequest {
|
private func makeRenameRequest(for account: Account) -> AccountRenameRequest {
|
||||||
AccountRenameRequest(
|
guard let validator = appEnvironment?.dataContextValidator else {
|
||||||
id: account.id,
|
return .forAccount(account, fallingBackTo: instance.displayName)
|
||||||
defaultName: appEnvironment?.dataContextValidator.defaultDisplayName(for: account)
|
}
|
||||||
?? instance.displayName,
|
|
||||||
initialText: displayName(for: account)
|
return .forAccount(account, resolvedBy: validator)
|
||||||
)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private func status(for account: Account) -> AccountStatus {
|
private func status(for account: Account) -> AccountStatus {
|
||||||
|
|||||||
@@ -490,29 +490,166 @@ struct AccountRenameTests {
|
|||||||
@Test("A typed name replaces the fallback")
|
@Test("A typed name replaces the fallback")
|
||||||
func typedNameIsStored() {
|
func typedNameIsStored() {
|
||||||
#expect(
|
#expect(
|
||||||
DataContextValidator.renamedDisplayName(" Main i01 ", fallingBackTo: "i01") == "Main i01"
|
DataContextValidator.renamedDisplayName(
|
||||||
|
" Main i01 ",
|
||||||
|
prefilledWith: "i01",
|
||||||
|
hasUserSetLabel: false
|
||||||
|
) == "Main i01"
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test("An empty field clears the label")
|
@Test("An empty field clears the label")
|
||||||
func emptyClearsTheLabel() {
|
func emptyClearsTheLabel() {
|
||||||
#expect(DataContextValidator.renamedDisplayName("", fallingBackTo: "i01") == nil)
|
for hasLabel in [true, false] {
|
||||||
#expect(DataContextValidator.renamedDisplayName(" \n ", fallingBackTo: "i01") == nil)
|
#expect(
|
||||||
|
DataContextValidator.renamedDisplayName(
|
||||||
|
"",
|
||||||
|
prefilledWith: "i01",
|
||||||
|
hasUserSetLabel: hasLabel
|
||||||
|
) == nil
|
||||||
|
)
|
||||||
|
#expect(
|
||||||
|
DataContextValidator.renamedDisplayName(
|
||||||
|
" \n ",
|
||||||
|
prefilledWith: "Main i01",
|
||||||
|
hasUserSetLabel: hasLabel
|
||||||
|
) == nil
|
||||||
|
)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test("Saving the prefilled fallback untouched sets no name at all")
|
@Test("Saving an unnamed account's prefill untouched sets no name at all")
|
||||||
func unchangedFallbackIsNotStored() {
|
func unchangedFallbackIsNotStored() {
|
||||||
// The field starts on the label the row is showing, and that label can
|
// The field starts on the label the row is showing, and on an account
|
||||||
// be device-local — the login identifier only exists where the
|
// with no user-set label that is device-local — the login identifier
|
||||||
// credential does. Storing it would publish one device's fallback to
|
// only exists where the credential does. Storing it would publish one
|
||||||
// every other device as if the user had chosen it.
|
// device's fallback to every other device as if the user had chosen it.
|
||||||
#expect(DataContextValidator.renamedDisplayName("i01", fallingBackTo: "i01") == nil)
|
|
||||||
#expect(
|
#expect(
|
||||||
DataContextValidator.renamedDisplayName(
|
DataContextValidator.renamedDisplayName(
|
||||||
"utest1@i01.v.yattee.stream",
|
"i01",
|
||||||
fallingBackTo: "utest1@i01.v.yattee.stream"
|
prefilledWith: "i01",
|
||||||
|
hasUserSetLabel: false
|
||||||
) == nil
|
) == nil
|
||||||
)
|
)
|
||||||
|
#expect(
|
||||||
|
DataContextValidator.renamedDisplayName(
|
||||||
|
" utest1@i01.v.yattee.stream ",
|
||||||
|
prefilledWith: "utest1@i01.v.yattee.stream",
|
||||||
|
hasUserSetLabel: false
|
||||||
|
) == nil
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test("Typing a name that happens to match this device's fallback still stores it")
|
||||||
|
func editedNameEqualToTheDefaultIsStored() {
|
||||||
|
// The rule is editedness, not equality with the default. On a second
|
||||||
|
// device the string that device falls back to is an ordinary name to
|
||||||
|
// choose — and the prefill there is the *synced* label, so typing over
|
||||||
|
// it is a real edit that has to store and sync like any other.
|
||||||
|
#expect(
|
||||||
|
DataContextValidator.renamedDisplayName(
|
||||||
|
"i01",
|
||||||
|
prefilledWith: "Main i01",
|
||||||
|
hasUserSetLabel: true
|
||||||
|
) == "i01"
|
||||||
|
)
|
||||||
|
// Same on an account with no label yet: the prefill was the fallback
|
||||||
|
// "utest1@…", and the user replaced it with something else entirely.
|
||||||
|
#expect(
|
||||||
|
DataContextValidator.renamedDisplayName(
|
||||||
|
"i01",
|
||||||
|
prefilledWith: "utest1@i01.v.yattee.stream",
|
||||||
|
hasUserSetLabel: false
|
||||||
|
) == "i01"
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test("Saving a named account untouched keeps its name rather than clearing it")
|
||||||
|
func unchangedNameIsKept() {
|
||||||
|
// Decided semantics for "Save with no edit" on an account that already
|
||||||
|
// carries a label: re-state the label. The manager drops the write as a
|
||||||
|
// no-op, so the name survives and nothing is queued — an idle tap must
|
||||||
|
// never read as "clear the name".
|
||||||
|
#expect(
|
||||||
|
DataContextValidator.renamedDisplayName(
|
||||||
|
"Main i01",
|
||||||
|
prefilledWith: "Main i01",
|
||||||
|
hasUserSetLabel: true
|
||||||
|
) == "Main i01"
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test("Both rename surfaces offer the shown label and fall back to the default")
|
||||||
|
func renameRequestIsBuiltFromTheValidator() {
|
||||||
|
let (accounts, instances, validator) = makeFixture()
|
||||||
|
var instance = Instance(type: .invidious, url: URL(string: "https://inv.example.com")!)
|
||||||
|
instance.name = "My Invidious"
|
||||||
|
instances.add(instance)
|
||||||
|
|
||||||
|
let account = accounts.addAccount(instanceID: instance.id)
|
||||||
|
|
||||||
|
let unnamed = AccountRenameRequest.forAccount(account, resolvedBy: validator)
|
||||||
|
#expect(unnamed.id == account.id)
|
||||||
|
#expect(unnamed.defaultName == validator.defaultDisplayName(for: account))
|
||||||
|
#expect(unnamed.initialText == validator.displayName(for: account))
|
||||||
|
#expect(unnamed.hasUserSetLabel == false)
|
||||||
|
|
||||||
|
accounts.updateDisplayName("Main i01", for: account.id)
|
||||||
|
let renamed = accounts.account(id: account.id)!
|
||||||
|
|
||||||
|
let named = AccountRenameRequest.forAccount(renamed, resolvedBy: validator)
|
||||||
|
// The placeholder stays the *default*, never the label being edited:
|
||||||
|
// clearing the field has to advertise where it lands.
|
||||||
|
#expect(named.defaultName == "My Invidious")
|
||||||
|
#expect(named.initialText == "Main i01")
|
||||||
|
#expect(named.hasUserSetLabel)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test("An orphan's rename request offers only its own label")
|
||||||
|
func orphanRenameRequestSkipsTheValidator() {
|
||||||
|
let (accounts, instances, _) = makeFixture()
|
||||||
|
let instance = Instance(type: .invidious, url: URL(string: "https://inv.example.com")!)
|
||||||
|
instances.add(instance)
|
||||||
|
|
||||||
|
let account = accounts.addAccount(instanceID: instance.id)
|
||||||
|
let unnamed = AccountRenameRequest.forAccount(account, fallingBackTo: "Unnamed")
|
||||||
|
#expect(unnamed.defaultName == "Unnamed")
|
||||||
|
#expect(unnamed.initialText.isEmpty)
|
||||||
|
#expect(unnamed.hasUserSetLabel == false)
|
||||||
|
|
||||||
|
accounts.updateDisplayName("Abandoned login", for: account.id)
|
||||||
|
let named = AccountRenameRequest.forAccount(
|
||||||
|
accounts.account(id: account.id)!,
|
||||||
|
fallingBackTo: "Unnamed"
|
||||||
|
)
|
||||||
|
#expect(named.initialText == "Abandoned login")
|
||||||
|
#expect(named.hasUserSetLabel)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test("Re-saving the same name writes nothing")
|
||||||
|
func noOpSaveIsNotWritten() {
|
||||||
|
let (accounts, instances, _) = makeFixture()
|
||||||
|
let instance = Instance(type: .invidious, url: URL(string: "https://inv.example.com")!)
|
||||||
|
instances.add(instance)
|
||||||
|
|
||||||
|
let account = accounts.addAccount(instanceID: instance.id)
|
||||||
|
|
||||||
|
// An untouched save on an account with no label: nil over nil.
|
||||||
|
accounts.confirmUpload(account.id)
|
||||||
|
let untouchedAt = accounts.account(id: account.id)?.updatedAt
|
||||||
|
accounts.updateDisplayName(nil, for: account.id)
|
||||||
|
#expect(!accounts.pendingUploads.contains(account.id))
|
||||||
|
#expect(accounts.account(id: account.id)?.updatedAt == untouchedAt)
|
||||||
|
|
||||||
|
accounts.updateDisplayName("Main i01", for: account.id)
|
||||||
|
accounts.confirmUpload(account.id)
|
||||||
|
let renamedAt = accounts.account(id: account.id)?.updatedAt
|
||||||
|
|
||||||
|
// And on one that carries a label: the same string over itself.
|
||||||
|
accounts.updateDisplayName("Main i01", for: account.id)
|
||||||
|
#expect(!accounts.pendingUploads.contains(account.id))
|
||||||
|
#expect(accounts.account(id: account.id)?.updatedAt == renamedAt)
|
||||||
|
#expect(accounts.account(id: account.id)?.displayName == "Main i01")
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test("The default name ignores the user-set one, and the shown name prefers it")
|
@Test("The default name ignores the user-set one, and the shown name prefers it")
|
||||||
|
|||||||
Reference in New Issue
Block a user