UPDATE - discussion on this is converging on adding a new json field 'metdata' to the line item table, permitting data related to that line item to be stored at point of order, and used when confirming. (UPDATE - this is not really the convergence now - proposal updated below)
We have a few issues open ( #28 (moved) , core#1402 (closed) , financial#53 ) that are to my mind blocked by us not having a data model that holds the user's intent when renewing. The focus of this gl is the data model and exposing it via the back office renewal form & doing what is needed in the apis.
The problem
When renewing an expired membership the back office user has a fairly blunt way of influencing the calculated dates - they can set the 'renewal date'. However, if the payment is not made in the same form submission that intent is not captured anywhere and cannot be respected when completing the transaction. Likewise the receipt text and number of renewal terms are not captured.
The proposal data model
add membership_num_terms to civicrm_line_item table.
add 'payment_completion_metadata' to the civicrm_line_item civicrm_contribution table for data that is only used in the order->payment flow. Limit what can be stored here to tested values used in that flow - likely to be the message details and effective date the renewal form uses but doesn't sotre.
1. New field added to civicrm_line_item called 'metadata' which stores information required to complete the membership renewal - e.g. the field might store ``json_encode(['start_date' => '2010-01-01', 'end_date' => '2020-01-01', 'membership_status_id' => 'Current', 'membership_num_terms' => 2']
- note this is the new proposal - but I'm wondering what happens if it's renewed in the meantime & then paid
- New status 'Pending renewal' used when there is a need to record the dates (is_current_member = 0, is_reserved = 1, is_admin = 1 too I think)- When a contribution linked to a membership in 'Pending renewal' is completed the status is updated but the dates are not changed- If a membership has 'Pending renewal' status and status_override_end_date is set then any processing of that membership after that date would result in it being reverted to previous dates and status - this information should be available in the membership_log table.
Proposed form exposure
No proposed form changes - this is just storing data already submitted in the renewal form.
~~- the logic could be used outside of existing core forms but within core I think this is something we are exposing to the back-office user doing a renewal - they already have considerable ability to edit dates. So I think this is a form layer thing not some site-wide setting.
when renewing an expired membership the renewal form would present the user with what will happen to the start date, end date etc if payment is received on that day and offer a checkbox to specify dates - if that is checked then date fields would be exposed an on save the Pending Renewal status would be used with the selected dates.
potentially the user can also enter the date the 'offer' is available until - ie status_override_end_date~~
Edge cases
Multiple terms
We would add an extra field membership_num_terms to the line items column
#28 (moved) throws up the edge case that more than one renewal term might be selected. Both @jptillman and I assumed that the right way to represent that would be setting qty = n and then picking that when confirming. @andrewhunt didn't think that was the right assumption https://github.com/civicrm/civicrm-core/pull/18618. If we come down on NOT setting qty then we ALSO need a way to record intent for non-expired renewals - this could be an extra membership_num_terms column on the line_item table. (I'm still inclined to my original assumption but more points of view needed).
About to expire rather than actually expired
Out of scope for now - just do what the form currently does
I think we would still manage this in the form ie - "the membership is due to to expire in 4 days, if payment is received after that then .... ' and present the same options as for an actually expired membership
Edited
To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information
I don't think the membership status is the place to hold this. If a membership is pending renewal, there is a possibility that the member will fail to send payment. In this case, we would see a lot of pending renewal memberships stacking up.
It's also ambiguous what the underlying status is. Is this an eager beaver who would otherwise have a current membership for months and months? Is this someone in a grace period who should retain some member privileges? Is this a membership that is long-expired? This is complicated by the first concern: someone might go to renew several months early, fail to send a check, and then sit in pending renewal for the next year, having passed through what would otherwise be several membership statuses.
With site functionality and member benefits tied to membership statuses, it would be difficult to grant or revoke benefits/privileges based upon this status.
Finally, it's unclear how this would interact with the override feature. If override is set, should that override pending renewal? If not, how will we know what status to restore the membership to?
I see your points about the status of the membership being clouded by this new approach. I have run into this problem previously with "status"-driven systems where all manner of statuses get introduced to accommodate things that are better tracked elsewhere.
I would suggest we take a step back and focus on what people do when they were doing this manually, without the support of Civi:
In my client's world, a pending payment implies an invoice that someone intends to pay by check. They are an odd duck, however, in that they allow folks who expire to "resurrect" them by catching up on payments. So when someone pays, my client will look at the payment and see if it covers the necessary catch-up, and if not, they'll ask for more.
For a more standard situation, I would imagine there's a time period during which that check would be welcomed, but it's not indefinite, probably a month or 90 days. This correlates to an "expired invoice". So, perhaps this indicates that pending payments should have an expiration date after which they transition to some other status.
So, what is the difference between an invoice that is paid immediately in the same moment as a request for membership renewal and one that is paid months later? In the business world, it's simply time. A paper invoice is just real-world state data, and the actions that will be carried out when that invoice is paid are understood by both parties and are the same regardless of the time taken to pay it (unless the invoice expires). This makes me think it should be the same case in the Civi code.
So, coming back to the code, then.
My main problem with adding a column to the line items table is the problem I've had with how Civi managed memberships all along: We seem to be polluting the financial subsystem with the specifics of memberships, or vice versa.
Because of that, I lean towards some sort of "payment completion" action state data getting stored which is linked to the membership component and which would be populated at the time the payment is created (but prior to its completion, EVEN in the situation where checks are entered directly, or PayPal payments are made) and then, when the transaction completes, whatever component has registered itself as responding to the payment completion action would get notified with its state data. Then it could do whatever it deems fit with the information, such as renewing a membership, making an event participant "registered", or whatever else the component's developer has dreamed up.
One nice side effect about this approach is that a single payment could be tied to multiple completion actions (one to many data records), such as both membership and event registration, without the financial code having to know a single thing about any of it.
From what I've seen in the Civi codebase in my adventure with this topic, it suffers from a strange split personality where a payment entered directly by the admin during renewal is handled in an entirely different way from pending payments that are completed later. It's no wonder we're having this problem of scenario-specific behavior when this is the case. And this, for me, points up the need for what I've described here, because there's not much difference between a payment completed when the membership is entered and one that is paid months later, apart from time -- and time simply implies the need for maintaining state.
And finally, on the topic of administrative override, I've considered that an "all bets are off" kind of thing. If you override, you take the process into your own hands and it's on you to determine what is the appropriate action, if any, regardless of payment or timing.
Instead, it might be more helpful to add a boolean field onto civicrm_membership_payment that would indicate whether the membership change is pending. When the referenced contribution is completed such as to complete the renewal, the pending flag would be switched back to 0.
This is similar to my thoughts back on PR 18618 where the number of terms associated with the contribution-membership combo could be stored there on civicrm_membership_payment.
@andrewhunt so my thinking was that we would use the status only when they would otherwise be expired and it would be something selected from the renewal form, rather than the new normal flow, when the user wants to pre-set what the dates will be when confirmed. The underlying status & dates would be determined by the last entry in the membership log table & this is what it would revert back to. I suspect Pending Renewal might itself get the is_override flag set.
The problem with a field on the membership_payment table (or the line_item table which I think is better but serves the same thing) is we have no way to record the intended preferred dates when the payment comes in. This is something you can alter via the form for real time (via some fairly sledgehammery form use)
Ah I wasn't thinking about this: you go to renew, the form could potentially expose end_date and let you pick whatever date you want. Is that what you are thinking about?
If so, that seems like more of an edge case than someone (a few days before expiration) filling the form to renew and then taking a few weeks to get the check in. I've never heard of an organization letting people pick their own membership dates.
The same principle would apply to other fields, though, so it is a consideration. One client has a field on the membership of whether the journal subscription should be delivered by mail or just online. (They have a couple of journals, so this preference is specific to the membership rather than the contact.) However, my feeling is that this stuff--anything that won't affect the status--is fine to overwrite on the membership in real time.
In saying this, I did think about membership upgrades and don't have a ready answer. If I am a regular member and then go to renew as a super-awesome member with pay-later, how do I continue with the regular member status until I actually follow through with my super-awesome dues?
@andrewhunt I'm not thinking about front end forms - so no not letting people pick their own membership dates.
I'm thinking only about the back office renewal form and a data model to represent the administrator's intent when renewing expired or expiring memberships. This would be a no-change for other forms.
The administrator currently has 2 ways to adjust the outcome to fit their intent when they have a payment in front of them
they can enter a renewal date - the text encourages them to 'make one up' to adjust the outcome - ie if they don't want start date to change
they can enter the renewal but edit the dates straight after.
But if the payment is disconnected from the act of renewing then the intent is lost.
To come back to @jptillman - their intent is the start date won't change, but the end date will be 2 years after the original expired date. Depending on how they enter the payment the start date will or won't currently change. Both variants could be seen as a 'feature' - but fundamentally the back office user knows what is right for the scenario they are dealing with and having a way to record their intent is currently missing
That makes sense. Still, I think that using the membership status will create more problems than it solves.
Memberships are unusual in CiviCRM because they're the only situation where there's a pending action and both the pre-action and post-action states matter at their respective points in time. Regular contributions and new event registrations can be ignored if they're pending--they don't represent paid-up things. Modified event registrations are different because the state matters for only one point in time: aside from making sure the pricing adds up, we don't care that you used to want the walking tour package; we care that now you want the VIP ticket and awards dinner because that's what you're going to get.
I really am not comfortable with modifying the membership record until the renewal is paid. Doing so causes data loss. Even limiting it to "expired or expiring memberships" raises the question of how to operationalize that.
Is it any membership where the end date is in the past? That won't cover the "expiring" case.
Is it any membership in a status with that problematic "current" checkbox blank? We advise clients to mark grace periods (1-12 months, depending) as "current" so renewals during that time continue the membership uninterrupted.
Is it Grace or Expired memberships? People have all kinds of membership statuses, both before and after the end date, with different names.
This just leaves me with the feeling that the only way to resolve this is to have a place to stash data about a pending renewal so it can be applied when that renewal contribution is completed.
I had felt that stashing the number of terms might have been sufficient, since it seems like an edge case even for an admin to want to modify the end date of an unpaid renewal. It strikes me as a UI problem that the form offers that field. However, I have no objection to stashing any/all data about a membership.
It would also allow us to give clear guidance to a user: if you want to modify a membership in real time, edit the membership record. If you want to tee up changes to take effect upon renewal, use the renewal form.
If I'd read this one before I replied above, I would have seen that Andrew's statement that...
This just leaves me with the feeling that the only way to resolve this is to have a place to stash data about a pending renewal so it can be applied when that renewal contribution is completed.
...fits exactly with my thoughts on "payment completion state data". I think we are converging on an "event oriented" design with some stashed state data to help the event handlers do their thing. In my experience, this has always yielded good results in workflow situations.
I'll reiterate that it should be the case for ALL transactions, pending or immediate. That will keep the behaviors consistent regardless of scenario.
I'd also like to suggest that we should consider the scope of this "payment completion action" dilemma to go beyond that of the features we are currently familiar with. Right now, for example, I'm shoe-horning a "certification tracking" function for one of my clients into the membership tracking feature. It's starting to show some seams, however, and I'm considering creating a custom extension for it. And its payment behaviors will very closely mirror those of memberships.
Let's not freeze out the extension developers who have ideas we haven't even dreamt of yet!
@andrewhunt so one thing I think has been unclear is that I ONLY envisaged the Pending Renewal being used in cases where the admin wants that extra control over the final outcome. If they just want the normal flow to happen depending on the payment date when it comes in they wouldn't check the box to put it into Pending Renewal status and it would remain expired until they do so.
If they do it's almost the equivalent of an offer - 'as long as the payment is received within x timeframe then these dates apply'.
I should also draw your attention to core#1402 (closed) - the issue is the final dates (& treatment of start date) for completing a contribution depend which form they use. You could make a case that either is right - but we shouldn't do 2 different things
I don't really understand the issue fully here but I do know that memberships are used in multiple different ways by different organisations and one set of rules generally doesn't work universally.
I don't really see a need for adding an additional membership status and would have thought that the use-case being described should be handled either by custom data or some other form of data that can be linked to the membership.
I would really like to see all membership logic pulled out into a (core) extension that could be overridden/replaced to suit an individual organisation. It would also make it much easier to see exactly how it works.
I think the approach that I and @andrewhunt each independently suggested (above) would achieve the data segregation you want. It would also solve the blocker that @eileen mentions, because the "renewal date" "number of terms", et al, could be kept with the context data stored for the final action after payment completion. It seems to cleanly cut through the knot of logic we are struggling with.
The more I mull this over, the more strongly I feel that the membership payment completion code should be the same regardless of whether payment is completed immediately or later. It just makes no sense to have the code split here, when one approach can handle both.
I agree that migrating the membership logic to a nice independent extension would be great. Having that would certainly have helped me in getting up to speed. I'm still not there yet.
@jptillman @andrewhunt so if we are looking for an existing place to store the intent it might be possible in the membership_log table. Failing that I think we would need to add a new table. I don't think we can extend the membership_payment table because the data model doesn't preclude having more than one purchase of the same membership type on different line items (buy 2 terms get one free). How that could work in practice is a bit beyond me but the data model does permit for payment matching by line item - so the entity we would want to extend is the line item - so it would be more like a table
line_item_metadata which would have a line item id & a json blob of additional data that would be used when fulfilling payment on the line item
( I note the membership log approach wouldn't have the line item vs contribution nuance but I think if we can fit it into the existing schema we can punt on it - it's only if adding to the schema I feel we need to consider that)
I'm likely the greenest one in this discussion when it comes to the codebase and its legacy issues, so I'm certainly willing to defer to the more experienced folks on this. However, I'm really intrigued by the concept of making this independent of membership. If there were a generic payment completion function which utilized the "line_item_metadata" to determine what completion action to call and passed that context data along to it, it would make the process so much more powerful and less prone to design troubles later on.
I may be just causing annoyance if there are constraints I don't know about, such as a taboo on adding new tables. Please let me know if this is so. I want to be helpful and not distracting.
@jptillman so we do have a generic payment completion function - the issue it is doesn't have all the data it needs in this case- which is our limitation
Adding to / altering the schema is not something we do lightly - but at the same time there may be a gap only a schema change will help with - @JoeMurray might also have thoughts
I don't think we can extend the membership_payment table because the data model doesn't preclude having more than one purchase of the same membership type on different line items (buy 2 terms get one free).
You're right that the data model could theoretically support this, but the form logic prohibits it, and I'm not confident that the membership processing code would handle this in a way that anyone might reasonably expect.
Backend:
Frontend:
Unless and until the business logic can handle something like this, I don't think we need to dance around it in the data schema. Moreover, one natural way to resolve a contribution with two line items with the same membership type might be to resolve it at the form level and record the consolidated membership effect into civicrm_membership_payment.
But at the same time I'm not opposed to a new line_item_metadata field, and I think it could do a good job for other purposes via extension like @jptillman has suggested. In fact, this would help permanently store data that might change on the price field/option. Right now, there's no way to be sure that the membership type or number of terms on the price option are the values that were there when a contribution was recorded.
@andrewhunt so to be clear - we are talking about a new line_item_metadata field in a new table - that could store line item metadata for any line item.
My only reason for thinking a new table rather than adding to the line_item table is that the line_item table is large already & this field might not be needed for all rows - but of course if it's not indexed then that might not really matter.
The membership_payment table duplicates the line item table for linking memberships to payments & we are moving away from referring to it in favour of the line item table in the code.
@mattwire so the issue here is fundamentally - how do we switch to an Order->payment flow for membership renewals when some of the form submitters intent is not stored beyond the submission of the form - ie the form permit setting 'renewal date' and 'number of terms' but neither are stored beyond the form submission so if payment follows later there is no way to know the submitter's intent.
To my mind this is the blocker on switching to that preferred flow
Wow, lots of interesting thoughts on a two-day old conversation. @jptillman, thanks for your great contributions as a newer member, they are most welcome. I particularly liked the objective of an event oriented design.
A few responses here and there:
I too am very uneasy about trying to introduce a new pending renewal membership status. It feels like it will lead to grief similar to our overloading of contribution status.
@andrewhunt On backoffice contact summary form, an info warning comes up when trying to create a second membership of the same type for the same contact, but the second membership is created rather than the first one being extended, at least by default. Wasn't how I did it long ago with dgg, but heh, it's been merged.
@eileen When trying to create a checkbox price field for memberships, trying to add two for the same membership type leads to 'You have selected multiple memberships for the same organization or entity. Please review your selections and choose only one membership per entity.' I would support better validation across the fields in a priceset to prevent different fields from including the same membership type; or at a minimum, validating at time of purchase (both via API and form) that only one membership per type is being purchased.
I tend to think that membership_payment record would be a more appropriate place to put fields relating only to memberships, rather than on line items which might be donations or event registrations. At least it is more normalized. I don't personally like having required data for operations only appear in a log table. Looking at the line_item table, it doesn't seem like the number fields is that big of a concern yet as we're no where near the limit. But it does tend to get a huge number of records, and adding unnormalized fields would be a small performance hit on the very largest CiviCRM implementations like WMF. And there is a (bad) precedent for putting in https://github.com/civicrm/civicrm-core/blob/master/xml/schema/Price/LineItem.xml#L119 which only relates to event registration line items. Slightly different consideration: when calling the order api (which doesn't correspond to an order table) we would want the arguments specifying the intent to override the membership date to be passed as line item parameters. So I would prefer to add fields to override the membership start date and membership end date, that would only be applied to a membership when a payment completes payment for the amount owing for the membership line item, on: a) membership payment, b) less preferred, but still good, line item. I haven't seen so far at least a good enough justification to create a whole new table.
Hmm - there is one more type of metadata it would be nice to store & re-use when confirming - receipt text. ie the text the form-filler enters
I guess if there were multiple line items we could just store it on 'pick on, any one' and concatenate it. In practice it's only relevant to the renewal form at the moment - so the multiple line item wouldn't come up.
Also - would we empty out the field once the renewal is complete? (I don't think we need to answer now but if the concept grows to other types of line items it might avoid bloat)
Actually, no, the storage of a VARCHAR(255) is not rows x space for max length of string. For each row, one calculates, with L length of string and M the maximum size of the field and L is the length of a particular's row's string: Sum of( L + 1 bytes if column values require 0 − 255 bytes, L + 2 bytes if values may require more than 255 bytes)