Skip to content
This repository

Added the forcedefault option #7

Closed
wants to merge 1 commit into from

2 participants

Mohamad El-Husseini Colin MacKenzie IV
Mohamad El-Husseini

I added the forcedefault or f option. This allows you to force a gravatar to any of the available defaults even if the user has a defined gravatar image--it will be ingored.

I don't really know what I'm doing, and I've only been doing Ruby for 3 days... but the commit is fairly small, and it works for me.

Mohamad El-Husseini

More of the f option can be found here: http://en.gravatar.com/site/implement/images/ under "Force Default"

Colin MacKenzie IV

Thanks very much for this pull request! However, there are a few minor problems. First of all (and the only real reason I didn't merge this), there is no corresponding unit test. I'm big on tests. :) Also, it looks like you've mistakenly committed the gem file, and I'm also not a big fan of bumping version before release -- otherwise we end up skipping version numbers and other kinds of madness.

So, I've implemented your changes separately in a0cb351.

As for installing the gem, you can do it the traditional way with gem install path/to/gravatar.gem --local or with the project's Rake task using rake install.

Colin MacKenzie IV sinisterchipmunk closed this January 12, 2012
Mohamad El-Husseini

Yeah, sorry about that. I read the pull-request notes in the README but belatedly. I'm also very new to git, Ruby, RoR, and consoles, so doing unit tests was beyond me :D

Cheers for implementing the change.

Colin MacKenzie IV

No worries, it is still a valuable contribution since it'd have gone unnoticed otherwise. I updated the contributors at the bottom of the readme to include you.

Mohamad El-Husseini
Colin MacKenzie IV

Well, what technically works is a different story from best practices. You've already discovered that there are a hundred ways it'll work, so let's discuss best practices.

Following the "Rails Way", the goal should always be to keep the models fat, the controllers skinny, and the views logic-less. If you absolutely must use logic in the views, put that logic into a helper and call it from the view, to keep clutter down.

With this principle in mind, I'd say treat the Gravatar instance as a model. Instantiate it from the controller, and use it in the view as if you were embedding the user's actual email address (or any other attribute) into the page. Example:

# controller code
def show
  @user = User.find 1 # just some arbitrary user
  @gravatar = Gravatar.new(@user.email)
end

# view code
<%= link_to image_tag @gravatar.image_url %>

Having written all that, it becomes apparent that we could make this a bit more natural. It stands to reason (and shows in the code) that "a user has a gravatar". Therefore, Gravatar becomes an attribute of User. Let's fix this:

# controller code
def show
  @user = User.find 1
end

# model code
class User < ActiveRecord::Base
  # ...

  def gravatar
    @gravatar ||= Gravatar.new(email)
  end
end

# view code
<%= link_to image_tag @user.gravatar.image_url %>

This is much more conventional, and keeps the code DRY (Don't Repeat Yourself!) because you don't have to constantly create a new Gravatar instance throughout your controllers.

As an aside, the ||= syntax means "if it exists already, return it; otherwise, create it, assign it, and then return it."

Mohamad El-Husseini

Colin, a big thank you for this great explanation. It makes a lot of sense. I need to think more OO, I suppose.

Cheers!

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

Showing 1 unique commit by 1 author.

Jan 11, 2012
Mohamad El-Husseini added forcedefault option 5770f9a
This page is out of date. Refresh to see the latest.
2  VERSION
... ...
@@ -1 +1 @@
1  
-1.0.2
  1
+1.0.3
BIN  gravatar-ultimate-1.0.2.gem
Binary file not shown
3  lib/gravatar.rb
@@ -176,6 +176,7 @@ def email_hash(email = self.email)
176 176
   #   :default or :d         a default URL for this image to display if the specified user has no image;
177 177
   #                          or this can be one of [ :identicon, :monsterid, :wavatar, 404 ]. By default a generic
178 178
   #                          Gravatar image URL will be returned.
  179
+  #   :forcedefault or :f    force a default image and ignore the user's specified image [ :identicon, :monsterid, :wavatar, 404 ]
179 180
   #   :filetype              an extension such as :jpg or :png. Default is omitted.
180 181
   #
181 182
   # See http://en.gravatar.com/site/implement/url for much more detailed information.
@@ -272,7 +273,7 @@ def options
272 273
 
273 274
   def query_for_image(options)
274 275
     query = ''
275  
-    [:rating, :size, :default, :r, :s, :d].each do |key|
  276
+    [:rating, :size, :default, :forcedefault, :r, :s, :d, :f].each do |key|
276 277
       if options.key?(key)
277 278
         query.blank? ? query.concat("?") : query.concat("&")
278 279
         query.concat("#{key}=#{CGI::escape options[key].to_s}")
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.