[REF] Move entityRef filters into their respective BAOs #13625
Conversation
| if (class_exists($baoName)) { | ||
| $entityFilters = $baoName::getEntityRefFilters(); | ||
| if ($entityFilters) { | ||
| $filters[_civicrm_api_get_entity_name_from_camel($entity)] = $entityFilters; |
There was a problem hiding this comment.
if we are calling this helper function from outside the api I think we should move it to CRM_Core_DAO_AllCoreTables
There was a problem hiding this comment.
Sorry what should be moved?
There was a problem hiding this comment.
the internal api function
There was a problem hiding this comment.
I'd prefer to keep it like this for this iteration as the final refactor will not require this function anymore. The plan is to standardize EntityRef fields on the CamelCase entity name as the lowercase names are deprecated as of Api v4.
| * | ||
| * @return array | ||
| */ | ||
| public static function getEntityRefFilters() { | ||
| $filters = array(); | ||
| $config = CRM_Core_Config::singleton(); | ||
|
|
||
| if (in_array('CiviEvent', $config->enableComponents)) { |
There was a problem hiding this comment.
Let's add in some caching - Civi::cache()-> get & set is preferred I believe - this could be doing a bit of php iteration to check all DAOs that exist
There was a problem hiding this comment.
The output of this function gets cached in the browser - it's part of the l10n.js dynamic script.
There was a problem hiding this comment.
OK - I guess that's fine it it's also never loaded in batch processes
|
So I've gone through & recreated this & am pretty confident on the line by line. The other comment I have is that I feel it would be cleaner (and save one query) to have a function on the BAO for isEnabled & have the BAO early return if they are not enabled - this seems like something that could be used in other types of DAO iterations too |
|
Test fail unrelated. |
|
@eileenmcnaughton that's a good suggestion and I'd like to think about how to best implement that. One option would be to add a function to In the meantime I want to keep going with my refactoring work so could we please merge this first (this PR was intended to be a fairly easy to review "simple" refactor -- the more tricky stuff comes later). |
|
Ok - I'll let that lie :-) |
Overview
Fixes an old FIXME in the code by moving entityRef filters into a more logical place.
Before
Code all jammed into an illogical place.
After
Code in a better place. More future-proof and extensible.
Technical Details
New entities (including entities from extensions) can now declare entityRef filters simply by placing them in their BAO class.
Comments
This is the beginning of a larger cleanup/refactor.