Skip to content

Fix missing_presence_validation for polymorphic associations#164

Closed
fatkodima wants to merge 1 commit into
gregnavis:masterfrom
fatkodima:fix-missing_presence_validation-for-polymorphic
Closed

Fix missing_presence_validation for polymorphic associations#164
fatkodima wants to merge 1 commit into
gregnavis:masterfrom
fatkodima:fix-missing_presence_validation-for-polymorphic

Conversation

@fatkodima

Copy link
Copy Markdown
Contributor

Fixes #163.

@gregnavis

Copy link
Copy Markdown
Owner

This doesn't look right. See the discussion on the corresponding issue.

Context.create_table(:images) do |t|
t.references :imageable, null: false, polymorphic: true
end.define_model do
belongs_to :imageable, polymorphic: true, required: true

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Required is deprecated in newer versions of Rails. Let's add a conditional here and pass required for older versions use optional for newer versions.

Additionally, we should be testing for all combinations:

  1. Columns are required, association is required (with covered this already)
  2. Columns are required, association is optional.
  3. Columns are optional, association is required.
  4. Columns are optional, association is optional.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Deprecation was introduced in rails/rails@6576f73

Pushed more tests.
Better to have a message mentioning to add :optional/:required to the association, but it requires too many changes to the checker. Maybe this is good enough to mention just columns?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do you think we can proceed with this fix guys?

@fatkodima fatkodima force-pushed the fix-missing_presence_validation-for-polymorphic branch from b816e91 to a6a07ee Compare December 8, 2023 14:13
@gregnavis gregnavis force-pushed the master branch 3 times, most recently from afcda83 to 4ee897c Compare November 10, 2024 14:43
@gregnavis

Copy link
Copy Markdown
Owner

I'm closing this in favor of #230 which adds support for associations.

@gregnavis gregnavis closed this Aug 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing Presence Validation with Polymorphic Type Column

3 participants