Skip to content

Conversation

@tobiasKaminsky
Copy link
Member

@tobiasKaminsky tobiasKaminsky commented Jan 8, 2020

Fix #5112

I also removed finish as it was not in there before.

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

@tobiasKaminsky
Copy link
Member Author

/backport to stable 3.10

@nextcloud-android-bot
Copy link
Collaborator

Copy link
Collaborator

@ezaquarii ezaquarii left a comment

Choose a reason for hiding this comment

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

I believe, the safest approach with minimal regression risk would be to disable this particular code path in FirstRunActivity using a if and boolean flag:

class BaseActivity ... {
    protected enableAccountHandling = true;

    @Override
    protected void onCreate(@Nullable Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        if (enableAccountHandling) {
            Account account = accountManager.getCurrentAccount();
            setAccount(account, false);
        }
    }

...
     if (newAccount == null) {
            /// no account available: force account creation
            createAccount(true);
            if (enableAccountHandling) {
                finish();
            }
        } else {
            currentAccount = newAccount;
        }
...
}

and in FirstRunActivity:

public void onCreate(...) {
    enableAccountHandling = false;
    super.onCreate();
    ...
}

}

@Override
protected void onCreate(@Nullable Bundle savedInstanceState) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This part was moved in this commit:

d7d1c47

If you loot at it, you'll see that this code was previously in a wrong onCreate(@Nullable Bundle savedInstanceState, @Nullable PersistableBundle persistentState) method.

It was a bug that I introduced while refactoring Account-related code in BaseActivity, caused by IDE autocompletion - I simply typed wrong method signature while overriding onCreate. We didn't notice this as it works by accident simply becuase numerious calls to setAccount in derived activities.

if (newAccount == null) {
/// no account available: force account creation
createAccount(true);
finish();
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is what closes the FirstRunActivity.

Removing it wil break FileListActivity, causing empty file list after creating new account:
#5031 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup, that's indeed what happens :-/ @tobiasKaminsky ?

@tobiasKaminsky
Copy link
Member Author

/backport to stable-3.10

Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>
@nextcloud-android-bot
Copy link
Collaborator

Codacy Here is an overview of what got changed by this pull request:

Issues
======
+ Solved 1
           

See the complete overview on Codacy

@nextcloud-android-bot
Copy link
Collaborator

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/12215.apk

qrcode

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

@nextcloud-android-bot
Copy link
Collaborator

Codacy

341

Lint

TypemasterPR
Warnings7474
Errors00

SpotBugs (new)

Warning TypeNumber
Bad practice Warnings26
Correctness Warnings71
Internationalization Warnings13
Malicious code vulnerability Warnings5
Multithreaded correctness Warnings9
Performance Warnings107
Security Warnings44
Dodgy code Warnings139
Total414

SpotBugs (master)

Warning TypeNumber
Bad practice Warnings26
Correctness Warnings71
Internationalization Warnings13
Malicious code vulnerability Warnings5
Multithreaded correctness Warnings9
Performance Warnings107
Security Warnings44
Dodgy code Warnings139
Total414

@codecov
Copy link

codecov bot commented Jan 10, 2020

Codecov Report

Merging #5122 into master will increase coverage by 0.02%.
The diff coverage is 71.42%.

@@             Coverage Diff              @@
##             master    #5122      +/-   ##
============================================
+ Coverage     17.71%   17.74%   +0.02%     
  Complexity        3        3              
============================================
  Files           392      392              
  Lines         33116    33120       +4     
  Branches       4651     4653       +2     
============================================
+ Hits           5868     5878      +10     
+ Misses        26292    26288       -4     
+ Partials        956      954       -2
Impacted Files Coverage Δ Complexity Δ
.../nextcloud/client/onboarding/FirstRunActivity.java 38.04% <100%> (+0.68%) 0 <0> (ø) ⬇️
...com/owncloud/android/ui/activity/BaseActivity.java 37.63% <66.66%> (+0.96%) 0 <0> (ø) ⬇️
...oud/android/ui/activity/SyncedFoldersActivity.java 26.31% <0%> (-0.51%) 0% <0%> (ø)
.../third_parties/daveKoeller/AlphanumComparator.java 82.14% <0%> (ø) 0% <0%> (ø) ⬇️
...ncloud/android/ui/fragment/OCFileListFragment.java 25.6% <0%> (+0.4%) 0% <0%> (ø) ⬇️
...loud/android/datamodel/ThumbnailsCacheManager.java 32.84% <0%> (+0.54%) 0% <0%> (ø) ⬇️
...ain/java/com/owncloud/android/ui/TextDrawable.java 81.48% <0%> (+7.4%) 0% <0%> (ø) ⬇️
...m/nextcloud/client/account/UserAccountManager.java 33.33% <0%> (+33.33%) 0% <0%> (ø) ⬇️

@tobiasKaminsky tobiasKaminsky merged commit b7a7772 into master Jan 10, 2020
@delete-merged-branch delete-merged-branch bot deleted the firstRun branch January 10, 2020 11:27
@backportbot-nextcloud
Copy link

backport to stable-3.10 in #5147

@AndyScherzinger AndyScherzinger added this to the Nextcloud App 3.11.0 milestone Jan 10, 2020
tobiasKaminsky added a commit that referenced this pull request Jan 12, 2020
ec515d0 [tx-robot] updated from transifex
422a61d Merge pull request #5136 from nextcloud/systemDefault
6e96e99 Merge pull request #5130 from nextcloud/createRichWorkspace
b7a7772 Merge pull request #5122 from nextcloud/firstRun
603825e update screenshot
c4d5e41 Merge pull request #5143 from nextcloud/background
c6d89ed only allow rich workspace creation on >= Lollipop
87dae2e show first run again
9125285 manually increasing lint due to "autoMirrored" warning
4c10cf5 launcher: correct folder position according to AS
51fdf94 [tx-robot] updated from transifex
e0b77f9 daily dev 20200110
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.

FirstRun is not shown

6 participants