Skip to content

Conversation

@ezaquarii
Copy link
Collaborator

@ezaquarii ezaquarii commented Apr 13, 2019

The implementation delegates calls to old code, so there should
be no regressions in functionality.

This change is huge, because AccountUtils.getCurrentOwnCloudAccount()
is a magic call used almost everywhere.

Not everything is migrated as there are some deep calls of static helpers
that need conversion first: connectivity, themes and whats new.

Signed-off-by: Chris Narkiewicz [email protected]

@ezaquarii ezaquarii force-pushed the ezaquarii/move-current-account-getter-to-user-account-manager branch 4 times, most recently from 69dd965 to 1ec4f1c Compare April 14, 2019 00:45
@nextcloud nextcloud deleted a comment Apr 14, 2019
@nextcloud nextcloud deleted a comment Apr 14, 2019
@nextcloud nextcloud deleted a comment Apr 14, 2019
@nextcloud nextcloud deleted a comment Apr 14, 2019
@nextcloud nextcloud deleted a comment Apr 14, 2019
@nextcloud nextcloud deleted a comment Apr 14, 2019
@nextcloud nextcloud deleted a comment Apr 14, 2019
@nextcloud nextcloud deleted a comment Apr 14, 2019
@nextcloud nextcloud deleted a comment from ezaquarii Apr 14, 2019
@nextcloud nextcloud deleted a comment Apr 14, 2019
@nextcloud nextcloud deleted a comment Apr 14, 2019
@nextcloud nextcloud deleted a comment Apr 14, 2019
@nextcloud nextcloud deleted a comment Apr 14, 2019
@nextcloud nextcloud deleted a comment Apr 14, 2019
@nextcloud nextcloud deleted a comment Apr 14, 2019
@nextcloud nextcloud deleted a comment Apr 14, 2019
@nextcloud nextcloud deleted a comment Apr 14, 2019
@nextcloud nextcloud deleted a comment Apr 14, 2019
@nextcloud nextcloud deleted a comment Apr 14, 2019
@nextcloud nextcloud deleted a comment Apr 14, 2019
@ezaquarii ezaquarii force-pushed the ezaquarii/move-current-account-getter-to-user-account-manager branch from 1ec4f1c to aed82dc Compare April 14, 2019 00:57
@nextcloud nextcloud deleted a comment Apr 14, 2019
@AndyScherzinger
Copy link
Member

Will need to leave this one to @tobiasKaminsky since I am currently without my IDE / laptop.

@ezaquarii ezaquarii force-pushed the ezaquarii/move-current-account-getter-to-user-account-manager branch from aed82dc to 05b38a6 Compare April 14, 2019 14:43
@nextcloud nextcloud deleted a comment Apr 14, 2019
@nextcloud nextcloud deleted a comment Apr 14, 2019
@nextcloud nextcloud deleted a comment from ezaquarii Apr 14, 2019
@ezaquarii ezaquarii force-pushed the ezaquarii/move-current-account-getter-to-user-account-manager branch from 05b38a6 to 23c31de Compare April 14, 2019 19:45
@nextcloud nextcloud deleted a comment Apr 14, 2019
@nextcloud nextcloud deleted a comment Apr 14, 2019
@ezaquarii ezaquarii force-pushed the ezaquarii/move-current-account-getter-to-user-account-manager branch from 23c31de to 8385b88 Compare April 14, 2019 19:58
@codecov
Copy link

codecov bot commented Apr 14, 2019

Codecov Report

Merging #3897 into master will increase coverage by 0.02%.
The diff coverage is 8.78%.

@@             Coverage Diff             @@
##             master   #3897      +/-   ##
===========================================
+ Coverage      6.45%   6.47%   +0.02%     
  Complexity        1       1              
===========================================
  Files           325     324       -1     
  Lines         31247   31286      +39     
  Branches       4479    4481       +2     
===========================================
+ Hits           2018    2027       +9     
- Misses        28931   28963      +32     
+ Partials        298     296       -2
Impacted Files Coverage Δ Complexity Δ
...java/com/nextcloud/client/di/ComponentsModule.java 0% <ø> (ø) 0 <0> (ø) ⬇️
.../java/com/owncloud/android/utils/DisplayUtils.java 6.74% <ø> (+0.02%) 0 <0> (ø) ⬇️
...i/activities/data/files/RemoteFilesRepository.java 100% <ø> (ø) 0 <0> (ø) ⬇️
...m/nextcloud/client/account/UserAccountManager.java 0% <ø> (ø) 0 <0> (ø) ⬇️
...m/owncloud/android/ui/adapter/TemplateAdapter.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...ndroid/ui/dialog/ChooseTemplateDialogFragment.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...oud/android/ui/activity/NotificationsActivity.java 0% <0%> (ø) 0 <0> (ø) ⬇️
.../owncloud/android/files/services/FileUploader.java 0.2% <0%> (ø) 0 <0> (ø) ⬇️
...owncloud/android/ui/adapter/OCFileListAdapter.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...com/owncloud/android/ui/activity/BaseActivity.java 1.35% <0%> (-0.02%) 0 <0> (ø)
... and 46 more

@codecov
Copy link

codecov bot commented Apr 14, 2019

Codecov Report

Merging #3897 into master will increase coverage by 0.02%.
The diff coverage is 10.04%.

@@             Coverage Diff             @@
##             master   #3897      +/-   ##
===========================================
+ Coverage      6.45%   6.48%   +0.02%     
  Complexity        1       1              
===========================================
  Files           325     324       -1     
  Lines         31247   31288      +41     
  Branches       4479    4481       +2     
===========================================
+ Hits           2018    2030      +12     
- Misses        28931   28962      +31     
+ Partials        298     296       -2
Impacted Files Coverage Δ Complexity Δ
...java/com/nextcloud/client/di/ComponentsModule.java 0% <ø> (ø) 0 <0> (ø) ⬇️
.../java/com/owncloud/android/utils/DisplayUtils.java 6.74% <ø> (+0.02%) 0 <0> (ø) ⬇️
...i/activities/data/files/RemoteFilesRepository.java 100% <ø> (ø) 0 <0> (ø) ⬇️
...m/nextcloud/client/account/UserAccountManager.java 0% <ø> (ø) 0 <0> (ø) ⬇️
...m/owncloud/android/ui/adapter/TemplateAdapter.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...ndroid/ui/dialog/ChooseTemplateDialogFragment.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...oud/android/ui/activity/NotificationsActivity.java 0% <0%> (ø) 0 <0> (ø) ⬇️
.../owncloud/android/files/services/FileUploader.java 0.2% <0%> (ø) 0 <0> (ø) ⬇️
...owncloud/android/ui/adapter/OCFileListAdapter.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...com/owncloud/android/ui/activity/BaseActivity.java 1.35% <0%> (-0.02%) 0 <0> (ø)
... and 47 more

@nextcloud nextcloud deleted a comment Apr 14, 2019
The implementation delegates calls to the old code, so there should
be no regressions in functionality.

Signed-off-by: Chris Narkiewicz <[email protected]>
@ezaquarii ezaquarii force-pushed the ezaquarii/move-current-account-getter-to-user-account-manager branch from 8385b88 to 442394f Compare April 14, 2019 20:02
@nextcloud nextcloud deleted a comment Apr 14, 2019
@nextcloud nextcloud deleted a comment Apr 14, 2019
@nextcloud-android-bot
Copy link
Collaborator

Codacy

294

Lint

TypemasterPR
Warnings5757
Errors00

SpotBugs (new)

Warning TypeNumber
Bad practice Warnings25
Correctness Warnings84
Internationalization Warnings15
Malicious code vulnerability Warnings4
Multithreaded correctness Warnings9
Performance Warnings124
Security Warnings48
Dodgy code Warnings130
Total439

SpotBugs (master)

Warning TypeNumber
Bad practice Warnings25
Correctness Warnings84
Internationalization Warnings15
Malicious code vulnerability Warnings4
Multithreaded correctness Warnings9
Performance Warnings124
Security Warnings48
Dodgy code Warnings132
Total441


String AUTHORITY = getContext().getResources().getString(R.string.users_and_groups_search_authority);
ACTION_SHARE_WITH = getContext().getResources().getString(R.string.users_and_groups_share_with);
setActionShareWith(getContext());
Copy link
Member

Choose a reason for hiding this comment

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

Why did you changed this, but not AUTHORITY in line 125?

@tobiasKaminsky
Copy link
Member

👍
This is indeed a big change in terms of code, but as you pointed out no functional change.
I really love it ❤️!

Thank you for this great effort!

I did a quick search to see where getCurrentOwncloudAccount(…) is still used:
image

In RichDocumentsWebView it should be possible to use the new AccountManager, as you already did in this class.

Same for app sort order in AppPreferencesImpl? As we use DI to instantiate it, we can also create AccountManager meanwhile, or?

@tobiasKaminsky
Copy link
Member

If you base this PR onto #3895 then the diff to review would have been a lot less, or am I wrong?

@AndyScherzinger
Copy link
Member

@tobiasKaminsky might be able to help you with restarting builds. Afaik you should be able to just log into the ci server with your Github account and be able to restart builds.

@tobiasKaminsky
Copy link
Member

As discussed remaining migration will follow in upcoming PR.

@tobiasKaminsky tobiasKaminsky merged commit d544fa5 into master Apr 15, 2019
@tobiasKaminsky tobiasKaminsky deleted the ezaquarii/move-current-account-getter-to-user-account-manager branch April 15, 2019 13:01
@tobiasKaminsky
Copy link
Member

Thanks again for this awesome contribution!

@AndyScherzinger AndyScherzinger added this to the Nextcloud App 3.7.0 milestone Apr 15, 2019
nextcloud-android-bot pushed a commit that referenced this pull request May 2, 2019
71f3370 Drone: update FindBugs results to reflect reduced error/warning count [skip ci]
0f50088 Merge pull request #3916 from jmue/cleanup/string_compare
7bb07a0 Update wording for store text (#3903)
4c72f3f Merge pull request #3923 from nextcloud/editOnShareLink
81ed32f allow edit on link share on a file
7d01ff7 - fix IT tests - re-organized build.gradle a bit
a9afff7 Merge pull request #3929 from nextcloud/dependabot/gradle/org.powermock-powermock-core-2.0.2
71c6d99 bump all other powermock parts to 2.0.2
0421d26 Bump powermock-core from 2.0.0 to 2.0.2
1d28d1b Update with new wording from frontpage
de3d3de Merge pull request #3924 from nextcloud/warnOnStable
69dd243 warn for wrong library branch on stable
2379b49 Merge pull request #3921 from nextcloud/dependabot/gradle/com.android.tools.build-gradle-3.4.0
c2730e8 Bump gradle from 3.3.2 to 3.4.0
b69169f unify empty string compare
8bb654b fix test & review comments
d8b9d10 unify empty string compare
2f23a30 remove unused variable (#3920)
92df32e Merge pull request #3918 from nextcloud/codacyCleanup
091c003 Drone: update FindBugs results to reflect reduced error/warning count [skip ci]
15ec487 Drone: update FindBugs results to reflect reduced error/warning count [skip ci]
da3c497 Merge commit '334155e707975e740a0a13b33d9b7bbedf40e9bc'
b22f517 Merge pull request #3915 from jmue/cleanup/ocfile
c7c6a98 use constant
e6604c8 use constant
caa2a91 Merge pull request #3912 from nextcloud/libraryUsage
334155e some cleanups
28f80cd Merge pull request #3907 from nextcloud/ezaquarii/initialize-main-app-context-before-content-providers-start
2afb934 Drone: update FindBugs results to reflect reduced error/warning count [skip ci]
8be1157 Merge pull request #3899 from jmue/cleanup/uriutils
52020a9 direct usage of library project
4062d40 Merge pull request #3908 from nextcloud/missingInjectable
2a08cb4 Initialize global context before MainApp.onCreate()
f02f482 Missing Injectable lead to half-stored account
02cbafe Merge pull request #3765 from nextcloud/showSharees
dc7a2c0 show sharees
03e04e5 Merge pull request #3889 from nextcloud/checkUserId
d65e94b Update wording for store text
585a3d0 Drone: update FindBugs results to reflect reduced error/warning count [skip ci]
533debf - removed useNextcloudUserAgent -> we now use this everytime - removed any oAuth and saml stuff, as we rely on weblogin flow
d544fa5 Merge pull request #3897 from nextcloud/ezaquarii/move-current-account-getter-to-user-account-manager
5b4ffba Merge pull request #3900 from nextcloud/codacyOnMaster
2757244 remove unused code
428a8bb show codacy count only on master
6b44074 Drone: update FindBugs results to reflect reduced error/warning count [skip ci]
c62e89b Merge commit '442394f949ff2de86bcccae172d2e9d997ab2ef8'
442394f Migrate current account getter from AccountUtils to UserAccountManager
182b65e daily dev 20190414
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants