totten / civix

CiviCRM Extension Builder
http://civicrm.org/
Other
56 stars 56 forks source link

Add FKColumnName to fields metadata #328

Closed colemanw closed 9 months ago

colemanw commented 9 months ago

This supports https://github.com/civicrm/civicrm-core/pull/28827 by adding the extra bit of info it needs.

colemanw commented 9 months ago

@totten I can confirm that this works... I used it to generate https://github.com/civicrm/civicrm-core/pull/28846 and https://lab.civicrm.org/extensions/cividiscount/-/merge_requests/304

totten commented 9 months ago

We had chatted briefly last week about some of the example diff's produced on other ext's . (I had been concerned about whether the dropped functions would affect compat.) Coleman pointed that (basically) this change doesn't cause that. Which was a good point...

I guess the dropped-function part points to a general issue around generate:entity-boilerplate, civicrm-core:dao.tpl, and civicrm-core:CRM/Core/DAO.php -- i.e. the extensions can get out-of-sync with the templates and the live framework.

Anyway, it does seem better for entity-boilerplate to provide the additional metadata here. (It probably does help folks who select a newer dao.tpl, and it probably doesn't hurt folks who select an older dao.tpl.)

colemanw commented 9 months ago

I sometimes think that these DAO's should work like Smarty

I sometimes think they shouldn't exist at all! They're nothing but boilerplate that could be replaced with generic functions which read directly from schema metadata in the xml files.

Also, there are growing use-cases for "virtual" entities that have a db table but no corresponding DAO file (ECK, CiviImport, and SearchSegments all generate such entities). And those all work!