Missing Injectable lead to half-stored account

[tobiasKaminsky]

Otherwise this

android/src/main/java/com/owncloud/android/authentication/AuthenticatorActivity.java

Line 1669 in 33d8b66

AccountdefaultAccount = accountManager.getCurrentAccount();

lead to a NPE.

Signed-off-by: tobiasKaminsky tobias@kaminsky.me

[tobiasKaminsky]

I am merging this right away (after CI), otherwise more user/dev could stumble across it.

[AndyScherzinger]

@tobiasKaminsky builds fail with compile errors!

[tobiasKaminsky]

@tobiasKaminsky builds fail with compile errors!

Yes, needs #3765 first. Sorry for the mess :/

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (master@02cbafe). Click here to learn what that means.
The diff coverage is 0%.

@@ Coverage Diff @@## master #3908 +/- ##
========================================
Coverage ? 6.57% Complexity ? 1 ========================================
Files ? 320 Lines ? 30876 Branches ? 4411 ========================================
Hits ? 2031 Misses ? 28549 Partials ? 296

Impacted Files	Coverage Δ	Complexity Δ
.../android/authentication/AuthenticatorActivity.java	1.87% <ø> (ø)	0 <0> (?)
.../authentication/ModifiedAuthenticatorActivity.java	0% <0%> (ø)	0 <0> (?)

[nextcloud-android-bot]

Codacy

300

Lint

Type	master	PR
Warnings	57	57
Errors	0	0

SpotBugs (new)

Warning Type	Number
Bad practice Warnings	25
Correctness Warnings	80
Internationalization Warnings	15
Malicious code vulnerability Warnings	4
Multithreaded correctness Warnings	9
Performance Warnings	124
Security Warnings	47
Dodgy code Warnings	132
Total	436

SpotBugs (master)

Warning Type	Number
Bad practice Warnings	25
Correctness Warnings	80
Internationalization Warnings	15
Malicious code vulnerability Warnings	4
Multithreaded correctness Warnings	9
Performance Warnings	124
Security Warnings	47
Dodgy code Warnings	132
Total	436

[ezaquarii]

This is the right solution.

This problem is also fixed by #3907 (otherwise I coudn't run it) in exactly the same way. However, I reformatted the long implements list vertically, so there might be conflict if you try to merge both.

[AndyScherzinger]

So which one should we merge then? :)

[ezaquarii]

So which one should we merge then? :)

@AndyScherzinger If you ask me, #3907 will make my life easier. I have 2 other PRs that depend on it that could be made ready by tomorrow business hours. ;)

[AndyScherzinger]

I am fine with that. Unfortunately @tobiasKaminsky won't respond before tomorrow, business hours ;)

[AndyScherzinger]

@ezaquarii just looked at the changes in both PRs. Are you sure we are talking about the same PRs. Them being #3907 and #3908?

[ezaquarii]

DOH! You are right, I just confused it with other PR I'm working on. Too many branches...

[ezaquarii]

It's #3898 that fixes the authenticator activity.

[AndyScherzinger]

No worries! So merge this one then?

[ezaquarii]

I had to fix SpotBugs. It found a new issue in code I didn't touch, but I cargo-culted it by taking down something else. Let's wait for CI and merge.

[tobiasKaminsky]

However, I reformatted the long implements list vertically, so there might be conflict if you try to merge both.

Personally I prefer to have it in one row, as it is not that often read and otherwise lengthen the code, but I have no hard feelings about it ;-)
We should just have a common rule set for such stuff.

[tobiasKaminsky]

I merged this as a quick fix and will have a look at #3989 soon.