Permalink
Browse files

accepts_nested_attributes_for + :finder_sql => parent_id included

When using accepts_nested_attributes_for on an association that uses
:finder_sql, the "#{parent.class.name.to_s.downcase}_id" column gets
included in the generated SQL query.

The object id suffices in itself, there is no need to include the parent
id, which doesn't exist, as the association is synthetic.
There is no need to even have a valid path from a Host to the
LookupValue: if the LookupValue references the Host name, the former
should be listed in the latter
(hence many :through associations cannot do).

    Host -> Puppetclass -> Lookup key -> Lookup value
         `----------------------------´
  • Loading branch information...
ofavre committed Aug 7, 2012
0 parents commit f7d7fda89d69c2fb99f0ec0eaec6639b73cec253
Showing with 2,402 additions and 0 deletions.
  1. +21 −0 .gitignore
  2. +42 −0 Gemfile
  3. +112 −0 Gemfile.lock
  4. +58 −0 README.rdoc
  5. +7 −0 Rakefile
  6. BIN app/assets/images/rails.png
  7. +15 −0 app/assets/javascripts/application.js
  8. +20 −0 app/assets/javascripts/helper.js
  9. +2 −0 app/assets/javascripts/home.js
  10. +2 −0 app/assets/javascripts/host_classes.js
  11. +2 −0 app/assets/javascripts/hosts.js
  12. +2 −0 app/assets/javascripts/lookup_keys.js
  13. +2 −0 app/assets/javascripts/lookup_values.js
  14. +2 −0 app/assets/javascripts/puppetclasses.js
  15. +13 −0 app/assets/stylesheets/application.css
  16. +16 −0 app/assets/stylesheets/helper.css
  17. +4 −0 app/assets/stylesheets/home.css
  18. +4 −0 app/assets/stylesheets/host_classes.css
  19. +4 −0 app/assets/stylesheets/hosts.css
  20. +4 −0 app/assets/stylesheets/lookup_keys.css
  21. +4 −0 app/assets/stylesheets/lookup_values.css
  22. +4 −0 app/assets/stylesheets/puppetclasses.css
  23. +56 −0 app/assets/stylesheets/scaffold.css
  24. +3 −0 app/controllers/application_controller.rb
  25. +4 −0 app/controllers/home_controller.rb
  26. +83 −0 app/controllers/host_classes_controller.rb
  27. +83 −0 app/controllers/hosts_controller.rb
  28. +83 −0 app/controllers/lookup_keys_controller.rb
  29. +83 −0 app/controllers/lookup_values_controller.rb
  30. +83 −0 app/controllers/puppetclasses_controller.rb
  31. +25 −0 app/helpers/application_helper.rb
  32. +2 −0 app/helpers/home_helper.rb
  33. +2 −0 app/helpers/host_classes_helper.rb
  34. +2 −0 app/helpers/hosts_helper.rb
  35. +2 −0 app/helpers/lookup_keys_helper.rb
  36. +2 −0 app/helpers/lookup_values_helper.rb
  37. +2 −0 app/helpers/puppetclasses_helper.rb
  38. 0 app/mailers/.gitkeep
  39. 0 app/models/.gitkeep
  40. +12 −0 app/models/host.rb
  41. +5 −0 app/models/host_class.rb
  42. +11 −0 app/models/lookup_key.rb
  43. +9 −0 app/models/lookup_value.rb
  44. +11 −0 app/models/puppetclass.rb
  45. +1 −0 app/views/home/index.html.erb
  46. +25 −0 app/views/host_classes/_form.html.erb
  47. +6 −0 app/views/host_classes/edit.html.erb
  48. +25 −0 app/views/host_classes/index.html.erb
  49. +5 −0 app/views/host_classes/new.html.erb
  50. +15 −0 app/views/host_classes/show.html.erb
  51. +49 −0 app/views/hosts/_form.html.erb
  52. +6 −0 app/views/hosts/edit.html.erb
  53. +23 −0 app/views/hosts/index.html.erb
  54. +5 −0 app/views/hosts/new.html.erb
  55. +28 −0 app/views/hosts/show.html.erb
  56. +27 −0 app/views/layouts/application.html.erb
  57. +32 −0 app/views/lookup_keys/_fields.html.erb
  58. +19 −0 app/views/lookup_keys/_form.html.erb
  59. +6 −0 app/views/lookup_keys/edit.html.erb
  60. +27 −0 app/views/lookup_keys/index.html.erb
  61. +5 −0 app/views/lookup_keys/new.html.erb
  62. +20 −0 app/views/lookup_keys/show.html.erb
  63. +24 −0 app/views/lookup_values/_fields.html.erb
  64. +20 −0 app/views/lookup_values/_form.html.erb
  65. +6 −0 app/views/lookup_values/edit.html.erb
  66. +27 −0 app/views/lookup_values/index.html.erb
  67. +5 −0 app/views/lookup_values/new.html.erb
  68. +20 −0 app/views/lookup_values/show.html.erb
  69. +30 −0 app/views/puppetclasses/_form.html.erb
  70. +6 −0 app/views/puppetclasses/edit.html.erb
  71. +23 −0 app/views/puppetclasses/index.html.erb
  72. +5 −0 app/views/puppetclasses/new.html.erb
  73. +15 −0 app/views/puppetclasses/show.html.erb
  74. +4 −0 config.ru
  75. +62 −0 config/application.rb
  76. +6 −0 config/boot.rb
  77. +25 −0 config/database.yml
  78. +5 −0 config/environment.rb
  79. +37 −0 config/environments/development.rb
  80. +67 −0 config/environments/production.rb
  81. +37 −0 config/environments/test.rb
  82. +7 −0 config/initializers/backtrace_silencers.rb
  83. +15 −0 config/initializers/inflections.rb
  84. +5 −0 config/initializers/mime_types.rb
  85. +7 −0 config/initializers/secret_token.rb
  86. +8 −0 config/initializers/session_store.rb
  87. +14 −0 config/initializers/wrap_parameters.rb
  88. +5 −0 config/locales/en.yml
  89. +70 −0 config/routes.rb
  90. +9 −0 db/migrate/20120806143308_create_hosts.rb
  91. +9 −0 db/migrate/20120806143329_create_puppetclasses.rb
  92. +12 −0 db/migrate/20120806143338_create_lookup_keys.rb
  93. +12 −0 db/migrate/20120806143343_create_lookup_values.rb
  94. +10 −0 db/migrate/20120806143359_create_host_classes.rb
  95. +56 −0 db/schema.rb
  96. +7 −0 db/seeds.rb
  97. 0 lib/assets/.gitkeep
  98. 0 lib/tasks/.gitkeep
  99. 0 log/.gitkeep
  100. +26 −0 public/404.html
  101. +26 −0 public/422.html
  102. +25 −0 public/500.html
  103. 0 public/favicon.ico
  104. +5 −0 public/robots.txt
  105. +6 −0 script/rails
  106. 0 test/fixtures/.gitkeep
  107. +9 −0 test/fixtures/host_classes.yml
  108. +7 −0 test/fixtures/hosts.yml
  109. +11 −0 test/fixtures/lookup_keys.yml
  110. +16 −0 test/fixtures/lookup_values.yml
  111. +7 −0 test/fixtures/puppetclasses.yml
  112. 0 test/functional/.gitkeep
  113. +9 −0 test/functional/home_controller_test.rb
  114. +49 −0 test/functional/host_classes_controller_test.rb
  115. +78 −0 test/functional/hosts_controller_test.rb
  116. +49 −0 test/functional/lookup_keys_controller_test.rb
  117. +49 −0 test/functional/lookup_values_controller_test.rb
  118. +49 −0 test/functional/puppetclasses_controller_test.rb
  119. 0 test/integration/.gitkeep
  120. +12 −0 test/performance/browsing_test.rb
  121. +13 −0 test/test_helper.rb
  122. 0 test/unit/.gitkeep
  123. +4 −0 test/unit/helpers/home_helper_test.rb
  124. +4 −0 test/unit/helpers/host_classes_helper_test.rb
  125. +4 −0 test/unit/helpers/hosts_helper_test.rb
  126. +4 −0 test/unit/helpers/lookup_keys_helper_test.rb
  127. +4 −0 test/unit/helpers/lookup_values_helper_test.rb
  128. +4 −0 test/unit/helpers/puppetclasses_helper_test.rb
  129. +7 −0 test/unit/host_class_test.rb
  130. +7 −0 test/unit/host_test.rb
  131. +7 −0 test/unit/lookup_key_test.rb
  132. +7 −0 test/unit/lookup_value_test.rb
  133. +7 −0 test/unit/puppetclass_test.rb
@@ -0,0 +1,21 @@
+# See http://help.github.com/ignore-files/ for more about ignoring files.
+#
+# If you find yourself ignoring temporary files generated by your text editor
+# or operating system, you probably want to add a global ignore instead:
+# git config --global core.excludesfile ~/.gitignore_global
+
+# Ignore bundler config
+/.bundle
+
+# Ignore the default SQLite database.
+/db/*.sqlite3
+
+# Ignore all logfiles and tempfiles.
+/log/*.log
+/tmp
+
+# Ignore the vendor path, where bundler installs its stuff
+/vendor
+
+# Ignore generated documentation
+/doc
42 Gemfile
@@ -0,0 +1,42 @@
+source 'https://rubygems.org'
+
+gem 'rails', '3.2.7'
+# Bundle edge Rails instead:
+#gem 'rails', :git => 'git://github.com/rails/rails.git'
+
+gem 'therubyracer', :require => 'v8'
+#gem 'active_record_deprecated_finders', :git => 'git://github.com/rails/active_record_deprecated_finders.git'
+#gem 'journey', :git => 'git://github.com/rails/journey.git'
+
+gem 'sqlite3'
+
+gem 'json'
+
+# Gems used only for assets and not required
+# in production environments by default.
+group :assets do
+ #gem 'sass-rails', '~> 3.2.3'
+ #gem 'coffee-rails', '~> 3.2.1'
+
+ # See https://github.com/sstephenson/execjs#readme for more supported runtimes
+ # gem 'therubyracer', :platforms => :ruby
+
+ gem 'uglifier', '>= 1.0.3'
+end
+
+gem 'jquery-rails'
+
+# To use ActiveModel has_secure_password
+# gem 'bcrypt-ruby', '~> 3.0.0'
+
+# To use Jbuilder templates for JSON
+# gem 'jbuilder'
+
+# Use unicorn as the app server
+# gem 'unicorn'
+
+# Deploy with Capistrano
+# gem 'capistrano'
+
+# To use debugger
+gem 'ruby-debug'
@@ -0,0 +1,112 @@
+GEM
+ remote: https://rubygems.org/
+ specs:
+ actionmailer (3.2.7)
+ actionpack (= 3.2.7)
+ mail (~> 2.4.4)
+ actionpack (3.2.7)
+ activemodel (= 3.2.7)
+ activesupport (= 3.2.7)
+ builder (~> 3.0.0)
+ erubis (~> 2.7.0)
+ journey (~> 1.0.4)
+ rack (~> 1.4.0)
+ rack-cache (~> 1.2)
+ rack-test (~> 0.6.1)
+ sprockets (~> 2.1.3)
+ activemodel (3.2.7)
+ activesupport (= 3.2.7)
+ builder (~> 3.0.0)
+ activerecord (3.2.7)
+ activemodel (= 3.2.7)
+ activesupport (= 3.2.7)
+ arel (~> 3.0.2)
+ tzinfo (~> 0.3.29)
+ activeresource (3.2.7)
+ activemodel (= 3.2.7)
+ activesupport (= 3.2.7)
+ activesupport (3.2.7)
+ i18n (~> 0.6)
+ multi_json (~> 1.0)
+ arel (3.0.2)
+ builder (3.0.0)
+ columnize (0.3.6)
+ erubis (2.7.0)
+ execjs (1.4.0)
+ multi_json (~> 1.0)
+ hike (1.2.1)
+ i18n (0.6.0)
+ journey (1.0.4)
+ jquery-rails (2.0.2)
+ railties (>= 3.2.0, < 5.0)
+ thor (~> 0.14)
+ json (1.7.4)
+ libv8 (3.3.10.4)
+ linecache (0.46)
+ rbx-require-relative (> 0.0.4)
+ mail (2.4.4)
+ i18n (>= 0.4.0)
+ mime-types (~> 1.16)
+ treetop (~> 1.4.8)
+ mime-types (1.19)
+ multi_json (1.3.6)
+ polyglot (0.3.3)
+ rack (1.4.1)
+ rack-cache (1.2)
+ rack (>= 0.4)
+ rack-ssl (1.3.2)
+ rack
+ rack-test (0.6.1)
+ rack (>= 1.0)
+ rails (3.2.7)
+ actionmailer (= 3.2.7)
+ actionpack (= 3.2.7)
+ activerecord (= 3.2.7)
+ activeresource (= 3.2.7)
+ activesupport (= 3.2.7)
+ bundler (~> 1.0)
+ railties (= 3.2.7)
+ railties (3.2.7)
+ actionpack (= 3.2.7)
+ activesupport (= 3.2.7)
+ rack-ssl (~> 1.3.2)
+ rake (>= 0.8.7)
+ rdoc (~> 3.4)
+ thor (>= 0.14.6, < 2.0)
+ rake (0.9.2.2)
+ rbx-require-relative (0.0.9)
+ rdoc (3.12)
+ json (~> 1.4)
+ ruby-debug (0.10.4)
+ columnize (>= 0.1)
+ ruby-debug-base (~> 0.10.4.0)
+ ruby-debug-base (0.10.4)
+ linecache (>= 0.3)
+ sprockets (2.1.3)
+ hike (~> 1.2)
+ rack (~> 1.0)
+ tilt (~> 1.1, != 1.3.0)
+ sqlite3 (1.3.6)
+ therubyracer (0.10.1)
+ libv8 (~> 3.3.10)
+ thor (0.15.4)
+ tilt (1.3.3)
+ treetop (1.4.10)
+ polyglot
+ polyglot (>= 0.3.1)
+ tzinfo (0.3.33)
+ uglifier (1.2.7)
+ execjs (>= 0.3.0)
+ multi_json (~> 1.3)
+
+PLATFORMS
+ ruby
+
+DEPENDENCIES
+ jquery-rails
+ json
+ rails (= 3.2.7)
+ ruby-debug
+ sqlite3
+ therubyracer
+ uglifier (>= 1.0.3)
@@ -0,0 +1,58 @@
+= PoB: +parent_id+ gets included with +:finder_sql+ association
+
+This sample Rails web-application tends to reproduce a part of Foreman's architecture
+in order to highlight a potential bug of Rails.
+
+== How to fire the bug
+
+=== Using functional testing
+
+run `rake test`, or the following command:
+
+ $ rake test TEST=test/functional/hosts_controller_test.rb
+
+You will get:
+
+ DEPRECATION WARNING: The :finder_sql association option is deprecated. Please find an alternative (such as using scopes). (called from /data/theforeman/test-shortcut/app/models/host.rb:5)
+ Loaded suite /data/theforeman/test-shortcut/vendor/ruby/1.8/gems/rake-0.9.2.2/lib/rake/rake_test_loader
+ Started
+ ........E
+ Finished in 0.328863 seconds.
+
+ 1) Error:
+ test_should_update_lookup_value(HostsControllerTest):
+ ActiveRecord::StatementInvalid: SQLite3::SQLException: no such column: lookup_values.host_id: SELECT "lookup_values".* FROM "lookup_values" WHERE "lookup_values"."host_id" = 980190962 AND "lookup_values"."id" IN (980190962)
+ app/controllers/hosts_controller.rb:62:in `update'
+ app/controllers/hosts_controller.rb:61:in `update'
+ test/functional/hosts_controller_test.rb:56:in `test_should_update_lookup_value'
+
+ 9 tests, 11 assertions, 0 failures, 1 errors
+ rake aborted!
+ Command failed with status (1): [/usr/bin/ruby1.8 -I"lib:test" -I"/data/the...]
+
+ Tasks: TOP => test:single
+ (See full trace by running task with --trace)
+
+=== Manually through the web UI
+
+1. Create a puppetclass
+
+2. Create a lookup key
+
+3. Create a lookup value, with matcher "hostname=foo"
+
+4. Create a host of name "foo"
+
+5. Associate the host to the created puppetclass
+
+6. Edit the host
+
+ Eventually modify the lookup value's value
+
+ Save
+
+You will get:
+
+ ActiveRecord::StatementInvalid in HostsController#update
+
+ SQLite3::SQLException: no such column: lookup_values.host_id: SELECT "lookup_values".* FROM "lookup_values" WHERE "lookup_values"."host_id" = 1 AND "lookup_values"."id" IN (1)
@@ -0,0 +1,7 @@
+#!/usr/bin/env rake
+# Add your own tasks in files placed in lib/tasks ending in .rake,
+# for example lib/tasks/capistrano.rake, and they will automatically be available to Rake.
+
+require File.expand_path('../config/application', __FILE__)
+
+TestShortcut::Application.load_tasks
Binary file not shown.
@@ -0,0 +1,15 @@
+// This is a manifest file that'll be compiled into application.js, which will include all the files
+// listed below.
+//
+// Any JavaScript/Coffee file within this directory, lib/assets/javascripts, vendor/assets/javascripts,
+// or vendor/assets/javascripts of plugins, if any, can be referenced here using a relative path.
+//
+// It's not advisable to add code directly here, but if you do, it'll appear at the bottom of the
+// the compiled file.
+//
+// WARNING: THE FIRST BLANK LINE MARKS THE END OF WHAT'S TO BE PROCESSED, ANY BLANK LINE SHOULD
+// GO AFTER THE REQUIRES BELOW.
+//
+//= require jquery
+//= require jquery_ujs
+//= require_tree .
@@ -0,0 +1,20 @@
+$(function() {
+ $('form a.add_child').click(function() {
+ var association = $(this).attr('data-association');
+ var template = $('#' + association + '_fields_template').html();
+ var regexp = new RegExp('new_' + association, 'g');
+ var new_id = new Date().getTime();
+
+ $(this).parent().before(template.replace(regexp, new_id));
+ return false;
+ });
+
+ $('form a.remove_child').live('click', function() {
+ var hidden_field = $(this).prev('input[type=hidden]')[0];
+ if(hidden_field) {
+ hidden_field.value = '1';
+ }
+ $(this).parents('.fields').hide();
+ return false;
+ });
+});
@@ -0,0 +1,2 @@
+// Place all the behaviors and hooks related to the matching controller here.
+// All this logic will automatically be available in application.js.
@@ -0,0 +1,2 @@
+// Place all the behaviors and hooks related to the matching controller here.
+// All this logic will automatically be available in application.js.
@@ -0,0 +1,2 @@
+// Place all the behaviors and hooks related to the matching controller here.
+// All this logic will automatically be available in application.js.
@@ -0,0 +1,2 @@
+// Place all the behaviors and hooks related to the matching controller here.
+// All this logic will automatically be available in application.js.
@@ -0,0 +1,2 @@
+// Place all the behaviors and hooks related to the matching controller here.
+// All this logic will automatically be available in application.js.
@@ -0,0 +1,2 @@
+// Place all the behaviors and hooks related to the matching controller here.
+// All this logic will automatically be available in application.js.
@@ -0,0 +1,13 @@
+/*
+ * This is a manifest file that'll be compiled into application.css, which will include all the files
+ * listed below.
+ *
+ * Any CSS and SCSS file within this directory, lib/assets/stylesheets, vendor/assets/stylesheets,
+ * or vendor/assets/stylesheets of plugins, if any, can be referenced here using a relative path.
+ *
+ * You're free to add application-wide styles to this file and they'll appear at the top of the
+ * compiled file, but it's generally better to create a new file per style scope.
+ *
+ *= require_self
+ *= require_tree .
+ */
@@ -0,0 +1,16 @@
+.topbar {
+ left: 0px;
+ top: 0px;
+ right: 0px;
+
+ background-color: black;
+ color: white;
+}
+
+.topbar a, .topbar a:visited {
+ color: white;
+}
+
+.inline, ul.menu li {
+ display: inline-block;
+}
@@ -0,0 +1,4 @@
+/*
+ Place all the styles related to the matching controller here.
+ They will automatically be included in application.css.
+*/
@@ -0,0 +1,4 @@
+/*
+ Place all the styles related to the matching controller here.
+ They will automatically be included in application.css.
+*/
@@ -0,0 +1,4 @@
+/*
+ Place all the styles related to the matching controller here.
+ They will automatically be included in application.css.
+*/
@@ -0,0 +1,4 @@
+/*
+ Place all the styles related to the matching controller here.
+ They will automatically be included in application.css.
+*/
@@ -0,0 +1,4 @@
+/*
+ Place all the styles related to the matching controller here.
+ They will automatically be included in application.css.
+*/
@@ -0,0 +1,4 @@
+/*
+ Place all the styles related to the matching controller here.
+ They will automatically be included in application.css.
+*/
Oops, something went wrong.

0 comments on commit f7d7fda

Please sign in to comment.