From cabe584938dbacdd358099564a72479cd4195b13 Mon Sep 17 00:00:00 2001 From: Bryan Montz Date: Tue, 14 Mar 2023 12:08:18 -0500 Subject: [PATCH] fix "Replying to..." issues and improve related tests --- damus/Nostr/Nostr.swift | 9 ++- damus/Views/Events/ReplyDescription.swift | 10 +-- damusTests/ReplyDescriptionTests.swift | 76 ++++++++++------------- 3 files changed, 46 insertions(+), 49 deletions(-) diff --git a/damus/Nostr/Nostr.swift b/damus/Nostr/Nostr.swift index e068d27b..3d909ec0 100644 --- a/damus/Nostr/Nostr.swift +++ b/damus/Nostr/Nostr.swift @@ -144,8 +144,13 @@ struct Profile: Codable { if pubkey == "anon" { return "Anonymous" } - let pk = bech32_nopre_pubkey(pubkey) ?? pubkey - return profile?.name ?? abbrev_pubkey(pk) + + if let name = profile?.name, !name.isEmpty { + return name + } else { + let pk = bech32_nopre_pubkey(pubkey) ?? pubkey + return abbrev_pubkey(pk) + } } } diff --git a/damus/Views/Events/ReplyDescription.swift b/damus/Views/Events/ReplyDescription.swift index fa8f471a..69447c1e 100644 --- a/damus/Views/Events/ReplyDescription.swift +++ b/damus/Views/Events/ReplyDescription.swift @@ -41,17 +41,19 @@ func reply_desc(profiles: Profiles, event: NostrEvent, locale: Locale = Locale.c let prof = profiles.lookup(id: $0) return Profile.displayName(profile: prof, pubkey: $0) } + + let uniqueNames = NSOrderedSet(array: names).array as! [String] - if names.count > 1 { + if uniqueNames.count > 1 { let othersCount = n - pubkeys.count if othersCount == 0 { - return String(format: NSLocalizedString("Replying to %@ & %@", bundle: bundle, comment: "Label to indicate that the user is replying to 2 users."), locale: locale, names[0], names[1]) + return String(format: NSLocalizedString("Replying to %@ & %@", bundle: bundle, comment: "Label to indicate that the user is replying to 2 users."), locale: locale, uniqueNames[0], uniqueNames[1]) } else { - return String(format: bundle.localizedString(forKey: "replying_to_two_and_others", value: nil, table: nil), locale: locale, othersCount, names[0], names[1]) + return String(format: bundle.localizedString(forKey: "replying_to_two_and_others", value: nil, table: nil), locale: locale, othersCount, uniqueNames[0], uniqueNames[1]) } } - return String(format: NSLocalizedString("Replying to %@", bundle: bundle, comment: "Label to indicate that the user is replying to 1 user."), locale: locale, names[0]) + return String(format: NSLocalizedString("Replying to %@", bundle: bundle, comment: "Label to indicate that the user is replying to 1 user."), locale: locale, uniqueNames[0]) } diff --git a/damusTests/ReplyDescriptionTests.swift b/damusTests/ReplyDescriptionTests.swift index 4dbf226d..4342f4e8 100644 --- a/damusTests/ReplyDescriptionTests.swift +++ b/damusTests/ReplyDescriptionTests.swift @@ -11,57 +11,47 @@ import XCTest final class ReplyDescriptionTests: XCTestCase { let enUsLocale = Locale(identifier: "en-US") - - override func setUpWithError() throws { - // Put setup code here. This method is called before the invocation of each test method in the class. + let profiles = test_damus_state().profiles + + private func descriptionForEvent(withTags tags: [[String]]) -> String { + var allTags = [["e", "123"]] + allTags.append(contentsOf: tags) + let replyingToOne = NostrEvent( + content: "hello there https://jb55.com/s/Oct12-150217.png https://jb55.com/red-me.jpg cool", + pubkey: "pk", + tags: allTags, + createdAt: Int64(Date().timeIntervalSince1970 - 100) + ) + + Bundle.main.localizations.map { Locale(identifier: $0) }.forEach { + XCTAssertNoThrow(reply_desc(profiles: profiles, event: replyingToOne, locale: $0)) + } + return reply_desc(profiles: profiles, event: replyingToOne, locale: enUsLocale) } - - override func tearDownWithError() throws { - // Put teardown code here. This method is called after the invocation of each test method in the class. - } - + // Test that English strings work properly with argument substitution and pluralization, and that other locales don't crash. func testReplyDesc() throws { - let profiles = test_damus_state().profiles - let replyingToSelfEvent = test_event XCTAssertEqual(reply_desc(profiles: profiles, event: replyingToSelfEvent, locale: enUsLocale), "Replying to self") Bundle.main.localizations.map { Locale(identifier: $0) }.forEach { XCTAssertNoThrow(reply_desc(profiles: profiles, event: replyingToSelfEvent, locale: $0)) } - - let replyingToOne = NostrEvent( - content: "hello there https://jb55.com/s/Oct12-150217.png https://jb55.com/red-me.jpg cool", - pubkey: "pk", - tags: [["e", "123"], ["p", "123"]], - createdAt: Int64(Date().timeIntervalSince1970 - 100) - ) - XCTAssertEqual(reply_desc(profiles: profiles, event: replyingToOne, locale: enUsLocale), "Replying to \(Profile.displayName(profile: nil, pubkey: "123"))") - Bundle.main.localizations.map { Locale(identifier: $0) }.forEach { - XCTAssertNoThrow(reply_desc(profiles: profiles, event: replyingToOne, locale: $0)) - } - - let replyingToTwo = NostrEvent( - content: "hello there https://jb55.com/s/Oct12-150217.png https://jb55.com/red-me.jpg cool", - pubkey: "pk", - tags: [["e", "123"], ["p", "123"], ["p", "456"]], - createdAt: Int64(Date().timeIntervalSince1970 - 100) - ) - XCTAssertEqual(reply_desc(profiles: profiles, event: replyingToTwo, locale: enUsLocale), "Replying to \(Profile.displayName(profile: nil, pubkey: "456")) & \(Profile.displayName(profile: nil, pubkey: "123"))") - Bundle.main.localizations.map { Locale(identifier: $0) }.forEach { - XCTAssertNoThrow(reply_desc(profiles: profiles, event: replyingToTwo, locale: $0)) - } - - let replyingToTwoAndOneOther = NostrEvent( - content: "hello there https://jb55.com/s/Oct12-150217.png https://jb55.com/red-me.jpg cool", - pubkey: "pk", - tags: [["e", "123"], ["p", "123"], ["p", "456"], ["p", "789"]], - createdAt: Int64(Date().timeIntervalSince1970 - 100) - ) - XCTAssertEqual(reply_desc(profiles: profiles, event: replyingToTwoAndOneOther, locale: enUsLocale), "Replying to \(Profile.displayName(profile: nil, pubkey: "789")), \(Profile.displayName(profile: nil, pubkey: "456")) & 1 other") - Bundle.main.localizations.map { Locale(identifier: $0) }.forEach { - XCTAssertNoThrow(reply_desc(profiles: profiles, event: replyingToTwoAndOneOther, locale: $0)) - } + + // replying to one + XCTAssertEqual(descriptionForEvent(withTags: [["p", "123"]]), + "Replying to \(Profile.displayName(profile: nil, pubkey: "123"))") + + // replying to two + XCTAssertEqual(descriptionForEvent(withTags: [["p", "123"], ["p", "456"]]), + "Replying to \(Profile.displayName(profile: nil, pubkey: "456")) & \(Profile.displayName(profile: nil, pubkey: "123"))") + + // replying to two that are the same + XCTAssertEqual(descriptionForEvent(withTags: [["p", "123"], ["p", "123"]]), + "Replying to \(Profile.displayName(profile: nil, pubkey: "123"))") + + // replying to two and one other + XCTAssertEqual(descriptionForEvent(withTags: [["p", "123"], ["p", "456"], ["p", "789"]]), + "Replying to \(Profile.displayName(profile: nil, pubkey: "789")), \(Profile.displayName(profile: nil, pubkey: "456")) & 1 other") for othersCount in 2...10 { var tags: [[String]] = [["e", "123"]]