[Bug Fix] Fix oauth2 token for widget didn't been setup when login

[tzef]

Description

The widget extension shares the auth token through the keychain that is configured in the TodayExtensionService. The auth token is obtained when:

1. The app is launched
2. The account is changed

The issue happens when the account is changed, because in AccountService -> updateAccountWithID -> updateDefaultBlogIfNeeded, we only configure the token if the account has a defaultBlog.

However, the defaultBlog is empty when the user just logs in, which means the widget service API token is lost until the next time the token sync-up, resulting in the error response: "The operation couldn't be completed. (Foundation_GenericObjCError error 0.)"

Impact

This issue causes inconsistent results for users, they will see a value of 0 until the app is launched again.

Users may try to click on the widget and be redirected to the stats insight page. The widget value will update at this time, but won't autorefresh because the token is not yet synced. The current time updated value comes from the cache, and the next auto fetch after 30 minutes will keep failing.

It is not severe because it can be recovered when the app is launched again. However, it may be a problem for users with multiple accounts.

Reproduce Steps

1. Log in to an account that has at least one website
2. Add any widget to the home screen
3. The widget value is always 0 due to the API request missing token error (fall back to use the initial model)

Solution

Separate configure widget and configure token, I think it makes sense always configure the token for logged-in users.
For the chase that users without websites, we have checked another config AppConfiguration.Widget.Stats.userDefaultsSiteIdKey in the timeline provider to present another status.

Testing Instructions

Given logged in account with at least one website and has stats value (not 0)

1. Add any widget to the home screen
2. Choose the website to have the value > 0
3. Log out to clear the token
4. Log in to the same account
5. The widget should display the correct value as step 2 instead of the default 0 value

Regression Notes

1. Potential unintended areas of impact
   Home screen widgets data fetch

2. What I did to test those areas of impact (or what existing automated tests I relied on)
   As same as Testing Instructions

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.

[tzef]

@staskus I found an existing issue during testing #20405
can you check if this fixe makes sense to you?

[staskus]

@tzef thanks for the bug fix, I'll review it! 👍

We can target trunk if it's not directly related to the lock widgets.

[tzef]

Okay! rebase and update the base branch to trunk

[staskus]

Thanks again for catching and attempting to fix the issue, @tzef!

Separate configure widget and configure token, I think it makes sense always configure the token for logged-in users.

I agree, tokens shouldn't depend on defaultBlog existing 👍

However, I have a questions:

However, the defaultBlog is empty when the user just logs in..

So, in your testing, defaultBlog is always empty when the user logs? If yes, then do we have a bad timing of setupWordPressExtensions call?

I'm wondering if there're more issues related to that then, I see we're setting authToken in this else for other extension [NotificationSupportService insertServiceExtensionToken:defaultAccount.authToken];. Maybe we should move away all the code that does not depend on the defaultBlog from these conditional statements. What is your view?

[tzef]

Good question! I tried to answer based on my research, cmiiw.

1. The defaultBlog is always empty when the setupWordPressExtensions comes from AccountChangedNotification, because the account does not have any blog data yet.

2. We have to rely on BlogService to syncBlogsForAccount which is triggered by views (BlogSelectorViewController, BlogListViewController, MySiteViewController).

3. Once the sync-up is completed, BlogService will ask AccountService to update the blogs to accounts and setup the auth token again, if everything collaborated well, the app extensions should be able to access the token at this step.

Apparently, there was something wrong in step 3 before, I just found that was fixed in #20394 four days ago 😅, that fixes all extensions not only for the widget extension as my PR.

So we can consider

1. close this PR, I verified the trunk branch, the sync-up works well without this token loss issue.
2. convert this PR to improvement, because I still think it's is better to decouple the token from the account syncing.
   what do you think?

[staskus]

Thanks for explaining the issue!

Apparently, there was something wrong in step 3 before, I just found that was fixed in #20394 four days ago 😅, that fixes all extensions not only for the widget extension as my PR.

Okay, good catch. There were a lot of changes with Core Data and syncing recently, it might've been the reason. That's why I wanted to make sure we're fixing the right thing.

1. close this PR, I verified the trunk branch, the sync-up works well without this token loss issue.
2. convert this PR to improvement, because I still think it's is better to decouple the token from the account syncing.
   what do you think?

Yes, it could be an option. I'm thinking about whether we would come back to this issue, especially if it wouldn't give an immediate value. I'd say we either finish decoupling defaultBlog and token syncing in this PR, or close it if we don't see a value. Either way, we can prioritize lock screen widgets over this given the rootcause issue is fixed.

[tzef]

Okay!! agree with you,
the root cause issue is fixed, so this PR is not the blocker for widgets anymore,
let's go back to focusing on the widget tasks. 👍