update readme for disabling papertrail within initializer

[oehlschl]

Fixes #1038

Updating the readme to clarify that PaperTrail.config.enabled cannot be set within an initializer since it's eventually overwritten by the environment-specific config (or by the internal default of true).

I tried to be clear without giving unnecessary details. Also tried to use style consistent with the rest of the readme, but please let me know if you have suggestions.

• Wrote [good commit messages][1].
• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests.
• Added an entry to the Changelog if the new
  code introduces user-observable changes.
• The PR relates to only one subject with a clear title
  and description in grammatically correct, complete sentences.

[oehlschl]

Also note that I did mention both PaperTrail.enabled and PaperTrail.config.enabled for clarity, since that they seemed to be used interchangeably (and without preference) in the readme.

[jaredbeck]

Thanks for taking a stab at this. I've worked hard on the documentation for PT's various on/off switches, and I'm always open to suggestions on how to improve the docs.

.. PaperTrail.enabled (or its alias PaperTrail.config.enabled) ..

To say that they are aliases is effectively correct, and a useful addition to the docs. They are not technically aliases, but the former does delegate to the later. We can fine-tune the wording and placement.

.. PaperTrail.config.enabled cannot be set within an initializer since it's eventually overwritten by the environment-specific config ..

Hmmm. According to my testing, environment files (config/environments) happen before initializers (config/initializers). See also The Rails Initialization Process guide.

[oehlschl]

Thanks for the response, @jaredbeck. It could just be poor wording on my part; it's actually that any custom initialization of .enabled gets overwritten by PaperTrail's own internal initialization hook as described here: #1038

You're correct on the order, but from my experience (and from the other report), what happens is:

1. environment-specific initialization
2. config/initializers/paper_trail.rb
3. /paper_trail/lib/paper_trail/frameworks/rails/engine.rb

Due to the PaperTrail.enabled = app.config.paper_trail.fetch(:enabled, true) line, any .enabled value in a custom initializer from (2) will be overwritten by the environment-specific value defined in (1) or by the true default defined in (3).

Obviously that's a lot of detail to put in the README, but the effect is still the same as what I described; custom initialization gets overwritten by the environment config (even though the environment config is evaluated first).

[jaredbeck]

Thanks for the clarification.

1. environment-specific initialization
2. config/initializers/paper_trail.rb
3. [the initializer defined by] /paper_trail/lib/paper_trail/frameworks/rails/engine.rb

I'm seeing the same order.

Maybe we should we deprecate config.paper_trail.enabled. I don't see what value it provides, and having two ways to do the same thing is confusing. Let's compare the two:

PaperTrail.enabled=	config.paper_trail.enabled=
happens immediately	happens later, when the initializer defined in engine.rb runs. The order in which this happens, relative to other initializers is not known. In our testing it is after custom initializers, but is this guaranteed?
about 5 LoC	hundreds of LoC, including arcane "rails engine" stuff that few people on Earth really understand, myself excluded
can be used in custom initializer or standard environment file	can only be used in standard environment file?
can be used after startup	cannot be used after startup
works in all frameworks	only works in rails

I think we should keep PaperTrail::Rails::Engine (it allows us to register our paths["app/models"] so that's nice) but maybe we should deprecate config.paper_trail.enabled=. Thoughts?

[oehlschl]

I hadn't thought too much about it before now, but it seems like there are some good arguments for deprecating config.paper_trail.

For counter arguments, I could imagine something related to Rails magic and specifics with config objects and threads under the hood, but that's just thinking hypothetically; I can't actually think of anything concrete. That said, it also looks from the source like PaperTrail already accounts for threads well enough on its own.

For other pro arguments, removing the config.paper_trail option would also keep all the library configuration on the module itself and collapse two ways of doing something into one source of truth.

If you do want to go forward with this, I'd suggest first raising/logging a deprecation messaging in the engine if config.paper_trail.enabled is set.

[jaredbeck]

@oehlschl please review #1183, thanks!

[jaredbeck]

Closed by #1183.

Everyone, please comment here if you'd like to see config.paper_trail.enabled kept.

[oehlschl]

Thanks @jaredbeck. Happy we were able to improve this for the next person.

[HoneyryderChuck]

Hi. I'm on 10.3.2, and I'm going to keep this option, mostly because using the recommend PaperTrail.enabled = does not work, i.e. my tests are storing versions.

[iberianpig]

@oehlschl @jaredbeck

This issue has been unresolved yet.

At first, PaperTrail.enabled= is loaded in the following order.

1. config/environments/*.rb
2. config/initializers/*.rb
3. lib/paper_trail/frameworks/rails/engine.rb:35 (in PaperTrail Gem)
4. config.after_initialize { } in config/environments/*.rb

Even if you set PaperTrail.enabled=false to config/environments/development.rb,

lib/paper_trail/frameworks/rails/engine.rb will override PaperTrail.enabled=true.

      initializer "paper_trail.initialisation" do |app|
        enable = app.config.paper_trail[:enabled] # => nil
        if enable.nil?
          unless PaperTrail.enabled? # =>false
            ::ActiveSupport::Deprecation.warn(DPR_RUDELY_ENABLING)
            PaperTrail.enabled = true # !!!override here!!!
          end
        else
          ::ActiveSupport::Deprecation.warn(DPR_CONFIG_ENABLED)
          PaperTrail.enabled = enable
        end
      end

Therefore, it will never become PaperTrail.enabled=false.

A workaround is to set config.after_initialize { PaperTrail.enabled = false } in config/environments/*.rb.

However, is this behavior correct?

If this behavior is correct, the documentation should explain where should we set PaperTrail.config.enabled=.