Figure out contexts and donors

This is much harder than I initially realised, so thank you for pushing me to consider it more carefully. I have 3 new suggestions. Apologies this might seem like I'm banging on again with ideas you already indicated that you don't favour, however I still see very strong reasons for them so I feel we need to revisit please. Maybe I've missed some important points, or maybe the new evidence I present will change the balance for you too.

1. I'm struggling to understand the idea of related contexts. I can see that we need to be flexible with context. Staff might detect duplicate donors and combine, or the website might even do it automatically (name/address similar). We might also need to split a donor apart if two individuals got accidentally merged. Or correct an error when a declaration was assigned to the wrong donor in case of two that are similar. However I see all of these as direct edits of the entities, rather than storing a separate entity to record that two contexts are related. I would like to have a goal of 1-to-1 correspondence Donor <=> individual. Could we then remove the idea of related contexts? It would mean that the list of declarations could be found by direct SQL query and we can use standard mechanisms like EVA to show the declarations for a Donor.
2. Store the address on the Declaration, correct at time of declaration. This is important if we need to move declarations between donors, or to unpick any errors in combining donors. The Donor address is used whenever we want the most recent known address. HMRC explicitly state that we don't need to update declarations if the address changes.
3. Create an Entity type for Records such as cancellation or change of address. The alternative strategy of using revisions on the Donor is inherently inflexible because they are then locked into the specific Donor. NB I feel this is likely quicker to develop: the extra entity type is balanced by avoiding the need for Donor revisions and we get forms for free from the entity rather than having to design custom forms. In particular Cancellation was a bit strange as a Donor revision because it didn't change any Donor fields; it carried different information which probably we could only store by converting to a string and putting it in the log message.

I'm struggling to understand the idea of "related contexts". I can see that we need to be flexible with context. Staff might detect duplicate donors and combine, or the website might even do it automatically (name/address similar). We might also need to split a donor apart if two individuals got accidentally merged. Or correct an error when a declaration was assigned to the wrong donor in case of two that are similar. However I see all of these as direct edits of the entities, rather than storing a separate entity to record that two contexts are related. I would like to have a goal of 1-to-1 correspondence Donor <=> individual. Could we then remove the idea of related contexts? It would mean that the list of declarations could be found by direct SQL query and we can use standard mechanisms like EVA to show the declarations for a Donor.

Imagine an organisation that has
1. CRM contact entities who may have offline ddeclararions recorded for them
2. Users able to access the website and make or cancel declarations for themselves
3. A commerce checkout where donors can mae a declaration at checkout

This creates a family of nasty edge cases:
A. Not all CRM contacts will have corresponding user entities.
B. A CRM contact may begin without a user but with an offline declaration, but later become linked up with a user and get a declaration or cancellation from that's users actions
C. A donation may be made with a declararion at checkout without being logged in. The user account could be created at the end of the checkout, or it could even never be set as the order customer but still identifiable as the same donor on the basis of email address.

The idea of 'contexts' was an atempt to handle these edge cases. The declaration stores the original context, there's no attempt to dynamically update what's stored on the declaration when further user/contact entitie are added/merged becaus that gets too complex and fragile. But nonetheless the system is able to pull these declarations together for a single donor when needed.

The only one of these edge cases that I actually care about in my project is the anonymous donor whose order can later be identified by having the order email match one of the users emails.

Store the address on the Declaration, correct at time of declaration. This is important if we need to move declarations between donors, or to unpick any errors in combining donors. The Donor address is used whenever we want the most recent known address. HMRC explicitly state that we don't need to update declarations if the address changes.

I've no problem with that as an implementation. I don't have clarity whether it's needed, but I've no problem with it and it feels plausible to me that it's a good solution.

Create an Entity type for Records such as cancellation or change of address. The alternative strategy of using revisions on the Donor is inherently inflexible because they are then locked into the specific Donor. NB I feel the Record entity approach is likely quicker to develop: the extra entity type is balanced by avoiding the need for Donor revisions and we get forms for free from the entity rather than having to design custom forms. In particular Cancellation was a bit strange as a Donor revision because it didn't change any Donor fields; it carried different information which probably we could only store by converting to a string and putting it in the log message.

The idea of a records entity type doesn't worry me as long as we also have a declarations entity type.

I think you probably still want donor revisions anyway as they're the most natural way of handling donor change of address.

I agree cancellation is strange as a donor revision. But there would be an evidence entity that would need linking to the donor as part of the cancelling which might hold a lot of the information.

I agree that a records entity type could well end up a nice clean solution. If you're going this way, I'd suggest not having a bundle entity type for the records. Instead maybe use bundle plugins from the contrib entity module. This means no config needed, and you get a class for each bundle where you can put specific logic, like putting cancellation logic in a postSave() on the cancel bundle.

The only one of these edge cases that I actually care about in my project is the anonymous donor whose order can later be identified by having the order email match one of the users emails.

Given that I could workround this by updating the order entity retrospectively to add the customer, I'm sympathetic if you want to say "this contexts business is a nightmare, how about if I create the module without it and then we see if you'v got any budget left to worry about it?"

If you're going this way, I'd suggest not having a bundle entity type for the records. Instead maybe use bundle plugins from the contrib entity module.

That sounds like a good idea I didn't even know it existed. Please can you explain more?

I found this https://www.drupal.org/node/3191609 → . Or perhaps a capability of https://www.drupal.org/project/entity → ?

See https://git.drupalcode.org/project/entity/-/tree/8.x-1.x/src/BundlePlugin

And

https://git.drupalcode.org/project/entity/-/tree/8.x-1.x/tests/modules/e...

Commerce_payment uses this for bundles of the payment gateway or payment method entity type I think.

But also when I said about putting logic in postSave() on the bundle, I was mixing it up with the bundle classes of https://www.drupal.org/node/3191609 → .

Unfortunately a class can't be both a bundle plugin and a bundle class as they need a different parent class.

But we could have an onCancel() method on a GiftAidRecordBundlePluginInterface and call that from the GiftAidRecord::postSave() entity method.

The idea of 'contexts' was an attempt to handle these edge cases. The declaration stores the original context, there's no attempt to dynamically update what's stored on the declaration when further user/contact entities are added/merged because that gets too complex and fragile. But nonetheless the system is able to pull these declarations together for a single donor when needed.

Thanks for that. I have my doubts though, because it seems to bypass many mechanisms that we are putting (or could put) in this module to meet the HMRC guidelines:

1. Explicit record of all changes with evidence
2. Preserve a copy of the evidence within this module in case the original data gets deleted
3. Provide access control and warnings for changing data that may have already been used for a declaration
4. Maintain a list of any such changes to allow retroactively updating claims with HMRC.

With the dynamic contexts, I feel that the charity could face major problems during audit because the required evidence is no longer present. So actually we do have to do the dynamic updates, which is exactly the solution you reached anyway in #4. Perhaps it could be "complex and fragile", but I am not convinced that it is any more so than the related contexts option.

I think you probably still want donor revisions anyway as they're the most natural way of handling donor change of address. blockquote>Donor revisions would definitely work. I agree they are the most obvious solution and I would be happy to do that. However with the approach I have outlined, the address history is provided by the declaration and change of address entities - so the Donor revisions are unnecessary, which saves time and complexity. The historical addresses are accessible without having to dive into revisions, which could be an advantage. If the Donor made one Commerce order with address A then another with B, then we can't necessarily be sure which is the "best" address, so it could be useful to have both available. Anyway I can always add the revisions in later if we want them.

Happy to leave dynamic contexts and donor revisions aside for now if they're not the approach you're most comfortable with.

The case I'm most concerned about if we don't have donor revisions and use change of address records instead is if we (by default, or a site chooses) to update the donor address to match the default commerce address in the address book. Then it's not as simple as custom code simply updating the address on the donor entity, there's also the change of address record to create.

We could of course have the door entity autocreate a change of address record as a kind of pseudo-revision. But maybe the right response is to say: it's right to give custom code the problem of creating a change of address record, because they need to consider audit and provide an explanation of why they made the change they made. But I'm still left wondering: isn't revision log the API Drupal already gives us for this ...?

because they need to consider audit and provide an explanation of why they made the change they made

Yes I agree - whether it's a revision or a record, then custom code needs to be audit-compliant and provide an explanation and evidence.

I agree cancellation is strange as a donor revision.

And I agree that change change of address is strange not as a revision.

But I'm still left wondering: isn't revision log the API Drupal already gives us for this ...?

Yes you could be right, I will consider it again.

We have the change of address, cancellation and misc record all sharing various concepts including storage of evidence, access checking, various routes/forms etc. So we'd like to share code and UI. Whether we choose revisions or records, either way somewhere there is a part that's a bit strange.

There is information to store for these changes, especially for the cancellation. If we have record entities, then we can use fields. With revisions it's less obvious. We could have fields on the evidence, only used for cancellation. Or we could make more fields on the donor that are like revision log - they get reset for each new revision. Anyway, the evidence is specific to the revision, so it could be clearer to treat it the same way - it makes the correlation of evidence to revision more obvious.

At the moment I'm still not really all that attracted to revisions as there seem to be more compromises. It's potentially stretching revision API beyond its intended scope. Declarations and the other records have similarities: both require evidence, dates, have similar access restrictions, etc. With records, the 2 entities can potentially have a common base type whereas with revisions, one is handled as an entity and the other as a revision.

Anyway when I reach the decision point I will do an evaluation.

For the cancellation, I would expect the following fields to meet the same standard of flexibility as the Declaration (i.e. some are for future-proofing and we don't create a UI now)

1. donor
2. charity
3. evidence
4. declared_date
5. date_based (later on we could cancel based on something other than dates)
6. start_date
7. end_date (someone could ask not to claim Gift Aid on their £25000 donation from savings as they didn't pay enough tax, but keep claiming on the £50 direct debit; one way we could handle this is a cancellation to exclude a date range)
8. reference to declaration (cancel that one specifically)

I don't see that it can really be a revision. It shares about 70% of the fields of a declaration, so we can make a new entity type, and give them a common base class.

On the other hand, change of address, and misc correspondence are simple things: they carry evidence and perhaps a date. We could fairly easily use a Donor revision: we'd need to enable revisions and add the evidence field. In return we would save a little compared with my previous idea because instead of the record entity with multiple bundles we can have a non-bundled cancellation entity.

In this way we avoid doing either of the strange things (one of which we have each been uncomfortable about for a valid reason). It adds a little work developing entities, but likely saves as much by avoiding forcing strangeness such as services with API functions that create entities under the covers.

What do you think?

end_date (someone could ask not to claim Gift Aid on their £25000 donation from savings as they didn't pay enough tax, but keep claiming on the £50 direct debit; one way we could handle this is a cancellation to exclude a date range)

Very interesting. An obscure use case I know I will have, but have avoided mentioning to keep things simple as it could be handled manually, is ceilings on what can be claimed. e.g. in the year 19-20 don't claim gift aid worth more than £xx as that was all the tax I paid.

Your cancellation entities seem well suited to being extended for this kind of use.

I'm happy with the idea of donor revisions + cancellation entities. Seems quite a strong solution to me.

As you say, cancellation has quite fiddly requirements unique to it.