> On Aug 27, 2018, at 8:43 PM, Dana Sherson wrote:
>
> Currently many of the persistence methods all have different behaviours when
> it comes to whether setters are called, and validations and or callbacks are
> run,
> Knowing what happens requires deeper knowledge of rails than just reading the
> method name
>
> This can be confusing for newcomers and even people very familiar with rails
> will need to double check which does what.
IMO most of these are clarified by clicking the “source” link under the method
in the docs to see how they use the core primitives - i.e., `update` calls
`save` internally.
> save(validate: false) => callbacks: yes, validations: no,
> only_save_mentioned_columns: N/A
> save => callbacks: yes, validations: yes, touch: yes,
> only_save_mentioned_columns: N/A
> save(touch: false) => callbacks: yes, validations: yes, touch: no,
> only_save_mentioned_columns: N/A
> save(validate: false, touch: false) => callbacks: yes, validations: no,
> touch: no, only_save_mentioned_columns: N/A
I doubt this is anybody’s favorite interface, but it’s better than the old
`save(false)`. AFAIK it’s partly an artifact of how `save` is implemented as a
stack of included modules.
> update/update_attributes => callbacks: yes, validations: yes, setters: yes,
> touch: yes, only_save_mentioned_columns: no
> update_attribute => callbacks: yes, validations: no, setters: yes, touch:
> yes, only_save_mentioned_columns: no
`update_attributes` is deprecated now on master, because it was confusing.
Calling `update_attribute` on a record with unsaved changes is generally an
antipattern, but its behavior is clearly documented.
> update_columns/update_column => callbacks: no, validations: no, setters: no,
> touch: no, only_save_mentioned_columns: yes
> increment!/decrement!/toggle! => callbacks: no, validations: no, setters: no,
> touch: no, only_save_mentioned_columns: yes
> increment!/decrement!(touch: true) => callbacks: no, validations: no,
> setters: no, touch: yes, only_save_mentioned_columns: yes
> touch => callbacks: some, validations: no, setters: no, touch: yes,
> only_save_mentioned_columns: yes
These are special-purpose methods and don’t belong in this list, IMO.
> update_all/Class update => callbacks: no, validations: no, setters: no,
> touch: no, only_save_mentioned_columns: yes
These last two aren’t the same - the former generates a single UPDATE query,
but the latter loads each object and calls `update`. This may count as evidence
for your point about “need[ing] to double check which does what” above. :)
> Proposal A:
>
> Follow what the `save` pattern started, and provide explicit instructions to
> update, and a more complete list to save
>
> 1. Add `callbacks: false` option to save.
Which callbacks run when you specify `callbacks: false, validate: true`? In
particular, does this skip `before_validation`/`after_validation`?
IMO if you’re skipping callbacks on a generic call to `save` you’ve got
callbacks that might not really be callbacks.
> 2. Add `validate:`,`touch:`,`callbacks:`,`only_save_mentioned_columns:` to
> update, with the option of providing the instructions in a separate hash for
> models where those are columns/attributes
> e.g. `update(column: whatever, callbacks: false)`, or `update(**params,
> callbacks: false)` or `update({ column: whatever }, callbacks: false)`, or
> `update(params, callbacks: false)`
Not a fan of taking this many boolean flags.
> 6. validate the keyword arguments passed to save/update, so people don't
> accidentally write 'skip_callbacks: true' or 'validates: false' or etc
This is the best suggestion in this proposal IMO, but it’s hard to do since the
flags are consumed by different parts of the `save` method implemented in
different files.
> Proposal B:
>
> Rather than modifying update_*/save signatures, deprecate and replace with a
> method called `persist` and `persist!` that takes those options
IMO if Proposal B was the status quo, we’d be talking about giving
frequently-used combinations of those boolean flags specific names like
“update_column” and deprecating `persist`.
To sum up:
* the only things that currently skip callbacks are specialized methods
explicitly intended to do very limited updates to the DB. IMO this shouldn’t
change
* similarly for `only_save_mentioned_columns`; only the specialized methods
skip saving dirty columns
* `validate` and `touch` are already boolean options to `save`. They aren’t
available in `update`, but `update` is literally just “assign attributes and
call save” so IMO it’s trivial to get a similar result if needed.
* in conclusion, I don’t see how exposing more of the internal implementation
as controllable options *reduces* the need to understand the internals
—Matt Jones
--
You received this message because you are subscribed to the Google Groups "Ruby
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it,
