Expose if the room has a custom avatar#9423

Merged

nickvergessen merged 19 commits intomasterfrom

bugfix/9320/expose-if-is-custom-avatar

May 4, 2023

Contributor

vitormattos commented May 2, 2023

Fixes to be able identify if have a custom avatar

Added the property isCustomAvatar to conversation object
Return every time a non empty string as avatarVersion

☑️ Resolves

Fix AvatarVersion should be populated for all conversation types #9320

🏁 Checklist

⛑️ Tests (unit and/or integration) are included or not required
📘 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

vitormattos added 3. to review feature: api 🛠️ php labels

vitormattos added this to the 💙 Next Major (27) milestone

vitormattos requested review from SystemKeeper and nickvergessen

May 2, 2023 12:55

vitormattos self-assigned this

vitormattos mentioned this pull request

feat(avatar): Use the first emoji of room names as avatar emoji #9424

Merged

4 tasks

SystemKeeper reviewed

View reviewed changes

Contributor

SystemKeeper left a comment

Tested and works besides small remarks

lib/Service/AvatarService.php Outdated Show resolved Hide resolved

docs/avatar.md Outdated Show resolved Hide resolved

nickvergessen reviewed

View reviewed changes

docs/conversation.md Outdated Show resolved Hide resolved

vitormattos added a commit that referenced this pull request


          Change text after code review

#9423 (comment)

Signed-off-by: Vitor Mattos <vitor@php.rio>

vitormattos added a commit that referenced this pull request


          Changes after code reivew

6e34509

#9423 (comment)
#9423 (comment)

Signed-off-by: Vitor Mattos <vitor@php.rio>

vitormattos requested review from SystemKeeper and nickvergessen

May 2, 2023 16:22

SystemKeeper approved these changes

View reviewed changes

Contributor

SystemKeeper left a comment

Nice!

SystemKeeper mentioned this pull request

Use is custom avatar on room nextcloud/talk-ios#1209

Merged

mahibi commented May 2, 2023

what happens if an emoji is set (as implemented in #9424 )?
will isCustomAvatar be true or false?

for the android talk app, it would be great if it's true for this case

Contributor Author

vitormattos commented May 2, 2023

We will need to do changes after merge this or #9424 but we can set isCustomAvatar to true when the avatar of room need to be an emoji.

@nickvergessen I think that will be best to merge your PR first and after this I will rebase this PR and implement the necessary to solve the @mahibi point.

Contributor

SystemKeeper commented May 2, 2023

what happens if an emoji is set (as implemented in #9424 )? will isCustomAvatar be true or false?

for the android talk app, it would be great if it's true for this case

We agreed to not do this ;)

nickvergessen requested changes

View reviewed changes

docs/conversation.md Outdated Show resolved Hide resolved

docs/conversation.md Outdated Show resolved Hide resolved

nickvergessen reviewed

View reviewed changes

docs/avatar.md Outdated Show resolved Hide resolved

vitormattos added 9 commits

May 3, 2023 04:58


          Fixes to be able identify if have a custom avatar

cb3fa78

* Added the header X-NC-IsCustomAvatar
* Return every time a non empty string as avatarVersion

Signed-off-by: Vitor Mattos <vitor@php.rio>


          Document the new header

abe35b3

Signed-off-by: Vitor Mattos <vitor@php.rio>


          Make possible check more than one property from room object

54176a0

Signed-off-by: Vitor Mattos <vitor@php.rio>


          Reduce IO

7f50f59

When we retrieve the room avatar sometimes is necessary to check if the file
exists and in other places we only need to get the avatar file name.
With this refactor I separated the logic to get the path of file in a
different method.

Signed-off-by: Vitor Mattos <vitor@php.rio>


          Replace the header X-NC-IsCustomAvatar by conversation property isCus…

7a0ac60

…tomAvatar

With the header isn't possible to check if `isCustomAvatar` without
retrieve the avatar to verify the header. Because this, isCUstomAvatar
was moved to a conversation property.

Signed-off-by: Vitor Mattos <vitor@php.rio>


          Remove unused step definition

92fe810

Signed-off-by: Vitor Mattos <vitor@php.rio>


          Simplify code after refactor

d102271

Signed-off-by: Vitor Mattos <vitor@php.rio>


          Change text after code review

#9423 (comment)

Signed-off-by: Vitor Mattos <vitor@php.rio>


          Prevent future problems with empty return

bd40421

Signed-off-by: Vitor Mattos <vitor@php.rio>

vitormattos added 2 commits

May 3, 2023 05:00


          Text fixes after code review

21bc57c

#9423 (comment)
#9423 (comment)

Signed-off-by: Vitor Mattos <vitor@php.rio>


          Return isCustomAvatar=true when the name of room start with an emoji

0ce2366

Signed-off-by: Vitor Mattos <vitor@php.rio>

vitormattos force-pushed the bugfix/9320/expose-if-is-custom-avatar branch from 6e34509 to 0ce2366 Compare

May 3, 2023 09:27

Contributor Author

vitormattos commented May 3, 2023

Rebased to solve conflicts.

vitormattos requested a review from nickvergessen

May 3, 2023 09:39

vitormattos added 2 commits

May 3, 2023 06:56


          Return isCustomAvatar=false when the name of room start with an avatar

ee52c47

Didn't make sense to return true because this property is only used to
display the trash icon to remove the customized avatar.

Signed-off-by: Vitor Mattos <vitor@php.rio>


          Cover with integration tests the room avatar with emoji

c92f66c

Signed-off-by: Vitor Mattos <vitor@php.rio>

SystemKeeper reviewed

View reviewed changes

tests/integration/features/bootstrap/FeatureContext.php Outdated Show resolved Hide resolved


          Fix typo

#9423 (comment)

Signed-off-by: Vitor Mattos <vitor@php.rio>

nickvergessen changed the title ~~Expose if the room have a custom avatar~~ Expose if the room has a custom avatar

nickvergessen reviewed

View reviewed changes

docs/conversation.md Outdated Show resolved Hide resolved


          Add capability requrement

9e47533

#9423 (comment)

Signed-off-by: Vitor Mattos <vitor@php.rio>

nickvergessen requested changes

View reviewed changes

tests/integration/features/bootstrap/FeatureContext.php Outdated Show resolved Hide resolved

tests/integration/features/bootstrap/FeatureContext.php Outdated Show resolved Hide resolved

tests/integration/features/bootstrap/FeatureContext.php Outdated Show resolved Hide resolved

tests/integration/features/bootstrap/FeatureContext.php Outdated Show resolved Hide resolved

tests/integration/features/bootstrap/FeatureContext.php Outdated Show resolved Hide resolved

vitormattos requested a review from nickvergessen

May 3, 2023 13:24

nickvergessen reviewed

View reviewed changes

tests/integration/features/bootstrap/FeatureContext.php Outdated Show resolved Hide resolved


          Fix texts

6659f1b

Signed-off-by: Vitor Mattos <vitor@php.rio>

vitormattos force-pushed the bugfix/9320/expose-if-is-custom-avatar branch from 15c8bf3 to 6659f1b Compare

May 3, 2023 14:43

vitormattos requested a review from nickvergessen

May 3, 2023 14:43


          fix(tests): Adjust function names to be unique

8bc0906

Signed-off-by: Joas Schilling <coding@schilljs.com>

nickvergessen approved these changes

View reviewed changes

nickvergessen enabled auto-merge

May 3, 2023 20:41

Member

nickvergessen commented May 4, 2023

Failed scenarios:

/drone/server/apps/spreed/tests/integration/features/integration/dashboard.feature:11


          Fix integration test

602ff96

I prefered to put a placeholder at URL from step to make splicit that
the URL have a version at the place of placeholder.

Signed-off-by: Vitor Mattos <vitor@php.rio>

nickvergessen merged commit 31082f9 into master

nickvergessen deleted the bugfix/9320/expose-if-is-custom-avatar branch

May 4, 2023 08:35

mishamosher pushed a commit to mishamosher/spreed that referenced this pull request


          Change text after code review

41114ef

nextcloud#9423 (comment)

Signed-off-by: Vitor Mattos <vitor@php.rio>

mishamosher pushed a commit to mishamosher/spreed that referenced this pull request


          Changes after code reivew

7a02e2e

nextcloud#9423 (comment)
nextcloud#9423 (comment)

Signed-off-by: Vitor Mattos <vitor@php.rio>

mishamosher pushed a commit to mishamosher/spreed that referenced this pull request


          Text fixes after code review

d29fa35

nextcloud#9423 (comment)
nextcloud#9423 (comment)

Signed-off-by: Vitor Mattos <vitor@php.rio>

mishamosher pushed a commit to mishamosher/spreed that referenced this pull request


          Fix typo

734ce67

nextcloud#9423 (comment)

Signed-off-by: Vitor Mattos <vitor@php.rio>

mishamosher pushed a commit to mishamosher/spreed that referenced this pull request


          Add capability requrement

0a89ba4

nextcloud#9423 (comment)

Signed-off-by: Vitor Mattos <vitor@php.rio>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: api 🛠️ php