Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix crash caused UserAccountManager dependency not being injected #4111

Merged
merged 1 commit into from
Jun 3, 2019
Merged

Fix crash caused UserAccountManager dependency not being injected #4111

merged 1 commit into from
Jun 3, 2019

Conversation

ezaquarii
Copy link
Collaborator

@ezaquarii ezaquarii commented Jun 2, 2019 •
edited by AndyScherzinger
Loading

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

Resolves #4107

@ezaquarii ezaquarii mentioned this pull request Jun 2, 2019
Closed
@ezaquarii ezaquarii force-pushed the ezaquarii/fix-crash-in-text-preview-fragment branch from 32bc73e to 30af6a9 Compare June 2, 2019 20:11
@ezaquarii
Copy link
Collaborator Author

@tobiasKaminsky @AndyScherzinger I think this should be merged ASAP.

ezaquarii
ezaquarii commented Jun 2, 2019
View reviewed changes
src/main/java/com/owncloud/android/ui/preview/PreviewTextFragment.java
@@ -67,7 +68,7 @@
import androidx.appcompat.widget.SearchView;
import androidx.core.view.MenuItemCompat;

public class PreviewTextFragment extends FileFragment implements SearchView.OnQueryTextListener {
public class PreviewTextFragment extends FileFragment implements SearchView.OnQueryTextListener, Injectable {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Missing Injectable caused the user account to be null and - surprise, surprise - NPE.
To test this code path just try opening any txt file. It should not crash.

@ezaquarii ezaquarii self-assigned this Jun 2, 2019
@ezaquarii
Copy link
Collaborator Author

ezaquarii commented Jun 2, 2019 •
edited
Loading

It looks like emulator died. There is no chance this fix causes a build failure.

Restarting...

@ezaquarii
Copy link
Collaborator Author

Build fails again, despite no errors in log.
Analysis is stuck for 90 minutes already...

@nextcloud-android-bot
Copy link
Collaborator

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/9603.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

310

Lint

TypemasterPR
Warnings5858
Errors00

SpotBugs (new)

Warning TypeNumber
Bad practice Warnings25
Correctness Warnings68
Internationalization Warnings12
Malicious code vulnerability Warnings4
Multithreaded correctness Warnings9
Performance Warnings121
Security Warnings47
Dodgy code Warnings137
Total423

SpotBugs (master)

Warning TypeNumber
Bad practice Warnings25
Correctness Warnings68
Internationalization Warnings12
Malicious code vulnerability Warnings4
Multithreaded correctness Warnings9
Performance Warnings121
Security Warnings47
Dodgy code Warnings137
Total423

@codecov
Copy link

codecov bot commented Jun 3, 2019 •
edited
Loading

Codecov Report

Merging #4111 into master will decrease coverage by 0.04%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #4111      +/-   ##
============================================
- Coverage     12.81%   12.77%   -0.05%     
  Complexity        1        1              
============================================
  Files           332      332              
  Lines         31068    31068              
  Branches       4431     4431              
============================================
- Hits           3981     3968      -13     
- Misses        26413    26430      +17     
+ Partials        674      670       -4
Impacted Files Coverage Δ Complexity Δ
...ncloud/android/ui/preview/PreviewTextFragment.java 0% <ø> (ø) 0 <0> (ø) ⬇️
...owncloud/android/ui/adapter/DiskLruImageCache.java 31.16% <0%> (-7.8%) 0% <0%> (ø)
...in/java/com/owncloud/android/utils/ThemeUtils.java 52.31% <0%> (-1.86%) 0% <0%> (ø)
...loud/android/datamodel/FileDataStorageManager.java 25.65% <0%> (-0.48%) 0% <0%> (ø)
.../java/com/owncloud/android/utils/DisplayUtils.java 18.72% <0%> (-0.38%) 0% <0%> (ø)
.../third_parties/daveKoeller/AlphanumComparator.java 82.14% <0%> (+1.19%) 0% <0%> (ø) ⬇️
...in/java/com/owncloud/android/datamodel/OCFile.java 61.92% <0%> (+1.37%) 0% <0%> (ø) ⬇️

@AndyScherzinger AndyScherzinger merged commit 81993ee into master Jun 3, 2019
@AndyScherzinger
Copy link
Member

Latest build worked 👍

@AndyScherzinger AndyScherzinger deleted the ezaquarii/fix-crash-in-text-preview-fragment branch June 3, 2019 03:53
@AndyScherzinger AndyScherzinger added this to the Nextcloud App 3.7.0 milestone Jun 3, 2019
@ralfi
Copy link

ralfi commented Jun 3, 2019

Yeah, worked also here ...

tobiasKaminsky added a commit that referenced this pull request Jun 4, 2019
@tobiasKaminsky
daily dev 20190604
09cca69 
81993ee Merge pull request #4111 from nextcloud/ezaquarii/fix-crash-in-text-preview-fragment
30af6a9 Fix crash caused UserAccountManager dependency not being injected
4a2a686 [tx-robot] updated from transifex
b002f11 daily dev 20190601
@tobiasKaminsky
Copy link
Member

Thank you @ezaquarii for finding the cause and also directly provide a PR ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndyScherzinger AndyScherzinger AndyScherzinger approved these changes

@tobiasKaminsky tobiasKaminsky Awaiting requested review from tobiasKaminsky

Assignees

@ezaquarii ezaquarii

Labels
3. to review
Projects
None yet
Milestone
Nextcloud App 3.7.0
Development

Successfully merging this pull request may close these issues.

Nextcloud-dev crashed while open txt files
5 participants
@ezaquarii @nextcloud-android-bot @AndyScherzinger @ralfi @tobiasKaminsky