IMO the most flexible and powerful tagging system would allow a site builder to manage multiple vocabularies for tagging, including to which entity types and bundles each can apply. This idea is only half-baked...
There could also be one default vocabulary (for each entity type?) for tags that don't require additional context/grouping that could support free-tagging. I'm not convinced it would be best to use Drupal's built-in concept of vocabulary (probably doesn't need to be fieldable, revisionable, or mixed in with the various non-CRM tag vocabs).
I'm also thinking of mutually exclusive labels e.g. GitLab's Scoped Labels.
Examples that a site builder might create:
"Lead source" vocab that applies to Contacts of type Organization
"VIP" term in the default vocab that applies to Contacts of type Person (note that this introduces the concept of specific terms applying to only certain entities/bundles
"Industry" vocab that applies to Contacts of type Organization or Person
"Eldest" term in the default vocab that applies to Relationships of type Household Member
"Past" term in the default vocab that applies to Relationships (assuming CRM doesn't provide an OOTB way to track this)
"No longer valid" term in the default vocab that applies to Contact Details (though I think CRM should provide a semantic field for this)
"Type" vocab that applies to Contacts of type Organization (e.g. "non-profit")
Probably worth some discussion whether a (non-dev) dependency like Tagify be adopted by CRM. Alternatively there could be a more opinionated, dependency-heavy "CRM Starter" kit that adds these sorts of things, leaving CRM itself very low dependency. Feels like an overall policy would be helpful so that each such thing doesn't require a bespoke debate.
Looking good!
Could you clarify your last comment? It works for Paragraphs, doesn't it?
Why direct? Wouldn't that restrict CRM to SQL backends?
I would assume that entity validation would be the only way to prevent ECA actions from being naughty.
I see the challenge with merging. IIRC there's a way to bypass entity constraints. If that's the case (do you know?), I think entity validation is the way to go.
Sorry, my previous question was unclear. I now get what we're doing here. This addresses my concern about creating Views showing a Contact's relationships so long as the table can be relied upon to be up-to-date. The term "cache" implies to me that it can't. Assuming that this table is always updated whenever relationships are added/removed/updated, I think it might make sense to find a name without "cache".
I don't think it's necessary to add the bundle name. A given Contact is unlikely to have a ton of Relationships so an extra join from this table to the Relationships table to filter by Relationship Type won't be too onerous. However, I'm not opposed to including it and, eventually, performance testing might suggest doing so.
Given the apparent need for this table, I wonder whether it would make sense to revisit the concept of one Relationship entity per direction of the relationship. Only because it would eliminate the need for this table.
Looks great!
I agree. Should we enforce it with validation too (e.g. to prevent ECA actions from doing things they shouldn't)?
Is an example of the reason to do this creating a view that lists a contact's siblings and having a single table to join to find all the siblings?
I was thinking at `/admin/structure/crm/contact-detail-types` (etc.) analogous to Content Types. Provides useful context for someone deciding whether to create a new one.
I found https://mkdocs-mermaid2.readthedocs.io/en/latest/superfences/#usage-for-... which proposed a different way to get Mermaid rendering. Might be worth a try.
Not sure how to verify whether it will render on GitLab Pages pre-merge (created 📌 Ability to preview GitLab Pages in CI Active ). I do see that your diagram isn't rendering on GitLab Pages. That feels like a GitLab Pages bug, but I can't find anything recent online about it.
https://git.drupalcode.org/issue/crm-3522273/-/blob/3522273-document-crm... shows a syntax error for mine, but I think that's expected because I'm using a GitLab Pages specific syntax to include the diagram and I wouldn't expect it to function at the above URL.
jdleonard → created an issue.
Missing analogous changes for ContactDetailType
Ugh, organization logins... I propose that we say that one needs to delete the existing mapping and create a new one.
It's possible a good use case for making User Contact fieldable comes along at a later date. If we were to make it fieldable in the future, it would simplify things to say that the Contact and User references cannot be changed (doing so could cause problems for future use cases' fields, which might assume this).
Merging...
I have encountered the following with my neighborhood association (ZNA) probably a dozen times:
- Someone new to ZNA pays dues on our payment processor's hosted form
- Zapier looks up the contact by email or, failing that, by exact name match in our accounting system, Xero (de facto CRM), and doesn't find them
- Zapier causes a contact to be created for this person in Xero
- The following year that person pays dues the same way but with a different email address (and slightly different name)
- Zapier causes a second contact to be created in Xero
- At some point I discover this and merge the contacts
In Drupal CRM / Member Platform, something similar could happen if the person isn't required to log in first (e.g. to optimize for collecting the dues), but in addition to the two Contacts (Contact A and Contact B), there could be two Users (User A and User B) and two User Contact mappings. At least one User Contact would need to be deleted. If the other one "needs" to be edited because the desired merge result is to keep User A and Contact B, I think we could adopt the same stance I propose for the organization login case: delete and re-create the User Contact entity.
In summary, I propose:
- Not fieldable
- Not editable
- Not revisionable
- Remove changed base field
Easy enough to re-add those things later.
Will need to follow up to document changes from:
📌
Rename Individual to Person
Active
📌
Make External Identifiers field's addition optional
Active
Now I understand the current design :)
It's more work, but I propose that we adopt Search API. This will be something commonly adopted for more intense use cases and it will benefit performance for the small ones too. Even the default database Search API backend (paired with a Search API Views-based autocomplete) would solve the autocomplete performance issue you describe CRM Core having faced, right?
Provide an optional "Contact name format" field on the Drupal User entity that overrides the Default contact name format for that User when they're logged in
I intended this as a User preference for how all Contacts' names should be rendered for that User ("I live in [country] and American name formats detract from a positive user experience")
Created an MR that uses the Read-only Field Widget → module. Not sure I'm crazy about my approach.
Reviewed all code changes and committed minor tweaks.
Performed some manual testing in the UI.
Discovered bug (not caused by your changes): 🐛 Error when looking up a Contact during User creation Active
I remain neutral on the conceptual change but I don't think this warrants further input from folks.
I think this is RTBC if you agree with my tweaks.
jdleonard → created an issue.
jdleonard → created an issue.
What I've done is ready for review. I'm sure plenty more could be written.
If what's there is good enough, I propose getting it merged promptly so we can include architecture docs updates in each issue that affects the architecture.
I'm creating a follow-up issue to document and tidy up access control and permissions.
I don't feel strongly about this at present. It can always be tweaked later. I'll proactively WF this though I think there's value to be had.
If there's nothing to be edited (i.e. no reason to be fieldable) then I'd propose removing the "changed" field.
An external system in which a CRM Contact is represented is not necessarily a CRM. I suggest adjusting naming to be more general e.g. "external system" rather than "external CRM".
Also consider spinning the external system field out into a module outside the CRM namespace as the logic doesn't seem particularly specific to CRM.
Not sure whether the External Entities → module might be relevant / already incorporate some of the needed functionality. It looks pretty intense.
Should a Contact Detail have a field representing the Contact that "owns" it? This would make the access control logic simpler as we could just delegate to ContactAccessControlHandler
. Should that field be read only (would that interfere with future sharing)?
I suppose we could still delegate to ContactAccessControlHandler
without such a field, but it would require a query to find the Contact referencing the Contact Detail. I err on the side of setting a read only back reference as described above.
I'm struggling to wrap my head around the various levels of indirection at play here, but let me try proposing an approach that provides maximum flexibility and seems easy enough to implement:
- Provide some sensible name formats, catering to known name format preferences around the world (and accounting for preferred name)
- Provide a CRM setting "Default contact name format" that lets the CRM admin select the default name format from the list of name formats for CRM functionality
- Provide an optional "Contact name format" field on the Drupal User entity that overrides the Default contact name format
- Scrap the name-related logic in
Contact::preSave()
- Implement
Contact::label()
to return the appropriately formatted value from thefull_name
field (using the User's Contact name format if set, otherwise the Default contact name format) in the case of an Individual and otherwise the value of thename
base field - Remove the
label
key from Contact'sentity_keys
- superseded by thelabel()
override
Note that the full_name
field is now locked, so you'll need to temporarily unlock it to make the "Preferred component source" change via the UI.
Great, thanks!
Or "View corresponding contact", but that loses the connection with the naming of the mapping entity.
I think this terminology is intertwined with 📌 Rename CRM User to be more clear Active .
If we end up referring to a crm_user_contact
entity as a "User Contact Mapping", then a permission "View mapped contact" would be relatively clear. That's my best suggestion on naming.
Everything "User" should become "UserContact" e.g. "UserListBuilder" should become "UserContactListBuilder". Thanks!
Created follow-up ✨ Customize display of contacts' names Active
jdleonard → created an issue.
Okay, I've updated the IS to reflect next steps here, restricted to adding a preferred name / nickname field and referencing it from the full_name field. I also added locking these fields (inspired by other issues about locking) as they are foundational.
I'll create a stub issue to follow up about name formats etc.
@tyler-ashbaugh, I think this is ready for you if you're still game!
If we were to pursue this, I don't think it would be any time soon. There is definite complexity. Happy to WF pending demand.
Should we lock the address and addres_type fields on the address bundle?
Yes!
Should the primary and type fields be locked on crm_contact_detail bundles?
If you mean the telephone and telephone_type fields on the telephone bundle and the email and email_type fields on the email bundle, yes! If not, I'm not sure what you're referring to.
jdleonard → created an issue.
I'm totally here to make things difficult...
Let's say I want to create a view that shows a Contact's siblings. This is straightforward if the Contact has a Relationship from that Contact to each sibling Contact. It is much less straightforward if the siblings might be referenced on Relationships from the Contact and/or on Relationships to the Contact.
Queries would be much simpler if all Relationships were one-way.
Would it make sense for a symmetrical relationships to be represented as two one-way relationships (possibly referencing each other)?
I realize I'm posting this on an issue about asymmetrical relationships...
Just a quick note that I'd advocate for the use of Tagify → as a modern accessible solution (adopted by Drupal CMS) for handling tagging UX.
Aside: this could also be used for entity reference fields targeting Contacts and we could implement a Tagify CRM Contact List module in the same vein as
Tagify User List →
to provide a rich experience:
The current External Identifiers field has a "Source" and an "External ID" for each value.
CiviCRM has a single "Contact Source" and a single "External ID". The tooltip for Contact Source is "Use this field to store a description of how/why this contact was added to the database. This field is usually auto-filled when a contact is created from a public form i.e. Event Registration."
That seems to be a different intent than the "Source" defined in CRM's External Identifiers field so I'm double checking what your intent was and whether we should adjust.
Separately, while I see some value in accommodating external IDs for multiple systems, I wonder whether that would better be accommodated using dedicated fields for each system, which could be provided by relevant integration modules. Should we ditch the External Identifiers field in favor of a simple single value External ID field like Civi has?
+1 to Event Platform Starter
Looks good to me. I haven't tested in any way, but seems pretty obvious what the effect of these changes are.
I think the top priority should be providing a field so that nickname / preferred name can be stored. And might as well connect it to the Full Name field via the field reference there.
I'm less bothered about whether it is included in a Contact's label / in a name format OOTB though I think it would be sensible to provide some options for name formats. Perhaps name format options should be a follow-up issue that incorporates i18n considerations, which makes things more complicated, but is important to maintain a global scope.
Maiden name is interesting. While nickname / preferred name seems very generally applicable, I suspect that the value of tracking a maiden name varies significantly by end user and surely is irrelevant for some countries/cultures. My gut says no to maiden name as an OOTB offering, at least unless and until we've tackled more generally valuable needs. FWIW Civi does not ship with a maiden name or alternative name field.
Not a use case I'm expecting to encounter personally. I'm just playing devil's advocate in an effort to avoid potential problems caused by validation.
Thinking through a hypothetical setting on the Relationship, I would suggest that it default to preventing a relationship to oneself. It could even be hidden during Relationship creation and only be visible when editing a Relationship to support the option without cluttering the main flow, but I don't think it makes much difference.
/admin/config/crm/user/list
Drupal\Core\Database\DatabaseExceptionWrapper: SQLSTATE[42S02]: Base table or view not found: 1146 Table 'db.crm_user' doesn't exist: SELECT COUNT(*) FROM "crm_user"; Array ( ) in Drupal\crm\UserListBuilder->render() (line 79 of /var/www/html/local_projects/crm/src/UserListBuilder.php).
UserListBuilder::render references the crm_user database table, which has changed.
UserListBuilder, UserStorageSchema, UserViewBuilder, UserForm should also be renamed.
Brainstorming a counter argument to the assumption that a relationship can't be between one Contact and itself...
Imagine a doctor's office with a Relationship "financially responsible for".
JD is financially responsible for JD
JD is financially responsible for Laura
JD is financially responsible for Ramona
JD is financially responsible for Sam
This wouldn't necessarily flow through a Household and could involve the proposed forbidden relationship.
Perhaps instead of limiting the relationship, there should be a warning to catch accidents but allow these sorts of exceptions.
Unfortunately that will undoubtedly depend on the organization using CRM.
In one system I use, I track people as:
LAST, FIRST [MIDDLE] [NICKNAME]
For me, that's:
Leonard, John Daniel Hayes "JD"
Eventually I think it would be nice for there to be a configurable default for the site and an option for a User to override this.
jdleonard → created an issue.
Updating IS from discussion during today's Zoom meeting.
Also added some ideas to feature brainstorming
Should the submodules' info.yml files also be updated?
Thanks you @jacobupal and @joegml!
I did a very quick code review. Should be a quick fix to get the tests working.
jdleonard → created an issue.
jdleonard → created an issue.
Contrived example coming up! Let's say I want to create a family tree (and there's no missing links). I create a module that can traverse sibling and parent Relationships to generate a tree visual. That task is easier (and can benefit from existing data if any) if Sibling and Parent Relationship Types have well-known IDs and are known to be present. CRM could do that.
The (good) reason I can think of to not do so is to not force the presence of the Relationship Types upon end users. Perhaps it would also be helpful to have a Relationship Type base field "Hidden from UI" or "Disabled" or something like that. A way to "get this out of my way" while maintaining its presence for any modules that depend on it.
I'm not full throated on this. A decision on it could certainly be postponed.
Thanks for jumping in Tyler! This seemed relatively simple until it didn't, haha!
Steve, I'm late to the game on the Name Field module. Thanks for your recent contributions there!
My perception is that the Name field has purposefully avoided hosting the text of a preferred/nickname, but I don't understand why. Maybe a philosophy that a nickname is "not a name"? It feels like it might be an uphill battle to get Name to host a nickname.
However the module appears to provide a passthrough of sorts...
After adding a nickname field to the Individual bundle, it looks like one can then reference it via the field config form ("Preferred component source") at /admin/structure/crm/contact-types/manage/individual/fields/crm_contact.individual.full_name
. Then it becomes available via the name formatters / field formatter.
Do I have that right? It sure feels awkward, but also workable.
What's your recommendation for how we proceed?
Made a good start. Unassigning so others can pick it up until I'm next available.
Interestingly, CiviCRM has "nickname" on Individual, Household, and Organization. I'm not sure I understand the use cases for Household or Organization having a nickname.
Adding 📌 Determine which Relationship Types (if any) should be locked Active as related if grants might rely on specific Relationship Types, which would thus presumably need to be locked.
jdleonard → created an issue.
Added rename of Phone Type to Telephone Type to IS.
Thanks!
jdleonard → created an issue.
jdleonard → created an issue.
jdleonard → created an issue.
jdleonard → created an issue.
jdleonard → created an issue.
Thanks Haritha.
Please "close" MR 30 to avoid confusion.
I have not performed a detailed review, but here are some outstanding needs.
For the remaining MR, please note items 2 and 4 from Creating Merge Requests → .
Classes such as CrmUserEvent
and CrmUserInterface
need renaming.
Variables such as `crm_user` and `$crm_user_storage` need renaming.
Any use of "Contact User" should be "User Contact". I updated the issue summary to remove a contradiction that may have introduced this.
It would also be helpful to hear what steps you took to test that the relevant features continue to function.
Looking good!
Re-opening to suggest a transparent background. It doesn't look as good as it could on the project page, for example.
I missed that issue, but looking at it, it's not clear that it's the same as this. It mentions "Create external crm entity" but is lacking detail. I based this issue off of what's in the 1.0.x branch. Feel free to close one issue if you think it makes sense.
Haritha, thanks for your contribution. Please close one MR and focus on just one MR/branch.
There remain many references in the project to crm_user_contact
, "CRM User", etc. These all need to reflect the name change. This should help get the tests passing.
To me, "status" in Drupal represents whether content is published (e.g. EntityPublishedInterface
). I think it is confusing and limiting to use "status" to mean something other than that and I don't think any of the entities in CRM conceptually get "published" or "unpublished".
An alternative for some use cases, adopted by Drupal CMS, is the Trash → module, which moves deleted content into a Trash can rather than being deleted outright from the start. This (or, more generally an "Archived" state) feels like a more modern approach that I've observed in CRMs. However, it is currently incompatible with Content Moderation (that's being worked on) and non-SQL storage backends (no evidence that's being worked on) so I'm not proposing adoption of Trash at this time.
Past employment is a great example of a useful thing to represent, but for which overloading the published "status" could cause problems. If a past employment relationship has status 0, should it not be rendered when viewing a contact? That seems very specific to a given site's needs. Relationships already have "start" and "end" timestamps, which would seem to already serve the need of identifying whether an employment relationship is in the past.
An Individual dying is a notable thing to track, but I would propose that this be tracked in a semantically named field on the Individual bundle, perhaps along with an optional date. A User might want to "archive" / soft delete a Contact independent of whether they died.
Similarly, that an Address is temporarily unavailable (maybe it's seasonal?) is a specific thing worthy of tracking more semantically.
While I'm open to the use of "status" if it's important and there's not a better solution, I'm not convinced that it's important yet and I think we should spend some more time addressing specific use cases in more semantic ways.
I propose proceeding to remove "status" as specified in the IS and opening follow-up issues for each entity that has use cases for which "status" could be used so that we select the best solution for each use case. In some cases this might be a workflow. In others it might be a field like "Dead". This will help facilitate a 1.0 release without committing to overloaded uses of "status".
I dig it!
I was going for shorter, but no objection.
Good change!
Sorry I missed your question earlier!
Honestly I have not focused on permissions yet. I think the ability to create/edit/delete an Address/Phone/Email should be inherited from the ability to create/edit the Contact and it should only be possible in the UI via inline Entity Form. I created 📌 Address/Phone/Email CRUD access control Active to follow-up.
jdleonard → created an issue.
jdleonard → created an issue.
Added follow-up issues to IS.