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

ajax/share.php should return correct list of suggestions #15182

Merged
merged 1 commit into from
Apr 9, 2015
Merged

ajax/share.php should return correct list of suggestions #15182

merged 1 commit into from
Apr 9, 2015

Conversation

rullzer
Copy link
Contributor

@rullzer rullzer commented Mar 25, 2015

Partial solves #8231

We should take advantage of the provided list of users and groups we already share with to filter the suggestions. This makes it harder to do double shares with the same users wich leads to bad UX.

Note to self: This should probably also be done in #13587

We should use the provided list of users and groups that we already
shared with to filter suggestions.
@scrutinizer-notifier
Copy link

The inspection completed: 2 new issues, 1 updated code elements

@ghost
Copy link

ghost commented Mar 25, 2015

Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng-simple/10815/
Test PASSed.

@jancborchardt
Copy link
Member

Does this also help to fix #7128 by any chance? :)

@rullzer
Copy link
Contributor Author

rullzer commented Mar 25, 2015

@jancborchardt I think for most situations yes.

@rullzer
Copy link
Contributor Author

rullzer commented Mar 25, 2015

@jancborchardt after reading it a bit more careful, no it does not!

@jancborchardt
Copy link
Member

Good stuff, fixes the second part of #8231 for me. 👍 Another review please @owncloud/designers?

@rullzer
Copy link
Contributor Author

rullzer commented Mar 25, 2015

So, the other part is slightly harder. Since we currently do not fetch any data about the share. We should probably try to solve that while we try to fix #7128 et al.

@jancborchardt
Copy link
Member

@rullzer do you want to do that in this PR or a separate one?

Another review please @owncloud/designers @schiesbn :)

@rullzer
Copy link
Contributor Author

rullzer commented Mar 26, 2015

@jancborchardt that fix is far less trivial, so different PR.
I do however want to add some unit tests here. But that will not be tonight :)

@DeepDiver1975
Copy link
Member

I do however want to add some unit tests here. But that will not be tonight :)

would be great to get this done these days - THX a lot!

@rullzer
Copy link
Contributor Author

rullzer commented Mar 31, 2015

@DeepDiver1975 I think this code it not unit tested at all currently. So my ini tiny tests will not improve it much. And I do not really feel like adding unit tests for the whole file.

I suggest to just go forward and get rid of all the old ajax/*.php stuff before 8.2

@rullzer rullzer changed the title [WIP] ajax/share.php should return correct list of suggestions ajax/share.php should return correct list of suggestions Mar 31, 2015
@DeepDiver1975
Copy link
Member

👍

DeepDiver1975 added a commit that referenced this pull request Apr 9, 2015
ajax/share.php should return correct list of suggestions
@DeepDiver1975 DeepDiver1975 merged commit 20eaadd into owncloud:master Apr 9, 2015
@rullzer rullzer deleted the fix-8231 branch May 22, 2015 11:51
@lock lock bot locked as resolved and limited conversation to collaborators Aug 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants