Commit Graph

13848 Commits

Author SHA1 Message Date
Rafael França
e1170909e2
Merge pull request #41442 from p8/improve-help-for-rake-tasks
Improve help for Rake tasks
2021-07-28 17:43:33 -04:00
Jean Boussier
f7cedc6b43
Merge pull request #42626 from Shopify/as-file-store-remove-local-cache
Remove LocalCache in FileStore
2021-07-28 17:06:56 +02:00
Jean Boussier
5ac9366699
Merge pull request #42702 from hahmed/add-favicon-to-internal-routes
Add favicon to internal routes `/favicon.ico`
2021-07-28 15:31:12 +02:00
Jorge Manrubia
a29884d733 Remove mechanism to disable test parallelization when runnin gonly 1 test
#42761 made the old system of disable paralleling testing when
only one test file was included obsolete.
2021-07-27 10:33:09 -07:00
Eugene Kenny
cca3911c16
Merge pull request #42860 from dixpac/dix/remove_deprecated_time_zone_initializer
FIX: Setting remove_deprecated_time_with_zone_name in new framework defaults
2021-07-27 08:36:35 +01:00
David Heinemeier Hansson
9c73d4aeab
Make Action Text + Trix JS/CSS available via the asset pipeline (#42857)
* Action Text JS should be available via the asset pipeline too

* Main was a module anyway, no need to reference that twice

* Fix rollup references

* Precompile action text JS for asset pipeline

* No JavaScript dependencies needed with the asset pipeline

* Stub Webpacker::Engine to trigger webpack path for testing

* Extract asset paths

* Exercise asset pipeline path

* Terser doesn't do anything useful on this small package

* Make trix directly available to the asset pipeline

* Indirect doesn't carry its worth

* Reminder for development about keeping things in sync for the asset pipeline

* Ensure this isn't turned into undefined while mirroring

* Mirror Trix CSS for asset pipeline

* Add the needed JS include tag automatically under the asset pipeline

* Please RuboCop

* Keep the peer dependency

Even though we also need it explicitly as a dev dependency in order to generate the mirror output for trix.

* Fix test

* Add CHANGELOG entry
2021-07-26 18:57:42 -04:00
Eugene Kenny
25f6a8e84f
Merge pull request #42809 from eugeneius/rm_request_response_requires
Don't require ActionDispatch::{Request,Response}
2021-07-26 02:06:02 +01:00
Eugene Kenny
e9b7403bb9
Merge pull request #42808 from eugeneius/log_rescued_responses_env_config
Pass log_rescued_responses as environment config
2021-07-26 02:05:48 +01:00
Haroon Ahmed
49332c9537 Add favicon to header dom element for welcome controller.
Co-authored-by: Sunny Ripert <sunny@sunfox.org>
Co-authored-by: Petrik de Heus <petrik@deheus.net>
2021-07-24 21:26:49 +01:00
Dino Maric
a6c0e84f02 FIX: Setting remove_deprecated_time_with_zone_name in new framework defaults
Setting `remove_deprecated_time_with_zone_name` didn't work because
the value is checked in a framework initialiser, which runs before application initialiser.

This PR moves the setting inside the `config.after_initialize` block.
I've also added 2 tests for this behaviour.

Fixes #42820
2021-07-24 16:47:49 +02:00
Gannon McGibbon
5e93cff835
Raise error on unpermitted open redirects.
Add `allow_other_host` options to `redirect_to`.
Opt in to this behaviour with `ActionController::Base.raise_on_open_redirects = true`.
2021-07-22 14:03:59 -04:00
Petrik
718814a4f2 Improve help for Rake tasks
When running `bin/rails -h` the following messages is shown:

> All commands can be run with -h (or --help) for more information.

This doesn't apply to the commands that are Rake tasks like db:migrate.
If you run `bin/rails db:migrate -h` you'll get the Rake help, which
can be confusing...

Instead, if we replace the `-h` argument with `--describe db:migrate`
Rails outputs the Rake task descriptions, which are a lot more helpful:

    rails db:migrate
        Migrate the database (options: VERSION=x, VERBOSE=false, SCOPE=blog).

Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
2021-07-21 22:12:08 +02:00
Ryuta Kamizono
e50b0e3ab3 Fixup CHANGELOGs [ci skip] 2021-07-21 10:08:08 +09:00
Ryuta Kamizono
8dc4e2316c Fix referencing a capture in a string [ci skip]
```irb
irb(main):001:0> '\1en'
=> "\\1en"
irb(main):002:0> "\1en"
=> "\u0001en"
irb(main):003:0> "\\1en"
=> "\\1en"
```
2021-07-21 09:24:27 +09:00
Ryuta Kamizono
a206aecedf chore: Use e.g. which is the more used spelling
```
% git grep -i '\be\.g\.' | wc -l
290
```
2021-07-21 09:17:54 +09:00
eileencodes
10ca60a16b
Add option to disable schema dumb per-database
Dumping the schema is on by default for all databases in an application. To turn it off for a
specific database use the `schema_dump` option:

```yaml
  # config/database.yml

  production:
  schema_dump: false
```

Co-authored-by: Luis Vasconcellos <vasconcelloslf@gmail.com>
2021-07-19 07:58:49 -04:00
Eugene Kenny
5e9fb65485 Don't require ActionDispatch::{Request,Response}
Since 2801786e1a51b7cf7d7c3fd72b5fc9974f83f435 these are eager
autoloaded, so there's no reason to require them explicitly.

Both classes have configuration applied via load hooks, so requiring
them too early could cause load order issues. In particular, an
application that referenced any of the middleware that required these
classes would be prevented from configuring them afterwards.
2021-07-19 01:30:54 +01:00
Eugene Kenny
d6b66ebb6e Pass log_rescued_responses as environment config
This lets us remove the reference to DebugExceptions from the Action
Dispatch railtie, which was causing load order issues since it depends
on ActionDispatch::Request.
2021-07-19 00:08:30 +01:00
Jorge Manrubia
675d9ffb6e
Add an option threshold: to .parallel() setup method (#42789)
This adds an additional method to configure the parallelization
threshold. Before this, the only way of configuring the threshold was
via an option:

```
config.active_support.test_parallelization_minimum_number_of_tests
```
2021-07-16 11:32:23 -07:00
Akshay Mohite
b3aacdc512 Used already defined method parallelized? and fixed semantics of the test name.
- Related to https://github.com/rails/rails/pull/42761
- Used `parallelized?` method instead of calling a method `should_parallelize?` to figure out if parallezation is enabled.
- Fixed semantics of the test name corresponding to the change
- Updated test name as per the code review suggestion.
2021-07-14 22:35:21 +05:30
Eileen M. Uchitelle
c18339791f
Merge pull request #42761 from basecamp/smart-parallel-tests
Parallelize tests only when there are enough to justify the parallelization overhead
2021-07-14 10:18:24 -04:00
Jorge Manrubia
ecc5afed30 Parallelize tests only when overhead is justified
Parallelizing tests has a cost in terms of database setup and fixture
loading. This change makes Rails disable parallelization when the number
of tests is below a configurable threshold.

When running tests in parallel each process gets its own database
instance. On each execution, each process will update each database
schema (if needed) and load all the fixtures. This can be very expensive
for non trivial datasets.

As an example, for HEY, when running a single file with 18 tests,
running tests in parallel in my box adds an overhead of 13 seconds
versus not parallelizing them. Of course parallelizing is totally worthy
when there are many tests to run, but not when running just a few tests.

The threshold is configurable via
config.active_support.test_parallelization_minimum_number_of_tests,
which is 30 50 by default.

This also adds some tracing to know how tests are being executed:

When in parallel:

```
Running 2829 tests in parallel in 8 processes
```

When not in parallel:

```
Running 15 tests in a single process (parallelization threshold is 30)
```
2021-07-14 13:36:28 +02:00
Petrik
6ec0a974ee Fix spelling of whether to fix the CI 2021-07-13 17:27:29 +02:00
Jean Boussier
7bcfe447eb Add an entry for active_record.partial_inserts in new_framework_defaults 2021-07-13 16:06:30 +02:00
Jean Boussier
91082730cc Disable Active Record partial_inserts by default in Rails 7.0
Ref: https://github.com/rails/rails/pull/42355

The justification for `partial_inserts` back in 2012
(144e8691cbfb8bba77f18cfe68d5e7fd48887f5e) was:

> This is more efficient, and also means that it will be safe to remove
> database columns without getting subsequent errors in running app processes
> (so long as the code in those processes doesn't contain any references to the
> removed column).

But since then `ignored_columns` is a much more reliable way to safely remove a
column, and I doubt the reduced query size really help much.

Additionally, `partial_inserts` prevent removing the default value of a column
in a safe way.
2021-07-13 11:02:12 +02:00
Eileen M. Uchitelle
5e41b028c8
Merge pull request #42763 from eileencodes/revert-40445
Revert "Merge pull request #40445 from robertomiranda/destroy_all-in_…
2021-07-12 13:36:42 -04:00
Ryuta Kamizono
adb9fc5773 Remove undefined ensure_zeitwerk_mode
Follow up to 8db23109bf61052eef437629b6ef27a94e0b5bd9.
2021-07-13 02:19:47 +09:00
eileencodes
eb50860fe9
Revert "Merge pull request #40445 from robertomiranda/destroy_all-in_batcches"
This reverts commit 1cdee90d385399083596c22343cc46f51283aca3, reversing
changes made to c3a1bfe187d594ed2c2348cc0ae6c5c894368b8e.

We are reverting this because it caused a change in behavior and we need
to discuss how to address that behavior change and find a work around
that doesn't break existing applications.
2021-07-12 13:15:13 -04:00
Xavier Noria
8db23109bf Removes unnecessary calls to zeitwerk_enabled?
In Rails 7 zeitwerk_enabled? returns true unconditionally. It exists to let 3rd
party code supporting multiple Rails versions check, but we no longer need to use
it internally.
2021-07-11 22:40:23 +02:00
Alex Ghiculescu
5ca37eae12 Handle paths with spaces when editing credentials
Fixes https://github.com/rails/rails/issues/41617

Co-authored-by: Alexander Riccio <alexander@riccio.com>
Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
2021-07-08 11:47:44 -05:00
Alex Ghiculescu
47467fe33d Verify foreign keys after loading fixtures
When writing fixtures, it's currently possible to define associations that don't exist, even if a foreign key exists. For example:

```yml
george:
  name: "Curious George"
  pirate: redbeard

blackbeard:
  name: "Blackbeard"
 ```

When the fixtures are created, `parrots(:george).pirate` will be nil, but it's not immediately clear why. This can make it hard to debug tests and can give false confidence in passing ones.

This can happen because Rails [disables referential integrity](f263530bf7/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb (L407)) when inserting fixtures. This makes the fixtures algorithm much simpler - it can just create the fixtures in alphabetical order and assume that the other side of a foreign key constraint will *eventually* be added.

Ideally we would check foreign keys once all fixtures have been loaded, so that we can be sure that the foreign key constraints were met. This PR introduces that. To enable it:

```ruby
config.active_record.verify_foreign_keys_for_fixtures = true
```

I'm proposing we enable this in 7.0 for new apps and have added it to new framework defaults. When run against our app, it found 3 fixture files with unmet FK constraints - turns out all those fixtures weren't being used and were safe to delete.
2021-07-07 15:41:05 -05:00
Rafael França
e9559f45a8
Merge pull request #42722 from p8/railties/add-missing-new-defaults
Add missing new defaults to new_framework_defaults_7_0.rb.tt
2021-07-07 13:32:34 -04:00
Petrik
4d3a8a0bf0 Add missing new defaults to new_framework_defaults_7_0.rb.tt
`railties/lib/rails/application/configuration.rb` contains defaults that
haven't been added to the `new_framework_defaults_7_0.rb.tt`.
This adds these defaults to ease migration.
2021-07-07 19:08:10 +02:00
Petrik
34d0b3d7cf Clarify new_framework_defaults instructions
Framework defaults cause confusion for contributors when adding new
defaults. Should the defaults be defined in:
`railties/lib/rails/application/configuration.rb` and/or
`config/initializers/new_framework_defaults_7_0.rb.tt`
And should these define the new or old defaults?

Hopefully this clarifies it by making it clear the
new framework defaults are commented out.
By mentioning `config.load_defaults` it hopefully clarifies that these
defaults should be similar to the onces defined by `load_defaults`.

Also reword "flip" to "uncomment" to make it clear you should uncomment
the config instead of setting `true` to `false`.
2021-07-07 12:29:27 +02:00
Zachary Scott
7373b5819a 💅 complete sentence in railties/CHANGELOG 2021-07-07 08:20:18 +09:00
Loïc Delmaire
056b70ee4f
config_for accepts root shared as an array
Fix a bug introduced by
3fe0ab52df
that raised an undefined method when trying to deep merge an array with
an empty config hash

It also adds a test to clarify config_for behaviour with root arrays: when there's
an env array and a shared array, it should only returns the env key (and not a concatenation)

Closes #42698
2021-07-06 11:57:21 +02:00
Ryuta Kamizono
398561bfbb
Merge pull request #42694 from p8/railties/generators-raise-on-invalid-index
Generators should raise an error if an attribute has an invalid index
2021-07-05 22:57:53 +09:00
Ryuta Kamizono
70bbc3706b
Merge pull request #42693 from p8/railties/format-framework-defaults
Remove linebreak from hash_digest_class framework defaults for consistency
2021-07-05 22:38:16 +09:00
Ryuta Kamizono
5f45aeaaae :nodoc: for corrections
Like all others for corrections already does.
2021-07-05 17:59:22 +09:00
Petrik
76f9b7531f Generators should raise an error if an attribute has an invalid index type
When passing an invalid index type to a generator, the index is silently
ignored. For example when misspelling the index:

    bin/rails g model post title:string:indxe

Instead of silently ignoring the invalid index, the generator should
raise an error.

This continues the work in d163fcd6208c9b0507746888c7fb4a6f934303ce
where we started raising errors if the attribute types are invalid.
2021-07-04 19:13:24 +02:00
Petrik
e61ff2baa2 Remove linebreak from hash_digest framework defaults for consistency
All new 7.0 framework defaults don't have an extra linebreak between the
explanation and the actual config.
For consistency we should remove it for `hash_digest_class` as well.
2021-07-04 13:45:17 +02:00
Petrik
0409ed57ac Clean up checks to see if DidYouMean is defined
As of Ruby 2.7 DidYouMean is included as a default gem, so there is no
need to check if DidYouMean is defined in the test suite. We still need
to check if the DidYouMean modules are defined in the actual code, as
someone might run Rails with DidYouMean disabled by using the
`--disable-did_you_mean` flag. This is ussually done for performance
reasons.

This commit also includes some of the changes made by Yuki in:
https://github.com/rails/rails/pull/39555
These changes include replacing Jaro with the more accurate
SpellChecker, and using DidYouMean::Correctable for simplere
corrections.

The DidYouMean::SpellChecker does have a treshold for corrections.
If there is not enough similarity it might not return a suggestion.
To stop the tests from failing some test data had to be changed.

For example, `non_existent` does not meet the treshold for `hello`, but
`ello` does:

DidYouMean::SpellChecker.new(dictionary: %w[hello]).correct('non_existent')
=> []
DidYouMean::SpellChecker.new(dictionary: %w[hello]).correct('ello')
=> ["hello"]

The treshold makes sense for spelling errors. But maybe we should add a
different SpellChecker that helps to get a suggestion even if there is
little overlap. For example for when a model only has 2 attributes
(title and body), it's helpful to get a suggestion for `name`

Co-Authored-By: Yuki Nishijima <yk.nishijima@gmail.com>
2021-07-04 13:43:50 +02:00
Nat Morcos
0ebb7084f4 unsafe_load secrets.yml with psych 4 2021-07-03 09:19:22 -04:00
Eileen M. Uchitelle
56d8ff8372
Merge pull request #42673 from ghiculescu/patch-4
Fix new framework defaults for `destroy_all_in_batches`
2021-07-02 09:05:59 -04:00
Ryuta Kamizono
db767c332a Fix CI failure caused by error_highlight gem
Since https://github.com/ruby/ruby/pull/4586, error.message includes the
line of code that raised.

https://bugs.ruby-lang.org/issues/17930
2021-07-02 17:44:10 +09:00
Alex Ghiculescu
4e30b8d3b5
Fix new framework defaults for destroy_all_in_batches
Small issue from https://github.com/rails/rails/pull/40445, the configuration should be commented out by default.
2021-07-01 16:08:57 -05:00
Andrew White
f263530bf7
Merge pull request #42634 from HackerIntro/actionmailbox-storage-service
Problem: ActionMailbox uses default ActiveStorage service
2021-07-01 16:15:37 +01:00
Jean Boussier
ef747e9f7e
Merge pull request #42652 from ghiculescu/remove-local-cache-middleware
Allow removing LocalCache middleware in application.rb
2021-07-01 15:20:53 +02:00
Alex Ghiculescu
ff66477182 Allow removing LocalCache middleware in application.rb
All other middlewares can be removed from the stack in `config/application.rb`. `ActiveSupport::Cache::Strategy::LocalCache::Middleware` cannot, because it is added to the stack as an instance rather than a class, here: https://github.com/rails/rails/blob/main/railties/lib/rails/application/bootstrap.rb#L62. As a result, [this check](https://github.com/rails/rails/blob/main/actionpack/lib/action_dispatch/middleware/stack.rb#L133) fails unless you also pass the same instance. The instance is not available in `config/appliaction.rb` because `Rails.cache` has not been initialized yet (it's initialized when the middleware is set).

This means that if you want to remove the local cache middleware you have to make an initiailzer just for that. Unlike all other middlewares which can be dealt with as classes in `config/application.rb`.

The fix is to check the middleware's `name`, not `klass`, for delete operations. The name is [provided explicitly](https://github.com/rails/rails/blob/main/activesupport/lib/active_support/cache/strategy/local_cache.rb#L162) presumably for this reason.
2021-06-30 12:28:51 -05:00
Yurii Rashkovskii
855e08d22d Problem: ActionMailbox uses default ActiveStorage service
This is imperfect in situations when a separation
between regular files (such as uploads) and emails
is necessary (for the purposes of regulatory compliance,
proper compartmentalization, etc.)

Solution: allow configuring ActionMailbox's storage service
2021-06-30 09:33:29 -07:00