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

Add DAO files for compatibility with SearchKit #172

Open
colemanw opened this issue Sep 14, 2023 · 14 comments
Open

Add DAO files for compatibility with SearchKit #172

colemanw opened this issue Sep 14, 2023 · 14 comments

Comments

@colemanw
Copy link
Contributor

It looks like missing DAO files are the reason this extension doesn't work with SearchKit.
Adding them might not be too hard.

@AHowiller
Copy link

This would be particularly important for the upcoming replacement of the contribution tab with SearchKit.

@jensschuppe
Copy link
Collaborator

Hm ... DAO files for ...? Receipts? DonrecSnapshot records? I'm not sure I understand what exactly "doesn't work" and how this would not imply extensive refactoring of current data structures. Could you line out what you have in mind?

@colemanw
Copy link
Contributor Author

colemanw commented Sep 15, 2023

@jensschuppe this extension installer creates 2 tables: donrec_snapshot and donrec_profile through a direct sql query. The normal process for this would be to create schema xml files for them, and then the sql and DAO files would be generated by civix generate:entity-boilerplate. Then they'd be picked up by SearchKit.
Any reason not to do that?

@Detsieber
Copy link

imho, this issue is mainly not about donrec_snapshot and donrec_profile! These both are very special entities that are most probably not needed outside of the donrec extension.

However, for the information whether a contribution is already included in a donation receipt (let's call this "donation receipt status"), the situation is completely different:

As you know, this "donation receipt status" is included in the contribution list of a contact by some kind of "hijacking" by donrec to display an extra column. This has worked well for a long time.

But currently, the core team is making strong efforts to replace the old quickform stuff with SearchKit displays. So very soon, the current way of displaying the donrec receipt status of a contribution won't work anymore, which makes this issue PRETTY URGENT:

-> The underlying issue is that the "donrec status" of a contribution currently is not easily available with SearchKit!

As far as I understand @colemanw 's issue, this is because of missing DAOs. But maybe I am missing something...
@bjendres @jensschuppe , do you understand what is needed here?

@jensschuppe
Copy link
Collaborator

Any reason not to do that?

Those two tables are not used for storing whether a contribution has been receipted etc., this information lives in custom fields in multi-value custom groups on contacts and contributions and is being injected into Core views of contributions via hook_civicrm_searchColumns(). The donrec_profiles table stores configuration profiles (that we'd rather like to refactor into an implementation of Configuration Profiles, see #173) and the donrec_snapshot table stores active receipting processes (like a lock mechanism).

I'd say that SearchKit could be used to display receipt information using the Contact and Contribution entities with custom fields, see https://github.com/systopia/de.systopia.donrec/blob/c797d69ee3675a815d5d708072a8fabc859348c0/CRM/Donrec/DataStructure.php#L20-L447C5 for their definition.

Necessary steps:

  • make the receipt_id custom field an EntityRef field (although not necessary to replicate current displays)
  • provide new SearchKit display for receipts per contact (the Donation Receipts tab on the contact summary screen)
  • Adjust SearchKit display for contributions per contact, once that's in admin_ui I guess (the Receipted column in the Contributions tab on the contact summary screen)

Also, multi-value custom groups need to be imlicitly JOINable in SearchKit in order to display records of those groups along with the contact/contribution. I'm not sure this can be done already.

I suppose the current old-style views of contributions will not disappear soon and their SearchKit replacements will first be implemented in the admin_ui extension, right, @colemanw?

@colemanw
Copy link
Contributor Author

I suppose the current old-style views of contributions will not disappear soon and their SearchKit replacements will first be implemented in the admin_ui extension, right, @colemanw?

Not right away, but it also sounds like it won't be too hard to get this extension working with SearchKit

make the receipt_id custom field an EntityRef field

Should be simple enough - just one word of caution. EntityRef fields get a real foreign-key-constraint in the database. So they must contain only valid foreign key references. Since the field currently has no constraint, it could be corrupted as easily as the referenced entity being deleted.

More work in the short-term but better in the long-term would be to convert your DataStructure to use .mgd.php declarations.

@jensschuppe
Copy link
Collaborator

More work in the short-term but better in the long-term would be to convert your DataStructure to use .mgd.php declarations.

Yep, good point, already created #175 for that. That's really old code which nobody bothered to refactor ...

Ad making receipt_id field an EntityRef - this might actually not really be necessary, but I'll just note it for later: #176

@colemanw, what would be required for those multi-value custom groups to be available as With entities in SearchKit?

@bjendres
Copy link
Member

Yep, good point, already created #175 for that. That's really old code which nobody bothered to refactor ...

"bothered" = "dared" :-)

@colemanw
Copy link
Contributor Author

what would be required for those multi-value custom groups to be available as With entities in SearchKit?

Maybe I've missed the bigger picture here. If this extension keeps all meaningful data in custom fields, then it should already work fine with SearchKit without any refactoring.
@jensschuppe? Multi-record custom fields are already available to join With Contacts in SearchKit... What have I missed?

@jensschuppe
Copy link
Collaborator

Multi-record custom fields are already available to join With Contacts in SearchKit... What have I missed?

I tried to set up a Contribution search, which does not seem to allow joining with multi-value custom groups obviously. For contacts, this works. I now recognize we've created a multi-value custom group for contributions which is not supposed to exist, at least the UI does not allow this.

@colemanw
Copy link
Contributor Author

I now recognize we've created a multi-value custom group for contributions which is not supposed to exist, at least the UI does not allow this.

@jensschuppe I thought I'd do a quick PR as an answer. Well that was a rabbit hole... 36 hours later I fought and won against the bad Quickform code. Here it is: civicrm/civicrm-core#27549
Now CiviCRM will cope with multi-record custom fields for any entity, and there is a metadata flag you can set which will allow it in the UI :)

@jensschuppe
Copy link
Collaborator

👍 Nice job, @colemanw!

Now CiviCRM will cope with multi-record custom fields for any entity

Re: https://github.com/civicrm/civicrm-core/pull/27549/files#diff-d20bbdac5d9d1d3ec28be6f1972f8cb747ead74dea90c9076daa2de2a627873cR2466-R2472

What would be needed to officially allow Contribution entities to have multi-value custom groups so that they're eventually joinable in SearchKit? As is_multiple is not a real property of OptionValue, will this be override-able with Managed Entities?

@colemanw
Copy link
Contributor Author

@jensschuppe I believe that PR does make them joinable in SearchKit. That was the intention anyway, as it lifts the hard-coded assumption that the multi-record group always joins to Contact.

@RoWo-DS
Copy link

RoWo-DS commented Nov 10, 2023

@jensschuppe I believe that PR does make them joinable in SearchKit. That was the intention anyway, as it lifts the hard-coded assumption that the multi-record group always joins to Contact.

With CiviCRM 5.67.0 (includes the PR as far as I understand) and donrec 2.2.4 I am still not able to join Contribution with a multi-value custom group for Contributions. Our usecase is what Detsieber explained above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants