Blaze: Update blaze instance from the correct thread

[momo-ozawa]

Fixes #20397

Description

Fixes an issue where Blog managed object was being updated from the wrong thread.

Queries the Blog managed object using the context provided by the performAndSave closure to ensure the managed object is updated correctly.

Ref: pbArwn-602-p2

How to test

0. Switch to trunk
1. Add -com.apple.CoreData.ConcurrencyDebug 1 to your launch arguments
2. Build and run the Jetpack app
3. The app crashes

4. Switch to this branch
5. Build and run the Jetpack app
6. The app doesn't crash

Regression Notes

1. Potential unintended areas of impact
   n/a

2. What I did to test those areas of impact (or what existing automated tests I relied on)
   n/a

3. What automated tests I added (or what prevented me from doing so)
   n/a

PR submission checklist:

• I have completed the Regression Notes.
• I have considered adding unit tests for my changes.
• I have considered adding accessibility improvements for my changes.
• I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

[peril-wordpress-mobile]

	Warnings
⚠️	This PR is assigned to a milestone which is closing in less than 4 days Please, make sure to get it merged by then or assign it to a later expiring milestone

Generated by 🚫 dangerJS

[wpmobilebot]

📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.

	App Name	WordPress Alpha
	Configuration	Release-Alpha
	Build Number	pr20413-20eddd8
	Version	22.0
	Bundle ID	org.wordpress.alpha
	Commit	20eddd8
	App Center Build	WPiOS - One-Offs #5344

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

[wpmobilebot]

📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.

	App Name	Jetpack Alpha
	Configuration	Release-Alpha
	Build Number	pr20413-20eddd8
	Version	22.0
	Bundle ID	com.jetpack.alpha
	Commit	20eddd8
	App Center Build	jetpack-installable-builds #4371

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

[crazytonyli]

The completion block can't be called here, otherwise it'll cause the same issue in #20196.

[crazytonyli]

An alternative would be declaring contextManager as CoreDataStackSwift, so that you can catch the error thrown by the Core Data operation closure:

contextManager.performAndSave({ context in
    let blog = try context.existingObject(with: objectID) as! Blog
    blog.isBlazeApproved = isBlazeApproved
}, completion: { (result: Result<Void, Error>) in
    completion?()
}, on: .main)

[momo-ozawa]

The completion block can't be called here

Fixed! 20eddd8

We're only logging the error, so I decided to use guard let blog = try? ...

[crazytonyli]

This completion block will be called regardless if the Core Data operation in the closure above success or not.

[momo-ozawa]

That's the desired behavior - I've amended the documentation to clarify this 🙇‍♀️

[hassaanelgarem]

The changes look good to me, and it works as described 🚀

[mokagio]

Taking over merge to ship a new beta

[hypest]

Thank you for the fix @momo-ozawa !

Can you add a link to a convo with context about going with a beta fix instead, for example, of a hotfix? Thanks!

[momo-ozawa]

@hypest 👋

We didn't have a conversation about whether to do a betafix vs hotfix for this crash. Sorry - I should have checked in with the release rotation team! Since this crash affected ~0.05% of users (low impact), and this isn't a critical flow (medium severity), I figured this would be a betafix instead of a hotfix.

Sentry Issue: JETPACK-IOS-Z5

[hypest]

Thanks for elaborating @momo-ozawa !

Since this crash affected ~0.05% of users (low impact), and this isn't a critical flow (medium severity), I figured this would be a betafix instead of a hotfix.

Makes sense.

To try to understand the impact, I wonder, are the users affected all on the Blaze flow, or some may be on an irrelevant flow? If all on the Blaze flow, would all Blaze flow users encounter the issue? I would elevate the impact assessment depending on those questions.

For the severity assessment part, I wonder if the issue has been a blocker for the Blaze users for, say, reaching the check-out stage. If yes, I'd elevate the severity here as well.

I understand that the fix will be out with the 22.0 release scheduled for today so, no further action needed, but I think we probably want to update the crash priority matrix, to include the new revenue flows, the least.

[momo-ozawa]

@hypest Those are good points, thanks for sharing.

To try to understand the impact, I wonder, are the users affected all on the Blaze flow, or some may be on an irrelevant flow?

If all on the Blaze flow, would all Blaze flow users encounter the issue?

For the severity assessment part, I wonder if the issue has been a blocker for the Blaze users for, say, reaching the check-out stage

This crash can happen when assessing whether users are eligible for Blaze. So this is before users begin the blaze purchase flow. Also, it doesn't happen all the time.

I think we probably want to update the crash priority matrix, to include the new revenue flows, the least.

Agreed! I've updated the crash priority matrix to include this info. pbArwn-1gZ-p2