Skip to content

Conversation

barisbasar1209
Copy link
Contributor

This pull request addresses issue #7814.

Previously:

Whenever the user searches for and requests a file or folder through the traymenu-searchbar...

  • Nextcloud Server Version 20: ...the containing folder is opened in the local file explorer, instead of the requested file or folder itsself.
  • Nextcloud Server Version above 20: ...the requested file or folder is opened, but in the webinterface filesystem application.

Change:

For any server version using the unified search (20+) the requested file or folder is opened in the local file explorer

Authors are @mike0609king and me.

Thanks a lot in advance for review and suggestions!

Copy link
Collaborator

@mgallien mgallien left a comment

Choose a reason for hiding this comment

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

see my inline comments
can you also make sure that there are new automated tests covering the new code branches ?
we will need to have tests to understand if the changes are working as expected

@barisbasar1209
Copy link
Contributor Author

@mgallien I have two more questions:

  1. I used the sign off and commit suggestions button to accept your suggestions. Is this the preferred way to go? (Maybe I should have asked in advance, I am sorry)
  2. Are the parameters in 64f2b0b really supposed to be formatted this way?

Thanks for your help!

@barisbasar1209 barisbasar1209 force-pushed the issue7814 branch 3 times, most recently from bd594c8 to bd7d413 Compare August 27, 2025 19:14
Copy link

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

@mike0609king
Copy link
Contributor

@mgallien The automated tests have been implemented; the newly implemented code has 100% coverage. We have quite a few edge cases in terms of what nextcloud returns in the subline and title section. Furthermore, the behavior is different for nextcloud version 20 and nextcloud version 20+.

We also decided to make tests independent of the results that have been defined in initSearchResultData, because

  1. we wanted to avoid breaking already implemented tests,
  2. the existing data in the tests is very redundant for our test cases; same entries in subline and title and
  3. we wanted to avoid to do the typical setup of those tests, i.e. setSearchTerm and `wait. This would make the code less readable.

@mgallien mgallien force-pushed the issue7814 branch 2 times, most recently from 9ba7ea1 to 1eee657 Compare September 11, 2025 07:06
Copy link

Artifact containing the AppImage: nextcloud-appimage-pr-8575.zip

Digest: sha256:5ac666978c7c488498bd210891a0644417d1bc60281065cd54523cce45ebc9d9

To test this change/fix you can download the above artifact file, unzip it, and run it.

Please make sure to quit your existing Nextcloud app and backup your data.

@mike0609king
Copy link
Contributor

Forgot to restore old accountState value after setting it to a null pointer. This caused tests after testSearchResultClicked to fail. IS fixed now.

barisbasar1209 and others added 12 commits September 23, 2025 15:58
Co-authored-by: Matthieu Gallien <[email protected]>
Signed-off-by: Barış <[email protected]>
Co-authored-by: Matthieu Gallien <[email protected]>
Signed-off-by: Barış <[email protected]>
Co-authored-by: Matthieu Gallien <[email protected]>
Signed-off-by: Barış <[email protected]>
Co-authored-by: Matthieu Gallien <[email protected]>
Signed-off-by: Barış <[email protected]>
Co-authored-by: Matthieu Gallien <[email protected]>
Signed-off-by: Barış <[email protected]>
Co-authored-by: Matthieu Gallien <[email protected]>
Signed-off-by: Barış <[email protected]>
Co-authored-by: Matthieu Gallien <[email protected]>
Signed-off-by: Barış <[email protected]>
Co-authored-by: Matthieu Gallien <[email protected]>
Signed-off-by: Barış <[email protected]>
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.

3 participants