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.

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

Don't re-filter autocomplete collaborators results for remote user #2569

Merged
merged 2 commits into from
Nov 29, 2019

Conversation

PVince81
Copy link
Contributor

Description

The autocomplete for adding collaborators was filtering results that
were already filtered on the server side.

This makes the filterRecipients function return true as no filtering is
necessary.

Related Issue

Fixes #2509

Motivation and Context

How Has This Been Tested?

Manual test with "user1", "user11" and "user2".
Typing "user1" properly filters out "user2" as it's done on the server already.

Also tested "user@x" to make sure a federated share appears in the list as expected in #2509

Expecting that all autocomplete tests in CI will still pass.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

@PVince81 PVince81 added the Status:Needs-Review Needs review from a maintainer label Nov 26, 2019
@PVince81 PVince81 added this to the Milestone 1: Phoenix for users milestone Nov 26, 2019
@PVince81 PVince81 self-assigned this Nov 26, 2019
@ownclouders
Copy link
Contributor

💥 Acceptance tests SharingAutocompletion failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/6801/

20191126-091909-910.png
20191126-092123-874.png
20191126-092512-196.png

@PVince81
Copy link
Contributor Author

Seems there's more to it...

Tests are expecting the filtering to also happen on display names. However as far as I remember the API doesn't always do this, depending on the setup.

@PVince81 PVince81 changed the title Don't re-filter autocomplete collaborators results Don't re-filter autocomplete collaborators results for remote user Nov 26, 2019
@PVince81
Copy link
Contributor Author

I've adjusted the PR to only disable client side filtering for remote users, with the goal to unblock #2509 for now.

We should still look into the general autocomplete filter thing.

@ownclouders
Copy link
Contributor

💥 Acceptance tests SharingAutocompletion failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/6804/

20191126-094700-359.png
20191126-095058-940.png

@PVince81
Copy link
Contributor Author

I've raised owncloud/core#36478 for the server side bug

When testing users and groups we are not expecting remote users to
appear. To make it work, we disable outgoing federated shares.
@PVince81
Copy link
Contributor Author

To fix the failing tests I had to disable federation because when testing the autocomplete for users containing an "@" character, there would be two results instead of one because of federation.

@ownclouders
Copy link
Contributor

💥 Acceptance tests Files failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/6811/

20191126-163839-200.png
20191126-163941-597.png

@dpakach
Copy link
Contributor

dpakach commented Nov 27, 2019

WebUIFiles failed for some reason I have restared drone.

@ownclouders
Copy link
Contributor

💥 Acceptance tests Trashbin failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/6819/

20191127-042843-303.png
20191127-042942-113.png

@PVince81
Copy link
Contributor Author

trashbin also is a random fail. passes locally

@PVince81 PVince81 merged commit 955009d into master Nov 29, 2019
@delete-merged-branch delete-merged-branch bot deleted the fix-remote-user-autocomplete branch November 29, 2019 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status:Needs-Review Needs review from a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Autocomplete not showing in federated share
4 participants