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

Add avatar of server database version upload to S3, removing SSO dependency for avatar management #7152

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

RICHQAQ
Copy link

@RICHQAQ RICHQAQ commented Mar 25, 2025

๐Ÿ’ป ๅ˜ๆ›ด็ฑปๅž‹ | Change Type

  • โœจ feat
  • ๐Ÿ› fix
  • โ™ป๏ธ refactor
  • ๐Ÿ’„ style
  • ๐Ÿ‘ท build
  • โšก๏ธ perf
  • ๐Ÿ“ docs
  • ๐Ÿ”จ chore

๐Ÿ”€ ๅ˜ๆ›ด่ฏดๆ˜Ž | Description of Change

This PR introduces a feature that allows direct upload of avatars to S3 for the server database version. Previously, avatar changes had to be made through the SSO provider, which was a cumbersome process. With the new implementation, users can now upload avatars directly to S3 and store the image URL in the database, removing the SSO dependency for avatar management. Other data will still remain synchronized with the SSO provider.

๐Ÿ“ ่กฅๅ……ไฟกๆฏ | Additional Information

No other functionality was affected. Only the avatar-related mechanism has been modified, making the process more efficient and user-friendly.

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Mar 25, 2025
@lobehubbot
Copy link
Member

๐Ÿ‘ @RICHQAQ

Thank you for raising your pull request and contributing to our Community
Please make sure you have followed our contributing guidelines. We will review it as soon as possible.
If you encounter any problems, please feel free to connect with us.
้žๅธธๆ„Ÿ่ฐขๆ‚จๆๅ‡บๆ‹‰ๅ–่ฏทๆฑ‚ๅนถไธบๆˆ‘ไปฌ็š„็คพๅŒบๅšๅ‡บ่ดก็Œฎ๏ผŒ่ฏท็กฎไฟๆ‚จๅทฒ็ป้ตๅพชไบ†ๆˆ‘ไปฌ็š„่ดก็ŒฎๆŒ‡ๅ—๏ผŒๆˆ‘ไปฌไผšๅฐฝๅฟซๅฎกๆŸฅๅฎƒใ€‚
ๅฆ‚ๆžœๆ‚จ้‡ๅˆฐไปปไฝ•้—ฎ้ข˜๏ผŒ่ฏท้šๆ—ถไธŽๆˆ‘ไปฌ่”็ณปใ€‚

@dosubot dosubot bot added the ๐ŸŒ  Feature Request New feature or request | ็‰นๆ€งไธŽๅปบ่ฎฎ label Mar 25, 2025
Copy link

vercel bot commented Mar 25, 2025

@RICHQAQ is attempting to deploy a commit to the LobeChat Desktop Team on Vercel.

A member of the Team first needs to authorize it.

@RICHQAQ
Copy link
Author

RICHQAQ commented Mar 25, 2025

I believe that some information, such as avatars, doesnโ€™t need to be tightly synced with the SSO provider. Just like many apps (e.g., WeChat, Google login), the username and avatar can be modified directly by the user without affecting the authentication service. This separation allows for a more flexible and user-friendly experience.

Copy link
Contributor

@arvinxx arvinxx left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! There is some part need to be improved~

Copy link
Contributor

Choose a reason for hiding this comment

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

Loading shouldn't use message loading. need another uploading ui

Copy link
Author

Choose a reason for hiding this comment

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

It has been changed to use the Spin to indicate the uploading process. Is this approach feasible?


const lobeUser = {
// ๅคดๅƒไฝฟ็”จ่ฎพ็ฝฎ็š„๏ผŒ่€Œไธๆ˜ฏไปŽ next-auth ไธญ่Žทๅ–
avatar: userAvatar || '',
Copy link
Contributor

Choose a reason for hiding this comment

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

should use userAvatar first and then fallback to next-auth

Copy link
Author

@RICHQAQ RICHQAQ Mar 29, 2025

Choose a reason for hiding this comment

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

I previously used: avatar: userAvatar || nextUser.image || ''.

However, I noticed a user experience issue: when refreshing the page after updating the avatar, since nextUser.image still maintains the old value, there would be a momentary flash - showing the old avatar briefly before switching to the updated one.

I considered two solutions:

  1. Use userAvatar directly while skipping nextUser.image, since during initial page load, useInitUserState reads the latest avatar from the database and saves it to the Store, which basically ensures userAvatar's availability. I've tested this and found no bugs.
  2. Synchronously update nextUser.image when the avatar is updated.

I chose the first, as nextUser.image should preserve the original data from the identity provider, while custom uploaded avatars are provided through userAvatar.

If the reviewer thinks another implementation approach would be better, or has a better solution, I'm happy to make the corresponding adjustments.

@RICHQAQ RICHQAQ requested a review from arvinxx March 30, 2025 06:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
๐ŸŒ  Feature Request New feature or request | ็‰นๆ€งไธŽๅปบ่ฎฎ size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants