[SHARE-511][Feature] Store user favicons as a blob#565
[SHARE-511][Feature] Store user favicons as a blob#565chrisseto merged 19 commits intoCenterForOpenScience:developfrom
Conversation
aaxelb
changed the title
aaxelb
changed the title
| @@ -0,0 +1,20 @@ | |||
| # -*- coding: utf-8 -*- | |||
There was a problem hiding this comment.
The bot migrations can be removed.
There was a problem hiding this comment.
I'll only add to apps with ProviderAppConfigs.
| with open(os.path.join(self.config.path, 'favicon.ico'), 'rb') as f: | ||
| user.favicon.save(user.username, File(f)) | ||
| except OSError: | ||
| pass |
There was a problem hiding this comment.
This exception shouldn't be caught. Anything that has this migration should have a favicon as well.
There was a problem hiding this comment.
Makes sense. I'll add the missing favicons, then, too.
| return ContentFile(favicon.image) | ||
|
|
||
| def _save(self, name, content): | ||
| user = ShareUser.objects.get(username=name) |
There was a problem hiding this comment.
Would a join in update_or_create work?
There was a problem hiding this comment.
I'd tried user__username=name but it complained that user_id was missing. Apparently it's not smart enough for that.
| @@ -75,6 +81,37 @@ def create_robot_user(self, username, robot, long_title='', home_page=''): | |||
| return user | |||
|
|
|||
|
|
|||
| class FaviconImage(models.Model): | |||
| user = models.OneToOneField('ShareUser', on_delete=DATABASE_CASCADE, primary_key=True) | |||
There was a problem hiding this comment.
Let django handle the primary key. Things have a tendency to get weird when you don't.
| @@ -26,7 +26,7 @@ def resolve_title(cls, instance, context, request, info): | |||
|
|
|||
| @classmethod | |||
| def resolve_favicon(cls, instance, context, request, info): | |||
| return '/static/{}/img/favicon.ico'.format(instance.robot.replace('providers.', '', 1)) | |||
There was a problem hiding this comment.
Just for sanity's sake, can you use django's reverse URL look up here?
| @@ -55,3 +55,4 @@ newrelic==2.68.0.50 | |||
| raven==5.23.0 | |||
| graphene==1.0.2 | |||
| graphene-django==1.0 | |||
| pillow==4.0.0 | |||
There was a problem hiding this comment.
Is pillow actually being used for anything?
There was a problem hiding this comment.
It's required by ImageField, to validate that uploaded files are images.
|
@chrisseto changes made |
aaxelb
force-pushed
the
feature/blob-favicons
branch
from
78c825a to
d3b4f6a
Compare
|
|
||
|
|
||
| def user_favicon_view(request, username): | ||
| user = ShareUser.objects.get(username=username) |
There was a problem hiding this comment.
Looks like this guy 500's rather than 404's if it fails
This should also only respond to GETs, just for semantic correctness.
There was a problem hiding this comment.
Oh, derp. Didn't think about the user not existing. Fixed.
2 participants
Store favicons as a blob in the database.
ShareUser.faviconfieldImageFieldwith custom storage to save image as a blob inFaviconImagemodel/favicons/{username}.icofavicon.icoto app root, out of sight of staticfileshttps://openscience.atlassian.net/browse/SHARE-511