Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bojanz created an issue. See original summary.

Kazanir’s picture

Usage groups in 1.x were an add-on to licenses which allowed additional information about the consumption of electronic resources to be tracked and charged for. The idea here was to allow more full-featured recurring billing, where a license represented not just one electronic resource but a subscription plan that tracked multiple elements. Common examples might include:

- Cell phone plans which track text messages, minutes used, or bandwidth consumption
- Metered billing for physical resources like utilities (i.e. power, gas, etc.)
- Consumption of server resources (storage, bandwidth, CPU time, etc.) for a hosting provider
- Customer ticketing or support systems which charge on a per-ticket basis
- Organization-level SaaS subscriptions that charge on a per-user basis

The attached patch is an initial attempt at working out this within the context of the new 2.x module. It has several components:

- The UsageType plugin type, which handles the creation of usage records and generating matching charges for them on the recurring order for a given subscription. Two default implementations are provided:
- Counter: Usage where all records are added up and charged together, optionally with free quantities depending on the subscription
- Gauge: Usage which tracks a level of consumption for a period of time, with each charge pro-rated against the time interval when consumption was at that level
- Future goal: A "field" usage type which handles usage on the basis of a field value on an entity rather than using separate storage.
- A default storage engine and record type which uses a custom Drupal database table. (We considered using entities for this, but it seemed too heavy.) The goal is for storage to ultimately be swappable, so an interface is defined. (The idea here is that one of the default UsageType plugins could be swapped to some other storage system which delivers the records already recorded there.)
- A UsageRecord class and interface which define the components of a usage record and is used to generate charges.
- The concept of a "usage group" which is defined on the subscription type. Usage groups leverage a particular UsageType plugin but are defined at the SubscriptionType level, usually with a particular name and product variation attached.
- A set of SubscriptionType add-on interfaces which allow a given subscription type to define custom behavior for its usage groups. A given subscription type might want to define different free or initial quantities based on the current subscription plan and these interfaces allow them to do so without storing callbacks in the plugin definition.

Open questions are:

- In the original 1.x implementation, usage groups depended heavily on license revisions. By using revisions, usage groups were able to conform to changes in the underlying subscription plan -- the code would load the appropriate license revision, get the usage group definitions (including things like the product, free/initial quantity, etc.) from the revision, and proceed with its normal logic. With subscriptions not using the revision system, it is an open question how to handle this. I went with the "subscription type implements custom interface and the usage type plugin passes the relevant information back to the interface method to determine per-group information" at Bojan's suggestion, but this is still up for debate -- as is whatever we're doing to track subscription history in general.

- We haven't decided on the complete charge logic, so I have avoided writing those methods until I'm more familiar. Based on the current code though, it should be apparent how to load the usage records for a particular usage group, subscription, and billing cycle combiation and generate charges from there. Feedback is welcome.

- As noted, the next move (other than writing tests) is to generate a field-based usage type. A usage group leveraging this UsageType plugin would (under the current model) be attached to a subscription type implementing something like a SubscriptionFieldUsageInterface. The method for this interface would be able to generate usage records based on the value of a particular field on the subscription or some other entity. The motivation for this is that in 1.x usage groups were not super-easy to use -- you had to write code to call the addUsage method somewhere. Allowing subscriptions to leverage the field system is an easy way to make a limited version of this functionality available with more Drupal-ism and less custom code required.

bojanz’s picture

Status: Active » Needs review
wizonesolutions’s picture

Status: Needs review » Needs work
  1. +++ b/commerce_recurring.install
    @@ -13,18 +13,21 @@ function commerce_recurring_schema() {
    +      'usage_group' => [
    

    The issue summary says that we agreed on UsageType...

  2. +++ b/src/Plugin/Commerce/UsageType/UsageTypeInterface.php
    @@ -94,14 +102,16 @@ interface UsageTypeInterface {
    +  public function onSubscriptionChange();
    

    My gut is kind of saying that we ought to be using event subscribers here, but I guess the underlying implementation that actually calls this method can use them. It is kind of a "usage type" concern, and I guess it's nice to do it this way for context reasons.

+++ b/commerce_recurring.install
@@ -49,4 +52,6 @@ function commerce_recurring_schema() {
+  return $schema;

Going to need an update hook now that the module is actually released. At least it's just the database and not fields (shudder). Will look at this.

This functionality also needs at least basic tests, obviously. Issue summary edits welcome on what those should be. There are various use cases at play. We have probably discussed these loosely before, but I'm not sure if they've been captured anywhere easily accessible...so we probably have to think about it again, and maybe just as well.

My thought? Counter and gauge. Those are our main usage cases, anyway.

Counter: you have X allowed Things (API calls, website checks, etc.), and we want to track how many of them you have used.
Gauge: you pay per Thing (virtual server, monitored website), and we want to know which Things you used during the billing period and when (since we might prorate).

There's also prepaid and postpaid on the subscription, which ties in with how we handle usage tracking. In my case, for example, subscriptions are prepaid, and usage tracking is tacked onto next month's bill. In a postpaid case, both the regular subscription amount and usage would be invoiced together. Counter can be both, but gauge is almost inherently postpaid.

As for gauge + free usage...does such a use case really exist? Would the Free Tier in Amazon AWS be free gauge usage or a "free trial"? I suppose it is the former, since it is possible to exceed it (for example, using more than 5 GB on S3). So...worth testing.

So some big, "does-this-work-at-all" cases are probably:

- counter, no free usage, postpaid: 300 usages at $0.05/usage = $15 invoiced at end of billing cycle
- same but prepaid, $5/mo. = $20 invoiced at end of billing cycle
- counter, free usage 150, postpaid, $5/mo.: 300 usages at $0.05/extra usage = $12.50 invoiced at end of billing cycle
- same but prepaid, $5/mo. = $12.50 invoiced at end of billing cycle
(no real difference on the last two...so not even sure if the prepaid/postpaid distinction is worth it for our tests, or if we should just focus on calculations)

- gauge, no free usage, 4 usages @ $5/usage-month = $20 invoiced at end of billing cycle
- gauge, no free usage, 2 usages from April 1 to 15, 4 usages from April 16 to April 30 @ $5/usage-month = $15 invoiced at end of billing cycle (2 all month = $10, 2 half-month = $5)
- gauge, free usage 5, 5 usages @ $0.02/usage-month = $0
- gauge, free usage 5, 10 usages @ $0.02/usage-month = $0.10 at end of cycle
(No prepaid tests for gauge as I can't think of any cases where this makes sense. Open to suggestions if I've forgotten something.)

I'll see how far I get on this. I only really care about the prepaid + counter case for myself, and I might only do that and leave the rest of it to someone else if it's taking me too long.

wizonesolutions’s picture

...yeah, the format of this patch is confusing me.

can't find file to patch at input line 350
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/src/Plugin/Commerce/UsageType/UsageTypeBase.php b/src/Plugin/Commerce/UsageType/UsageTypeBase.php
|index 8fe01b1..7099444 100644
|--- a/src/Plugin/Commerce/UsageType/UsageTypeBase.php
|+++ b/src/Plugin/Commerce/UsageType/UsageTypeBase.php
--------------------------

The dev branch doesn't have a commit 8fe01b1...so this patch must have been rolled against something that never got committed. I'll dig around in GitHub and try to produce something that at least fails...meaningfully?

wizonesolutions’s picture

FileSize
41.54 KB
62.94 KB
48.14 KB

Trying to rebuild this. I'm not sure what happened to Commerce Recurring, but the histories of Kazanir's GitHub branch and the Drupal project are totally separate:

Anyway, https://github.com/Kazanir/commerce_recurring/commit/056e38910c25c01ddc5... doesn't seem important; I guess this was scaffolding that ultimately got redone/thrown away.

https://github.com/Kazanir/commerce_recurring/commit/0c384d3573f834048c5... seems partially relevant, especially https://github.com/Kazanir/commerce_recurring/commit/0c384d3573f834048c5...

(I went through a couple other commits in the repo. All of Kazanir's + a couple of bojanz cleanup commits. Probably not that important.)

Now I'm trying to use git checkout against the commits in Kaz's repo in order to create new files in a temp branch, and I'm manually making edits to existing files. I think this will go well.

...OK, there were some cleanups from bojanz that messed with/disabled things (probably because of usage tracking not getting into Recurring right away), but I think I got everything. It wasn't done to begin with.

From doing this, my next steps are:

- Try writing a test in order to understand the codebase.
- Write an example module for usage or something? Or think about if it could work without requiring a custom subscription type. Maybe we could provide one integrated with basic use cases, such that calling code would only need to call UsageTypeInterface::addUsage() at the right time but not deal with all the boilerplate if they're following a somewhat common workflow (subjective, I know). Such a subscription type would need to be configurable: how many free uses, how many initial uses, etc. Need to look at if that architecture is already in place and think about/discuss what would be best if not (and if it's worth it, since someone implementing this definitely has to write some code anyway and since presumably not many people are implementing it?)

For now, here's a broken patch with usage group changed back to usage type and the services and plugin types re-added. Also fixed the wrong class name being used for the database storage class in the original patch. There's obviously more to do.

My interdiff shows a missing PlanHistory object from the original patch, but this is not used anywhere in the rest of the patch or in the reconstructed code I have. It's also not on the usage branch on GitHub and thus not in https://github.com/bojanz/commerce_recurring/pull/29/files. I'm not sure where it's coming from. I do notice that bojanz/commerce_recurring did this stuff on an 8.x-2.x branch, but it's on a 8.x-1.x branch on d.o. So I guess there was application of patches rather than a straight merge from GitHub, or something (thus changing all the SHA's). Gonna just leave it out.

And I'm attaching the interdiff without -p1 and with it. Because these two diffs are based upon different versions of history, interdiff gets super-confused about what the heck happened here and doesn't fully make sense. So take the interdiff as a very fast-and-loose idea of the differences between the patches.

If anyone knows what this new patch is missing or if it deviates from something agreed-upon, lmk ASAP. I'm not sure if there's any documentation where I can read a good spec about what's supposed to happen? I think maybe an old one was floating around somewhere?

wizonesolutions’s picture

FileSize
41.66 KB
58.5 KB
41.78 KB

Was re-reading Kazanir's first comment and realized I messed up a little on this:

- The concept of a "usage group" which is defined on the subscription type. Usage groups leverage a particular UsageType plugin but are defined at the SubscriptionType level, usually with a particular name and product variation attached.

So we have both usage types (counter and gauge) and usage groups, which are basically what they were in 1.x (discrete combinations of product variation, etc. that ultimately appear on the invoice). So I'll just revert those naming changes based on the last commit on GitHub...

OK, here's the updated patch and interdiffs.

wizonesolutions’s picture

I've been rubber-ducking this out in the #commerce-recurring channel on Drupal Slack.

I forked the repo temporarily just to make it easier to work (and in case it takes a while to get the final patch committed or I stop working on it before it's Done and someone else finishes, etc.):

https://gitlab.com/fillpdfservice/commerce_recurring

I will give other people access later if it makes sense.

I am also tracking my main thoughts/Slack messages about the work in this Google Doc: https://docs.google.com/document/d/1zn-oITWf0CvmsXBsUwpkgMXymOAc7eRAbCj5...

So far, I'm working on the first test to check that Counter usage works. I have the overall structure I want for the API in mind and am working on a UsageHelperTrait class that most usage-tracking subscription types will simply be able to use. It will enrich them with convenience methods for usage tracking so that they don't have to deal with loading the relevant plugins directly.

As far as the test goes, I left off basically needing to fix up everything to actually record the usage in the DB :) specifically, I'm considering removing the concept of named usage groups from 2.x. Usage types (counter, gauge) will of course stay, but when we're already tracking which product variation provides the price for usage, that model already allows adding several usage variations to the same subscription. Requiring an additional name seems like an unnecessary burden for 2.x, and it was one of the more confusing parts in 1.x. I need to review the 1.x code, and I may change my mind. I won't have to integrate usage tracking with Commerce Recurring's invoice for the very first assertion ("was usage recorded") , but I'll need to for the first test in general (see my notes about what the tests are above).

I will also want to have a test where I specifically record multiple types of usage for the same subscription (maybe two counters and two gauges) in order to make sure that works. Probably also multiple subscriptions on the same order, but I have to check the model for that. If separate recurring orders are spawned, no need...if it's the same one, worth testing.

More general notes are in the doc.

I welcome comments/questions from anyone if they are planning to use this and want to make sure it will work. I would love to know more specific use cases in order to make this as little code as makes sense to implement on sites. It is very much a developer-facing API, and there won't be any UI in the initial release (other than charges appearing on invoices), at least not from me.

wizonesolutions’s picture

(can't believe I hadn't posted the above like two days ago. good thing Chrome saves in-progress forms when you quit...)

wizonesolutions’s picture

FileSize
0 bytes
39.79 KB

Attaching another WIP patch + interdiff.

I have counter usage being represented on the order now. However, in the test, I think I have a prorating issue. I think I'll use a prepaid rolling billing schedule instead of the postpaid fixed one that RecurringKernelTestBase provides. That will probably resolve or mostly resolves the issues. I may have some BillingPeriod-related issues as well, but I'll see next time.

Once the charges are shown on the order, the other tests should be more straightforward. I'm not sure if I will actually be the one to test postpaid scenarios. I may try but will likely give up if it gets too complex, as I don't intend to use them. I just need prepaid rolling to work (and really only counter, for that matter, but I will make a best effort at gauge).

wizonesolutions’s picture

FileSize
67.05 KB

lol. Oops. Let's try that again. Not attaching an interdiff because it didn't seem to come out cleanly. Maybe I changed too much had a weird previous patch.

wizonesolutions’s picture

Counter tests are working. They're mostly the same as what I specced, and test the same things.

Starting now on gauge. I'm also dropping the initial usage interface and hooks. This can just be achieved with SubscriptionTypeInterface::onSubscriptionActivate().

wizonesolutions’s picture

FileSize
80.3 KB

I think the earlier patches might've been incomplete. Realized I had two WIP commits on my branch rather than one squashed/amended one. So I'm not attaching an interdiff here.

This is another unfinished patch, but it's almost done!

Remaining:
- update hook for commerce_recurring_usage table
- test for incomplete gauge usage (UsageTypeInterface::isComplete()). I'm not 100% sure about this one. I asked this in Slack:

wizonesolutions [13:01]
@jsacksick @bojanz Can subscriptions be blocked from renewing? It seems like Commerce License Billing in 1.x had a completeness check for metered usage. Wondering how I'd implement this in 2.x. I probably should.

  if ($can_close_order) {
    $order->status = 'recurring_payment_pending';
    commerce_order_save($order);
    // Clean-up usage.
    $license_ids = array_keys($licenses);
    commerce_license_billing_usage_history_clear($license_ids, $billing_cycle);

    return array(
      'status' => ADVANCEDQUEUE_STATUS_SUCCESS,
      'result' => 'Closed the billing cycle.',
    );
  }
  else {
    if ($order->status != 'recurring_usage_pending') {
      $order->status = 'recurring_usage_pending';
      commerce_order_save($order);
    }
    else {
      // Release the lock.
      entity_get_controller('commerce_order')->resetCache();
    }

    // Advancedqueue will retry every 24h, until 5 retries have failed, as
    // specified in commerce_license_billing_advanced_queue_info().
    return array(
      'status' => ADVANCEDQUEUE_STATUS_FAILURE_RETRY,
      'result' => 'Incomplete usage information.',
    );
  }

- tests for prepaid + gauge

Next week I hope to finish testing gauge usage and field test this work by integrating metered usage with my store (probably also with integration tests on my end or something).

Other than what's remaining though, I'm fairly sure that this is working well. But I won't ask for testers until I have the update hook done (not that it'll take long, just haven't done it yet).

It's fairly safe to start reviewing this patch too. Not a lot is going to change, and I'll include interdiffs from now on for the last parts that'll be coming.

wizonesolutions’s picture

Status: Needs work » Needs review
FileSize
86.2 KB
23.54 KB

OK, I'm finally ready to set this to Needs Review.

I have not implemented checking if usage is complete. I don't need this, and it seemed that I would have to add a new workflow state, etc. Couldn't be bothered, and it doesn't seem critical for shipping this feature. It can easily be added later. I left in the interface and logic to help with that.

There is one database update to install the new table.

Locally, all Commerce Recurring tests are passing. Let's see what testbot thinks.

I'm still not that happy about the $order_period = NULL hack (you'll see what I mean), but I think it's balanced enough for what I'm doing. I do think that being able to override the prorater on a per-charge (and thus per-order-item) basis would be the right solution. That could be done later, though. I don't think it would particularly break BC with this approach.

I can finally attach a decent interdiff too.

Tests should pass, but I expect PAReview to fail. I'll fix what it flags.

Interdiff should work now.

Status: Needs review » Needs work

The last submitted patch, 14: 2919598_usage_tracking_14.patch, failed testing. View results

wizonesolutions’s picture

Issue tags: +Needs documentation

For implementing, just look at the \Drupal\commerce_recurring\_test\Plugin\Commerce\SubscriptionType\UsageTestProductVariation subscription type. Your main job is to implement ::getFreeQuantity() properly from your site. Likely, you'll have a field on the variation that you can use to check this. That's what I plan to do. The UsageHelperTrait gives you a convenience method for adding usage. See UsageTrackingTest for how to call it.

That's basically it. You may need to add your own UsageType plugin as well. See the included Counter and Gauge plugins for this. You may be able to just subclass one of them, e.g. if you just want to override generateCharge() and change the title pattern for how usage charges look on the order.

Since it's a different subscription type, you have to be sure to configure this appropriately in your actual shop (make sure your subscription product uses it, etc.).

People using Commerce License should extend \Drupal\commerce_license\Plugin\Commerce\SubscriptionType\LicenseSubscription rather than ProductVariation as the test class does.

This should be enough to get started. I'm also implementing this, so I'll add more after that. Time permitting, I'll PR a draft of some documentation to Commerce Docs depending on the final form of the code. Much more likely if I can get a review of this before Drupal Mountain Camp.

wizonesolutions’s picture

Adds test annotation.

wizonesolutions’s picture

Status: Needs work » Needs review
FileSize
86.01 KB
764 bytes

Pruning old comments.

wizonesolutions’s picture

I don't know what's up with testbot, but I ran PHPCS locally and fixed all the warnings. So I consider this patch fully ready for review.

wizonesolutions’s picture

There's another tweak I made where the usage type manager in SubscriptionTypeBase should be an optional param, since third-party code extending it might override the constructor but not pass that param into the parent call. No existing code does that, but it'd be a BC break. It's one that core would allow (https://www.drupal.org/core/d8-bc-policy#classes), but it's not very hard to prevent in this case.

Leaving status alone here because it'd be great to get this reviewed so that I don't have to change my own implementation of it too much (e.g. if we make architectural changes).

bojanz’s picture

Commerce Guys is having trouble finding the resources for the maintenance of this module. To make it possible to get this module stable (and continue maintenance), we are going to need to reduce the module's scope. That means no usage-based/metered billing. It took us months to stabilize this properly in D7 (Commerce License Billing), which was sponsored by Platform.sh back then. I have no doubt it would take weeks here, even if the current 90kb patch is perfect. And there is no sponsor for any of the D8 recurring issues.

So, we should aim for you to create a commerce_recurring_metered module, and focus on making any necessary API changes on the recurring side to support it.

We can then re-evalute for a future 2.x branch if a sponsor appears.

wizonesolutions’s picture

Status: Needs review » Needs work

That's fine. I'll create that and move this issue this week. I'll have to look at how to integrate with SubscriptionTypeBase when it collects charges. Definitely some API changes.

wizonesolutions’s picture

Got namespace https://www.drupal.org/project/commerce_recurring_metered and will flesh this out this week (Drupal Mountain Camp, etc.)

wizonesolutions’s picture

Project: Commerce Recurring Framework » Commerce Recurring Metered Billing

This also means I can move this over to that project now. I will open a separate issue for the need to override prorating. That, and potentially hooks into SubscriptionTypeBase::collectCharges() (but I assume those exist in a way that would let me implement this).

wizonesolutions’s picture

Version: 8.x-1.x-dev »

Hiding patches, since they are for Commerce Recurring directly and need to be rerolled/re-namespaced (along with tests...and tests have to be set up too).

wizonesolutions’s picture

Priority: Normal » Critical
Status: Needs work » Needs review
FileSize
87.56 KB

Here is the code turned into its own module. The tests for fixed billing schedule counter usage are commented out because Commerce Recurring does not yet support overriding the prorater per order item. (See related issue.) I don't need that, so it's a lower priority for me to solve it. I will just note the issue on the project page once I've implemented this module in my project and am ready to release a stable version.

Attaching a patch with all the code. Need to set up automated tests, then will queue the test. No interdiff since this is a different module now.

wizonesolutions’s picture

Version: » 8.x-1.x-dev
wizonesolutions’s picture

FileSize
87.56 KB

Whoops. Let's try this.

  • wizonesolutions authored 5767309 on 8.x-1.x
    Issue #2919598 by wizonesolutions, Kazanir, bojanz: Implement usage...
  • wizonesolutions authored 6e9333e on 8.x-1.x
    Issue #2919598 by wizonesolutions, Kazanir, bojanz: Implement usage...
wizonesolutions’s picture

Pushed some code to dev, but not RTBC'ing this until I've tested it some more. I'm also making some refinements because I realized that the usage-adding logic needs to be reusable for classes that basically have to extend SubscriptionTypeBase (e.g. mine, which will extend Commerce License's LicenseSubscription).

So in that case, I plan to add a service that simply collects usage charges. I will then use that in MeteredBillingBase and privately in my subclass of LicenseSubscription.

Update from before I posted this comment: I'm actually going to fold all of UsageHelperTrait into this service, which I will call UsageProxy. This makes the most sense and frees people from having to integrate two things. Open to ideas on how to make integrating usage tracking as easy as possible. Right now it's:

  • Create subscription type that EITHER extends MeteredBillingBase (if not license-based) or extends whatever class is being used for subscriptions and adds usage charges itself in ::collectCharges() (see MeteredBillingBase for how
  • In custom/project code, use UsageProxyInterface::addUsage() to add metered usage

That is enough for most people. Some implementers may also use getUsageForPeriod() to show the user their current usage.

wizonesolutions’s picture

Status: Needs review » Needs work

Still working on some tweaks here. I added some more tests of free quantity wiping out all usage and did some cleanup. Will send patch later. There's a bug in Commerce that breaks this in conjunction with taxes, so I have to look at that more and discuss with @bojanz.

  • 5723ea3 committed on 8.x-1.x
    Issue #2919598: Get rid of Usage sub-namespace.
    
  • 1cd8c78 committed on 8.x-1.x
    Issue #2919598: Move the usage helper into a service.
    
    A trait turned...
  • 1dfa6fc committed on 8.x-1.x
    Issue #2919598: Fix order refresh a little.
    
  • 14ed5bb committed on 8.x-1.x
    Issue #2919598: Pin to ^1.0 again.
    
    beta4 is out.
    
  • 81e9e0d committed on 8.x-1.x
    Issue #2919598: Fix proration since Recurring did.
    

  • 4148fc2 committed on 8.x-1.x
    Issue #2919598: Remove outdated test dependency.
    
    It's ours now!
    
wizonesolutions’s picture

Tests are passing. Released beta1. Working on project page and documentation, then will close this issue.

wizonesolutions’s picture

Status: Needs work » Fixed

OK. I think this is good enough to close this issue: https://www.drupal.org/docs/8/modules/commerce-recurring-metered-billing...

Status: Fixed » Closed (fixed)

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