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/avatar upload #166

Merged
merged 32 commits into from
Sep 23, 2023
Merged

Conversation

Schnoop
Copy link
Contributor

@Schnoop Schnoop commented Sep 18, 2023

#8

@frank9999
Copy link
Member

What is your opinion about storing the avatar as base64encoded string in the database? I'm using multi-server deployments, so persisting user uploads in 1 directory on 1 server will cause issues. Several deployment tools also create a new checkout in a separate directory, which will cause the avatars to break. I can work around these issues by making the avatar upload directory a shared NFS share or use a shared object store... Storing the image in the database seems like the easiest solution, but it may cause performance issues and increase database disk usage/load. @Schnoop

@Schnoop
Copy link
Contributor Author

Schnoop commented Sep 19, 2023

Yeah - that would be a good option. If the database should get in trouble at some point in the future it should not be a problem to move the images back to files and think about other deployment options. Maybe a S3 storage is another option to store the user generated images.

@Schnoop
Copy link
Contributor Author

Schnoop commented Sep 19, 2023

Changed implementation to store image in database.

@Schnoop Schnoop closed this Sep 19, 2023
@Schnoop Schnoop reopened this Sep 19, 2023
@Schnoop Schnoop closed this Sep 19, 2023
@Schnoop Schnoop reopened this Sep 19, 2023
Copy link
Member

@frank9999 frank9999 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 this merge request! I left some review notes :)

composer.json Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
src/Entity/User.php Outdated Show resolved Hide resolved
src/Form/UploadAvatarType.php Show resolved Hide resolved
templates/game/profile.html.twig Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

Codecov Report

Merging #166 (8ece4f4) into master (fb3b777) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@             Coverage Diff             @@
##             master    #166      +/-   ##
===========================================
- Coverage      0.08%   0.08%   -0.01%     
- Complexity     1831    1845      +14     
===========================================
  Files           179     181       +2     
  Lines          6140    6220      +80     
===========================================
  Hits              5       5              
- Misses         6135    6215      +80     
Files Changed Coverage Δ
src/Controller/Game/UserController.php 0.00% <0.00%> (ø)
src/Entity/User.php 0.00% <0.00%> (ø)
src/Form/UploadAvatarType.php 0.00% <0.00%> (ø)
src/Twig/Base64EncodeExtension.php 0.00% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@frank9999 frank9999 merged commit 5465bb6 into FrankProjects:master Sep 23, 2023
4 checks passed
@Schnoop Schnoop deleted the feature/AvatarUpload branch September 24, 2023 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants