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

ActiveRecord::DangerousAttributeError: object_id is defined by Active Record #50160

Closed
misdoro opened this issue Nov 24, 2023 · 6 comments · Fixed by #50162
Closed

ActiveRecord::DangerousAttributeError: object_id is defined by Active Record #50160

misdoro opened this issue Nov 24, 2023 · 6 comments · Fixed by #50162

Comments

@misdoro
Copy link
Contributor

misdoro commented Nov 24, 2023

With ActiveRecord Before https://github.com/rails/rails/pull/45883/files
it was possible to have a polymorphic relation object,
implicitly defining object_type and object_id columns.

Expected behavior

Able to use object as a name for a polymorphic relationship

Actual behavior

An exception

ActiveRecord::DangerousAttributeError:
       object_id is defined by Active Record. Check to make sure that you don't have an attribute or method with the same name.

is raised

System configuration

Rails version:
updating from 7.0 to 7.1

Can the object_id column be removed from the dangerous attributes list?

Having the column with that name was not breaking anything for us for many years, and now we are blocked from upgrading to rails 7.1.

@fatkodima
Copy link
Member

I think the action here is to rename the column in the app to something else.

@misdoro
Copy link
Contributor Author

misdoro commented Nov 24, 2023

Using the object_id as a column name was explicitly allowed here

@fatkodima
Copy link
Member

Ah, ok.

I see object_id is used in a few (3-5) places in Active Record. So if we change these to use __id__ instead, then we can reconsider https://github.com/rails/rails/pull/45883/files (by @byroot) by removing object_id from the list?

Still not sure this is a good idea, but object_id seems like a popular attribute name.

misdoro added a commit to misdoro/rails that referenced this issue Nov 24, 2023
Fixes rails#50160

The `object_id` name may be used by polymorphic relations where `object` is the best name, like `notification.object`.
In that case two columns are created: `object_id` and `object_type`.

Update tests and comments to reference `__id__` instead of `object_id` for consistency.
misdoro added a commit to misdoro/rails that referenced this issue Nov 24, 2023
Fixes rails#50160

The `object_id` name may be used by polymorphic relations where `object` is the best name, like `notification.object`.
In that case two columns are created: `object_id` and `object_type`.

Update tests and comments to reference `__id__` instead of `object_id` for consistency.
misdoro added a commit to misdoro/rails that referenced this issue Nov 24, 2023
Fixes rails#50160

The `object_id` name may be used by polymorphic relations where `object` is the best name, like `notification.object`.
In that case two columns are created: `object_id` and `object_type`.

Update tests and comments to reference `__id__` instead of `object_id` for consistency.
@byroot
Copy link
Member

byroot commented Nov 24, 2023

I don't mind allowing it again, but we should find some way to test that active record itself doesn't rely on it :)

@fatkodima
Copy link
Member

Custom rubocop cop probably. Or redefining object_id in active record's test suite to raise 💥

@byroot
Copy link
Member

byroot commented Nov 24, 2023

I think defining an object_id column on several of the most used fixtures models should work.

byroot pushed a commit to byroot/rails that referenced this issue Nov 28, 2023
Fixes rails#50160

The `object_id` name may be used by polymorphic relations where `object` is the best name, like `notification.object`.
In that case two columns are created: `object_id` and `object_type`.

Update tests and comments to reference `__id__` instead of `object_id` for consistency.
byroot pushed a commit to byroot/rails that referenced this issue Nov 28, 2023
Fixes rails#50160

The `object_id` name may be used by polymorphic relations where `object` is the best name, like `notification.object`.
In that case two columns are created: `object_id` and `object_type`.

Update tests and comments to reference `__id__` instead of `object_id` for consistency.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants