aboutsummaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* Merge pull request #35899 from ↵Eileen M. Uchitelle2019-04-152-17/+51
|\ | | | | | | | | eileencodes/fix-connection-when-handler-doesnt-exist Ensure a handler is set when using `connected_to`
| * Ensure a handler is set when using `connected_to`eileencodes2019-04-082-17/+51
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | After looking at #35800 there is definitely an issue in the `connected_to` method although it's generally behaving. Here are the details: 1) I added a default connection role - writing - to the connection handler lookup. I did this because otherwise if you did this: ``` connected_to(databse: :development) ``` And development wasn't a pre-established role it would create a new handler and connect using that. I don't think this is right so I've updated it to pick up the default (:writing) unless otherwise specified. To set a handler when using the database version pass a hash like you would to `connects_to`: ``` connected_to(database: { readonly_slow: :development }) ``` This will connect the `development` database to the `readonly_slow` handler/role. 2) I updated the tests to match this behavior that we expect. 3) I updated the documentation to clarify that using `connected_to` with a `database` key will establish a new connection every time. This is exactly how `establish_connection` behaves and I feel this is correct. If you want to only establish a connection once you should do that in the model with `connects_to` and then swap on the role instead of on the database hash/key. 4) In regards to #35800 this fixes the case where you pass a symbol to the db and not a hash. But it doesn't fix a case where you may pass an unknown handler to an abstract class that's not connected. This is tricky because technical AbstractFoo doesn't have any connections except for through ApplicationRecord, so in the case of the application that was shared we should only be swapping connections on ActiveRecord::Base because there are no other actual connections - AbstractFoo isn't needed since it's not establishing a new connection. If we need AbstractFoo to connect to a new handler we should establish that connection with the handler in AbstractFoo before trying to shard there.
* | Merge pull request #35974 from mrhead/remove-unused-modulesGeorge Claghorn2019-04-151-4/+0
|\ \ | | | | | | Remove unused modules from StrongParameters
| * | Remove unused modules from StrongParametersPatrik Bóna2019-04-151-4/+0
|/ / | | | | | | | | | | Unless I'm missing some undocumented use case, these modules aren't needed in `StrongParameters` anymore since 8e221127ab. Also, all actionpack tests are passing without them.
* | Fix dirty tracking for `touch`Ryuta Kamizono2019-04-155-9/+50
| | | | | | | | | | | | | | | | | | | | Before this fix, `touch` only clears dirty tracking for touched attributes, doesn't track saved (touched) changes. This fixes that tracks saved changes and carry over remaining changes. Fixes #33429. Closes #34306.
* | Correctly set application path in ↵yuuji.yaginuma2019-04-151-2/+2
| | | | | | | | | | | | `test_app_update_does_not_change_config_target_version` This is the follow up of 10fa3b3792153c2a213f837bcf51bbf6844c1661.
* | Make test application the same state as `app:update`yuuji.yaginuma2019-04-151-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | In `app:update`, it is decided whether to skip depending on whether `Spring` is defined or not. However, `spring` is not currently specified in Gemfile. As a result, `app:update` determines that `Spring` is not used, and diff appears in the result file. If there is a difference, the console for processing the difference is output and the test stops. To avoid this, do not include `Spring` in app. This is a bit strange approach, so I will revisit this later.
* | Merge pull request #35967 from ↵Yuji Yaginuma2019-04-151-49/+60
|\ \ | | | | | | | | | | | | ryohashimoto/do_not_app_update_in_app_generator_test Do not execute `rails app:update` in railties/test/generators/app_generator_test.rb
| * | use Rails::Generators::AppGenerator#update_config_files instead of `rails ↵Ryo Hashimoto2019-04-131-49/+60
| | | | | | | | | | | | app:update` in app_generator_test.rb
* | | Merge pull request #35970 from yskkin/reversible_commentRyuta Kamizono2019-04-159-7/+182
|\ \ \ | | | | | | | | make change_column_comment and change_table_comment invertible
| * | | make change_column_comment and change_table_comment invertibleYoshiyuki Kinjo2019-04-159-7/+182
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We can revert migrations using `change_column_comment` or `change_table_comment` at current master. However, results are not what we expect: comments are remained in new status. This change tells previous comment to these methods in a way like `change_column_default`.
* | | | Fix broken `AppGeneratorTest#test_spring_no_fork`yuuji.yaginuma2019-04-151-1/+1
| | | | | | | | | | | | | | | | | | | | The mock is called three times because the `spring_install?` call has been added in 65344f254cde87950c7f176cb7aa09c002a6f882.
* | | | Refactor `sync_with_transaction_state`Ryuta Kamizono2019-04-151-8/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | All transaction states (:committed, :fully_committed, :rolledback, :fully_rolledback) are mutually exclusive. And also, `force_clear_transaction_record_state` should clear `@transaction_state` as well.
* | | | generate config.cache_classes = false if SpringXavier Noria2019-04-143-5/+21
| | | |
* | | | Merge pull request #35951 from dv/fix-redis-fetch-multi-without-namesGeorge Claghorn2019-04-142-0/+7
|\ \ \ \ | | | | | | | | | | Make `Redis.cache.fetch_multi([])` not throw an error
| * | | | Redis fetch without names returns {}David Verhasselt2019-04-122-0/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When trying to call mget in Redis without any parameters, a Redis error is thrown. To avoid this, we circumvent Redis entirely when there are no key names given.
* | | | | Remove the Amazon SES ingressGeorge Claghorn2019-04-1411-149/+30
| | | | | | | | | | | | | | | It's unusable and not ready to ship in Rails 6.0. We'll rewrite it for 6.1.
* | | | | Merge pull request #35971 from ↵Ryuta Kamizono2019-04-141-46/+52
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | shioyama/avoid_acceptance_validator_module_inclusion_in_tests Ensure acceptance validator is not applied more than once to Person
| * | | | | Ensure acceptance validator is not applied more than once to PersonChris Salzberg2019-04-141-46/+52
| | | | | |
* | | | | | Remove useless `update_attributes_from_transaction_state` and ↵Ryuta Kamizono2019-04-141-14/+3
|/ / / / / | | | | | | | | | | | | | | | `set_transaction_state`
* | | | | Merge pull request #35956 from mrhead/fix-per-exception-retry-counterGeorge Claghorn2019-04-143-2/+26
|\ \ \ \ \ | | | | | | | | | | | | Use individual execution counters when calculating retry delay
| * | | | | Use individual execution counters when calculating retry delayPatrik Bóna2019-04-123-2/+26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Individual execution counters were introduced in #34352. However `#determine_delay` which is used to calculate retry delay still uses the global counter. This commit fixes it.
* | | | | | Don't expose `add_to_transaction`Ryuta Kamizono2019-04-144-18/+12
| |_|/ / / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | `add_to_transaction` was added at da840d1, but it should not be called by except internal, since `remember_transaction_record_state` should be called only once before saving. And also, currently `add_to_transaction` doesn't always add the record to transaction since da8de91, that is the reason hard to use that even in internal. Even if `add_to_transaction` ensure to add the record to transaction, that is an internal concern, people don't need to explicitly call `add_to_transaction`.
* | | | | deprecates autoloading constants during initialization [closes #35745]Xavier Noria2019-04-143-0/+96
| | | | | | | | | | | | | | | | | | | | See rationale in the warning message included in the patch.
* | | | | Merge pull request #35968 from ↵Ryuta Kamizono2019-04-141-18/+32
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | shioyama/apply_acceptance_validator_to_subclasses_in_tests Ensure multiple anonymous modules are not included into test classes
| * | | | | Ensure multiple anonymous modules are not included into Topic in testsChris Salzberg2019-04-141-18/+32
|/ / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Each acceptance validator applied to a model class includes an instance of a module builder (LazilyDefineAttributes) into that class. In tests, if the original model class is not subclassed, these modules pile up and cannot be removed, potentially leading to flakey specs and false positive/negatives. To avoid this, always use subclasses in tests whose names (constants) can be removed when the test is done.
* | | | | Merge pull request #35966 from shioyama/define_validator_on_topic_subclassRyuta Kamizono2019-04-141-5/+7
|\ \ \ \ \ | | | | | | | | | | | | Add validation to subclass in tests to avoid polluting parent class
| * | | | | Add validation to subclass in tests to avoid polluting parent classChris Salzberg2019-04-131-5/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | These two tests currently both define acceptance validators on the same class, Topic. This means that in either one test or the other, there are not one but *two* instances of the LazilyDefineAttributes module builder in the class' ancestors, which can result in unpredictable results. Subclassing Topic in each test avoids conflicts.
* | | | | | Merge pull request #35958 from yskkin/bulk_change_tableRyuta Kamizono2019-04-144-48/+43
|\ \ \ \ \ \ | |_|_|_|/ / |/| | | | | use PostgreSQL's bulk_alter_table implementation
| * | | | | use PostgreSQL's bulk_alter_table implementationYoshiyuki Kinjo2019-04-134-48/+43
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Running this migration on mysql at current master fails because `add_references_for_alter` is missing. ``` change_table :users, bulk: true do |t| t.references :article end ``` This is also true for postgresql adapter, but its `bulk_alter_table` implementation can fallback in such case. postgresql's implementation is desirable to prevent unknown failure like this.
* | | | | | Merge pull request #35963 from y-yagi/fix_code_climate_exclude_patternsYuji Yaginuma2019-04-131-0/+2
|\ \ \ \ \ \ | |_|/ / / / |/| | | | | Fix Code Climate exclude patterns config
| * | | | | Fix Code Climate exclude patterns configyuuji.yaginuma2019-04-131-0/+2
|/ / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | I removed `exclude_paths` by a737143. But I had mistaken that exclude paths would not be used if removed `exclude_paths` config. However, in the absence of `exclude_paths`, the default setting used and it contains `test`. https://docs.codeclimate.com/docs/advanced-configuration#section-exclude-patterns https://docs.codeclimate.com/docs/excluding-files-and-folders#section-auto-generated-file-and-folder-exclusions As we need to target the tests, specify an empty array to prevent the default from being used.
* | | | | Auto-correct `Style/StringLiterals` cop offencesRyuta Kamizono2019-04-131-4/+4
| | | | | | | | | | | | | | | | | | | | Somehow Code Climate is not working as expected for now?
* | | | | Merge pull request #35961 from shioyama/rename_method_missing_targetRyuta Kamizono2019-04-136-21/+21
|\ \ \ \ \ | | | | | | | | | | | | Rename method_missing_target to target
| * | | | | Improve wording of commentsChris Salzberg2019-04-135-11/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Most of the time, these methods are called from actual methods defined from columns in the schema, not from method_missing, so the current wording is misleading.
| * | | | | Rename method_missing_target to targetChris Salzberg2019-04-131-10/+10
|/ / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The target of matchers is used in two contexts: to define attribute methods which dispatch to handlers like attribute_was, and to match method calls in method_missing and dispatch to those same handler methods. Only in the latter context does the term "method_missing_target" make any sense; in the former context it is just confusing. "target" is not ideal as a term but at least it avoids this confusion.
* | | | | Do not treat `environment_desc` as commandsyuuji.yaginuma2019-04-131-1/+3
| | | | | | | | | | | | | | | | | | | | For avoid to show `environment_desc` in help.
* | | | | Fix `presicion` -> `precision`yuuji.yaginuma2019-04-132-6/+6
| | | | | | | | | | | | | | | | | | | | This fix is necessary to test precision's default correctly.
* | | | | Merge pull request #35957 from itsWill/reset_post_attributes_stateGannon McGibbon2019-04-121-0/+16
|\ \ \ \ \ | | | | | | | | | | | | Fix test flakyness due to `test_truncate_tables`
| * | | | | Fix test flakyness due to `test_truncate_tables`Guilherme Mansur2019-04-121-0/+16
| |/ / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | `Truncate Tables posts` will also reset the `AUTOINCREMENT` this causes a situation where if a test suite that uses the `taggings` fixtures runs and subsequently the `test_truncate_tables` run, newly created posts would reference the `tagging` in the database. This commit resest the state of the posts table after the `connection.truncate` call in the `test_truncate_tables`, as well as all other tests that call `trucate` This ensures the associations and db state remain consistent after the tests. Fixes: https://github.com/rails/rails/issues/35941
* | | | | Merge pull request #35955 from ↵Gannon McGibbon2019-04-122-3/+2
|\ \ \ \ \ | |/ / / / |/| | | | | | | | | | | | | | shioyama/remove_method_name_from_attribute_method_match Remove unused method_name from AttributeMethodMatch
| * | | | Remove unused method_name from AttributeMethodMatchChris Salzberg2019-04-122-3/+2
|/ / / /
* | | | Merge pull request #35952 from utilum/instance_varRafael França2019-04-121-1/+1
|\ \ \ \ | |/ / / |/| | | Squash `warning: instance variable @filename not initialized`
| * | | Squash `warning: instance variable @filename not initialized`utilum2019-04-121-1/+1
| |/ /
* | | Merge pull request #35918 from ↵Ryuta Kamizono2019-04-123-8/+10
|\ \ \ | | | | | | | | | | | | | | | | kamipo/lazy_sync_with_transaction_state_on_destroy Lazy sync with transaction state on destroy
| * | | Lazy sync with transaction state on destroyRyuta Kamizono2019-04-103-8/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This reverts commit 58410b3d566e6b93c7b71c0eec0fc11ec906b68e. If we have any implicit commit/rollback callbacks, it is necessary to add record to transaction explicitly like `:touch_deferred_attributes`. https://github.com/rails/rails/blob/5f261d04d6f857d49c75124df809adfbd6cd5b5e/activerecord/lib/active_record/touch_later.rb#L9 https://github.com/rails/rails/blob/5f261d04d6f857d49c75124df809adfbd6cd5b5e/activerecord/lib/active_record/touch_later.rb#L25 But I can't find any other implicit commit/rollback callbacks in our code base at least now. I think the `self.class.connection.add_transaction_record(self)` line doesn't cover any behavior.
* | | | Merge pull request #35920 from ↵Ryuta Kamizono2019-04-124-5/+43
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | kamipo/dont_call_commit_callbacks_for_invalid_record Don't call after_commit callbacks despite a record isn't saved
| * | | | Don't call after_commit callbacks despite a record isn't savedRyuta Kamizono2019-04-124-5/+43
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Regardless of a record isn't saved (e.g. validation is failed), `after_commit` / `after_rollback` callbacks are invoked for now. To fix the issue, this adds a record to the current transaction only when a record is actually saved. Fixes #29747. Closes #29833.
* | | | | Merge pull request #28830 from kamipo/dont_regard_extension_block_as_scopeRyuta Kamizono2019-04-126-40/+33
|\ \ \ \ \ | | | | | | | | | | | | Fix `automatic_inverse_of` not to be disabled if extension block is given
| * | | | | Fix `automatic_inverse_of` not to be disabled if extension block is givenRyuta Kamizono2019-04-126-40/+33
| | |_|/ / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If an association has a scope, `automatic_inverse_of` is to be disabled. But extension block is obviously not a scope. It should not be regarded as a scope. Fixes #28806.
generated by cgit v1.2.3 (git 2.25.1) at 2025-01-16 22:57:58 +0000