ryanflach / gif_generator Goto Github PK

@ryanflach

1. Use current_user in your controllers and views, no need to save it as an instance variable.
2. The logic behind generating the class below should be taken care of somewhere else, perhaps in a helper.

#users/show.html.erb

<%= image_tag gif.image_path, class: "#{gif.category.name}-image-#{index + 1} img-responsive img-rounded" %><br />

1. This logic should live in another helper: <% if current_user && !current_admin? %>. Whenever you find yourself doing && or || or > etc, you should create a new method that handles that logic.
2. You should never ever ever query your database in your views. Query in your controller, then pass the data in via an instance method.

# admin/gifs/index.html.erb
<% Category.all.each do |cat| %>

1. When validating our models, you'll want to validate the association not the foreign key. In your FavoriteGif model, you have:

  validates :gif_id, presence: true

Instead, you should be validating the association like so:

  validates :gif, presence: true

1. Check into dependent: :destroy when you're destroying a gif and potentially destroying the category if that category has no other gifs. I'm pretty sure dependent destroy can help you out with that!
2. Don't forget your unique constraints on the database level if you want your data to truly be unique 👍

Good job with this!