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

use singular table name if pluralize_table_names is setted as false whil... #19652

Merged
merged 1 commit into from Apr 6, 2015
Merged

use singular table name if pluralize_table_names is setted as false whil... #19652

merged 1 commit into from Apr 6, 2015

Conversation

meinac
Copy link
Contributor

@meinac meinac commented Apr 4, 2015

use singular table name if pluralize_table_names is setted as false while creating foreign key

Closes #19643

@senny
Copy link
Member

senny commented Apr 5, 2015

@meinac could you add a test-case to prevent against regressions?

@meinac
Copy link
Contributor Author

meinac commented Apr 5, 2015

@senny I've created a test case for this situation.

@@ -12,6 +12,7 @@ class ReferencesForeignKeyTest < ActiveRecord::TestCase
teardown do
@connection.drop_table "testings", if_exists: true
@connection.drop_table "testing_parents", if_exists: true
@connection.drop_table "testing", if_exists: true
Copy link
Member

Choose a reason for hiding this comment

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

can you move that drop_table into an ensure clause for the specific test. No need to run it for every other test if the table is only created in one of em.

@senny senny self-assigned this Apr 5, 2015
@senny senny added this to the 4.2.2 milestone Apr 5, 2015
@meinac
Copy link
Contributor Author

meinac commented Apr 5, 2015

@senny what about this?

assert_equal "testing", fk.to_table
ensure
ActiveRecord::Base.pluralize_table_names = original_pluralize_table_names
@connection.drop_table "testing_parents"
Copy link
Member

Choose a reason for hiding this comment

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

let's add if_exists: true. The test could fail on any line in the begin block. The tables might not exists and this could hide errors.

Copy link
Member

Choose a reason for hiding this comment

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

Aren't these two tables dropped in teardown already?

      teardown do
        @connection.drop_table "testings", if_exists: true
        @connection.drop_table "testing_parents", if_exists: true
      end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have to drop testing parent table here because I'm adding foreign key to it. If you don't drop it you can't drop testing table too.

@meinac
Copy link
Contributor Author

meinac commented Apr 5, 2015

@senny I believe this is ok now :)
I have to drop testing_parents table before the testing table because foreign key constraint will fail if I try to drop testing table before testing_parents table.

…hile creating foreign key

test case for use singular table name if pluralize_table_names is setted as false while creating foreign key

refactor references foreign key addition tests

use singular table name while removing foreign key

merge foreign key singular table name methods

remove unnecessary drop table from test
@meinac
Copy link
Contributor Author

meinac commented Apr 6, 2015

I've remove drop table for testing_parents table because I'm removing foreign key from this table inside of the test case so don't need to drop this table before testing table

@senny senny merged commit 8c11807 into rails:master Apr 6, 2015
senny added a commit that referenced this pull request Apr 6, 2015
…_bug

use singular table name if pluralize_table_names is setted as false whil...
@senny
Copy link
Member

senny commented Apr 6, 2015

@meinac thank you.

I'll look into a backport to 4-2-stable.

senny added a commit that referenced this pull request Apr 6, 2015
…_bug

use singular table name if pluralize_table_names is setted as false whil...

Conflicts:
	activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ActiveRecord::Migration table.references() ignores ActiveRecord::Base.pluralize_table_names = false
2 participants