Fix: ContactManager search with fullmatch#38907
Merged
JohannesGGE merged 1 commit intomasterfrom Jul 18, 2023
Merged
Conversation
Signed-off-by: Johannes Merkel <mail@johannesgge.de>
fffed18 to
846754e
Compare
ChristophWurst
approved these changes
Jun 29, 2023
Member
ChristophWurst
left a comment
There was a problem hiding this comment.
Works as expected in combination with nextcloud/mail#8551
Member
|
/backport to stable27 |
Member
|
/backport to stable26 |
Member
|
/backport to stable25 |
|
|
||
| if (!$enumeration && !$fullMatch) { | ||
| // No access to system address book AND no full match allowed | ||
| continue; |
Member
There was a problem hiding this comment.
I wonder if this PR could have any unexpected side effects. I don't think it does but let's double-check.
It would be interesting to set a breakpoint here and test if any other features that use contacts (file sharing, contacts menu) behave different with this change.
Contributor
Author
There was a problem hiding this comment.
Did some testing:
- file sharing and calender sharing work as before (seems they got the right result already from the UserPlugin)
- contact menu is working a bit better than before (change has effect and show same group users on group:true and fullmatch:false) but
\OC\Contacts\ContactsMenu\ContactsStore::getContactsis doing some filtering in the end so (fullmatch:true, group:true) doesn't show full matches.
\OC\ContactsManager::search seems to work properly :D
miaulalala
approved these changes
Jul 18, 2023
This was referenced Jul 18, 2023
Contributor
|
Looks like this is also fixed #34637 |
9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
If I understand the sharing options in the admin settings right, the following should lead to the suggestion of a SAB user in the same group, and don't show other SAB users even if there is a full match (name, email).
Currently, because of the 'fullmatch' option, I pass as false in mail, the if condition is triggered and no SAB user is suggested at all. To work correctly I think there should be the check for the access to the SAB as well.
Maybe I am wrong here, correct me if that's the case :)
Checklist