| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
| |
This reverts commit dd779c9686f49f5ed6dda8ad5a1cb3b0788e1dd4.
|
|
|
|
|
|
| |
ActiveSupport::TestCase now"
This reverts commit 98d0f7ebd34b858f12a12dcf37ae54fdbb5cab64.
|
|
|
|
| |
This reverts commit 8d2866bb80fbe81acb04f5b0c44f152f571fb29f.
|
| |
|
| |
|
|
|
|
|
|
|
|
| |
This hack prevails everywhere in the codebase by being copy & pasted, and it's actually not a negative thing but a necessary thing for framework implementors,
so it should better have a name and be a thing.
And with this commit, activesupport/test/abstract_unit.rb now doesn't silently autoload AS::TestCase,
so we're ready to establish clearner environment for running AS tests (probably in later commits)
|
|
|
|
| |
It's used everywhere, clean and mature enough
|
|
|
|
|
|
| |
In most cases it works now without explicit require because it's accidentally required through
active_support/core_ext/date_and_time/calculations.rb where we still call `try`,
but that would stop working if we changed the Calculations implementation and remove the require call there.
|
|
|
|
| |
and Git taught me that this crap was added via this commit... https://github.com/rails/rails/commit/68db6bc431fbff0b2291f1f60ccf974b4eece596
|
| |
|
|
|
|
| |
fstrings
|
| |
|
| |
|
|
|
|
| |
We're already running Performance/RegexpMatch cop, but it seems like the cop is not always =~ justice
|
|\
| |
| | |
Make plain matcher match first, not last
|
| | |
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
This code takes the "plain" matcher with no prefix and suffix and puts
it at the end of the matchers array such that it is de-prioritized among
all matchers. The comment explaining this code, originally from commimt
8b8b7143efe dated in 2011, is in a context where detection from matchers
happened immediately. In that situation, the plain matcher would indeed
always match first and no others would ever be used.
However, the current code does not immediately detect one match but
rather maps matchers to matches for the method_name. Detection only
happens for matches whose attribute name is valid.
In this context, there is no need to bump the plain matcher to the end
of the array, since it will only be selected if the attribute name it
catpures matches a valid attribute name.
|
| | |
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
- `AM::Error#to_h` was kind of broken before and would return in the
hash values a single error message.
```ruby
person = Person.new
person.errors.add(:name, "cannot be blank")
person.errors.add(:name, "too long")
puts person.errors.to_h # {name: 'too long'}
```
Since an attribute can have different errors, the previous behavior
didn't make much sense.
Now, `ActiveModel::Errors#to_hash` correctly returns an array of
error messages containing all the errors for an attribute.
However, one can easily be surprised by this change, so let's
deprecated it first.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
- In ef4d3215b1198c456780b8d18aa62be7795b9b8c I made a change to
pass `AM::Error` object in case the arity of the block passed to
`each` accepted less than 2 arguments.
This is causing one issue for `to_h` as it expects the argument
passed to the block to be an Array (and were are passing it an
instance of `AM::Error`).
There is no real reason to use `to_h` anymore since `to_hash` exists
Deprecating `to_h` inf favor of `to_hash`
Co-Authored-By: Rafael França <rafael@franca.dev>
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
- `AM::Errors#each` is implemented for the `Enumerator` module and
get called indirectly by a bunch of method in the ruby land
(map, first, select ...)
These methods have a `-1` arity as they are written in C and they
wrongly trigger a deprecation warning.
This commit fixes that and correctectly return a `AM::Error` object
when `each` is called with a negative arity.
|
| |
| |
| |
| |
| |
| |
| |
| |
| | |
- Since `ActiveModel::Error` can now be inherited by
`ActiveModel::NestedError`, when the latter generates a
`full_message`, the `i18n_customize_full_message` accessor set in
the parent class is not set.
This commit fixes that by using a `class_attribute` instead.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
- One regression introduced by the "AM errors as object" features is
about the `full_messages` method.
It's currently impossible to call that method if the `base` object
passed in the constructor of `AM::Errors` doesn't respond to the
`errors` method.
That's because `full_messages` now makes a weird back and forth trip
`AM::Errors#full_messages` -> `AM::Error#full_message` -> `AM::Errors#full_message`
Since `full_message` (singular) isn't needed by AM::Errors, I moved
it to the `AM::Error` (singular) class. This way we don't need to
grab the `AM::Errors` object from the base.
|
|\ \
| | |
| | | |
Returns `nil` when `AM::Errors#delete` doesn't delete anything:
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
- `AM::Errors#delete` currently returns an empty array when trying
to delete an error that doesn't exist.
This behaviour is surprising and I think it would be better
to no return a truthy value but instead return nil like
`Hash#delete` does.
|
|\ \ \
| | | |
| | | | |
Fix `AM::Errors.added?` trying to generate a message:
|
| |/ /
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
- When a ActiveRecord record get saved and validated as part of a
collection association, the errors attribute are changed to reflect
the children names. You end up with an error attribute that will
look like this:
`author.errors # {:'books.title' => [:blank]}`
https://github.com/rails/rails/blob/2fe20cb55c76e6e50ec3a4ec5b03bbb65adba290/activerecord/lib/active_record/autosave_association.rb#L331-L340
We then can't check if the `books.title` errors was added using
`ActiveModel::Errors#added?` because it tries to generate a message
to make the match and end up calling the "books.title" method
on the Author.
```
author.errors.added?(:'books.title', :blank) => NoMethodError: undefined method `books.title'
```
This patch modify the behaviour of `strict_match?` to not generate
a message to make the comparison but instead make a strict
comparison with the `options` from the error.
|
|\ \ \
| | | |
| | | | |
Fix errors getting duplicated when passed validations options:
|
| |/ /
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
- In 86620cc3aa8e2630bc8d934b1a86453276b9eee9, a change was made
on how we remove error duplication on a record for autosave
association
This fix has one caveat where validation having a `if` / `unless`
options passed as a proc would be considered different.
Example:
```ruby
class Book < ApplicationRecord
has_one :author
validates :title, presence: true, if -> { true }
validates :title, presence: true, if -> { true }
end
Book.new.valid? # false
Book.errors.full_messages # ["title can't be blank", "title can't be blank"]
```
While this example might sound strange, I think it's better to
ignore `AM::Validations` options (if, unless ...) when making the
comparison.
|
|/ /
| |
| |
| | |
active_support/rails.rb
|
| |
| |
| | |
`previously_changed` seems to actually be `previous_changes`
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Currently `type.serialize` and `connection.{quote|type_cast}` for a time
object always does `time.getutc` call regardless of whether it is
already utc time object or not, that duplicated proccess
(`connection.type_cast(type.serialize(time))`) allocates extra/useless
time objects for each type casting.
This avoids that redundant `time.getutc` call if it is already utc time
object. In the case of a model has timestamps (`created_at` and
`updated_at`), it avoids 6,000 time objects allocation for 1,000 times
`model.save`.
```ruby
ObjectSpace::AllocationTracer.setup(%i{path line type})
pp ObjectSpace::AllocationTracer.trace {
1_000.times { User.create }
}.select { |k, _| k[0].end_with?("quoting.rb", "time_value.rb") }
```
Before (c104bfe424e6cebe9c8e85a38515327a6c88b1f8):
```
{["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
203,
:T_ARRAY]=>[1004, 0, 778, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
220,
:T_STRING]=>[2, 0, 2, 1, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
209,
:T_ARRAY]=>[8, 0, 8, 1, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
57,
:T_ARRAY]=>[4, 0, 4, 1, 1, 0],
["~/rails/activemodel/lib/active_model/type/helpers/time_value.rb",
17,
:T_DATA]=>[4000, 0, 3096, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
120,
:T_DATA]=>[2000, 0, 1548, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
126,
:T_STRING]=>[4000, 0, 3096, 0, 1, 0]}
```
After (this change):
```
{["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
203,
:T_ARRAY]=>[1004, 0, 823, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
220,
:T_STRING]=>[2, 0, 2, 1, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
209,
:T_ARRAY]=>[8, 0, 8, 1, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
57,
:T_ARRAY]=>[4, 0, 4, 1, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
126,
:T_STRING]=>[2000, 0, 1638, 0, 1, 0]}
```
|
|/
|
|
|
|
|
|
|
|
|
| |
We sometimes say "✂️ newline after `private`" in a code review (e.g.
https://github.com/rails/rails/pull/18546#discussion_r23188776,
https://github.com/rails/rails/pull/34832#discussion_r244847195).
Now `Layout/EmptyLinesAroundAccessModifier` cop have new enforced style
`EnforcedStyle: only_before` (https://github.com/rubocop-hq/rubocop/pull/7059).
That cop and enforced style will reduce the our code review cost.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Currently, if `:datetime` type has a precision, type casting always does
round off subseconds regardless of whether necessary or not, it is a bit
slow.
Since #34970, `t.timestamps` with `precision: 6` by default, so
`pluck(:created_at)` in newly created app will always be affected by the
round off.
This avoids the round off if possible, it makes `pluck(:created_at)`
about 25% faster.
https://gist.github.com/kamipo/e029539f2632aee9f5e711fe66fc8842
Before (0a87d7c9ddb95cf7568baf889ff4091469ba9af4 with postgresql adapter):
```
Warming up --------------------------------------
users.pluck(:id) 344.000 i/100ms
users.pluck(:name) 336.000 i/100ms
users.pluck(:created_at) 206.000 i/100ms
Calculating -------------------------------------
users.pluck(:id) 3.620k (± 8.5%) i/s - 18.232k in 5.077316s
users.pluck(:name) 3.579k (± 9.4%) i/s - 17.808k in 5.020230s
users.pluck(:created_at) 2.069k (± 8.0%) i/s - 10.300k in 5.019284s
```
Before (0a87d7c9ddb95cf7568baf889ff4091469ba9af4 with mysql2 adapter):
```
Warming up --------------------------------------
users.pluck(:id) 245.000 i/100ms
users.pluck(:name) 240.000 i/100ms
users.pluck(:created_at) 180.000 i/100ms
Calculating -------------------------------------
users.pluck(:id) 2.548k (± 9.4%) i/s - 12.740k in 5.066574s
users.pluck(:name) 2.513k (± 8.0%) i/s - 12.480k in 5.011260s
users.pluck(:created_at) 1.771k (±11.2%) i/s - 8.820k in 5.084473s
```
After (this change with postgresql adapter):
```
Warming up --------------------------------------
users.pluck(:id) 348.000 i/100ms
users.pluck(:name) 357.000 i/100ms
users.pluck(:created_at) 254.000 i/100ms
Calculating -------------------------------------
users.pluck(:id) 3.628k (± 8.2%) i/s - 18.096k in 5.024748s
users.pluck(:name) 3.624k (±12.4%) i/s - 17.850k in 5.020959s
users.pluck(:created_at) 2.567k (± 7.0%) i/s - 12.954k in 5.081153s
```
After (this change with mysql2 adapter):
```
Warming up --------------------------------------
users.pluck(:id) 268.000 i/100ms
users.pluck(:name) 265.000 i/100ms
users.pluck(:created_at) 207.000 i/100ms
Calculating -------------------------------------
users.pluck(:id) 2.586k (±10.9%) i/s - 12.864k in 5.050546s
users.pluck(:name) 2.543k (±10.2%) i/s - 12.720k in 5.067726s
users.pluck(:created_at) 2.263k (±14.5%) i/s - 10.971k in 5.004039s
```
|
|\
| |
| | |
Improve doc for :root option in as_json()
|
| |
| |
| |
| |
| |
| |
| |
| | |
Remove trailing whitespace [ci skip]
Reword
Root value should be string [ci skip]
|
| |
| |
| |
| |
| |
| |
| |
| |
| | |
This fixes the following warnings.
```
/rails/activemodel/test/cases/nested_error_test.rb:9: warning: method redefined; discarding old test_initialize
/rails/activemodel/test/cases/error_test.rb:29: warning: previous definition of test_initialize was here
```
|
| |
| |
| |
| | |
This reverts commit 9c9c950d02af83742a5f76302d0faa99508f242c.
|
|/
|
|
| |
Otherwise we get deprecation warnings in the generated scaffold template files
|
|
|
|
|
|
|
|
| |
instead of 6.0 (#36087)
* Change the deprecation for Enumerating ActiveModel::Errors to Rails 6.1 instead of 6.0
* Changed the deprecation message for ActiveModel::Errors methods: slice, values, keys and to_xml
|
|\
| |
| | |
Model error as object
|
| | |
|
| |
| |
| |
| | |
maintaining behavior errors.details[:foo].any?
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| |
| |
| |
| |
| |
| | |
Fix double wrapping issue
Revert messages_for wrapping. It's a new method so no need to put
deprecation warnings.
|
| |
| |
| |
| | |
Revert some tests to ensure back compatibility
|