Right now, as soon as you enable Commerce Log, it's all or nothing. You either log everything or nothing at all. However, there may be good reasons for a site to reduce the amount of noise in the logs - particularly in high volume scenarios where they still want a payment log but not cart or order item manipulation logs.

I'm envisioning a checkboxes list of all log templates that I can enable or disable whose settings are respected by the log generator. I don't know that we can get any more fine-tuned by default, because we don't really know all the different ways a log template will be generated (i.e., the same template might be used by an event subscriber and a form submit handler). Perhaps in the future we also add a context variable to the log generator to refine log creation even further.

Issue fork commerce-3570388

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

rszrama created an issue. See original summary.

tbkot made their first commit to this issue’s fork.

tbkot opened merge request !618

tbkot’s picture

tbkot commented
Status: Active » Needs review

The settings form for the Commerce log module has been added. It allows configuring the list of disabled log templates that should not be saved to the database.

jsacksick’s picture

jsacksick commented

Looks mostly good, the only thing I dislike is the fact that we're harcoding a list of "manual" log templates.
I'm thinking introducing an extra property "source": "manual" for those events would help us filter out the log templates that aren't generated by events.

"source" could default to "event" so we don't have to manually specify the source for most existing templates.

It could also just be a "manual" boolean? "manual": true, but maybe "source" is better... Don't know, but right now we hardcode a list of log templates, including one defined by the commerce_license module.

jsacksick’s picture

jsacksick commented

@tbkot: Thanks for your new patch, actually the Enum approach wasn't a problem here, as we could have updated the processDefinition() method to disregard non allowed values:


  /**
   * {@inheritdoc}
   */
  public function processDefinition(&$definition, $plugin_id) {
    parent::processDefinition($definition, $plugin_id);

    $definition['id'] = $plugin_id;
    foreach (['label', 'category', 'template'] as $required_property) {
      if (empty($definition[$required_property])) {
        throw new PluginException(sprintf('The commerce_log_template %s must define the %s property.', $plugin_id, $required_property));
      }
    }

    if (!$this->isStringSafe($definition['template'])) {
      throw new PluginException(sprintf('The commerce_log_template %s does not have a valid template string.', $plugin_id));
    }
  }

With that said, what we really need for now is filtering out manual log templates. I'll think about this a little more but this looks like a reasonable approach.

jsacksick’s picture

jsacksick commented

Let's go with "can_disable" instead, as discussed with @rszrama. Defaulting to true, and set to false for the "manual" ones.

  • jsacksick committed a3b40cfe on 3.x authored by tbkot 
    feat: #3570388 Add a settings form to control what gets logged to orders...
jsacksick’s picture

jsacksick commented
Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.