| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| |
|
|
| |
Follow up of #32952.
|
| |
|
|
| |
- Fixes #32940
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Rollback parent transaction when children fails to update
Rails supports autosave associations on the owner of a `has_many`
relationship. In certain situation, if the children of the association
fail to save, the parent is not rolled back.
```ruby
class Employee < ActiveRecord::Base
end
class Company < ActiveRecord::Base
has_many(:employees)
end
company = Company.new
employee = company.employees.new
company.save
```
In the previous example, if the Employee failed to save, the Company
will not be rolled back. It will remain in the database with no
associated Employee.
I expect the `company.save` call to be atomic, and either create all or
none of the records.
The persistance of the Company already starts a transaction that nests
it's children. However, it didn't track the success or failure of it's
children in this very situation, and the outermost transaction is not
rolled back.
This PR makes the change to track the success of the child insertion and
rollback the parent if any of the children fail.
* Change the test to reflect what we expect
Once #32862 is merged, rolling back a record will rollback it's state to match
the state before the database changes were applied
* Use only the public API to express the tests
* Refactor to avoid reassigning saved for nested reflections
[Guillaume Malette + Rafael Mendonça França]
|
| | |
|
| |
|
|
|
| |
This autocorrects the violations after adding a custom cop in
3305c78dcd.
|
| |
|
|
| |
Closes #31998
|
| | |
|
| | |
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
| |
If `:readers` fixture is loaded before the test, it will be failed.
Use `firm.developer_ids` instead because we don't have `:contracts`
fixture for now.
https://travis-ci.org/rails/rails/jobs/268976230#L729
|
| |
|
|
|
|
|
| |
when other `AutomaticInverseFindingTests` load `:comments` fixture
but does not load `:posts`.
Refer #30385 for similar issue
|
| |\ |
|
| | | |
|
| | |
| |
| |
| |
| | |
This reverts commit 3420a14590c0e6915d8b6c242887f74adb4120f9, reversing
changes made to afb66a5a598ce4ac74ad84b125a5abf046dcf5aa.
|
| | | |
|
| |/
|
|
|
|
| |
Reproduction command -
ARCONN=postgresql be ruby -w -Itest test/cases/autosave_association_test.rb --seed 34101
|
| |
|
|
| |
Actually, private methods cannot be called with `self.`, so it's not just redundant, it's a bad habit in Ruby
|
| |
|
|
|
|
|
|
| |
This issue was introduced with d849f42 to solve #19782. However, we can
solve #19782 without causing the issue. It is enough to save only when
necessary.
Fixes #27338.
|
| |
|
|
|
|
| |
Some methods were added to public API in
5b14129d8d4ad302b4e11df6bd5c7891b75f393c and they should be not part of
the public API.
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
With the changes in #25337, double save bugs are pretty much impossible,
so we can just lift this restriction with pretty much no change. There
were a handful of cases where we were relying on specific quirks in
tests that had to be updated. The change to has_one associations was due
to a particularly interesting test where an autosaved has_one
association was replaced with a new child, where the child failed to
save but the test wanted to check that the parent id persisted to `nil`.
I think this is almost certainly the wrong behavior, and I may change
that behavior later. But ultimately the root cause was because we never
remove the parent in memory when nullifying the child. This makes #23197
no longer needed, but it is what we'll do to fix some issues on 5.0
Close #23197
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When the association is autosaved we were storing the details with
string keys. This was creating inconsistency with other details that are
added using the `Errors#add` method. It was also inconsistent with the
`Errors#messages` storage.
To fix this inconsistency we are always storing with symbols. This will
cause a small breaking change because in those cases the details could
be accessed as strings keys but now it can not.
The reason that we chose to do this breaking change is because `#details`
should be considered a low level object like `#messages` is.
Fix #26499.
[Rafael Mendonça França + Marcus Vieira]
|
| |
|
|
|
|
| |
assert [1, 3].includes?(2) fails with unhelpful "Asserting failed" message
assert_includes [1, 3], 2 fails with "Expected [1, 3] to include 2" which makes it easier to debug and more obvious what went wrong
|
| |
|
|
|
|
|
|
|
|
|
|
| |
"models/comment"`
to address BasicsTest#test_readonly_attributes failure #26368
It reproduces only when both of these conditions are satisfied:
- Other test files `autosave_association_test.rb` or `where_test.rb`
which executes `require "models/comment"` then `require "models/post"`
- When `autosave_association_test.rb` or `where_test.rb` executed before `base_test.rb`
|
| |
|
|
|
|
|
|
| |
Style/SpaceBeforeBlockBraces
Style/SpaceInsideBlockBraces
Style/SpaceInsideHashLiteralBraces
Fix all violations in the repository.
|
| | |
|
| | |
|
| |
|
|
|
| |
The current code base is not uniform. After some discussion,
we have chosen to go with double quotes by default.
|
| |
|
|
| |
- marked_for_destroyal -> marked_for_destruction
|
| | |
|
| |\
| |
| |
| |
| |
| | |
Errors can be indexed with nested attributes
Close #8638
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
`has_many` can now take `index_errors: true` as an
option. When this is enabled, errors for nested models will be
returned alongside an index, as opposed to just the nested model name.
This option can also be enabled (or disabled) globally through
`ActiveRecord::Base.index_nested_attribute_errors`
E.X.
```ruby
class Guitar < ActiveRecord::Base
has_many :tuning_pegs
accepts_nested_attributes_for :tuning_pegs
end
class TuningPeg < ActiveRecord::Base
belongs_to :guitar
validates_numericality_of :pitch
end
```
- Old style
- `guitar.errors["tuning_pegs.pitch"] = ["is not a number"]`
- New style (if defined globally, or set in has_many_relationship)
- `guitar.errors["tuning_pegs[1].pitch"] = ["is not a number"]`
[Michael Probber, Terence Sun]
|
| | |
| |
| |
| |
| |
| |
| |
| |
| | |
As per the docs, `mark_for_destruction` should do nothing if `autosave`
is not set to true. We normally persist associations on a record no
matter what if the record is a new record, but we were always skipping
records which were `marked_for_destruction?`.
Fixes #20882
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
This code is so fucked. Things that cause this bug not to replicate:
- Defining the validation before the association (we end up calling
`uniq!` on the errors in the autosave validation)
- Adding `accepts_nested_attributes_for` (I have no clue why. The only
thing it does that should affect this is adds `autosave: true` to the
inverse reflection, and doing that manually doesn't fix this).
This solution is a hack, and I'm almost certain there's a better way to
go about it, but this shouldn't cause a huge hit on validation times,
and is the simplest way to get it done.
Fixes #20874.
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
We deprecate the support for passing an argument to force reload in
6eae366d0d2e5d5211eeaf955f56bd1dc6836758. That led to several
deprecation warning when running Active Record test suite.
This commit silence the warnings by properly calling `#reload` on the
association proxy or on the association object instead. However, there
are several places that `ActiveSupport::Deprecation.silence` are used as
those tests actually tests the force reload functionality and will be
removed once `master` is targeted next minor release (5.1).
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
In 1f006c an option was added called :class to allow passing anonymous
classes to association definitions. Since using :class instead of
:class_name is a fairly common typo even amongst experienced developers
this can result in hard to debug errors arising in raise_on_type_mismatch?
To fix this we're renaming the option from :class to :anonymous_class as
that is a more correct description of what the option is for. Since this
was an internal, undocumented option there is no need for a deprecation.
Fixes #19659
|
| |/
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
To me it seems like this should only be the case if `autosave: true` is
set on the association. However, when implemented that way, it caused
issues with has many associations, where we have explicit tests stating
that child records are updated when the parent is new, even if autosave
is not set (presumably to update the parent id, but other changed
attributes would be persisted as well).
It's quirky, but at least we should be consistently quirky. This
constitutes a minor but subtle change in behavior, and therefore should
not be backported to 4.2 and earlier.
Fixes #19782
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
I’m renaming all instances of `use_transcational_fixtures` to
`use_transactional_tests` and “transactional fixtures” to
“transactional tests”.
I’m deprecating `use_transactional_fixtures=`. So anyone who is
explicitly setting this will get a warning telling them to use
`use_transactional_tests=` instead.
I’m maintaining backwards compatibility—both forms will work.
`use_transactional_tests` will check to see if
`use_transactional_fixtures` is set and use that, otherwise it will use
itself. But because `use_transactional_tests` is a class attribute
(created with `class_attribute`) this requires a little bit of hoop
jumping. The writer method that `class_attribute` generates defines a
new reader method that return the value being set. Which means we can’t
set the default of `true` using `use_transactional_tests=` as was done
previously because that won’t take into account anyone using
`use_transactional_fixtures`. Instead I defined the reader method
manually and it checks `use_transactional_fixtures`. If it was set then
it should be used, otherwise it should return the default, which is
`true`. If someone uses `use_transactional_tests=` then it will
overwrite the backwards-compatible method with whatever they set.
|
| |
|
|
|
|
| |
The test added in 85465ed3e6c582d25f0c8fafe21f7a2c182c2f67 was passing
when the file was run on its own, but failing when the entire suite was
run since this test modifies the class and doesn't clean up.
|
| |
|
|
|
|
|
| |
When associations checked for autosave have a cycle, and
none of them is dirty, then changed_for_autosave? will be an
infinite loop. We now remember if we're in the check and
will short circuit the recursion.
|
| | |
|
| |
|
|
|
| |
`Computer` class needs to be require
See #17217 for more details
|
| |
|
|
|
|
|
|
|
|
|
|
| |
068f092ced8483e557725542dd919ab7c516e567 registered autosave callbacks
as `after_save` callbacks. This caused the regression described in #17209.
Autosave callbacks should be registered as `after_update` and
`after_create` callbacks, just like before.
This is a partial revert of 068f092ced8483e557725542dd919ab7c516e567.
Fixes #17209.
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
alan/only_save_changed_has_one_objects""
This reverts commit e94e6c27af495a2460c811bb506459f1428dec6b.
Conflicts:
activerecord/CHANGELOG.md
The original commit was reverted only to be safe since #14407 were reported.
We don't have any proof we added a regression with the original commit
so reverting it now will give us more problem.
Closes #14407
|
| |
|
|
|
|
|
|
|
| |
This reverts commit 6e3ab3e15faf782f6a937ccf5574a4fb63e3e353, reversing
changes made to 39e07b64ce3f4bb55e60ba0266e677f8e4f4893a.
Conflicts:
activerecord/CHANGELOG.md
activerecord/test/cases/autosave_association_test.rb
|
