Migrate simple cases of get current account to get user#4853
Merged
tobiasKaminsky merged 3 commits intomasterfrom Nov 26, 2019
Merged
Conversation
ezaquarii
force-pushed
the
migrate-simple-cases-of-get-current-account-to-get-user
branch
from
097addd to
e6fc540
Compare
Collaborator
|
IT test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/11661 |
ezaquarii
force-pushed
the
migrate-simple-cases-of-get-current-account-to-get-user
branch
2 times, most recently
from
0779a6b to
84b0849
Compare
ezaquarii
changed the title
Member
|
I guess this needs to be rebased on master, as newUser PR is now in. |
ezaquarii
force-pushed
the
migrate-simple-cases-of-get-current-account-to-get-user
branch
from
84b0849 to
faa9d77
Compare
Codecov Report
@@ Coverage Diff @@
## master #4853 +/- ##
============================================
+ Coverage 17.62% 17.69% +0.06%
Complexity 3 3
============================================
Files 383 384 +1
Lines 32509 32528 +19
Branches 4587 4588 +1
============================================
+ Hits 5731 5757 +26
+ Misses 25857 25848 -9
- Partials 921 923 +2
|
ezaquarii
force-pushed
the
migrate-simple-cases-of-get-current-account-to-get-user
branch
from
522e79f to
33afcbc
Compare
ezaquarii
force-pushed
the
migrate-simple-cases-of-get-current-account-to-get-user
branch
3 times, most recently
from
c3e5f70 to
dc4f44a
Compare
Collaborator
|
Unit test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/11719 |
ezaquarii
force-pushed
the
migrate-simple-cases-of-get-current-account-to-get-user
branch
2 times, most recently
from
c2473b4 to
436b564
Compare
Collaborator
|
Unit test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/11725 |
| @Deprecated | ||
| boolean isSearchSupported(@Nullable Account account); | ||
|
|
||
| @Deprecated |
Member
There was a problem hiding this comment.
can you add here, what to use instead?
Collaborator
Author
There was a problem hiding this comment.
JavaDoc updated.
| public OwnCloudClient create(User user) throws CreationException { | ||
| try { | ||
| return OwnCloudClientFactory.createOwnCloudClient(user.toPlatformAccount(), context); | ||
| } catch (OperationCanceledException|AuthenticatorException|IOException|AccountUtils.AccountNotFoundException e) { |
Member
There was a problem hiding this comment.
as this is too long, please also line wrap it
Collaborator
Author
There was a problem hiding this comment.
Long exceptions list reformatted into vertical.
|
|
||
| FileDataStorageManager manager = new FileDataStorageManager(account, getContext().getContentResolver()); | ||
| boolean federatedShareAllowed = manager.getCapability(account.name).getFilesSharingFederationOutgoing() | ||
| FileDataStorageManager manager = new FileDataStorageManager(user.toPlatformAccount(), getContext().getContentResolver()); |
| Account account = accountManager.getCurrentAccount(); | ||
| GetActivityListTask getActivityListTask = new GetActivityListTask(account, accountManager, lastGiven, callback); | ||
| User user = accountManager.getUser(); | ||
| GetActivityListTask getActivityListTask = new GetActivityListTask(user.toPlatformAccount(), accountManager, lastGiven, callback); |
src/main/java/com/owncloud/android/ui/adapter/TrashbinListAdapter.java
Outdated
Show resolved
Hide resolved
src/main/java/com/owncloud/android/ui/dialog/ChooseTemplateDialogFragment.java
Outdated
Show resolved
Hide resolved
src/main/java/com/owncloud/android/ui/fragment/ExtendedListFragment.java
Show resolved
Hide resolved
src/main/java/com/owncloud/android/ui/trashbin/RemoteTrashbinRepository.java
Outdated
Show resolved
Hide resolved
Member
|
Nice! Again this is a great PR! Some very small issues and then we can merge. |
Migrate trivially convertible uses of getCurrentAccount() to new user model - getUser(). Signed-off-by: Chris Narkiewicz <hello@ezaquarii.com>
ezaquarii
force-pushed
the
migrate-simple-cases-of-get-current-account-to-get-user
branch
from
1c2ae46 to
a7eb714
Compare
ezaquarii
commented
Nov 25, 2019
Collaborator
Author
ezaquarii
left a comment
ezaquarii
left a comment
There was a problem hiding this comment.
All issues addressed.
Collaborator
Author
|
@tobiasKaminsky Please bear in mind that #4884 is causing CI to fail sporadicaly. Just restart the build if this particular test fails. |
… [skip ci] Signed-off-by: nextcloud-android-bot <android@nextcloud.com>
Collaborator
|
Issues
======
+ Solved 3
- Added 3
Complexity increasing per file
==============================
- src/main/java/com/nextcloud/client/network/ClientFactoryImpl.java 2
- src/main/java/com/owncloud/android/ui/trashbin/RemoteTrashbinRepository.java 1
- src/main/java/com/owncloud/android/ui/activity/NotificationsActivity.java 2
Complexity decreasing per file
==============================
+ src/main/java/com/owncloud/android/providers/UsersAndGroupsSearchProvider.java -2
+ src/main/java/com/owncloud/android/ui/activities/data/files/FilesServiceApiImpl.java -3
See the complete overview on Codacy |
| String accountName = getIntent().getExtras().getString(NotificationJob.KEY_NOTIFICATION_ACCOUNT); | ||
| if(accountName != null && optionalUser.isPresent()) { | ||
| User user = optionalUser.get(); | ||
| if (user.getAccountName().equalsIgnoreCase(accountName)) { |
Collaborator
There was a problem hiding this comment.
| if (optionalUser.isPresent()) { | ||
| mLastDisplayedAccount = optionalUser.get().toPlatformAccount(); | ||
| } else { | ||
| mLastDisplayedAccount = null; |
Collaborator
There was a problem hiding this comment.
| Optional<User> optionalUser = getUser(); | ||
| if (optionalUser.isPresent() && accountName != null) { | ||
| User user = optionalUser.get(); | ||
| if (!accountName.equalsIgnoreCase(user.getAccountName())) { |
Collaborator
There was a problem hiding this comment.
Collaborator
|
IT test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/11734 |
Collaborator
|
APK file: https://www.kaminsky.me/nc-dev/android-artifacts/11743.apk |
Collaborator
Author
|
@tobiasKaminsky |
Collaborator
Codacy312Lint
SpotBugs (new)
SpotBugs (master)
|
AndyScherzinger
approved these changes
Nov 26, 2019
Member
|
Ha finally greeeen :-) |
delete-merged-branch
bot
deleted the
migrate-simple-cases-of-get-current-account-to-get-user
branch
tobiasKaminsky
added a commit
that referenced
this pull request
Nov 27, 2019
e92daa2 Merge pull request #4853 from nextcloud/migrate-simple-cases-of-get-current-account-to-get-user 99d9a69 Merge pull request #4885 from nextcloud/disable_android_backup_qa 10960bf [tx-robot] updated from transifex 9f24b25 [tx-robot] updated from transifex b7876c4 disable GoogleAppIndexingWarning lint check 11687c6 Drone: update FindBugs results to reflect reduced error/warning count [skip ci] 80fb800 Merge commit 'a7eb7148fa0ceb42981366eb2ddcf0ff921e6a55' b91136c Disable android backup on the QA app a7eb714 Migrate simple cases of getCurrentAccount() to getUser() 6249a06 daily dev 20191123
joshtrichards
added a commit
that referenced
this pull request
Feb 8, 2026
…ovider Remove getCurrentAccount() method from CurrentAccountProvider interface. The method was deprecated in favor of getUser() and had only one external caller (DrawerActivity), which has been migrated to use getUser() instead. The implementation in UserAccountManagerImpl remains as it's still used internally by getUser(), but removing it from the interface prevents new code from depending on the deprecated Account-based API. Refs: #4853, #4835 Signed-off-by: Josh <josh.t.richards@gmail.com>
1 task
alperozturk96
pushed a commit
that referenced
this pull request
Feb 9, 2026
…ovider Remove getCurrentAccount() method from CurrentAccountProvider interface. The method was deprecated in favor of getUser() and had only one external caller (DrawerActivity), which has been migrated to use getUser() instead. The implementation in UserAccountManagerImpl remains as it's still used internally by getUser(), but removing it from the interface prevents new code from depending on the deprecated Account-based API. Refs: #4853, #4835 Signed-off-by: Josh <josh.t.richards@gmail.com>
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.
This branch is an attempt to migrate all trivial use cases of
UserAccountManager.getCurrentAccount()toUserAccountManager.getUser(), leveragingUser.toPlatformAccount()andUser.toOwnCloudAccount()APIs whenever refactoring becomes problematic or too risky.Since
UserAccountManageris injected in top-level component, such asActivities(top), this PR shall open path to migration of internal components, deeper in the object hierarchy (bottom).My initial plan to start from internal componentsand taking migration up to
Activity/Fragment/Servicelevel - botton-top approach - turned out to not be feasible, because various internal componetns require refactoring of the callers anyway (top) atd those refactorings spill horribly all over the codebase.I try to keep the risk of this change in reasonable bounds, but it will require complehensive application testing nevertheless.