Make sure we're using the new account system everywhere#999
Merged
astralbodies merged 27 commits intodevelopfrom Jan 7, 2014
Merged
Make sure we're using the new account system everywhere#999astralbodies merged 27 commits intodevelopfrom
astralbodies merged 27 commits intodevelopfrom
Conversation
…dPress-iOS into issue/22-remove-comapi-warnings Conflicts: WordPress/Classes/Blog.m WordPress/Classes/NotificationsManager.m WordPress/Classes/WPMobileStats.m WordPress/Classes/WPcomLoginViewController.m WordPress/Classes/WordPressAppDelegate.m
…warnings #22 Remove WordPressComApi deprecation
If this in fact needed, it can be reverted. It was not called.
For account change #22
Note @koke, that for JetpackSettingsVC, the effect of creating a new account may not be the correct approach taken. signInWithUsername:password:blocks use to store the authtoken internally within WordPressComApi. Now, the only way to save a token is by creating a new account. We could make `refreshTokenWithSuccess:failure` useful again for these kinds of purposes.
- WordPressComOAuthClient should likely handle the keychain for auth token - Remove kWPcomXMLRPCUrl as self.xmlrpc for a dot com blog is the same - Collapse various const NSString keys that had the same value
Registers and gets settings via XML RPC API
- Move app usage into NotificationsManager - Make WordPressComApi app-independent
- Move app usage into NotificationsManager - Make WordPressComApi app-independent
- Move app usage into NotificationsManager - Make WordPressComApi app-independent
- Notification was posted for checkForUnseenNotifications - Also turns out that this method is called, but nothing observes the notification - Removed the call from the app in NotificationsManager.
- WPToast: present toast from callers instead - Constants, WPAccount, ContextManager, Note
- Calls to WordPressComApi discovered that relied on the Notes being synced
- Split out the API usage into a category for organization - Rename confusing ‘sync’ method to `mergeNewNotes:` like other models
- checkNotifications -> fetchRecentNotifications; fetches the last 20
- Remove redundant getter, use property getter instead
- While not ideal, it gives more meaning to the API calls - Most Notes related calls sync the notes after the response comes back, so keep this in Note - Remove redundant ‘user’ reference in controller that points to WordPressComApi
…nto feature/22-accounts Conflicts: WordPress/Classes/BlogListViewController.m WordPress/Classes/NotificationsFollowDetailViewController.m WordPress/Classes/ReaderPost.m WordPress/Classes/SettingsViewController.m WordPress/Classes/WPAccount.m
Move the default account URL as recipients of the notification fired attempt to use the default account while deletion is happening, even if it's through the same context. Fixes a visual issue where the sign out did not cause the visual status of settings to be correct.
Results controller should take care of noticing when account changes
Member
Author
There was a problem hiding this comment.
This looks unrelated to accounts and I think we're in string freeze now. Should we move this to a separate ticket/branch? @sendhil @xtreme-rebecca-putinski
Contributor
There was a problem hiding this comment.
@koke - we are in a string freeze, but I see both "Unfollowed" and "Followed" are already localized.
Contributor
|
I think I might be conflicting with some of the changes in PR #1000 - I'll review this PR now |
astralbodies
added a commit
that referenced
this pull request
Jan 3, 2014
Conflicts: WordPress/Classes/BlogSelectorViewController.m WordPress/Classes/Constants.h WordPress/Classes/CreateAccountAndBlogViewController.m WordPress/Classes/EditSiteViewController.m WordPress/Classes/JetpackSettingsViewController.m WordPress/Classes/Note.h WordPress/Classes/NotificationsCommentDetailViewController.m WordPress/Classes/NotificationsManager.m WordPress/Classes/NotificationsViewController.m WordPress/Classes/ReaderCommentFormView.m WordPress/Classes/ReaderPostsViewController.m WordPress/Classes/SettingsViewController.m WordPress/Classes/WPAccount.m WordPress/WordPressApi/WordPressComApi.m
Contributor
|
Overall this looks good - I'm going to wait to merge #1000 until this is in develop and I'll merge develop into there to make sure my APN fixes still apply. |
Contributor
There was a problem hiding this comment.
Was this accidentally excluded?
Contributor
There was a problem hiding this comment.
Hmmm, collateral damage from a merge?
Member
Author
There was a problem hiding this comment.
That constant isn't used anywhere in WPAccount
Contributor
|
So I'm seeing a bunch of compilation errors now that I finally checked out the branch. I'm assuming not intentional 😄. |
Member
Author
|
Code looks good after some merge hiccups, but needs more testing |
Member
Author
|
|
Contributor
|
Looks good for merging |
astralbodies
added a commit
that referenced
this pull request
Jan 7, 2014
Make sure we're using the new account system everywhere
Contributor
|
I'm deleting the branch but we can restore it if needed. |
Contributor
|
💥 |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Some parts of the code still use
[WordPressComApi sharedApi].usernameto get the WordPress.com account. Migrate everything to the newWPAccountsystem.Fixes #22