Skip to content

Fix external storage directory shown with shared folder icon overlay - #17997

Open
daniele-verducci wants to merge 4 commits into
masterfrom
fix/external-storage-icon
Open

Fix external storage directory shown with shared folder icon overlay#17997
daniele-verducci wants to merge 4 commits into
masterfrom
fix/external-storage-icon

Conversation

@daniele-verducci

@daniele-verducci daniele-verducci commented Nov 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Found this bug while working on the android side of nextcloud/server#22547
This fixes issue #1289

How to repro:

  • Use Nextcloud instance with at least an external storage folder
  • Open app: note external storage folder icon: it's the shared folder one (the one with 3 people) instead of the external storage one (the one with square and arrow pointing outside)

🖼️ Screenshots

🏚️ Before 🏡 After
image image

🏁 Checklist

  • ⛑️ Tests (unit and/or integration) are included or not needed
  • 🔙 Backport requests are created or not needed: /backport to stable-xx.x
  • 📅 Milestone is set
  • 🌸 PR title is meaningful (if it should be in the changelog: is it meaningful to users?)

🤖 AI (if applicable)

  • The content of this PR was partly or fully generated using AI (N/A)

@daniele-verducci daniele-verducci added this to the Nextcloud App 35.2.0 milestone Nov 9, 2026
@daniele-verducci daniele-verducci mentioned this pull request Nov 9, 2026
Open
@github-actions

github-actions Bot commented Nov 9, 2026 •
edited
Loading

Copy link
Copy Markdown

📱 QA build
Download app-qa-debug.apk
QR code Open the QR code for this download
Commit bacc779
Version 17997
Available until 7 days after this build

The QA build installs alongside a released Nextcloud app, so you can keep
using your existing install while testing.

Downloading the file requires a GitHub account, so open this link on the
device you want to test on, or transfer the APK to it.

alperozturk96
alperozturk96 requested changes Nov 9, 2026
View reviewed changes

@alperozturk96 alperozturk96 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Tests are failing and why mounted() removed?

Image

@daniele-verducci
daniele-verducci force-pushed the fix/external-storage-icon branch from 801590d to 446b8ba Compare November 10, 2026 08:49
@daniele-verducci

daniele-verducci commented Nov 10, 2026

Copy link
Copy Markdown
Contributor Author

Tests are failing and why mounted() removed?

About mounted(): it only checks if it has mounted permission, and that's true for every external storage dir, so it never evaluated the external storage branch. I think it's a leftover, it doesn't make a lot of sense to me... what do you think?
I tested with some different shares and external storages and the result is what I'd expect, but of course you know the project better.

About the tests, I'll check, thx.

@daniele-verducci
daniele-verducci force-pushed the fix/external-storage-icon branch from d451749 to 9a216a4 Compare November 10, 2026 15:12
@daniele-verducci

daniele-verducci commented Nov 10, 2026

Copy link
Copy Markdown
Contributor Author

About the tests: they were failing because I assumed MountType was always present, but it's not if the file doesn't come from the server (i.e. the ones mocked in tests). I fixed it

@codecov

codecov Bot commented Nov 10, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...ils/extensions/FileDataStorageManagerExtensions.kt 75.00% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@daniele-verducci
daniele-verducci force-pushed the fix/external-storage-icon branch from 4b299df to 59bce05 Compare November 11, 2026 09:29
@daniele-verducci
Fix external storage directory shown with shared folder icon overlay
274519f 
Signed-off-by: daniele-verducci <daniele.verducci@nextcloud.com>
@daniele-verducci
Do not default to MountType.INTERNAL, as can exist files with no moun…
53dc885 
…tType (i.e. the ones created by tests)

Signed-off-by: daniele-verducci <daniele.verducci@nextcloud.com>
@daniele-verducci
Clearer variable names
81f13c8 
Signed-off-by: daniele-verducci <daniele.verducci@nextcloud.com>
@daniele-verducci
Suppress spotbugs warnings for enums that are meant to be nullable
bacc779 
Signed-off-by: daniele-verducci <daniele.verducci@nextcloud.com>
@daniele-verducci
daniele-verducci force-pushed the fix/external-storage-icon branch from 4014c87 to bacc779 Compare November 13, 2026 08:09
alperozturk96
alperozturk96 requested changes Nov 13, 2026
View reviewed changes
Comment thread app/src/main/java/com/owncloud/android/datamodel/OCFile.java
encrypted = false;
readOnly = false;
mountType = WebdavEntry.MountType.INTERNAL;
mountType = null;

@alperozturk96 alperozturk96 Nov 13, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can you mark getMountType via @Nullable and check every call?

@github-actions

github-actions Bot commented Nov 13, 2026

Copy link
Copy Markdown

SpotBugs

CategoryBaseNew
Bad practice2424
Correctness4645
Dodgy code125125
Experimental11
Malicious code vulnerability11
Multithreaded correctness1313
Performance2525
Security1515
Total250249

@github-actions

github-actions Bot commented Nov 13, 2026

Copy link
Copy Markdown

stable22-IT test failed, but no output was generated. Maybe a preliminary stage failed.

@github-actions

github-actions Bot commented Nov 13, 2026

Copy link
Copy Markdown

master-IT test failed, but no output was generated. Maybe a preliminary stage failed.

@github-actions

github-actions Bot commented Nov 13, 2026

Copy link
Copy Markdown

stable35-IT test failed, but no output was generated. Maybe a preliminary stage failed.

This branch has not been deployed

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

Reviewers

@alperozturk96 alperozturk96 alperozturk96 requested changes
@tobiasKaminsky tobiasKaminsky Awaiting requested review from tobiasKaminsky

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

Nextcloud App 35.2.0

Development

Successfully merging this pull request may close these issues.

2 participants

@daniele-verducci @alperozturk96