Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fix for issue #1307: validates_attachment_presence doesn't change field-label #1339

Closed
wants to merge 3 commits into from

3 participants

@fernandomm

Simple pull request that fixes issue #1307 by adding attachment_presence to the validators in required? function.

@bbenezech
Collaborator

Thanks !
Can you duplicate to the ActiveRecord model part to the Mongoid model and move the spec to https://github.com/sferik/rails_admin/tree/master/spec/unit/config/fields/base_spec.rb?

It's not necessary to test against the whole stack for this. You can get inspiration from this instead: https://github.com/sferik/rails_admin/blob/master/spec/unit/config/fields/base_spec.rb#L11

@fernandomm fernandomm Adding image model to mongoid in dummy_app
Now tests should pass when ORM is set to mongoid.
147a20d
@fernandomm

Ok, just pushed a commit to add the image model for mongoid. Running rspec with CI_ORM=mongoid works now.

About the spec location. The problem is not that the field is required ( it is! ), it's only the label that shows "Optional" when it should show "Required".

For this reason i'm testing against the whole stack, to make sure that the correct label shows up.

I guess that if i test with be_required, it won't cover the modification that i did.

Let me know if i'm wrong :)

@bbenezech
Collaborator

You are not testing if the model column is required, you are testing if RailsAdmin is considering its inherited field as required. You don't need to test the full stack for that. A unit test is enough.
field.should be_required is testing if field.required? which happens to be the option you are modifying ;-)
https://github.com/fernandomm/rails_admin/blob/147a20d59cf2020ea8776bd62539967a39eafcd4/lib/rails_admin/config/fields/base.rb#L166

@bbenezech
Collaborator

@fernandomm Do you want me to finish this?

@bbenezech bbenezech was assigned
@fernandomm

Sorry for the lack of updates. Was very busy lately.

I just moved the test to base_spec.rb.

@bbenezech bbenezech closed this in a5a1f08
@coveralls

Coverage Status

Changes Unknown when pulling 0db4fe2 on fernandomm:paperclip-presence into ** on sferik:master**.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 28, 2012
  1. @fernandomm
Commits on Oct 3, 2012
  1. @fernandomm

    Adding image model to mongoid in dummy_app

    fernandomm authored
    Now tests should pass when ORM is set to mongoid.
Commits on Oct 27, 2012
  1. Moving paperclip presence tests from integration to base_spec.rb.

    Fernando Morgenstern authored
This page is out of date. Refresh to see the latest.
View
2  lib/rails_admin/config/fields/base.rb
@@ -168,7 +168,7 @@ def virtual?
(@required ||= {})[context] ||= !!([name] + children_fields).uniq.find do |column_name|
!!abstract_model.model.validators_on(column_name).find do |v|
!v.options[:allow_nil] and
- [:presence, :numericality].include?(v.kind) and
+ [:presence, :numericality, :attachment_presence].include?(v.kind) and
(v.options[:on] == context or v.options[:on].blank?)
end
end
View
6 spec/dummy_app/app/active_record/image.rb
@@ -0,0 +1,6 @@
+class Image < ActiveRecord::Base
+ attr_accessible :file
+
+ has_attached_file :file, :styles => { :medium => "300x300>", :thumb => "100x100>" }
+ validates_attachment_presence :file
+end
View
9 spec/dummy_app/app/mongoid/image.rb
@@ -0,0 +1,9 @@
+class Image
+ include Mongoid::Document
+ include Mongoid::Paperclip
+
+ attr_accessible :file
+
+ has_mongoid_attached_file :file, :styles => { :medium => "300x300>", :thumb => "100x100>" }
+ validates_attachment_presence :file
+end
View
8 spec/dummy_app/db/migrate/20120928075608_create_images.rb
@@ -0,0 +1,8 @@
+class CreateImages < ActiveRecord::Migration
+ def change
+ create_table :images do |t|
+ t.attachment :file
+ t.timestamps
+ end
+ end
+end
View
4 spec/factories.rb
@@ -68,4 +68,8 @@
factory :hardball do
color('blue')
end
+
+ factory :image do
+ file File.open(Rails.root.join('public', 'robots.txt'))
+ end
end
View
1  spec/integration/basic/new/rails_admin_basic_new_spec.rb
@@ -70,5 +70,4 @@
page.should have_css("select#team_player_ids option[selected='selected'][value='#{@player.id}']")
end
end
-
end
View
6 spec/unit/config/fields/base_spec.rb
@@ -11,6 +11,12 @@
RailsAdmin.config('Ball').fields.first.with(:object => Ball.new).should be_required
RailsAdmin.config('Ball').fields.first.with(:object => FactoryGirl.create(:ball)).should_not be_required
end
+
+ context 'on a Paperclip installation' do
+ it "should detect required fields" do
+ RailsAdmin.config('Image').fields.find{ |f| f.name == :file }.with(:object => Image.new).should be_required
+ end
+ end
end
describe "#name" do
Something went wrong with that request. Please try again.