Skip to content

Conversation

@toupper
Copy link
Contributor

@toupper toupper commented Oct 14, 2022

Closes: #7864

Description

Before, when the user tapped on "order details" universal link with a blog Id parameter value they couldn't access, the app opened and switched to an undetermined state where data couldn't be loaded (see video below).
The reason is that, when switching to the new store, we logged out from the previous one without checking if the user could actually access the new site. Once we do that, the app cannot load any data (stats, orders, products ...) of the new site because the user doesn't have access.
With this PR we check first if the switching store was previously synced and stored in CoreData, which means the user has access to it. Since we synchronize the sites remotely on every startup and we want the navigation to be quick, I consider enough checking locally for the store instead of syncing remotely after opening the app following a universal link.

Testing instructions

  1. Add a link to the Notes app with a blog id you don't have access to E.g https://woocommerce.com/mobile/orders/details?blog_id=1&order_id=240
  2. Tap on the Link
    3a. Before: App opens, the order load fails and the app cannot retrieve data anymore until you switch store again in settings.
    3b. After: App opens and navigates to orders.

Screenshots

Before

RPReplay_Final1665756881.MP4

After

RPReplay_Final1665757863.MP4

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@toupper toupper requested a review from Ecarrion October 14, 2022 14:44
@toupper toupper added type: bug A confirmed bug. feature: notifications Related to notifications or notifs. feature: deep links Related to the universal and deep links labels Oct 14, 2022
@toupper toupper added this to the 10.9 milestone Oct 14, 2022
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Oct 14, 2022

You can test the changes from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr7866-59b0d71 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

Copy link
Contributor

@Ecarrion Ecarrion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

/// - Returns: a boolean that indicates whether the site was changed.
///
func switchToStoreIfSiteIsStored(with storeID: Int64, onCompletion: @escaping (Bool) -> Void) {
self.refreshStoredSites()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self.refreshStoredSites()
refreshStoredSites()

func switchToStoreIfSiteIsStored(with storeID: Int64, onCompletion: @escaping (Bool) -> Void) {
self.refreshStoredSites()

let siteWasStored = self.wooCommerceSites.first(where: { $0.siteID == storeID }) != nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let siteWasStored = self.wooCommerceSites.first(where: { $0.siteID == storeID }) != nil
let siteWasStored = wooCommerceSites.first(where: { $0.siteID == storeID }) != nil

@toupper toupper enabled auto-merge October 17, 2022 12:57
@toupper toupper merged commit 3a33ce8 into trunk Oct 17, 2022
@toupper toupper deleted the issue/7864-app-in-wrong-state-link-wrong-blog-id branch October 17, 2022 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: deep links Related to the universal and deep links feature: notifications Related to notifications or notifs. type: bug A confirmed bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Universal Links - Notifications] App gets in undetermined state when opening universal link with unknown site id

4 participants