🖼️ Conversation avatars API

[vitormattos]

Fix #927

🚧 TODO

• We need OCS API, suggestion: /api/{apiVersion}/room/{token}/image - Similar to https://github.com/nextcloud/spreed/blob/master/lib/Controller/TempAvatarController.php
  • POST a new image
    • Needs to validate the file is an image
    • Needs to validate the image is a square and of a supported type (check User avatar code for what we should allow)
    • File should be stored with OCP\Files\IAppData with conversation token + file type as name
  • DELETE to remove an image
  • GET which serves the image as FileDisplayResponse
    • Not sure if that works as OCSController, if it does not we need another Controller with another route.
    • If AppData does not have a file with the conversation name, we fall back to returning an icon based on the conversation type
    • If the conversation is a one-to-one conversation we fall back to the user avatar?
    • /dark URL for dark avatars
• Add capability
• Add integration tests
• Adjust "icon url" usage in:
  • activities
  • notifications
  • search results
  • references
  • etc.

🏁 Checklist

• ⛑️ Tests (unit and/or integration) are included
• 📘 API documentation in docs/ has been updated or is not required
• 📗 User documentation in https://github.com/nextcloud/documentation/tree/master/user_manual/talk has been updated or is not required
• 🔖 Capability is added or not needed

[nickvergessen]

Added some todos in the PR description

[SystemKeeper]

If this in renamed to a more general "AvatarController", would that supersede the "TempAvatarController" (temp-user-avatar-api capability) ?

[nickvergessen]

would that supersede the "TempAvatarController" (temp-user-avatar-api capability) ?

No, as that is still an API that should not be in talk but in server

[nickvergessen]

I think this parameter is a bit problematic. It will make the avatar look updated on the conversation list, but e.g. search results, notifications, etc still show the old one. So basically we would need to tell clients to track this value and when it is different, they should invalidate their avatar image cache.

[nickvergessen]

cc @Ivansss @mahibi

Any idea what the best way would be? I'd like to have it similar to user avatars. They work without any additional information, you just need the userid (conversation token) and can make your URL. It's cached so can take some time until others see it. Should be fine, unless you did the change yourself.
Would you think it's okay to have it cached/old on the mobile apps when you changed it in the web? Or what would be a good way to handle this?

[vitormattos]

The user avatar also have a query string to expire with an incremental value called v too.
The endpoint /avatar/{userId}/{size} haven't this value because is only to expire and is send by query string.

When the backend delete the avatar, the version is incremented:
https://github.com/nextcloud/server/blob/master/lib/private/Avatar/UserAvatar.php#L194-L195
The same code is trigged when we set a new avatar:
https://github.com/nextcloud/server/blob/master/lib/private/Avatar/UserAvatar.php#L90-L96

The frontend every time get the version directly from userconfig:
https://github.com/nextcloud/server/blob/master/apps/settings/src/components/PersonalInfo/AvatarSection.vue#L149

The backend also retrieve the avatar version every time when is the current user that request:
https://github.com/nextcloud/server/blob/master/lib/private/TemplateLayout.php#L157
and when the current user need the avatar of other user:
https://github.com/nextcloud/server/blob/master/lib/private/Template/JSConfigHelper.php#L287-L290

Is a bit complex to identify this in the server repository because the code that do this is fragmented in a lot of places, but the server store and use the version number of avatar.

[nickvergessen]

The frontend every time get the version directly from userconfig:

This is only for your very own avatar. It is not done for the avatar of others, see:
https://github.com/nextcloud/nextcloud-vue/blob/master/src/components/NcAvatar/NcAvatar.vue#L627-L647

But that is exactly the problem here. It's basically always the avatar of others 😬

[vitormattos]

I think that this wont be a problem to Talk.

I already implemented to send the version of avatar in Room object, then, in all places that we need to display the current room avatar, will be possible.

In the places that we haven't the Room object, we will send the URL of avatar and we already have a contract to do this, have places in server that need a property called icon, other use iconUrl and we can do the same in Talk.

[nickvergessen]

That is exactly why I started the discussion here. We will NOT be able to add the avatar URL on all things. But anyway.

We can move this to a follow up issue so we can merge this PR

[nickvergessen]

Should add the file extension?

Suggested change

	$avatarName = $this->random->generate(10, ISecureRandom::CHAR_HUMAN_READABLE);
	$avatarName = $this->random->generate(6, ISecureRandom::CHAR_HUMAN_READABLE) . ($mimeType === 'image/jpeg' ? '.jpg' : '.png');

[vitormattos]

Is possible, but we will need change the current implementation a bit to consider the extension.
For now, the name that we stored on database is only to have a token to expire the previous image. Now, if we send a request without the version, will get the room image too.

[nickvergessen]

I meant for readability on the filesystem (also user avatars have it). But sure if it works, let's keep it in the current format.
It could still work without it.

[vitormattos]

Yes, will be more readable to save the filename with the extension.

Maybe we can change this approach in the future creating a table to store all room avatar, maybe room_avatar to make possible to have a gallery of room avatar. With room avatar gallery, we can maintain only the name with the 10 chars (without the extension) in room table following the current approach to store the current room avatar. In the table room_avatar we can store more details, by example, the full name of file in filesystem.

[nickvergessen]

Rebased to resolve conflicts from breakout rooms part1 merge

[nickvergessen]

Added a commit so the file can be viewed in the browser and does not trigger a download.

Other than that only the system messages are missing, but we can do that as a follow up.

[nickvergessen]

Follow up in #8443