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

Use getEntityNameForTable() instead of getClassForTable(), as this might yield ambiguous results #22116

Merged

Conversation

jensschuppe
Copy link
Contributor

Overview

Use CRM_Core_DAO_AllCoreTables::getEntityNameForTable() instead of CRM_Core_DAO_AllCoreTables::getBriefName(CRM_Core_DAO_AllCoreTables::getClassForTable()) constructs, as this might yield ambiguous results since entities can share a DAO class.

Before

The first result returned by array_search() lookups in CRM_Core_DAO_AllCoreTables::getBriefName() might not be the entity in question, because there can be multiple entities sharing a DAO class.

After

The correct entity is being returned.

Technical Details

See #21853 for why entities are now being indexed by brief name instead of class name.

Comments

This is toward work on virtual entities, followup to #21853

@civibot
Copy link

civibot bot commented Nov 23, 2021

(Standard links)

@civibot civibot bot added the master label Nov 23, 2021
@jensschuppe
Copy link
Contributor Author

Pinging @colemanw.

@colemanw
Copy link
Member

Yes, definitely +1 to this change.

@seamuslee001
Copy link
Contributor

I think test Fail maybe related here

@colemanw
Copy link
Member

@jensschuppe the test failure showed that we were not taking multilingual schema into account in our new function. I've pushed a fix to this PR.

@colemanw colemanw merged commit 208cc8d into civicrm:master Nov 24, 2021
@jensschuppe jensschuppe deleted the virtualEntities/getEntityNameForTable branch November 24, 2021 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants