Validate options when managing columns and tables in migration #46178
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tgxworld
force-pushed
the
validate_migration_options
branch
5 times, most recently
from
394166d to
f0698b8
Compare
tgxworld
force-pushed
the
validate_migration_options
branch
5 times, most recently
from
3e45e30 to
a790293
Compare
|
I'm generally in favor of this, but I need to take the time to review carefully (probably won't have time today, so cc @casperisfine). One concern might be backward compatibility, generally the stance on the migration API changes have been don't break previous versions. So we might need to scope this to Rails 7.1, but other than that I don't foresee much problems. |
casperisfine
reviewed
Oct 6, 2022
casperisfine
reviewed
Oct 6, 2022
activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
Outdated
Show resolved
Hide resolved
activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
Outdated
Show resolved
Hide resolved
activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
Outdated
Show resolved
Hide resolved
activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb
Outdated
Show resolved
Hide resolved
activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
Outdated
Show resolved
Hide resolved
tgxworld
force-pushed
the
validate_migration_options
branch
4 times, most recently
from
50f4cd0 to
f9107f4
Compare
|
@casperisfine I've updated the PR per your review comments and included a CHANGELOG entry. Thank you for reviewing 👍 |
|
Please squash your commits, and then it's good for me. |
This commit adds a step to validate the options that are used when managing columns and tables in migrations. The intention is to only validate options for new migrations that are added. Invalid options used in old migrations are silently ignored like they always have been. Fixes rails#33284 Fixes rails#39230 Co-authored-by: George Wambold <georgewambold@gmail.com>
tgxworld
force-pushed
the
validate_migration_options
branch
from
f9107f4 to
e6da3eb
Compare
|
@byroot I've rebased the PR. Thank you for taking the time to review this PR. |
9 tasks
5 tasks
matsales28
added a commit
to matsales28/shoulda-matchers
that referenced
this pull request
Oct 6, 2023
…ation This commit adjusts the way we manage columns and tables in migrations to account for the changes in Rails 7.1.0, it was introduced in this Rails version a validation around the options passed to tables and columns in migrations, so we need to adjust our code to skip this validation. This PR introduced this new validation on the migrations: rails/rails#46178
alpaca-tc
added a commit
to alpaca-tc/ridgepole
that referenced
this pull request
Oct 11, 2023
unsigned is not a valid syntax in PostgreSQL and has been prohibited since rails 7.1. Since it has not changed anything in PostgreSQL since before, I think it is appropriate to remove it from the code. related: rails/rails#46178
9 tasks
alpaca-tc
added a commit
to alpaca-tc/ridgepole
that referenced
this pull request
Oct 14, 2023
unsigned is not a valid syntax in PostgreSQL and has been prohibited since rails 7.1. Since it has not changed anything in PostgreSQL since before, I think it is appropriate to remove it from the code. related: rails/rails#46178
y-yagi
added a commit
to y-yagi/activerecord-import
that referenced
this pull request
Oct 26, 2023
Rails checks options in migration since Rails 7.1. Ref: rails/rails#46178
matsales28
added a commit
to thoughtbot/shoulda-matchers
that referenced
this pull request
Nov 17, 2023
* Add Rails 7.1 to appraisals * fix: Adjust validate options when managing columns and tables in migration This commit adjusts the way we manage columns and tables in migrations to account for the changes in Rails 7.1.0, it was introduced in this Rails version a validation around the options passed to tables and columns in migrations, so we need to adjust our code to skip this validation. This PR introduced this new validation on the migrations: rails/rails#46178 * Refine `ActiveRecord::SerializeMatcher` for Rails 7.1 compatibility This commit refines the `ActiveRecord::SerializeMatcher` to accommodate changes in the public API introduced in Rails 7.1. The `serialize` method's API has been updated, necessitating adjustments to our matcher specs to align with the new API. While this PR addresses the immediate need for compatibility, there is potential for further enhancements and refinements in the matcher's implementation to bring it in line with the revised public API. However, such improvements will be explored in a future PR. For reference, the changes in the public API can be found in the following PR: rails/rails#47463 This commit adjusts the `ActiveRecord::SerializeMatcher` the public API for the `serialize` method has changed on Rails 7.1, so we had to * feat: Upadte workflow matrix * fix: Adjust comparison between hash and `ActionController::Parameters` The behavior of the comparison between hash and a `ActionController::Parameters` was changed and now will be deprecated. Check this Rails PR for more context rails/rails#44826. * fix: Adjust usage of `has_secure_token` without a token attr The behaviour of this method was changed in Rails 7.1, and now it'll search for a `token` attr in the initialization of the record instead of the creation. To stay with the same behaviour as before it's necessary to pass a new argument `on: : create` to the method. * fix: Adjust specs for primary_key check * fix: Test updating error_highlight gem * fix: Add default Rails tables to acceptance spec This commit fixes the problem of not finding the tables when running the specs in eager load mode, this was introduced by Rails 7.1. Load the model schema when running test in eager load context rails/rails#49470
davinlagerroos
pushed a commit
to umn-asr/oracle-enhanced
that referenced
this pull request
Feb 21, 2024
See rails/rails#46178 and rails/rails#47609 These were the options that were failing in the test suite.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation / Background
This Pull Request has been created because invalid options used when managing columns and tables in migrations are silently ignored.
Previous attempt in #33347
Resolves #33284
Resolves #39230
Detail
This Pull Request adds a step to validate the options that are used when managing columns and tables in migrations. The intention is to only validate options for new migrations that are added. Invalid options used in old migrations are silently ignored like they always have been.
Checklist
Before submitting the PR make sure the following are checked:
[Fix #issue-number]main(if not - rebase it).