Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for validations #29

Merged
merged 3 commits into from
May 14, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion active_remote.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ Gem::Specification.new do |s|
s.add_development_dependency "rake"
s.add_development_dependency "rspec"
s.add_development_dependency "rspec-pride"
s.add_development_dependency "pry-nav"
s.add_development_dependency "pry"
s.add_development_dependency "protobuf-rspec", ">= 1.0"
s.add_development_dependency "simplecov"
end
5 changes: 5 additions & 0 deletions lib/active_remote/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
require 'active_remote/scope_keys'
require 'active_remote/search'
require 'active_remote/serialization'
require 'active_remote/validations'

module ActiveRemote
class Base
Expand All @@ -39,6 +40,10 @@ class Base
# so it needs to be included last.
include Dirty

# Overrides persistence methods, so it must included after
include Validations
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does including this after Dirty cause any issues with the dirty tracking?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After looking at the Validations module, this shouldn't be an issue since it calls super. However, if there are not specs around this already, it might a good idea to add some to verify.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can go before or after Dirty, but it must be after Persistence

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

include ActiveModel::Validations::Callbacks

attr_reader :last_request, :last_response

define_model_callbacks :initialize, :only => :after
Expand Down
11 changes: 11 additions & 0 deletions lib/active_remote/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,17 @@ class ActiveRemoteError < StandardError
class ReadOnlyRemoteRecord < ActiveRemoteError
end

# Raised by ActiveRemote::Validations when save is called on an invalid record.
class RemoteRecordInvalid < ActiveRemoteError
attr_reader :record

def initialize(record)
@record = record
errors = @record.errors.full_messages.join(', ')
super(errors)
end
end

# Raised by ActiveRemove::Base.find when remote record is not found when
# searching with the given arguments.
class RemoteRecordNotFound < ActiveRemoteError
Expand Down
12 changes: 6 additions & 6 deletions lib/active_remote/persistence.rb
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,9 @@ def readonly?
#
# Also runs any before/after save callbacks that are defined.
#
def save
def save(*args)
run_callbacks :save do
create_or_update
create_or_update(*args)
end
end

Expand All @@ -192,8 +192,8 @@ def save
#
# Also runs any before/after save callbacks that are defined.
#
def save!
save || raise(RemoteRecordNotSaved)
def save!(*args)
save(*args) || raise(RemoteRecordNotSaved)
end

# Returns true if the record doesn't have errors; otherwise, returns false.
Expand Down Expand Up @@ -245,9 +245,9 @@ def create
# are created, existing records are updated. If the record is marked as
# readonly, an ActiveRemote::ReadOnlyRemoteRecord is raised.
#
def create_or_update
def create_or_update(*args)
raise ReadOnlyRemoteRecord if readonly?
new_record? ? create : update
new_record? ? create : update(*args)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not splat the args on create as well? Does it not support skipping validations?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, after looking at this again, I'm not sure that either of these need to take args. Is there some AR functionality that this is replicating that I'm not aware of?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just structured it like ActiveRecord

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird. I'm guessing they're doing that so the method arity doesn't change when dirty tracking is included.

end

# Handles updating a remote object and serializing it's attributes and
Expand Down
64 changes: 64 additions & 0 deletions lib/active_remote/validations.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
module ActiveRemote
module Validations
extend ActiveSupport::Concern

# Attempts to save the record like Persistence, but will run
# validations and return false if the record is invalid
#
# Validations can be skipped by passing :validate => false
#
# example Save a record
# post.save
#
# example Save a record, skip validations
# post.save(:validate => false)
#
def save(options = {})
perform_validations(options) ? super : false
end

# Attempts to save the record like Persistence, but will raise
# ActiveRemote::RemoteRecordInvalid if the record is not valid
#
# Validations can be skipped by passing :validate => false
#
# example Save a record, raise and error if invalid
# post.save!
#
# example Save a record, skip validations
# post.save!(:validate => false)
#
def save!(options = {})
perform_validations(options) ? super : raise_validation_error
end

# Runs all the validations within the specified context. Returns true if
# no errors are found, false otherwise.
#
# Aliased as validate.
#
# example Is the record valid?
# post.valid?
#
# example Is the record valid for creation?
# post.valid?(:create)
#
def valid?(context = nil)
context ||= (new_record? ? :create : :update)
output = super(context)
errors.empty? && output
end

alias_method :validate, :valid?

protected

def raise_validation_error
fail(::ActiveRemote::RemoteRecordInvalid.new(self))
end

def perform_validations(options = {})
options[:validate] == false || valid?(options[:context])
end
end
end
2 changes: 1 addition & 1 deletion spec/lib/active_remote/dirty_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@

it "clears previous changes" do
new_record = post.instantiate(record.to_hash)
new_record.previous_changes.should be_nil
new_record.previous_changes.should eq({})
end

it "clears changes" do
Expand Down
56 changes: 56 additions & 0 deletions spec/lib/active_remote/validations_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
require 'spec_helper'

describe ActiveRemote::Validations do
let(:invalid_record) { ::Post.new }
let(:valid_record) { ::Post.new(:name => 'test') }

before { valid_record.stub(:create_or_update).and_return(true) }
before { invalid_record.stub(:create_or_update).and_return(true) }

describe 'save' do
context 'valid record' do
it 'returns true' do
result = valid_record.save
result.should be true
end
end

context 'invalid record' do
it 'returns false' do
result = invalid_record.save
result.should be false
end
end
end

describe 'save!' do
context 'valid record' do
it 'returns true' do
result = valid_record.save!
result.should be true
end
end

context 'invalid record' do
it 'raises invalid record error' do
expect { invalid_record.save! }.to raise_error(ActiveRemote::RemoteRecordInvalid)
end
end
end

describe 'valid?' do
context 'valid record' do
it 'returns true' do
result = valid_record.valid?
result.should be true
end
end

context 'invalid record' do
it 'returns false' do
result = invalid_record.valid?
result.should be false
end
end
end
end
2 changes: 2 additions & 0 deletions spec/support/models/post.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,6 @@ class Post < ::ActiveRemote::Base
belongs_to :coauthor, :class_name => '::Author'
belongs_to :bestseller, :class_name => '::Author', :foreign_key => :bestseller_guid
belongs_to :user, :class_name => '::Author', :scope => :user_guid

validates :name, :presence => true
end