Problem/Motivation

Upgrading from earlier versions to 2.0.0 or 8.x-1.2 turns off logging.

Steps to reproduce

Install an earlier version of the module, then upgrade. Then visit the settings page with the new "enable" checkbox. You will see that this is disabled by default.

Proposed resolution

The problem occurs because there is no "enable" config values in previous versions of the module. On updating the module, the config is untouched, and so where referencing this config value, we're getting a null returned. The null is a falsy value, so this expression returns false: https://git.drupalcode.org/project/lagoon_logs/-/blob/2.x/src/Logger/Lag...

Relying on explicitly setting the module to be enabled runs counter to the spirit of lagoon_logs' philosophy (zero config) - the proposed solution is to flip the `enabled` setting to a `disabled` setting, so that in cases where there are no appropriate configuration values, the default (if the module is enabled) is to log to lagoon.

CommentFileSizeAuthor
#10 Screenshot 2022-02-02 at 11.53.35.png223.73 KBtim.clifford
#10 Screenshot 2022-02-02 at 11.54.00.png417.14 KBtim.clifford

Issue fork lagoon_logs-3261904

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

bomoko created an issue. See original summary.

bomoko opened merge request !9

bomoko’s picture

bomoko commented
Status: Active » Needs review

I've created MRs for both 2.x and 8.x branches above.

Ready to test.

fjgarlin’s picture

fjgarlin commented

@bomoko, almost there. There is also this file where the "enable" field appears: https://git.drupalcode.org/project/lagoon_logs/-/blob/2.x/config/schema/...

Once that's changed, then it should be good to go.

fjgarlin’s picture

fjgarlin commented
Status: Needs review » Needs work

tim.clifford made their first commit to this issue’s fork.

bomoko’s picture

bomoko commented
Status: Needs work » Needs review

Thanks @fjgarlin - requested changes have been made.

fjgarlin’s picture

fjgarlin commented

Great, I think that @tim.clifford was going to test it all. If you need anything else just ping.

tim.clifford opened merge request !10

tim.clifford’s picture

tim.clifford commented
StatusFileSize
new Screenshot 2022-02-02 at 11.54.00.png417.14 KB
new Screenshot 2022-02-02 at 11.53.35.png223.73 KB

Testing patch against our drupal example on Lagoon against 2.0.x and 8.x1.* and can confirm there is now the 'disable' module flag and logs are automated on install.

tim.clifford opened merge request !11

tim.clifford closed merge request !11

tim.clifford’s picture

tim.clifford commented
Status: Needs review » Reviewed & tested by the community
tim.clifford’s picture

tim.clifford commented
Version: 2.0.0 » 8.x-1.3
fjgarlin’s picture

fjgarlin commented
Status: Reviewed & tested by the community » Fixed

Changes were merged to 8.x-1.x and 2.x. New releases of the module were created too.

Thanks @bomoko and @tim.clifford. Closing the issue.

fjgarlin’s picture

fjgarlin commented
Status: Fixed » Closed (fixed)