User model avatars #144

Closed
wants to merge 15 commits into
from

Conversation

Projects
None yet
3 participants
@codev
Contributor

codev commented Feb 8, 2012

I didn't want to use gravatar on a recent project for anonymity but wanted avatars from my current user class. So I've added the ability to set Forem.avatar_user_method to a method to be called on user to return an image url to use as an avatar. If you don't set avatar_user_method you keep the gravatar functionality.

@parndt

This comment has been minimized.

Show comment Hide comment
@parndt

parndt Feb 8, 2012

Contributor

Odd, it's saying there are merge conflicts with this pull request. Can you please rebase against master?

Thanks!

Contributor

parndt commented Feb 8, 2012

Odd, it's saying there are merge conflicts with this pull request. Can you please rebase against master?

Thanks!

codev added some commits Feb 8, 2012

@codev

This comment has been minimized.

Show comment Hide comment
@codev

codev Feb 8, 2012

Contributor

Any better?

Contributor

codev commented Feb 8, 2012

Any better?

@parndt

This comment has been minimized.

Show comment Hide comment
@parndt

parndt Feb 8, 2012

Contributor

Yeah, it now is clean.

Contributor

parndt commented Feb 8, 2012

Yeah, it now is clean.

@parndt

This comment has been minimized.

Show comment Hide comment
@parndt

parndt Feb 28, 2012

Contributor

Or was, sorry.

Contributor

parndt commented Feb 28, 2012

Or was, sorry.

@codev

This comment has been minimized.

Show comment Hide comment
@codev

codev Mar 29, 2012

Contributor

Merged it with all the moderation fixes

Contributor

codev commented Mar 29, 2012

Merged it with all the moderation fixes

@radar

This comment has been minimized.

Show comment Hide comment
@radar

radar Apr 1, 2012

Collaborator

Please extract changes pertinent to this pull request into another branch and re-submit a pull request. There are far too many commits in here with too many changes for my liking.

For instance, you have a commit that removes subscriber functionality and lib/forem.rb removes the forem_autocomplete attribute.

Collaborator

radar commented Apr 1, 2012

Please extract changes pertinent to this pull request into another branch and re-submit a pull request. There are far too many commits in here with too many changes for my liking.

For instance, you have a commit that removes subscriber functionality and lib/forem.rb removes the forem_autocomplete attribute.

@radar radar closed this Apr 1, 2012

@@ -0,0 +1,5 @@
+class AddCustomAvatarUrlToUsers < ActiveRecord::Migration

This comment has been minimized.

Show comment Hide comment
@radar

radar Apr 1, 2012

Collaborator

spec/dummy is re-generated with a call to rake forem:dummy_app, and so this migration would be nuked by that call. Recommended to move it into the dummy generator.

@radar

radar Apr 1, 2012

Collaborator

spec/dummy is re-generated with a call to rake forem:dummy_app, and so this migration would be nuked by that call. Recommended to move it into the dummy generator.

@@ -1,8 +1,16 @@
module Forem
module PostsHelper
def avatar(user, options = {})
- if (email = user.try(:email)).present?
- image_tag(avatar_url(email, options), :alt => "Gravatar")
+ if Forem.avatar_user_method.nil?

This comment has been minimized.

Show comment Hide comment
@parndt

parndt Apr 1, 2012

Contributor

if avatar_user_method just returned :email by default then you wouldn't need the conditional logic here at all :-)

@parndt

parndt Apr 1, 2012

Contributor

if avatar_user_method just returned :email by default then you wouldn't need the conditional logic here at all :-)

This comment has been minimized.

Show comment Hide comment
@parndt

parndt Apr 1, 2012

Contributor

By this, I mean that avatar_url would need to be modified to handle all cases automatically.

@parndt

parndt Apr 1, 2012

Contributor

By this, I mean that avatar_url would need to be modified to handle all cases automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment