updates_for: skip updates

[stimulusreflex]

updates_for: skip updates

[assuntaw]

@julianrubisch good idea, thanks!

I've been playing around with this and will hopefully submit a PR tonight or tomorrow morning CET (rm -rf / and all, @leastbad 😸 )

[leastbad]

I like these ideas, but I admit that I'm similarly stumped in terms of their implementation.

Is this something we could do on an environment basis? When are you running seeds? eg. is this a testing thing?

I do have an idea how we could implement @andrewerlanger's idea that will likely make us all 🤮 a little bit, but here goes: there's only 4 actual instances of ActionCable.server.broadcast in updatable.rb and model_updatable_callbacks.rb... what if we:

1. Create a single method that calls ActionCable.server.broadcast and convert those four instances to use it.
2. Wrap the above broadcast call in a conditional check for a @@global Boolean variable that gets set by
3. A new CableReady::Updateable.without_updates method that would look sort of like:

def without_updates do
  @@what_could_go_wrong = true
  yield
  @@what_could_go_wrong = false
end

It's putting perfume on a pig, but I think it would allow us to brute force this without boiling the ocean.

[assuntaw]

@leastbad that sounds and looks like a solid idea – I'd be happy to give it a try early next week and see what 🤮 -potential I can reach.

One concern that springs to mind is the class variable being (re-)set before its time, like if someone were to try and pass an async task in the block, e.g.

cable_ready.without_updates { RaceConditionJob.perform_later }

What do you think?

P.S:

Is this something we could do on an environment basis? When are you running seeds? eg. is this a testing thing?

I'm afraid this is a bit of an env-agnostic issue. We run seeds in development, data migrations in development, staging and production. We use MockRedis in our test environment anyway so haven't had any issues related to updates_for there (without the mock server we would).

[leastbad]

I think that if they run an async job inside a helper that is literally an escape hatch for the developer of a site to very intentionally run processes that shouldn't flood the UI with updates... then they deserve what's coming to them.

It's all about the intentionality. We could embed <%= `rm -rf /` %> in our view templates, but we have to give developers enough guidance that they don't want to do that to themselves.

As I often say to Julian, it all comes down to good documentation, IMO!

I can't wait to see what you come up with. 😺

[julianrubisch]

Maybe we could take inspiration from how https://api.rubyonrails.org/classes/ActiveRecord/NoTouching/ClassMethods.html is implemented

[internets]

I haven't used this feature yet, but it's an inspiring one so I've been digging in.

What if there were a configuration governing how Redis::CannotConnectError errors are handled during enable_updates broadcasts?

config.enable_updates_connection_errors = :log

where the available options are :log, :raise, or :silence

Other options in the future, potentially. Maybe crib some ideas from https://api.rubyonrails.org/classes/ActiveSupport/Deprecation/Behavior.html#method-i-behavior-3D

Another thing to consider is expanding the scope of the error handling beyond connection errors. If that's desirable there might be a better way to design the configuration options.

[leastbad]

@internets I like where you're going with this, Peter!

#168 has been merged (huzzah!) but I encourage you to open a draft PR for the API you'd like to see. 👍