Create new sensors to monitor payments count and turnover

1. +++ b/config/optional/payment/monitoring.sensor.payment_count.yml
   @@ -0,0 +1,11 @@
   +settings:
   +  entity_type: 'payment'
   +  time_interval_value: '86400'
   

   No need to specify the entity_type, as that is not used.

2. +++ b/config/optional/payment/monitoring.sensor.payment_turnover.yml
   @@ -0,0 +1,11 @@
   +value_label: 'CHF'
   

   We can't just hardcode CHF here :)

   Have a look at the existing commerce sensor. What we did there is expose a sensor for each enabled currency, but this works all a bit different in 8.x now.

   Check which currency is enabled by default in payment and use that for now. We will also need a setting to limit the sensor to payments of a specific currency.

3. +++ b/src/Plugin/monitoring/Sensor/SensorPaymentCount.php
   @@ -0,0 +1,44 @@
   + *   description = @Translation("Monitors how much money was transferred for payments with effective date."),
   

   The description here is copied from the other one and not correct. the two should also have a different label. label here is the same as for the sensors.

4. +++ b/src/Plugin/monitoring/Sensor/SensorPaymentCount.php
   @@ -0,0 +1,44 @@
   +    $ids = $statement->fetchCol();
   +
   +    /* @var \Drupal\payment\Entity\Payment[] $payments */
   +    $payments = \Drupal::entityManager()
   +      ->getStorage('payment')
   +      ->loadMultiple($ids);
   +
   +    $sensor_result->setValue(count($payments));
   

   You already have the count, there is no need to load them, count($ids) is enough, you can actually do a COUNT() expression here... Almost.

   The problem is that payment_status contains multiple rows per payment, so this will return too much.

   Which mean we need to do a count(DISTINCT payment_id).

   And we need to add an option to filter on a specific payment status. We only want to count completed payments her, so we need to filter by "completed" or what it is called. This is a sensor setting.

Hmm.. Testbot currently postponed checking due to a fail in HEAD. That's a strange thing.
I have pushed the debug statement from the issue referred...
#2382539: Rename SensorInfo to SensorConfig
Hopefully we will be able to see the bug origin when it fails again.

Yeah and now it passes again... grrr :-!

Ah yeah and testbot doesn't work with payment currently...

1. +++ b/src/Plugin/monitoring/Sensor/SensorPaymentCount.php
   @@ -0,0 +1,39 @@
   + * Contains \Drupal\monitoring\Plugin\monitoring\Sensor\SensorPaymentTurnover.
   ...
   + *   id = "payment_aggregator_count",
   ...
   +class SensorPaymentCount extends SensorDatabaseAggregatorBase {
   

   Comment does not match.

2. +++ b/src/Plugin/monitoring/Sensor/SensorPaymentCount.php
   @@ -0,0 +1,39 @@
   +    $statement = db_select('payment_status', 'ps')
   
   +++ b/src/Plugin/monitoring/Sensor/SensorPaymentTurnover.php
   @@ -0,0 +1,52 @@
   +    $statement = db_select('payment_status', 'ps')
   

   Please add a comment about what the query does.

3. +++ b/src/Plugin/monitoring/Sensor/SensorPaymentTurnover.php
   @@ -0,0 +1,52 @@
   +        if ($item->getCurrencyCode() == $this->sensorConfig->getValueLabel()) {
   

   Filtering by proper currency here seems to me pretty late. We are loading payments and cycling through them although they are unrelated to the current sensor.

   If earlier filter through db_select is not possible, explain in comment.

The sensor should alter the value_label of SensorForm to a select and offer the currencies only for selection. Alternatively, it should validate the input.

Ah and don't forget, i'm about to rename Sensor classes... so you should follow the new pattern SensorPaymentTurnover => PaymentTurnoverSensor... See
#2383993: Rename Sensor* to *Sensor, Sensor/Sensors/* to Sensor/*, Sensor to SensorBase

The sensor should alter the value_label of SensorForm to a select and offer the currencies only for selection. Alternatively, it should validate the input.

There is no way to do this other than a form_alter (which would be pointless as it's the same module. The interaction beween value type and the plugin settings forms is *very* problematic (cross-dependency on config forms for example), let's not go into this here.

And in this case, as written in #2368593: Currency value support, currency should be a value type, which would then completely replace a manual value label configuration. (Yet more complexity: hide the value_label form element if you select a value type that has a formatter callback).

1. +++ b/src/Plugin/monitoring/Sensor/PaymentCountSensor.php
   @@ -0,0 +1,41 @@
   +    // The follwoing query selects the payment_id column from payment_status
   
   +++ b/src/Plugin/monitoring/Sensor/PaymentTurnoverSensor.php
   @@ -0,0 +1,48 @@
   +    // The following query joins the tables 'ps' and payment_line_item (as pli).
   

   No "the following"...

2. +++ b/src/Plugin/monitoring/Sensor/PaymentCountSensor.php
   @@ -0,0 +1,41 @@
   +    // Table where the created time is within the past 24 hours.
   

   not past 24h. It's the configured interval.

3. +++ b/src/Plugin/monitoring/Sensor/PaymentTurnoverSensor.php
   @@ -0,0 +1,48 @@
   +    // Selects the payment_status as 'ps' from the database.
   

   My intention was more a summary documentation statement instead of interpreting line by line...

   I can read the code and see payment_status and payment_line_item already. But even though you documented it, it is not quickly clear to the reader why these two tables are needed.

   Example:
   Query payments from payment_status, filter for the currency.
   The payment amounts reside on payment_line_item.

4. +++ b/src/Plugin/monitoring/Sensor/PaymentTurnoverSensor.php
   @@ -0,0 +1,48 @@
   + * Monitors payment turnover stats.
   + *
   

   Add a comment here that we are using a custom "illegal" query to the database instead of using the EntityManager...

Yeah, now looks good. Possibly ready to merge in. Not tested though.

1. +++ b/src/Plugin/monitoring/Sensor/PaymentCountSensor.php
   @@ -0,0 +1,41 @@
   +    $statement = db_select('payment_status', 'ps')
   ...
   +      ->distinct(TRUE)
   ...
   +    $sensor_result->setValue(count($ids));
   

   Why are you not using a COUNT(*)?

   Also, can't we extend from database aggregator by now?

2. +++ b/src/Plugin/monitoring/Sensor/PaymentTurnoverSensor.php
   @@ -0,0 +1,49 @@
   +    $line_items_value = $statement->execute()->fetchCol();
   +    $amount = 0;
   +    foreach ($line_items_value as $line_item_value) {
   +      $amount += $line_item_value;
   +    }
   +    $sensor_result->setValue($amount);
   

   Same here, this should be possible as a SUM() query, if we query it directly anyway..

3. +++ b/src/Tests/MonitoringCoreTest.php
   @@ -22,7 +23,7 @@ class MonitoringCoreTest extends MonitoringTestBase {
       */
   -  public static $modules = array('dblog', 'image', 'node', 'taxonomy');
   +  public static $modules = array('dblog', 'image', 'node', 'taxonomy', 'payment');
   
   @@ -317,6 +318,39 @@ class MonitoringCoreTest extends MonitoringTestBase {
   +    // ======= payment sensors tests ======= //
   +    $payment_method = Payment::methodManager()->createInstance('payment_test');
   +    $payment = Generate::createPayment(4, $payment_method);
   +    $payment->save();
   +
   

   The class is named Monitoring*Core*Test, Payment is not core. So we need a separate integration test for this.

   And we need a test_dependencies[] in the info file.

   Also, commiting this is tricky, it will only work if testbot can get the proper dependency and it will be broken until we do. We can try as payment is on d.o, but I'm not sure if it will work.

So we have to commit this and then wait until the dependencies are rebuilt, then we should get payment and the test should work. Alternatively, we can commit without the test and do that as a follow-up.

search_api is now there, so we can add the test for that back.

Ok, I just committed the payment test dependency separately. Let's wait day, then re-roll this and then it should work.

Providing updated patch without test dependency.

Yay! Now it passes.
Committed and pushed.