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

Handle nullable foreign key in multiselect #2446

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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

### Fixed
- Add button for nested-many form in modal disappeared on click([#2372](https://github.com/sferik/rails_admin/issues/2372), [#2383](https://github.com/sferik/rails_admin/pull/2383))
- Prevent users to remove association's element when foreign key is not nullable ([#2446](https://github.com/sferik/rails_admin/issues/2446))


## [0.7.0](https://github.com/sferik/rails_admin/tree/v0.7.0) - 2015-08-16
Expand Down
44 changes: 25 additions & 19 deletions app/assets/javascripts/rails_admin/ra.filtering-multiselect.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
return { query: query };
},
sortable: false,
removable: true,
regional: {
up: "Up",
down: "Down",
Expand Down Expand Up @@ -78,24 +79,27 @@


this.add = $('<a href="#" class="ui-icon ui-icon-circle-triangle-e ra-multiselect-item-add">' + this.options.regional.add + '</a>');
this.columns.center.append(this.add);

this.remove = $('<a href="#" class="ui-icon ui-icon-circle-triangle-w ra-multiselect-item-remove">' + this.options.regional.remove + '</a>');
if (this.options.removable) {
this.remove = $('<a href="#" class="ui-icon ui-icon-circle-triangle-w ra-multiselect-item-remove">' + this.options.regional.remove + '</a>');
this.columns.center.append(this.remove);
}

this.columns.center.append(this.add).append(this.remove)
if (this.options.sortable) {
this.up = $('<a href="#" class="ui-icon ui-icon-circle-triangle-n ra-multiselect-item-up">' + this.options.regional.up + '</a>');
this.down = $('<a href="#" class="ui-icon ui-icon-circle-triangle-s ra-multiselect-item-down">' + this.options.regional.down + '</a>');
this.columns.center.append(this.up).append(this.down);
}

this.selection = $('<select class="form-control ra-multiselect-selection" multiple="multiple"></select>');
this.columns.right.append(this.selection);



this.removeAll = $('<a href="#" class="ra-multiselect-item-remove-all"><span class="ui-icon ui-icon-circle-triangle-w"></span>' + this.options.regional.clearAll + '</a>');

this.columns.right.append(this.selection)
.append(this.removeAll);
if (this.options.removable) {
this.removeAll = $('<a href="#" class="ra-multiselect-item-remove-all"><span class="ui-icon ui-icon-circle-triangle-w"></span>' + this.options.regional.clearAll + '</a>');
this.columns.right.append(this.removeAll);
}

this.selection.wrap('<div class="wrapper"/>');

Expand Down Expand Up @@ -127,19 +131,21 @@
widget.selection.trigger('change');
});

/* Remove all from selection */
this.removeAll.click(function(e){
widget._deSelect($('option', widget.selection));
e.preventDefault();
widget.selection.trigger('change');
});
if (this.options.removable) {
/* Remove all from selection */
this.removeAll.click(function(e){
widget._deSelect($('option', widget.selection));
e.preventDefault();
widget.selection.trigger('change');
});

/* Remove from selection */
this.remove.click(function(e){
widget._deSelect($(':selected', widget.selection));
e.preventDefault();
widget.selection.trigger('change');
});
/* Remove from selection */
this.remove.click(function(e){
widget._deSelect($(':selected', widget.selection));
e.preventDefault();
widget.selection.trigger('change');
});
}

var timeout = null;
if(this.options.sortable) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
:'edit-url' => (authorized?(:edit, config.abstract_model) ? edit_path(model_name: config.abstract_model.to_param, id: '__ID__') : ''),
remote_source: index_path(config.abstract_model, source_object_id: form.object.id, source_abstract_model: source_abstract_model.to_param, associated_collection: field.name, current_action: current_action, compact: true),
sortable: !!field.orderable,
removable: !!field.removable,
cacheAll: !!field.associated_collection_cache_all,
regional: {
chooseAll: t("admin.misc.chose_all"),
Expand Down
5 changes: 5 additions & 0 deletions lib/rails_admin/adapters/active_record/association.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ def foreign_key
association.foreign_key.to_sym
end

def foreign_key_nullable?
return if foreign_key.nil? || type != :has_many
klass.columns_hash[association.foreign_key].null
end

def foreign_type
options[:foreign_type].try(:to_sym) || :"#{name}_type" if options[:polymorphic]
end
Expand Down
5 changes: 5 additions & 0 deletions lib/rails_admin/adapters/mongoid/association.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ def foreign_key
association.foreign_key.to_sym rescue nil
end

def foreign_key_nullable?
return if foreign_key.nil?
true
end

def foreign_type
return unless polymorphic? && [:referenced_in, :belongs_to].include?(macro)
association.inverse_type.try(:to_sym) || :"#{name}_type"
Expand Down
5 changes: 5 additions & 0 deletions lib/rails_admin/config/fields/association.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ def association # rubocop:disable TrivialAccessors
@associated_collection_cache_all ||= (associated_model_config.abstract_model.count < 100)
end

# Reader whether association's elements can be removed
def removable
association.foreign_key_nullable?
end

# Reader for the association's child model's configuration
def associated_model_config
@associated_model_config ||= RailsAdmin.config(association.klass)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class ChangeFieldTestIdToNestedFieldTests < ActiveRecord::Migration
def change
change_column :nested_field_tests, :field_test_id, :integer, null: false
end
end
45 changes: 42 additions & 3 deletions spec/integration/config/edit/rails_admin_config_edit_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,8 @@ class HelpTest < Tableless
describe 'nested form' do
it 'works', js: true do
@record = FactoryGirl.create :field_test
@record.nested_field_tests = [NestedFieldTest.create!(title: 'title 1'), NestedFieldTest.create!(title: 'title 2')]
NestedFieldTest.create! title: 'title 1', field_test: @record
NestedFieldTest.create! title: 'title 2', field_test: @record
visit edit_path(model_name: 'field_test', id: @record.id)

find('#field_test_comment_attributes_field .add_nested_fields').click
Expand Down Expand Up @@ -743,7 +744,7 @@ class HelpTest < Tableless
end
end
@record = FieldTest.create
@record.nested_field_tests << NestedFieldTest.create!(title: 'title 1')
NestedFieldTest.create! title: 'title 1', field_test: @record
visit edit_path(model_name: 'field_test', id: @record.id)
expect(find('#field_test_nested_field_tests_attributes_0_title_field')).to have_content('NestedFieldTest')
end
Expand Down Expand Up @@ -776,7 +777,7 @@ class HelpTest < Tableless

it 'does not show destroy button except for newly created when :allow_destroy is false' do
@record = FieldTest.create
@record.nested_field_tests << NestedFieldTest.create!(title: 'nested title 1')
NestedFieldTest.create! title: 'nested title 1', field_test: @record
allow(FieldTest.nested_attributes_options).to receive(:[]).with(:nested_field_tests).
and_return(allow_destroy: false, update_only: false)
visit edit_path(model_name: 'field_test', id: @record.id)
Expand Down Expand Up @@ -817,6 +818,44 @@ class HelpTest < Tableless
end
end

describe 'has_many', active_record: true do
context 'with not nullable foreign key' do
before do
RailsAdmin.config FieldTest do
edit do
field :nested_field_tests do
nested_form false
end
end
end
@field_test = FactoryGirl.create :field_test
end

it 'don\'t allow to remove element', js: true do
visit edit_path(model_name: 'FieldTest', id: @field_test.id)
is_expected.not_to have_selector('a.ra-multiselect-item-remove')
is_expected.not_to have_selector('a.ra-multiselect-item-remove-all')
end
end

context 'with nullable foreign key' do
before do
RailsAdmin.config Team do
edit do
field :players
end
end
@team = FactoryGirl.create :team
end

it 'allow to remove element', js: true do
visit edit_path(model_name: 'Team', id: @team.id)
is_expected.to have_selector('a.ra-multiselect-item-remove')
is_expected.to have_selector('a.ra-multiselect-item-remove-all')
end
end
end

describe 'fields which are nullable and have AR validations', active_record: true do
it 'is required' do
# draft.notes is nullable and has no validation
Expand Down
15 changes: 15 additions & 0 deletions spec/rails_admin/adapters/active_record/association_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ class ARComment < ActiveRecord::Base
expect(association.type).to eq :has_many
expect(association.klass).to eq Division
expect(association.read_only?).to be_falsey
expect(association.foreign_key_nullable?).to be_truthy
end
end

Expand All @@ -113,6 +114,7 @@ class ARComment < ActiveRecord::Base
expect(association.type).to eq :has_many
expect(association.klass).to eq Team
expect(association.read_only?).to be_truthy
expect(association.foreign_key_nullable?).to be_truthy
end
end

Expand All @@ -127,6 +129,18 @@ class ARComment < ActiveRecord::Base
end
end

describe 'has_many association with not nullable foreign key' do
let(:field_test) { RailsAdmin::AbstractModel.new(FieldTest) }

context 'for direct has many' do
let(:association) { field_test.associations.detect { |a| a.name == :nested_field_tests } }

it 'returns correct values' do
expect(association.foreign_key_nullable?).to be_falsey
end
end
end

describe 'has_and_belongs_to_many association' do
subject { @post.associations.detect { |a| a.name == :a_r_categories } }

Expand All @@ -135,6 +149,7 @@ class ARComment < ActiveRecord::Base
expect(subject.klass).to eq ARCategory
expect(subject.primary_key).to eq :id
expect(subject.foreign_type).to be_nil
expect(subject.foreign_key_nullable?).to be_nil
expect(subject.as).to be_nil
expect(subject.polymorphic?).to be_falsey
expect(subject.inverse_of).to be_nil
Expand Down
7 changes: 7 additions & 0 deletions spec/rails_admin/adapters/mongoid/association_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ class MongoNote
expect(subject.klass).to eq MongoBlog
expect(subject.primary_key).to eq :_id
expect(subject.foreign_key).to eq :mongo_blog_id
expect(subject.foreign_key_nullable?).to be_truthy
expect(subject.foreign_type).to be_nil
expect(subject.foreign_inverse_of).to be_nil
expect(subject.as).to be_nil
Expand All @@ -111,6 +112,7 @@ class MongoNote
expect(subject.klass).to eq MongoPost
expect(subject.primary_key).to eq :_id
expect(subject.foreign_key).to eq :mongo_blog_id
expect(subject.foreign_key_nullable?).to be_truthy
expect(subject.foreign_type).to be_nil
expect(subject.foreign_inverse_of).to be_nil
expect(subject.as).to be_nil
Expand All @@ -130,6 +132,7 @@ class MongoNote
expect(subject.klass).to eq MongoCategory
expect(subject.primary_key).to eq :_id
expect(subject.foreign_key).to eq :mongo_category_ids
expect(subject.foreign_key_nullable?).to be_truthy
expect(subject.foreign_type).to be_nil
expect(subject.foreign_inverse_of).to be_nil
expect(subject.as).to be_nil
Expand All @@ -150,6 +153,7 @@ class MongoNote
expect(subject.klass).to eq [MongoBlog, MongoPost]
expect(subject.primary_key).to eq :_id
expect(subject.foreign_key).to eq :commentable_id
expect(subject.foreign_key_nullable?).to be_truthy
expect(subject.foreign_type).to eq :commentable_type
expect(subject.foreign_inverse_of).to be_nil
expect(subject.as).to be_nil
Expand All @@ -170,6 +174,7 @@ class MongoNote
expect(subject.klass).to eq MongoComment
expect(subject.primary_key).to eq :_id
expect(subject.foreign_key).to eq :commentable_id
expect(subject.foreign_key_nullable?).to be_truthy
expect(subject.foreign_type).to be_nil
expect(subject.foreign_inverse_of).to be_nil
expect(subject.as).to eq :commentable
Expand All @@ -194,6 +199,7 @@ class MongoNote
expect(subject.klass).to eq MongoNote
expect(subject.primary_key).to eq :_id
expect(subject.foreign_key).to be_nil
expect(subject.foreign_key_nullable?).to be_nil
expect(subject.foreign_type).to be_nil
expect(subject.foreign_inverse_of).to be_nil
expect(subject.as).to be_nil
Expand All @@ -213,6 +219,7 @@ class MongoNote
expect(subject.klass).to eq MongoNote
expect(subject.primary_key).to eq :_id
expect(subject.foreign_key).to be_nil
expect(subject.foreign_key_nullable?).to be_nil
expect(subject.foreign_type).to be_nil
expect(subject.as).to be_nil
expect(subject.polymorphic?).to be_falsey
Expand Down