Search also the email and displayname in user mangement for groups

[MorrisJobke]

Similar to #17800 but also searches the email address.

This streamlines the search in groups with the one in users:

server/lib/private/User/Database.php

Lines 266 to 278 in 145eee9

	$query->select('uid', 'displayname')
	->from($this->table, 'u')
	->leftJoin('u', 'preferences', 'p', $query->expr()->andX(
	$query->expr()->eq('userid', 'uid'),
	$query->expr()->eq('appid', $query->expr()->literal('settings')),
	$query->expr()->eq('configkey', $query->expr()->literal('email')))
	)
	// sqlite doesn't like re-using a single named parameter here
	->where($query->expr()->iLike('uid', $query->createPositionalParameter('%' . $this->dbConn->escapeLikeParameter($search) . '%')))
	->orWhere($query->expr()->iLike('displayname', $query->createPositionalParameter('%' . $this->dbConn->escapeLikeParameter($search) . '%')))
	->orWhere($query->expr()->iLike('configvalue', $query->createPositionalParameter('%' . $this->dbConn->escapeLikeParameter($search) . '%')))
	->orderBy($query->func()->lower('displayname'), 'ASC')
	->orderBy('uid_lower', 'ASC')

See #13951

• when testing this it doesn't show the results - but executing the query by hand finds the correct users. Any idea what causes the trouble here?

[blizzz]

when testing this it doesn't show the results - but executing the query by hand finds the correct users. Any idea what causes the trouble here?

WFM ™️

[MorrisJobke]

[x] when testing this it doesn't show the results - but executing the query by hand finds the correct users. Any idea what causes the trouble here?

Works for me now.

Unfortunately the unified search made the search field unavailable for the user management. Thus I based this on commit 7b91cb5 which is the one right before the unified search got merged. Then we can still test this and work on a solution for this in 20.

@skjnldsv Is this the wanted behavior of the unified search? Because some apps (like the user management here or the app management) use this for filtering the current view.

@juliushaertl @nickvergessen @kesselb Mind to review this?

@blizzz Does a backport make sense here?

[skjnldsv]

@skjnldsv Is this the wanted behavior of the unified search? Because some apps (like the user management here or the app management) use this for filtering the current view.

We're discussing this.
The apps are going to appear in the unified search #22102
For the users it's up to @jancborchardt

[MorrisJobke]

For the users it's up to @jancborchardt

Same for app management, right? Because it's an admin task vs an normal use case. But up to @jancborchardt

[nickvergessen]

Can we have a dedicated test added?

[MorrisJobke]

Can we have a dedicated test added?

I added some integration tests to verify the outside view of this. Otherwise I need to have mocked so much, that it barely is useful in the future.

[MorrisJobke]

I added some integration tests to verify the outside view of this. Otherwise I need to have mocked so much, that it barely is useful in the future.

I refined them a bit and they work locally. Also did some negative testing locally so that they actually indicate when something breaks and it is the case.

Ready for a final review :)

[MorrisJobke]

/backport to stable19

[jancborchardt]

Yep, so there we hit the other part of what we discussed – unified search works great overall, but it does have drawbacks when you are inside an app itself. There e.g. in Talk, Users and Apps, direct filtering of the content is nicer.

This is why in the spec we discussed that the apps which already have in-app search which do proper filtering:

• Can move their in-app search/filter to the top of the navigation, just like Talk – this would work nicely here for User management as well as App management
• Still get the benefit of the unified search in the top right.

Makes sense?

[MorrisJobke]

This is why in the spec we discussed that the apps which already have in-app search which do proper filtering:

• Can move their in-app search/filter to the top of the navigation, just like Talk – this would work nicely here for User management as well as App management
• Still get the benefit of the unified search in the top right.

Makes sense?

How to differentiate then between the filter and the unified search? Or should we maybe move the filtering from the header into the actual app content?

[faily-bot]

🤖 beep boop beep 🤖

Here are the logs for the failed build:

Status of 31405: failure

carddavtester-new-endpoint

• clone failure

samba

• clone failure

mysql5.6-php7.2

Show full log

There were 2 warnings:

1) Test\Files\ViewTest::testRenameFailDeleteTargetKeepSource
Trying to configure method "writeStream" which cannot be configured because it does not exist, has not been specified, is final, or is static

2) Test\Files\ViewTest::testCopyFailDeleteTargetKeepSource
Trying to configure method "writeStream" which cannot be configured because it does not exist, has not been specified, is final, or is static

--

There were 2 failures:

1) Test\Files\Storage\Wrapper\QuotaTest::testFWriteNotEnoughSpace
Failed asserting that 6 matches expected 3.

/drone/src/tests/lib/Files/Storage/Wrapper/QuotaTest.php:107

2) OCA\Files_Sharing\Tests\SharedMountTest::testPermissionMovedGroupShare with data set #110 ('folder', 21, 11)
Failed asserting that false is true.

/drone/src/apps/files_sharing/tests/SharedMountTest.php:367

postgres11-php7.2

Show full log

There were 2 warnings:

1) Test\Files\ViewTest::testRenameFailDeleteTargetKeepSource
Trying to configure method "writeStream" which cannot be configured because it does not exist, has not been specified, is final, or is static

2) Test\Files\ViewTest::testCopyFailDeleteTargetKeepSource
Trying to configure method "writeStream" which cannot be configured because it does not exist, has not been specified, is final, or is static

--

There was 1 failure:

1) OCA\Files_Versions\Tests\VersioningTest::testRestoreMovedShare
File content has not changed
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'version 2'
+'version 1'

/drone/src/apps/files_versions/tests/VersioningTest.php:729

[jancborchardt]

How to differentiate then between the filter and the unified search? Or should we maybe move the filtering from the header into the actual app content?

Yep, as said:

Can move their in-app search/filter to the top of the navigation, just like Talk

;)

[MorrisJobke]

Can move their in-app search/filter to the top of the navigation, just like Talk

Ah - I thought adding it to the "top navigation" (aka header) 🙈

[blizzz]

@blizzz Does a backport make sense here?

yes (:

[blizzz]

in Talk, Users and Apps, direct filtering of the content is nicer.

at least in Users search has to go via the server. Just too much to pull all of them into the browser.