Skip to content

Commit 668c85b

Browse files
itsmeichigoitsmeichigo
authored
Jetpack Setup: Fix crash upon magic link (#14897)
2 parents 4f57e3f + e886334 commit 668c85b

File tree

12 files changed

+339
-11
lines changed

12 files changed

+339
-11
lines changed

‎Networking/Networking.xcodeproj/project.pbxproj‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1120,6 +1120,8 @@
11201120
DEDA8DB52B187DE40076BF0F /* WordPressThemeMapper.swift in Sources */ = {isa = PBXBuildFile; fileRef = DEDA8DB42B187DE40076BF0F /* WordPressThemeMapper.swift */; };
11211121
DEDA8DB72B187E770076BF0F /* theme-mine-success.json in Resources */ = {isa = PBXBuildFile; fileRef = DEDA8DB62B187E770076BF0F /* theme-mine-success.json */; };
11221122
DEDA8DB92B187EC90076BF0F /* WordPressThemeMapperTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = DEDA8DB82B187EC90076BF0F /* WordPressThemeMapperTests.swift */; };
1123+
DEE55CCC2D3A0551006D0251 /* site.json in Resources */ = {isa = PBXBuildFile; fileRef = DEE55CCB2D3A0551006D0251 /* site.json */; };
1124+
DEE55CCE2D3A069C006D0251 /* site-error.json in Resources */ = {isa = PBXBuildFile; fileRef = DEE55CCD2D3A069C006D0251 /* site-error.json */; };
11231125
DEEDA4442975003800F845AB /* settings-general-without-data.json in Resources */ = {isa = PBXBuildFile; fileRef = DEEDA4432975003800F845AB /* settings-general-without-data.json */; };
11241126
DEEF8E6529C832B500D47411 /* wordpress-site-info-with-auth-url.json in Resources */ = {isa = PBXBuildFile; fileRef = DEEF8E6429C832B500D47411 /* wordpress-site-info-with-auth-url.json */; };
11251127
DEEF8E6829C858AD00D47411 /* OneTimeApplicationPasswordUseCase.swift in Sources */ = {isa = PBXBuildFile; fileRef = DEEF8E6729C858AD00D47411 /* OneTimeApplicationPasswordUseCase.swift */; };
@@ -2315,6 +2317,8 @@
23152317
DEDA8DB42B187DE40076BF0F /* WordPressThemeMapper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WordPressThemeMapper.swift; sourceTree = "<group>"; };
23162318
DEDA8DB62B187E770076BF0F /* theme-mine-success.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = "theme-mine-success.json"; sourceTree = "<group>"; };
23172319
DEDA8DB82B187EC90076BF0F /* WordPressThemeMapperTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WordPressThemeMapperTests.swift; sourceTree = "<group>"; };
2320+
DEE55CCB2D3A0551006D0251 /* site.json */ = {isa = PBXFileReference; lastKnownFileType = text.json; path = site.json; sourceTree = "<group>"; };
2321+
DEE55CCD2D3A069C006D0251 /* site-error.json */ = {isa = PBXFileReference; lastKnownFileType = text.json; path = "site-error.json"; sourceTree = "<group>"; };
23182322
DEEDA4432975003800F845AB /* settings-general-without-data.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = "settings-general-without-data.json"; sourceTree = "<group>"; };
23192323
DEEF8E6429C832B500D47411 /* wordpress-site-info-with-auth-url.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = "wordpress-site-info-with-auth-url.json"; sourceTree = "<group>"; };
23202324
DEEF8E6729C858AD00D47411 /* OneTimeApplicationPasswordUseCase.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OneTimeApplicationPasswordUseCase.swift; sourceTree = "<group>"; };
@@ -3444,6 +3448,8 @@
34443448
CC0786622678F79500BA9AC1 /* shipping-label-purchase-success.json */,
34453449
CC0786C6267BB10700BA9AC1 /* shipping-label-status-success.json */,
34463450
CE606D8A2BE38B04001CB424 /* shipping-methods.json */,
3451+
DEE55CCB2D3A0551006D0251 /* site.json */,
3452+
DEE55CCD2D3A069C006D0251 /* site-error.json */,
34473453
B56C1EB920EA7D2C00D749F9 /* sites.json */,
34483454
2670C3FD270F4E6A002FE931 /* sites-malformed.json */,
34493455
7426CA1221AF34A3004E9FFC /* site-api.json */,
@@ -4559,6 +4565,7 @@
45594565
CEF2DD9D2B56E04F00A3DD0B /* jetpack-settings-invalid-module.json in Resources */,
45604566
45B79AC62C9355F800DCCB2C /* meta-data-products-and-orders_nested_in_data.json in Resources */,
45614567
DE9D6BCC270D769C00BA6562 /* shipping-label-address-without-name-validation-success.json in Resources */,
4568+
DEE55CCC2D3A0551006D0251 /* site.json in Resources */,
45624569
74A1D263211898F000931DFA /* site-visits-day.json in Resources */,
45634570
31054710262E2EE400C5C02B /* wcpay-payment-intent-succeeded.json in Resources */,
45644571
EE9826922B17569400A3F57E /* product-subscription-sync-renewals-day-month-format-int-values.json in Resources */,
@@ -4633,6 +4640,7 @@
46334640
2683D71024456EE4002A1589 /* categories-extra.json in Resources */,
46344641
A69FE19D2588D70E0059A96B /* order-with-deleted-refunds.json in Resources */,
46354642
E18152C228F85E0A0011A0EC /* iap-products.json in Resources */,
4643+
DEE55CCE2D3A069C006D0251 /* site-error.json in Resources */,
46364644
02B41A90296BC85800FE3311 /* site-domains.json in Resources */,
46374645
DE2004702BFB535500660A72 /* product-report.json in Resources */,
46384646
EE57C13E297FBEE200BC31E7 /* product-tags-created-without-data.json in Resources */,

‎Networking/Networking/Remote/SiteRemote.swift‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,11 @@ public protocol SiteRemoteProtocol {
2626
/// - Returns: The site that matches the site ID.
2727
func loadSite(siteID: Int64) async throws -> Site
2828

29+
/// Loads a site.
30+
/// - Parameter domain: Domain of the site to load.
31+
/// - Returns: The site that matches the site ID.
32+
func loadSite(domain: String) async throws -> Site
33+
2934
/// Update a site title.
3035
/// - Parameters:
3136
/// - siteID: Remote ID of the site to update
@@ -135,6 +140,16 @@ public class SiteRemote: Remote, SiteRemoteProtocol {
135140
return try await enqueue(request)
136141
}
137142

143+
public func loadSite(domain: String) async throws -> Site {
144+
let path = Path.loadSite(domain: domain)
145+
let parameters = [
146+
SiteParameter.Fields.key: SiteParameter.Fields.value,
147+
SiteParameter.Options.key: SiteParameter.Options.value
148+
]
149+
let request = DotcomRequest(wordpressApiVersion: .mark1_1, method: .get, path: path, parameters: parameters)
150+
return try await enqueue(request)
151+
}
152+
138153
public func updateSiteTitle(siteID: Int64, title: String) async throws {
139154
let parameters = [
140155
Fields.title: title
@@ -307,6 +322,10 @@ private extension SiteRemote {
307322
"sites/\(siteID)"
308323
}
309324

325+
static func loadSite(domain: String) -> String {
326+
"sites/\(domain)"
327+
}
328+
310329
static func siteSettings(siteID: Int64) -> String {
311330
"sites/\(siteID)/settings"
312331
}

‎Networking/NetworkingTests/Remote/SiteRemoteTests.swift‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,4 +292,34 @@ final class SiteRemoteTests: XCTestCase {
292292
(error as? DotcomError) == .unauthorized
293293
})
294294
}
295+
296+
// MARK: - `loadSite`
297+
298+
func test_loadSite_by_domain_returns_site_info_on_success() async throws {
299+
// Given
300+
let domain = "example.com"
301+
network.simulateResponse(requestUrlSuffix: "sites/\(domain)", filename: "site")
302+
303+
// When
304+
let response = try await remote.loadSite(domain: domain)
305+
306+
// Then
307+
XCTAssertEqual(response.siteID, 1112233334444555)
308+
XCTAssertEqual(response.name, "Testing Blog")
309+
XCTAssertEqual(response.url, "https://some-testing-url.testing.blog")
310+
}
311+
312+
func test_loadSite_by_domain_returns_error_upon_failure() async throws {
313+
// Given
314+
let domain = "example.com"
315+
network.simulateResponse(requestUrlSuffix: "sites/\(domain)", filename: "site-error")
316+
317+
await assertThrowsError({
318+
// When
319+
_ = try await remote.loadSite(domain: domain)
320+
}, errorAssert: { ($0 as? DotcomError) == .unknown(
321+
code: "parse_error",
322+
message: "The Jetpack site is inaccessible or returned an error: parse error (local). not well formed [-32710]"
323+
)})
324+
}
295325
}

‎Networking/NetworkingTests/Responses/site-error.json‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"error": "parse_error",
3+
"message": "The Jetpack site is inaccessible or returned an error: parse error (local). not well formed [-32710]"
4+
}

‎Networking/NetworkingTests/Responses/site.json‎

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
{
2+
"ID": 1112233334444555,
3+
"name": "Testing Blog",
4+
"description": "Testing Tagline",
5+
"URL": "https:\/\/some-testing-url.testing.blog",
6+
"capabilities": {
7+
"edit_pages": true,
8+
"edit_posts": true,
9+
"edit_others_posts": true,
10+
"edit_others_pages": true,
11+
"delete_posts": true,
12+
"delete_others_posts": true,
13+
"edit_theme_options": true,
14+
"edit_users": false,
15+
"list_users": true,
16+
"manage_categories": true,
17+
"manage_options": true,
18+
"moderate_comments": true,
19+
"activate_wordads": true,
20+
"promote_users": true,
21+
"publish_posts": true,
22+
"upload_files": true,
23+
"delete_users": false,
24+
"remove_users": true,
25+
"own_site": false,
26+
"view_hosting": true,
27+
"view_stats": true,
28+
"activate_plugins": true
29+
},
30+
"jetpack": true,
31+
"jetpack_connection": true,
32+
"was_ecommerce_trial": true,
33+
"plan": {
34+
"product_id": 1008,
35+
"product_slug": "business-bundle",
36+
"features": {
37+
"active": [
38+
"akismet",
39+
"donations",
40+
"ai-assistant"
41+
]
42+
}
43+
},
44+
"options": {
45+
"timezone": "",
46+
"gmt_offset": 3.5,
47+
"blog_public": 1,
48+
"videopress_enabled": false,
49+
"upgraded_filetypes_enabled": true,
50+
"login_url": "https:\/\/some-testing-url.here\/wp-login.php",
51+
"admin_url": "https:\/\/some-testing-url.here\/wp-admin\/",
52+
"is_mapped_domain": true,
53+
"is_redirect": false,
54+
"unmapped_url": "https:\/\/some-testing-url.here",
55+
"featured_images_enabled": false,
56+
"theme_slug": "storefront",
57+
"header_image": false,
58+
"background_color": false,
59+
"image_default_link_type": "file",
60+
"image_thumbnail_width": 150,
61+
"image_thumbnail_height": 150,
62+
"image_thumbnail_crop": 0,
63+
"image_medium_width": 300,
64+
"image_medium_height": 300,
65+
"image_large_width": 1024,
66+
"image_large_height": 1024,
67+
"permalink_structure": "\/%year%\/%monthnum%\/%day%\/%postname%\/",
68+
"post_formats": [],
69+
"default_post_format": "0",
70+
"default_category": 12,
71+
"allowed_file_types": [
72+
"jpg",
73+
"jpeg",
74+
"png",
75+
"gif",
76+
"pdf",
77+
"doc",
78+
"ppt",
79+
"odt",
80+
"pptx",
81+
"docx",
82+
"pps",
83+
"ppsx",
84+
"xls",
85+
"xlsx",
86+
"key",
87+
"ogv",
88+
"mp4",
89+
"m4v",
90+
"mov",
91+
"wmv",
92+
"avi",
93+
"mpg",
94+
"3gp",
95+
"3g2"
96+
],
97+
"show_on_front": "page",
98+
"default_likes_enabled": true,
99+
"default_sharing_status": true,
100+
"default_comment_status": true,
101+
"default_ping_status": true,
102+
"software_version": "4.9.6",
103+
"created_at": "2014-03-28T15:03:03+00:00",
104+
"wordads": false,
105+
"publicize_permanently_disabled": false,
106+
"frame_nonce": "73ae7163d8",
107+
"page_on_front": 1455,
108+
"page_for_posts": 0,
109+
"headstart": false,
110+
"headstart_is_fresh": false,
111+
"ak_vp_bundle_enabled": false,
112+
"advanced_seo_front_page_description": "",
113+
"advanced_seo_title_formats": [],
114+
"verification_services_codes": null,
115+
"podcasting_archive": null,
116+
"is_domain_only": false,
117+
"is_automated_transfer": true,
118+
"is_wpcom_store": true,
119+
"woocommerce_is_active": true,
120+
"can_blaze": true,
121+
"design_type": null,
122+
"site_goals": null,
123+
"jetpack_version": "6.2.1",
124+
"main_network_site": "https:\/\/some-testing-url.here",
125+
"active_modules": [
126+
"after-the-deadline",
127+
"contact-form",
128+
"custom-content-types",
129+
"custom-css",
130+
"enhanced-distribution",
131+
"gravatar-hovercards",
132+
"json-api",
133+
"latex",
134+
"manage",
135+
"notes",
136+
"post-by-email",
137+
"protect",
138+
"publicize",
139+
"sharedaddy",
140+
"shortcodes",
141+
"shortlinks",
142+
"sitemaps",
143+
"stats",
144+
"subscriptions",
145+
"verification-tools",
146+
"widget-visibility",
147+
"widgets",
148+
"sso",
149+
"tiled-gallery",
150+
"likes",
151+
"comments",
152+
"comment-likes",
153+
"videopress",
154+
"carousel",
155+
"photon",
156+
"seo-tools",
157+
"google-analytics",
158+
"infinite-scroll",
159+
"masterbar",
160+
"vaultpress"
161+
],
162+
"max_upload_size": false,
163+
"wp_memory_limit": "268435456",
164+
"wp_max_memory_limit": "268435456",
165+
"is_multi_network": false,
166+
"is_multi_site": false,
167+
"file_mod_disabled": false
168+
},
169+
"updates": {
170+
"plugins": 3,
171+
"themes": 1,
172+
"wordpress": 0,
173+
"translations": 0,
174+
"total": 4
175+
}
176+
}

‎WooCommerce/Classes/AppDelegate.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -581,7 +581,7 @@ private extension AppDelegate {
581581
// MARK: - Magic link
582582
private extension AppDelegate {
583583
func handleAuthenticationUrl(_ url: URL, options: [UIApplication.OpenURLOptionsKey: Any], rootViewController: UIViewController) -> Bool {
584-
return if ServiceLocator.stores.isAuthenticatedWithoutWPCom {
584+
return if ServiceLocator.stores.isAuthenticated {
585585
handleAuthenticationUrlForJetpackSetup(url, rootViewController: rootViewController)
586586
} else {
587587
ServiceLocator.authenticationManager.handleAuthenticationUrl(url, options: options, rootViewController: rootViewController)

‎WooCommerce/Classes/ViewRelated/JetpackSetup/JetpackSetupCoordinator.swift‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -251,15 +251,17 @@ private extension JetpackSetupCoordinator {
251251

252252
func authenticateUserAndRefreshSite(with credentials: Credentials) {
253253
analytics.track(.jetpackSetupCompleted)
254-
stores.sessionManager.deleteApplicationPassword()
254+
let previousCredentials = stores.sessionManager.defaultCredentials
255255
stores.authenticate(credentials: credentials)
256+
256257
let progressView = InProgressViewController(viewProperties: .init(title: Localization.syncingData, message: ""))
257258
rootViewController.topmostPresentedViewController.present(progressView, animated: true)
258259

259-
let action = AccountAction.synchronizeSitesAndReturnSelectedSiteInfo(siteAddress: site.url) { [weak self] result in
260+
let action = SiteAction.syncSiteByDomain(domain: site.url.trimHTTPScheme()) { [weak self] result in
260261
guard let self else { return }
261262
switch result {
262263
case .success(let site):
264+
self.stores.sessionManager.deleteApplicationPassword()
263265
self.stores.updateDefaultStore(storeID: site.siteID)
264266
self.stores.synchronizeEntities { [weak self] in
265267
self?.stores.updateDefaultStore(site)
@@ -277,6 +279,11 @@ private extension JetpackSetupCoordinator {
277279
progressView.dismiss(animated: true, completion: { [weak self] in
278280
self?.showAlert(message: Localization.errorFetchingSites, onRetry: {
279281
self?.authenticateUserAndRefreshSite(with: credentials)
282+
}, onCancel: {
283+
// Revert the change to credentials
284+
if let previousCredentials {
285+
self?.stores.authenticate(credentials: previousCredentials)
286+
}
280287
})
281288
})
282289

@@ -436,7 +443,8 @@ private extension JetpackSetupCoordinator {
436443
/// Shows an error alert with a button to retry the failed action.
437444
///
438445
func showAlert(message: String,
439-
onRetry: (() -> Void)? = nil) {
446+
onRetry: (() -> Void)? = nil,
447+
onCancel: (() -> Void)? = nil) {
440448
let alert = UIAlertController(title: message,
441449
message: nil,
442450
preferredStyle: .alert)
@@ -446,7 +454,9 @@ private extension JetpackSetupCoordinator {
446454
}
447455
alert.addAction(retryAction)
448456
}
449-
let cancelAction = UIAlertAction(title: Localization.cancelButton, style: .cancel)
457+
let cancelAction = UIAlertAction(title: Localization.cancelButton, style: .cancel) { _ in
458+
onCancel?()
459+
}
450460
alert.addAction(cancelAction)
451461
rootViewController.topmostPresentedViewController.present(alert, animated: true)
452462
}

‎WooCommerce/WooCommerceTests/ViewRelated/JetpackSetup/JetpackSetupCoordinatorTests.swift‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,8 @@ final class JetpackSetupCoordinatorTests: XCTestCase {
156156
func test_handleAuthenticationUrl_proceeds_to_authenticate_user_if_jetpack_is_already_connected() throws {
157157
// Given
158158
let stores = MockStoresManager(sessionManager: .makeForTesting(authenticated: true, isWPCom: false))
159-
let testSite = Site.fake().copy(siteID: WooConstants.placeholderStoreID)
159+
let siteURL = "https://example.com"
160+
let testSite = Site.fake().copy(siteID: WooConstants.placeholderStoreID, url: siteURL)
160161
let expectedScheme = "scheme"
161162
let coordinator = JetpackSetupCoordinator(site: testSite, dotcomAuthScheme: expectedScheme, rootViewController: navigationController, stores: stores)
162163
let url = try XCTUnwrap(URL(string: "scheme://magic-login?token=test"))
@@ -174,11 +175,18 @@ final class JetpackSetupCoordinatorTests: XCTestCase {
174175
}
175176
}
176177

177-
let expectedSite = Site.fake().copy(siteID: 44, url: "https://example.com")
178+
let expectedSite = Site.fake().copy(siteID: 44, url: siteURL)
179+
stores.whenReceivingAction(ofType: SiteAction.self) { action in
180+
switch action {
181+
case let .syncSiteByDomain(domain, completion):
182+
XCTAssertEqual(domain, "example.com")
183+
completion(.success(expectedSite))
184+
default:
185+
break
186+
}
187+
}
178188
stores.whenReceivingAction(ofType: AccountAction.self) { action in
179189
switch action {
180-
case .synchronizeSitesAndReturnSelectedSiteInfo(_, let onCompletion):
181-
onCompletion(.success(expectedSite))
182190
case .synchronizeAccount(let onCompletion):
183191
onCompletion(.success(expectedAccount))
184192
case .synchronizeAccountSettings(_, let onCompletion):

0 commit comments

Comments
 (0)