This repository was archived by the owner on Feb 5, 2025. It is now read-only.
Site address discovery #666
Merged
Conversation
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
…ith creator closure
…racking accordingly
1 task
selanthiraiyan
approved these changes
Aug 18, 2022
Contributor
selanthiraiyan
left a comment
selanthiraiyan
left a comment
There was a problem hiding this comment.
The changes look good to me.
I have also tested the WCiOS PR, and it works as expected. 🎉
I left a nit and a question.
| func instantiateViewController<T: NSObject>(ofClass classType: T.Type, creator: ((NSCoder) -> UIViewController?)? = nil) -> T? { | ||
| let identifier = classType.classNameWithoutNamespaces | ||
| return instance.instantiateViewController(withIdentifier: identifier) as? T | ||
| return instance.instantiateViewController(identifier: identifier, creator: creator) as? T |
WordPressAuthenticator/Authenticator/WordPressAuthenticatorDelegateProtocol.swift
Show resolved
Hide resolved
Comment on lines
+442
to
+468
| // Checks that the site exists | ||
| var request = URLRequest(url: url) | ||
| request.httpMethod = "HEAD" | ||
| request.timeoutInterval = 10.0 // waits for 10 seconds | ||
| let task = URLSession.shared.dataTask(with: request) { [weak self] _, _, error in | ||
| DispatchQueue.main.async { [weak self] in | ||
| guard let self = self else { return } | ||
|
|
||
| if let error = error { | ||
| self.configureViewLoading(false) | ||
|
|
||
| if self.authenticationDelegate.shouldHandleError(error) { | ||
| self.authenticationDelegate.handleError(error) { customUI in | ||
| self.pushCustomUI(customUI) | ||
| } | ||
| return | ||
| } | ||
|
|
||
| return self.displayError(message: Localization.nonExistentSiteError, moveVoiceOverFocus: true) | ||
| } | ||
|
|
||
| // Proceeds to check for the site's WordPress | ||
| self.guessXMLRPCURL(for: self.loginFields.siteAddress) | ||
| } | ||
| } | ||
| task.resume() | ||
| } |
Contributor
There was a problem hiding this comment.
❓
Should we raise a PR in the WP iOS repository and test this change as this code will execute even if isSiteDiscovery is false? 🤔
Internal reference - p91TBi-49U-p2
Contributor
Author
There was a problem hiding this comment.
Makes sense! Added a PR here: wordpress-mobile/WordPress-iOS#19209.
Contributor
Author
|
Merging since wordpress-mobile/WordPress-iOS#19209 tests passed |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Part of woocommerce/woocommerce-ios#7468
Description
This PR adds a new flow for troubleshooting a given site address. This flow reuses the current site address UI and returns the
WordPressSiteInfoto a new delegate method for checking the problem of the site and show appropriate error message on the host app.While testing the integration in woocommerce/woocommerce-ios#7484, I found that for a non-existent site, the check is taking quite long to return an error - the library was checking for WordPress while we could check for its existence first. This PR adds that with a 10-second timeout and displays the error inline.
This PR also updates analytics by tracking a new flow
site_discovery.Testing
Please check woocommerce/woocommerce-ios#7484 for testing steps.