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

Fix missing_presence_validation for polymorphic associations #164

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fatkodima
Copy link
Contributor

Fixes #163.

@gregnavis
Copy link
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
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
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?

@fatkodima fatkodima force-pushed the fix-missing_presence_validation-for-polymorphic branch from b816e91 to a6a07ee Compare December 8, 2023 14:13
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
2 participants