Fix User profile picture when performing the search

[ghost]

Signed-off-by: Andy Xheli axheli@axtsolutions.com

Before

After

Fix for #31065

[juliusknorr]

This url should probably be built using IURLGenerator and we might need to have special handling for the viability settings, but the change in general makes sense I think.

@nickvergessen Didn't you mention a while back in chat a case on our instance where the contacts menu endpoint was not returning an avatar for a user? Could be related

[ghost]

Hey guys! Anyway we can look into this also. I think it would be nice to have this in NC 25.

Currently in NC 25

[ghost]

Signed-off-by: Andy Xheli axheli@axtsolutions.com

Tested on NC 24 & 25 and it works as it should

[ghost]

Hi all, any updates on this on being in NC 25 ?

[blizzz]

@andyxheli CI is unhappy:

There was 1 failure:

1. Tests\Contacts\ContactsMenu\ContactsStoreTest::testGetContactsWithoutBinaryImage
   Failed asserting that '' is null.

/drone/src/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php:166

that test needs adjustements apparently

[ghost]

Thank you! @blizzz

@Pytal Do you have any suggestions on what needs to be done on ?

There was 1 failure:

Tests\Contacts\ContactsMenu\ContactsStoreTest::testGetContactsWithoutBinaryImage
Failed asserting that '' is null.
/drone/src/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php:166

[Pytal]

This looks to be from the return value of linkToRouteAbsolute() so would say that urlGenerator needs to be mocked, @blizzz might know more here though?

[blizzz]

Actually the test was ensuring(!) that avatar is null. Now it is not anymore as every time an avatar url is being created. The mock would at least return an empty string, and not null. testGetContactsWithoutAvatarURI does not fail however? A bit confusing when your not totally in the logic, it feels a bit that the tests were out of touch with the changes of the ContactsStore.

[blizzz]

Best probably to do as @Pytal suggested: mock url generator to return a dummy url and expect this.

Something like this

diff --git a/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php b/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php
index aa609aedbb9..dfdd67fbb23 100644
--- a/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php
+++ b/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php
@@ -140,6 +140,10 @@ class ContactsStoreTest extends TestCase {
    public function testGetContactsWithoutBinaryImage() {
        /** @var IUser|MockObject $user */
        $user = $this->createMock(IUser::class);
+       $this->urlGenerator->expects($this->any())
+           ->method('linkToRouteAbsolute')
+           ->with('core.avatar.getAvatar', $this->anything())
+           ->willReturn('https://urlToNcAvatar.test');
        $this->contactsManager->expects($this->once())
            ->method('search')
            ->with($this->equalTo(''), $this->equalTo(['FN', 'EMAIL']))
@@ -163,7 +167,7 @@ class ContactsStoreTest extends TestCase {
        $entries = $this->contactsStore->getContacts($user, '');
 
        $this->assertCount(2, $entries);
-       $this->assertNull($entries[1]->getAvatar());
+       $this->assertSame('https://urlToNcAvatar.test', $entries[1]->getAvatar());
    }
 
    public function testGetContactsWithoutAvatarURI() {

(didn't run tests against it)

[ghost]

Thanks @blizzz I created a pull request for the changes that you suggested for the test

#34386

[blizzz]

@andyxheli could you add them as commit to this PR?

[ghost]

@blizzz All done :)

[blizzz]

/backport to stable25

[ghost]

@blizzz thank you! Can we also backpo this to NC 24 ?

[blizzz]

/backport to stable24

[szaimen]

This seemingly broke avatars of vcf-contacts.

See #36735

[ghost]

@szaimen would this also fix #34503 ? under See #36735

[szaimen]

@szaimen would this also fix #34503 ?

No.

[ghost]

Okay, thanks. Just checking so we did have duplicate issues.