Provide a way to disable creation of database log entries

Tagging this with Novice → as the instructions to proceed are well defined. I'm here to review and provide guidance if needed.

I will take a look on it

Sorry guys, I may have cause some chaos with the 3354081-drupal-11 branch. Anyway, the code for review should be in the 3354081-provide-a-way branch. The only thing missing is the test, which I will write as soon as I will manage to run PHPUnit tests in my lando docker machine.

Awesome @petiar! Many thanks.

Think we will need a CR for the change to the UI.

https://git.drupalcode.org/project/drupal/-/merge_requests/4375 didn't do a full review since it was in draft.

There are a few coding standards that needs to be fixed, but those are irrelevant now. The only thing that is not 100% to me right now is how to make the DblogServiceProvider::alter code run as soon the Dblog Form is submitted.

I'm not sure if \Drupal::service('kernel')->rebuildContainer(); is the way to go, or if there is something less aggressive. I was not able to find something in core that does this already. All the calls to rebuildContainer I found are part of tests.

I've been trying this today. It seems the approach to alter the service is not actually necessary. Dblog can provide the service using by DblogServiceProvider::register and provide the regular logger only when the config is not available.

The only example in core I found doing something kind of similar is the Drupal\language\LanguageServiceProvider.

The logic then would be something like this:

namespace Drupal\dblog;

use Drupal\Core\DependencyInjection\ContainerBuilder;
use Drupal\Core\DependencyInjection\ServiceProviderBase;

class DblogServiceProvider extends ServiceProviderBase {

  public function register(ContainerBuilder $container) {

    if (\Drupal::configFactory()->getEditable('dblog.settings')->get('logging_paused')) {
      return;
    }

    $container->register('dblog.logger', 'Drupal\dblog\Logger\DbLog')
    // ...
  }

}

\Drupal::configFactory() wont work at this register phase I'm afraid. But again LanguageServiceProvider provides some clues how what to do.

MR updated.
Please review.

Thanks for working on this! Tagging this with Usability tag as there are changes to the UI (new checkbox).

I would personally prefer to have a Usability review here as well, because the two proposed solutions (see the IS) can be evaluated from different point of views:

1) as a developer
I would agree that introducing an exception for dblog entries when uninstalling is not a great thing.

2) as a website admin
Currently you can uninstall the module in one click, without need to care about anything. But instead of one click, now you will need to do (at least) two more steps (2 + 3):

1. Go to the modules uninstall page - you will see that it is not possible to uninstall the module
2. Go to the dblog module settings and pause logging - the question also is whether users will try to delete the logs first and will need to repeat the steps 2 and 3 again
3. Delete all dblog entries
4. Only then you will be able to uninstall the module

Isn't it possible that it will get too complicated for normal use? I know that uninstalling this module is not something, that is done on a daily basis, but even so.. We can argue that it is needed for all modules using content enties, but I still consider dblog entries as something different like a real content (for me these are just logs).

The MR is in a draft state (so the tests did not go through) and has codestyle issues, so moving to NW.

My patch at 🐛 Dependency on config storage causes circular reference in service container Needs review has a pattern that might be useful for dblog module as well: A config subscriber checks if the config changed and if so, invalidates the container.

@poker10 I think you pointed out some valid inputs. I had the same thoughts as well and this is why I tried to avoid this in the first place while coding 📌 Make dblog entities Postponed

Unfortunately I don't see an easy way to avoid this, at least without not touching a lot of content related stuff involved with content deletion.

The way I see it, dblog entries will be the same than Comments now. If you have a non public site, deleting the logs will be enough to not need to pause them.

I'm thinking maybe we can avoid this and improve the UX at the same time to allow you to block temporally log entries while you are deleting them.

I think we can also play with ?redirect=to in the url to make this as simple as possible.

Eventually most developers will use drush to uninstall dblog, so I think we can provide a patch to make this in one go if we need to.

Another interesting approach to analyze is the UI provided by the paragraph module. As it allows you to delete the content "on the fly" when deleting a paragraph type.

Isn't it possible that it will get too complicated for normal use? I know that uninstalling this module is not something, that is done on a daily basis, but even so.. We can argue that it is needed for all modules using content enties, but I still consider dblog entries as something different like a real content (for me these are just logs).

@poker10 I found a way 📌 Make dblog entities Postponed to not need this to implement 📌 Make dblog entities Postponed

I will keep this issue open in case someone else want to work on this, but the original problem described in the issue is not a problem anymore.

Doesn't seem like a novice level issue