Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] Respect capability for Avatar support. #3438

Merged
merged 17 commits into from
Dec 13, 2021

Conversation

fesave
Copy link
Contributor

@fesave fesave commented Nov 10, 2021

Related Issues

App: #3285
Library: owncloud/android-library#442

  • Added changelog files for the fixed issues in folder changelog/unreleased. More info here

QA

QA checklist: #3438 (comment)

@fesave fesave self-assigned this Nov 10, 2021
@fesave fesave linked an issue Nov 10, 2021 that may be closed by this pull request
@fesave fesave marked this pull request as ready for review November 10, 2021 17:09
@fesave fesave force-pushed the feature/avatar_capability branch from 4bbfb72 to 4f1e312 Compare November 10, 2021 17:21
Copy link
Collaborator

@JuancaG05 JuancaG05 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No syntax error found 🥇 , however, let's wait for @abelgardep to review this

Copy link
Contributor

@abelgardep abelgardep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some suggestions here @fesave

@fesave fesave requested a review from abelgardep November 17, 2021 15:48
Copy link
Contributor

@abelgardep abelgardep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some suggestions here @fesave

@fesave fesave requested a review from abelgardep November 18, 2021 07:49
Copy link
Contributor

@abelgardep abelgardep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job @fesave! Let's move it to QA

@jesmrec
Copy link
Collaborator

jesmrec commented Nov 18, 2021

QA checks:

  • "profile_picture": false -> avatar is not retrieved
  • "profile_picture": true -> avatar is retrieved
  • No "profile_picture" in response -> avatar is retrieved

@jesmrec
Copy link
Collaborator

jesmrec commented Nov 18, 2021

(1) [FIXED]

Server returning

"user": {
            "expire_date": {
                   "enabled": false
            },
            "profile_picture": false,
            "send_mail": false
},

is also showing the avatar.

Nexus 6P
abced0f8

@fesave
Copy link
Contributor Author

fesave commented Nov 24, 2021

@jesmrec this is ready to test, but I have seen that sometimes the avatar call is called before the capabilities have been recovered. In the worst case the call is only made only once with this new check, avoiding saturating the server with constant calls.

@fesave fesave force-pushed the feature/avatar_capability branch 2 times, most recently from c7f6cf6 to 7e07298 Compare November 24, 2021 08:42
@CLAassistant
Copy link

CLAassistant commented Nov 24, 2021

CLA assistant check
All committers have signed the CLA.

@fesave fesave force-pushed the feature/avatar_capability branch from 7e07298 to f2da72f Compare November 24, 2021 09:20
@jesmrec
Copy link
Collaborator

jesmrec commented Dec 2, 2021

Regarding (1).

When browsing to root, the avatar endpoint request is done, no matter where do toy come from:

  • From first login
  • From other account
  • Browsing back from any folder
  • Using the Files tab
  • From sharing view
  • From settings view

will recheck this

@abelgardep
Copy link
Contributor

It should be fixed now. @jesmrec

The avatar was retrieved from the SyncProfileOperation where we ask for the user info, user quota, and also avatar.

From now on, we will ask for the user info, user quota, and depending on the capability, we will ask for the avatar.

@jesmrec
Copy link
Collaborator

jesmrec commented Dec 13, 2021

(1) is fixed.

Actually, capabilities are being moved to the new DB in the scope of the new architecture. For this reason, not every capability is supported with the new stuff yet. The current capability (profile_picture) is being refreshed together with the migrated ones, and that happens when browsing to the Sharing. That means, capability is not taken in account till user opens the Sharing view. This is like an intermediate status, that will be fixed when #3108 is implemented. Then, capabilities will keep the usual behaviour (updated when root folder is browsed)

With the current approach, this PR is ready to go.

@abelgardep abelgardep force-pushed the feature/avatar_capability branch from 6ce727d to 4187f11 Compare December 13, 2021 07:55
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@abelgardep abelgardep merged commit a21021c into master Dec 13, 2021
@abelgardep abelgardep deleted the feature/avatar_capability branch December 13, 2021 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE REQUEST] respect capability for Avatar support
5 participants