JDutil / contact_us

Gem providing simple Contact Form functionality with a Rails 3+ Engine.
http://contact-us-demo.herokuapp.com
MIT License
134 stars 57 forks source link

Use BasicModel as a base for ContactUs::Contact #1

Closed petebrowne closed 13 years ago

petebrowne commented 13 years ago

As a shameless suggestion, you could use BasicModel as a base for the ContactUs::Contact model. It's meant to be used for tableless models:

require "basic_model"

class ContactUs::Contact
  include BasicModel

  attr_accessor :email, :message

  validates :email,   :format => { :with => /^([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})$/i },
                      :presence => true
  validates :message, :presence => true

  def save
    if self.valid?
      ContactUs::ContactMailer.contact_email(self).deliver
      return true
    end
    return false
  end
end
JDutil commented 13 years ago

Thanks for the suggestion, but I would prefer to to avoid the extra dependency. Since I am already requiring Rails simply including the required ActiveModel module, and adding a couple methods to interact with the forms is not a big deal to me.

petebrowne commented 13 years ago

Understandable. Suggestion though: you should include ActiveModel::Conversion and define #persisted?. Then your model will pass the ActiveModel::Lint::Tests.

Also, I don't think defining #read_attribute_for_validation is necessary. By default it will use #send and get the correct attributes defined by attr_accessor.

JDutil commented 13 years ago

I will have to look into this more. Part of the model is based on an old blog post I read when Rails 3 was first being released with the great ActiveModel abstraction that allowed this. I'm not really aware of what the ActiveModel::Lint::Tests are to be honest so if you could explain how I can test against them or better yet submit a pull request that would pass them it would be greatly appreciated.

petebrowne commented 13 years ago

I used the method described here: http://library.edgecase.com/Rails/2010/10/30/activemodel-lint-test-for-rspec.html to use the ActiveModel::Lint::Tests in Rspec.

JDutil commented 13 years ago

Thanks for the suggestion I wasn't aware of the Conversion module and Lint tests that will be handy for this gem.