From 5899117bb2c955aeaf753a54375eceaec7a45dd6 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Fri, 8 May 2026 11:01:30 +1200 Subject: [PATCH 1/2] Format files in preparation for tag-selection bugfix --- .../Features/Posts/TagSelectionTests.swift | 32 ++++++++++++++----- .../ViewRelated/Tags/TagsViewModel.swift | 31 ++++++++++++++---- 2 files changed, 48 insertions(+), 15 deletions(-) diff --git a/Tests/KeystoneTests/Tests/Features/Posts/TagSelectionTests.swift b/Tests/KeystoneTests/Tests/Features/Posts/TagSelectionTests.swift index a9458f9db47a..251224b1f989 100644 --- a/Tests/KeystoneTests/Tests/Features/Posts/TagSelectionTests.swift +++ b/Tests/KeystoneTests/Tests/Features/Posts/TagSelectionTests.swift @@ -117,9 +117,13 @@ struct TagSelectionTests { func selectionCallbackFiltersOutPendingItems() async { let mock = MockService(tags: ["Foo", "Bar"]) var callbackTags: [TagsViewModel.SelectedTerm] = [] - let viewModel = TagsViewModel(taxonomy: nil, service: mock, mode: .selection(onSelectedTagsChanged: { tags in - callbackTags = tags - })) + let viewModel = TagsViewModel( + taxonomy: nil, + service: mock, + mode: .selection(onSelectedTagsChanged: { tags in + callbackTags = tags + }) + ) _ = viewModel.addNewTag(named: "Baz") @@ -132,9 +136,13 @@ struct TagSelectionTests { let mock = MockService(tags: ["Foo", "Bar"]) let tags = await mock.tags var callbackTags: [TagsViewModel.SelectedTerm] = [] - let viewModel = TagsViewModel(taxonomy: nil, service: mock, mode: .selection(onSelectedTagsChanged: { tags in - callbackTags = tags - })) + let viewModel = TagsViewModel( + taxonomy: nil, + service: mock, + mode: .selection(onSelectedTagsChanged: { tags in + callbackTags = tags + }) + ) viewModel.toggleSelection(for: tags[0]) @@ -217,7 +225,11 @@ private actor MockService: TaxonomyServiceProtocol { let lowercasedName = name.lowercased() if tags.contains(where: { $0.name.lowercased() == lowercasedName }) { - let error = NSError(domain: "MockService", code: -1, userInfo: [NSLocalizedDescriptionKey: "Tag already exists"]) + let error = NSError( + domain: "MockService", + code: -1, + userInfo: [NSLocalizedDescriptionKey: "Tag already exists"] + ) throw error } @@ -235,7 +247,11 @@ private actor MockService: TaxonomyServiceProtocol { return newTag } - func updateTag(_ term: AnyTermWithViewContext, name: String, description: String) async throws -> AnyTermWithViewContext { + func updateTag( + _ term: AnyTermWithViewContext, + name: String, + description: String + ) async throws -> AnyTermWithViewContext { guard let index = tags.firstIndex(where: { $0.id == term.id }) else { let error = NSError(domain: "MockService", code: -2, userInfo: [NSLocalizedDescriptionKey: "Tag not found"]) throw error diff --git a/WordPress/Classes/ViewRelated/Tags/TagsViewModel.swift b/WordPress/Classes/ViewRelated/Tags/TagsViewModel.swift index 7d6303e2b684..6ae6e59b3783 100644 --- a/WordPress/Classes/ViewRelated/Tags/TagsViewModel.swift +++ b/WordPress/Classes/ViewRelated/Tags/TagsViewModel.swift @@ -56,11 +56,27 @@ class TagsViewModel: ObservableObject { self.init(taxonomy: nil, service: TagsService(blog: blog), selectedTerms: selectedTags, mode: mode) } - convenience init(blog: Blog, client: WordPressClient, taxonomy: SiteTaxonomy, selectedTerms: [SelectedTerm] = [], mode: TagsViewMode) { - self.init(taxonomy: taxonomy, service: AnyTermService(client: client, endpoint: taxonomy.endpoint), selectedTerms: selectedTerms, mode: mode) + convenience init( + blog: Blog, + client: WordPressClient, + taxonomy: SiteTaxonomy, + selectedTerms: [SelectedTerm] = [], + mode: TagsViewMode + ) { + self.init( + taxonomy: taxonomy, + service: AnyTermService(client: client, endpoint: taxonomy.endpoint), + selectedTerms: selectedTerms, + mode: mode + ) } - init(taxonomy: SiteTaxonomy?, service: TaxonomyServiceProtocol, selectedTerms: [SelectedTerm] = [], mode: TagsViewMode) { + init( + taxonomy: SiteTaxonomy?, + service: TaxonomyServiceProtocol, + selectedTerms: [SelectedTerm] = [], + mode: TagsViewMode + ) { self.taxonomy = taxonomy self.tagsService = service self.mode = mode @@ -85,7 +101,7 @@ class TagsViewModel: ObservableObject { private func loadInitialTags() async { isLoading = true - defer { isLoading = false} + defer { isLoading = false } error = nil @@ -135,7 +151,7 @@ class TagsViewModel: ObservableObject { let remoteTags = try await tagsService.searchTags(with: searchText) return try await TagsPaginatedResponse { _ in - return TagsPaginatedResponse.Page( + TagsPaginatedResponse.Page( items: remoteTags, total: remoteTags.count, hasMore: false, @@ -171,7 +187,8 @@ class TagsViewModel: ObservableObject { do { let newTag: AnyTermWithViewContext if let existing = try await tagsService.searchTags(with: name) - .first(where: { $0.name.compare(name, options: .caseInsensitive) == .orderedSame }) { + .first(where: { $0.name.compare(name, options: .caseInsensitive) == .orderedSame }) + { newTag = existing } else { newTag = try await tagsService.createTag(name: name, description: "") @@ -190,7 +207,7 @@ class TagsViewModel: ObservableObject { } func isSelected(_ term: AnyTermWithViewContext) -> Bool { - return selectedTagsSet.contains(term.name.lowercased()) + selectedTagsSet.contains(term.name.lowercased()) } func isNotSelected(_ term: AnyTermWithViewContext) -> Bool { From f67070a16cc8561c5473365c8b4593dfe7336dd3 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Fri, 8 May 2026 11:22:25 +1200 Subject: [PATCH 2/2] Stop dropping pending tag names from the shared selector callback The shared tag selector filtered out pending (`id == 0`) terms before notifying its parent, which silently dropped user-typed tag names if the user published before the async search/create completed. Normal posts publish tag names directly through `RemotePostCreateParameters`, and the custom-post REST flow resolves IDs at save time via `TermResolutionService.resolveIDs(for:)`, so neither consumer needs the filter. Drop it so the parent always observes the user's full selection, and replace the old test that locked in the broken contract with one that asserts the callback fires synchronously with the pending term. --- .../Features/Posts/TagSelectionTests.swift | 17 +++++++++++++---- .../ViewRelated/Tags/TagsViewModel.swift | 7 ++++++- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/Tests/KeystoneTests/Tests/Features/Posts/TagSelectionTests.swift b/Tests/KeystoneTests/Tests/Features/Posts/TagSelectionTests.swift index 251224b1f989..9e7ad951aa36 100644 --- a/Tests/KeystoneTests/Tests/Features/Posts/TagSelectionTests.swift +++ b/Tests/KeystoneTests/Tests/Features/Posts/TagSelectionTests.swift @@ -114,7 +114,11 @@ struct TagSelectionTests { // MARK: - Selection callback @Test - func selectionCallbackFiltersOutPendingItems() async { + func selectionCallbackEmitsPendingItemsImmediately() async { + // Regression coverage for the missing-tags bug: if the user publishes before the + // async search/create completes, the parent's tag list must still contain the + // typed name. The callback must therefore fire synchronously on `addNewTag` with + // the pending (`id == 0`) term so the parent never observes an empty selection. let mock = MockService(tags: ["Foo", "Bar"]) var callbackTags: [TagsViewModel.SelectedTerm] = [] let viewModel = TagsViewModel( @@ -125,10 +129,15 @@ struct TagSelectionTests { }) ) - _ = viewModel.addNewTag(named: "Baz") + // Do not await — we want to observe the state before the create task can run. + let task = viewModel.addNewTag(named: "Baz") + + #expect(callbackTags.count == 1) + #expect(callbackTags[0].name == "Baz") + #expect(callbackTags[0].id == 0) + #expect(callbackTags[0].isPending) - // The pending item (id == 0) should be filtered out of the callback - #expect(callbackTags.isEmpty) + await task?.value } @Test diff --git a/WordPress/Classes/ViewRelated/Tags/TagsViewModel.swift b/WordPress/Classes/ViewRelated/Tags/TagsViewModel.swift index 6ae6e59b3783..4e2366e2c160 100644 --- a/WordPress/Classes/ViewRelated/Tags/TagsViewModel.swift +++ b/WordPress/Classes/ViewRelated/Tags/TagsViewModel.swift @@ -30,7 +30,12 @@ class TagsViewModel: ObservableObject { @Published private(set) var selectedTags: [SelectedTerm] { didSet { if case .selection(let onSelectedTagsChanged) = mode { - onSelectedTagsChanged?(selectedTags.filter { !$0.isPending }) + // Emit the full selection, including pending (`id == 0`) terms. + // Filtering them here would silently drop user-typed tag names from the parent's + // selection state if publish/save runs before the async search/create completes. + // Consumers that require resolved IDs (e.g. custom-post REST flows) resolve them + // at save time via `TermResolutionService.resolveIDs(for:)`. + onSelectedTagsChanged?(selectedTags) } } }