Initialize model callbacks on boot #37
Conversation
lib/dfe/analytics.rb
Outdated
| model = model_for_entity(entity) | ||
| if model.include?(DfE::Analytics::Entities) && !@shown_deprecation_warning | ||
| Rails.logger.info("DEPRECATION WARNING: DfE::Analytics::Entities was manually included in a model (#{model.name}), but it's included automatically since v1.4. You're running v#{DfE::Analytics::VERSION}. To silence this warning, remove the include from model definitions in app/models.") | ||
| @shown_deprecation_warning = true |
There was a problem hiding this comment.
It's tempting to include a deprecation warning for every model that does this. But, then, if it's only included in ApplicationRecord then that might be annoying.
I'd go for annoy and apologise later. 😛
There was a problem hiding this comment.
I agree, let's be nuisances
| # not. To avoid this specific conflict with devise and possibly other | ||
| # gems/engines, proactively load the routes unless | ||
| # configuration.eager_load is set. | ||
| Rails.application.reload_routes! unless Rails.configuration.eager_load |
There was a problem hiding this comment.
Did we only need to do this in development env? Or do we not need to check that because in non-dev the routes will be loaded at this point anyway? I don't quite remember what we said, but might be worth explaining why this is ok to do in production if there's anything interesting to say.
There was a problem hiding this comment.
Will beef up this comment to explain what default configs there are in prod/dev
|
Might be worth baseing this PR onto the |
|
OK: all updated |
We're going to use this on boot so we can be a bit more relaxed inside the gem
Now that apps will panic on boot if there's invalid config, it's necessary to put a correct analytics.yml file into the dummy app to make the specs work
This means that we attach callbacks to many-to-many association tables that don't have explicit models inheriting from ApplicationRecord For some reason, presumably due to execution order, it's necessary to move the route declarations from the around block to the before block in analytics_spec.rb
See commits.
Manually mixing the behaviour in to ActiveRecord classes does not achieve what we want to achieve: it missed out HABTM relations. So hook up everything automatically —
analytics.ymlwill determine what gets sent. As part of this piece of work we now refuse to boot if that file isn't correct, which should surface configuration issues as early as possible.Depends on #36.