Skip to content

Conversation

@PhilLab
Copy link
Contributor

@PhilLab PhilLab commented Jan 15, 2026

Original PR: #16215

  • Tests written, or not not needed

This streamlines the upload from files, e.g. from email attachments.
Closes #9215.
Also offers a convenient pre-selection of the filename without extension when entering the input field, as well as file name validation.

New input field No input field (e.g multi file upload). Also old behavior
Screenshot_20260102_121002 Screenshot_20260102_121341

Recording of the automated test:
(The upload failure in the end is expected, as there is no server mocked)

Screen_recording_20260102_124130.mp4

Technical details

  1. The approach via the listener mFileDisplayNameTransformer was chosen because this allows a potential extension to multiple files (with multiple input fields). Alternatively, ReceiveExternalFilesActivity#uploadFile() could've been used, which requires the storage to a temporary file first
  2. nextFocusForward and nextFocusDown are both needed to support navigation with the Tab key as well as the Enter key.
  3. The automated test is not yet verifying that the upload is actually using the user-provided name. I attempted to mock parts of the involved utility classes, but spent an inappropriate ammount of time without achieving anything.
  4. The automated test also changes the two screenshots, because the test files are prepared in a @Before method which apply to all tests. Screenshot tests are broken on master right now, so I did not feel confident to provide updated screenshots with no way of verifying them. Either they need to be updated later, or the tests should be separated in two files

This streamlines the upload from files, e.g. from email attachments.
Closes #9215.
The approach via the listener mFileDisplayNameTransformer was chosen
because this allows a potential extension to multiple files (with multiple
input fields). Alternatively, ReceiveExternalFilesActivity#uploadFile()
could've been used, which requires the storage to a temporary file
first.

nextFocusForward and nextFocusDown are both needed to support navigation
with the Tab key as well as the Enter key

Signed-off-by: Philipp Hasper <vcs@hasper.info>
This streamlines the renaming and speeds up the process.

Signed-off-by: Philipp Hasper <vcs@hasper.info>
When running the screenshot tests, the following error was raised:

com.owncloud.android.ui.activity.ReceiveExternalFilesActivityIT > openMultiAccount[uiComparison(AVD) - 9] FAILED
java.lang.AssertionError: Can't create multiple screenshots with the same name: com.owncloud.android.ui.activity.ReceiveExternalFilesActivityIT_open
at com.facebook.testing.screenshot.internal.AlbumImpl.addRecord

This happened because openMultiAccount() simply calls the open() test which
"hardcodes" the screenshot name

Signed-off-by: Philipp Hasper <vcs@hasper.info>
The UI only allows to proceed with the upload, if the permissions are
given accordingly, so the test setup needs access to the corresponding
permission keys.
Instead of only making one of the keys available, all were made public.
Because why should these magic strings be private after all? This only
invites duplication and hinders maintenance.

Signed-off-by: Philipp Hasper <vcs@hasper.info>
The automated test only tests this partially, because the only invalid
name was an emtpy name. All other characters which are typically unsuitable
for a filename, like the directory separator '/' are not properly rejected
during the test run, because the FileNameValidator is based on the user
capabilities, which are null because this is a sever-less test.

Signed-off-by: Philipp Hasper <vcs@hasper.info>
Signed-off-by: Philipp Hasper <vcs@hasper.info>
To reduce duplication, the file and folders are setup in a @before method,
but this also affects the existing screenshot tests.

Signed-off-by: Philipp Hasper <vcs@hasper.info>
Working around the limitation of returns as well as the condition
complexity was attempted but made the code just less readable.
Ignoring these two specific findings is more appropriate here.

Signed-off-by: Philipp Hasper <vcs@hasper.info>
Signed-off-by: Philipp Hasper <vcs@hasper.info>
@PhilLab
Copy link
Contributor Author

PhilLab commented Jan 15, 2026

@kra-mo I recreated the PR in the main repo now, so the automated tests are executed.

Regarding your comment:

There should be some more padding atop the entry since the label is quite close to the edge now, but other than that, looks good from the design side. Neat :)

I added one commit which introduces padding at the top. See the comparison:

No padding Padding (standard_half_margin)
image image

@github-actions
Copy link

Codacy

SpotBugs

CategoryBaseNew
Bad practice4343
Correctness7474
Dodgy code257257
Experimental11
Internationalization77
Malicious code vulnerability33
Multithreaded correctness3434
Performance4444
Security1818
Total481481

@github-actions
Copy link

blue-Light-Screenshot test failed, but no output was generated. Maybe a preliminary stage failed.

@github-actions
Copy link

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

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.

Change/choose name of files uploaded via sharing with Nextcloud app

3 participants