Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CIWEMB-487: Prevent deletion membership lines linked to one-off payments #504

Merged
merged 1 commit into from
Oct 26, 2023

Conversation

omarabuhussein
Copy link
Member

@omarabuhussein omarabuhussein commented Oct 20, 2023

Before

If you delete a membership line using the "manage instalments" form and that membership is linked to a one-off payment, then:

  • The membership line will be removed from the payment plan
  • The Membership end date will be altered so it = "Today's date" or to the date set by the user.
  • But the but the one-off payment (contribution) will remain.

This behavior is not great given the user might assume that the one-off payment (contribution) will be automatically removed, But contributions are "invoices" and they have a sequential number, so we cannot just remove them either.

So to make things a bit better for the user, we have two possible options:

1- Issue a credit note for the one-off payment.
2- Prevent the user from removing the membership line if it is linked to a one-off payment, and if that one-off payment has no any other line item except the membership line that we are trying to remove.

There is a 3rd option which is to keep the one-off payment (contribution) and just delete the line item from it, but this will result in a contribution with total amount = 0, so it is not a good option.

We decided to go with option 2 for now given it is more steightfoward to implement.

After

Trying to delete a membership line that is linked to a one-off payment does not work anymore and the following error shows up "This line item cannot be deleted as it is the last line item linked to a one-off contribution." :

aft1.mp4

But when the one-off payment has more that one line item (for example in case the user added another line item to it, which is unlikely to happen tbh) we just allow deleting the membership line, and it will be removed from the one-off payment as well:

aft2.mp4

Technical Details

We don't have a one-2-one link between the payment plan line item and the line item on the one-off payment (contribution), so here when the user tries to delete a membership line, I do the following checks to determine if it is linked to a one off payment or not:

1- Check across all the "Pending" contributions that are linked to the payment plan.
2- With only "single" line item.
3- And the "Total amount" of the line item equals the payment plan line amount.
4- And the "Source" of the one-off payment = Manage Instalments form - One off payment", which is "source" value we set by default to any one off payment.

The combination of all of these criteria together shows with great accuracy if the membership line that we trying to delete is linked to a one-off payment or not, and it is all done in a single SQL query inside isLineItemLinkedToOneOffPayment() method.

As part of my work above, I also found a problem in the way I create the one-off payments in general for membership lines, in which the created one-off payment has line item with partially incorrect information, here is an example of how the line item will look like before my fix:

beforebug

where the line item is linked to the "civicrm_contribution" table instead of the "civicrm_membership" table (which should not be the case for membership line items), and where the "price_field_id" and the "price_field_value_id" does not point to the correct price field and price field value that are associated with the membership type.

Here is how it looks now after my fix:

afterbug

@omarabuhussein omarabuhussein merged commit b4156fb into CIWEMB-84-workstream Oct 26, 2023
2 checks passed
@omarabuhussein omarabuhussein deleted the CIWEMB-487-proper-del branch October 26, 2023 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants