Fix double-callback and nil category in PostCategoryService create - #25970
Merged
Conversation
Contributor
|
| App Name | WordPress | |
| Configuration | Release-Alpha | |
| Build Number | 34254 | |
| Version | PR #25970 | |
| Bundle ID | org.wordpress.alpha | |
| Commit | 8eec585 | |
| Installation URL | 59ct1crp42po0 |
Contributor
|
| App Name | Jetpack | |
| Configuration | Release-Alpha | |
| Build Number | 34254 | |
| Version | PR #25970 | |
| Bundle ID | com.jetpack.alpha | |
| Commit | 8eec585 | |
| Installation URL | 40vg62emje1eg |
The prior fix guarded `success(newCategory)` behind `if (newCategory)`, which stopped the `success(nil)` crash but left a path where neither `success` nor `failure` fires: the blog resolves and the category saves, but the main-context lookup returns nil (e.g. a response that carried no usable category ID). The only caller, `PostCategoryCreateView`, clears `isSaving` solely in its failure block, so a dropped callback hangs the save spinner indefinitely. Adopt the `__block NSError *error` pattern already used by `syncCategoriesForBlog:`: the no-blog branch records the error and the main-queue completion delivers exactly one callback — `failure(error)`, `success(category)`, or `failure(serviceErrorCategoryNotFound)` when the created category can't be resolved locally. This also moves the no-blog failure onto the main queue, matching the success path; the regression test now asserts that delivery is on the main thread.
crazytonyli
approved these changes
Sep 2, 2026
`PostCategoryServiceTests` only exercised the no-blog error path; the happy path — blog resolves, the category is created and handed back through `success` — was never executed by any test. Save the blog so its objectID resolves in the background save context, then assert `success` fires once on the main queue with the created category's ID, name, and parent, and that `failure` does not fire.
`createCategoryWithName:…:success:failure:` has a single caller — `PostCategoryCreateView` (Swift) — which always provides both blocks, so the `nullable` annotations bought nothing but three `if (success)` / `if (failure)` guards. Mark both blocks non-null: Swift enforces `_Nonnull` at compile time, so the caller cannot pass nil, and the completion drops the guards while making "exactly one callback" a hard invariant. The sibling `syncCategoriesForBlog:` methods stay `nullable` — they are genuinely called with `success:nil failure:nil` (the XML-RPC post-create re-sync).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follows the pattern from #25964, and fixes a latent double-callback.
Summary
createCategoryWithName:…success:failure:runs its save inperformAndSaveUsingBlock:. On the!blogpath the save block callsfailure(...)andreturns — but thereturnonly exits the save block, socompletion:still ran and calledsuccess(...).completion:resolves the category vialookupWithBlogObjectID:…, which returnsnilwhen the category was never created, and passed thatnilinto the_Nonnull(PostCategory *) success block.Root Cause
Two bugs on one path: (1) both
failureandsuccessfire when the blog is missing — the alert shows and the screen dismisses viadidCreate— and (2)successcan receive anilcategory.Fix
!blogpath the lookup isnil, so onlyfailurefires; on successsuccessreceives a realPostCategory, keeping the_Nonnullannotation honest.Test plan
WordPressapp builds (generic iOS Simulator).Part of the Objective-C non-null nullability audit; see #25964.