feat: Added FXA profile image#4204
Conversation
mathjazz
left a comment
mathjazz
left a comment
There was a problem hiding this comment.
Nice work. I haven't tested yet, but found a potential improvement for code readability.
| @@ -23,6 +23,12 @@ def user_profile_url(self): | |||
|
|
|||
|
|
|||
| def user_gravatar_url(self, size): | |||
There was a problem hiding this comment.
The gravatar_url() can now return a non-Gravatar URL, which makes the name misleading. Could we either rename the helper or add a small wrapper so template code can use the more accurate name?
| def user_gravatar_url(self, size): | |
| def user_avatar_url(self, size): |
There was a problem hiding this comment.
I can rename it. The only reason I didnt do it before is because we add user_gravater_url as a property to the usermodel name gravatar_url and also alias it in the user_gravatar_url_small. Should I just change this function name and its refrences or also the property ? I suggest just the function name
There was a problem hiding this comment.
The properties as well, please.
|
@mathjazz A lot of the code for these changes has moved to a user_util.py and thus this PR has a lot of conflicts. I you give the go ahead I can close this PR and make a new one from main. I can rebase and try to resolve the merge conflicts but I would still have to duplicate my changes and by closing this PR an making a new one I can ensure no bug goes through |
I'm sorry about this. :/ We were just working on refactoring the User model. You can rebase here or open a new PR, either works. Thanks for understanding! |
No problem, I will open a new PR to make sure nor bugs creep in |
2 participants
Description
Added FXA avatar scope and the avatar will be loaded on runtime using the user_gravatar_url if available.
Linked Issue
Closes #4017
Screenshot
Caveats
There several caveats with this implementation