Add 'changed' field to the subscription entity

1. +++ b/commerce_recurring.install
   @@ -144,3 +144,21 @@ function commerce_recurring_update_8106() {
   +      ->setLabel(t('Changed'))
   

   The indentation here is wrong.

2. +++ b/src/Entity/Subscription.php
   @@ -742,6 +744,7 @@ class Subscription extends ContentEntityBase implements SubscriptionInterface {
   +    $this->setChangedTime(\Drupal::time()->getRequestTime());
   

   This shouldn't be needed as the field type plugin is reponsible for this already.

3. +++ b/templates/commerce-subscription.html.twig
   @@ -30,7 +30,7 @@
   +        {% for key in ['created', 'chnaged', 'trial_starts', 'trial_ends', 'starts', 'ends'] %}
   

   Typo here, should be "changed".

Hi @jsacksick, thanks for the reply.

I fixed the points you mentioned, and added tests.

I also modified the update hook implementation to update all existing subscriptions to set the initial value of 'changed' field to match the value of 'created' field; this is needed because we need to have a value for 'changed' field in order to update it when doing operations other than saving the form (e.g. cancel operation). Using setInitialValueFromField('created') when defining the field won't work since the field types don't match.

There's no way we can just load all subscriptions at once from an update hook. What about sites with millions of subscriptions???

My bad, on it.

Loaded them in chunks of size 20, please let me know if this should be changed.

Prevented access check when updating existing subscriptions.

Updating subscriptions like this is useless. You're only updating 20 subscriptions here. So maybe we should simply set the changed value to the current time, or not set the field and call it a day.

I'm not updating 20 subscriptions only, I'm updating all subscriptions but loading them in chunks of size 20 (loading each group of 20 subscriptions at once) to avoid memory issues.

Setting the value to the current time is an option, but setting it to match "created" field is more sensible since it will probably be much closer to the real last update.

Not setting a value at all is problematic as I explained before; if you performed any operation on the subscription other than saving the form and "changed" field didn't have a value at that time, it will not get updated and will remain empty.

This will be a valuable feature.

I think the update hook should set the changed timestamp to the most recent of the "renewed" or "created" fields. That will at least produce approximately sensible results.

Sites wishing to improve on that can implement their own update to set the changed date based on most recent paid order or whatever.

Thank you for the feedback, @John.

Attached is an updated patch that utilizes batch processing.

I think we should skip loading entities all together... Doing direct update queries to the DB makes the most sense here... We don't really want to involve loading / saving entities... It'd take hours for a site with hundreds of thousands of entities this way... So let's just write an update hook that performs direct update queries to the subscription table, similar to what we did for setting the balance field for orders in the latest Commece release.
Removing the "needs tests" tag as tests were added.

MR is ready to be checked
The failed test is not related to the made changes it can be fixed by replacing

  $values = [
    'payment_method[target_id]' => 2,
  ];

with

  $values = [
    'payment_method[target_id]' => '2',
  ];

the number should be a string.

Attaching a patch, just in case

@tBKoT: Can you look into using the highest value between the created and the renewed timestamp as suggested in comment #13?

@jsacksick
MR updated. The changed date will be the greatest value between the "created" and "renewed" dates.

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