dev/core#2486 Add in FinancialItem APIv4 Entity

[seamuslee001]

Overview

Does what it says on the tin

Before

No APIv4 entity

After

APIv4 Entity

ping @eileenmcnaughton @JoeMurray @monishdeb

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

@seamuslee001 did you check in search kit if this works in terms of it being able to discover the joins? I think that with the other financial entities I had to do some metadata fixes to make them bridgeable

[colemanw]

Since this isn't a bridge entity, it should work ok with joins... except...
I noticed that the field entity_table doesn't have a pseudoconstant, which means the API will have no clue what other entities it can potentially join onto.

[monishdeb]

Agree that this is not a bridge entity. I think the entity_table field shouldn't be required to have pseudoconstant like LineItem.entity_table or EntityFinancialTrxn.entity_table . Here entity_table holds civicrm_line_item|civicrm_financial_trxn

[colemanw]

If there are only 2 possibilities for the value of entity_table then we definitely should add a pseudoconstant. That would make it much easier to understand what's going on here. A simple function callback to return those 2 items would be fine.

[eileenmcnaughton]

I think it's fine to go with a callback but it would be nice to be able to just put options in the xml when there are only a couple

[colemanw]

Maybe, but IMO we already have too many ways to define a pseudoconstant list.

[monishdeb]

Currently we are able to do joins on entity_table as in

$financialItems = \Civi\Api4\FinancialItem::get()
  ->setJoin([
    ['LineItem AS line_item', 'INNER', NULL, ['line_item.id', '=', 'entity_id'], ['entity_table', '=', "'civicrm_line_item'"]],
  ])
  ->execute();

But I am thinking how to declare psueduconstant for field which can dynamically extend to entity_table fields. I didn't find any such pseudoconstant declaration in xmls. How about:

--- a/xml/schema/Financial/FinancialItem.xml
+++ b/xml/schema/Financial/FinancialItem.xml
@@ -147,6 +147,10 @@
   <title>Entity ID</title>
   <type>int unsigned</type>
   <comment>The specific source item that is responsible for the creation of this financial_item</comment>
+    <pseudoconstant>
+      <entityTable>civicrm_financial_trxn|civicrm_line_item</entityTable>
+    </pseudoconstant>
   <add>4.3</add>
 </field>
 <dynamicForeignKey> 

and there would be callback function to expose the two table options on JOIN , SELECT and WHERE entity_id

[eileenmcnaughton]

@monishdeb I think @colemanw is suggesting we stick with the approach in EntityFinancialTrxn

    <pseudoconstant>
      <callback>CRM_Financial_BAO_EntityFinancialTrxn::entityTables</callback>
    </pseudoconstant>
    <add>3.2</add>

Note search kit can figure out the join for that table based on the metadata - ie you can search for 'contributions with financial transactions' & it figures out the join based on the metadata

[monishdeb]

@monishdeb I think @colemanw is suggesting we stick with the approach in EntityFinancialTrxn

    <pseudoconstant>
      <callback>CRM_Financial_BAO_EntityFinancialTrxn::entityTables</callback>
    </pseudoconstant>
    <add>3.2</add>

Note search kit can figure out the join for that table based on the metadata - ie you can search for 'contributions with financial transactions' & it figures out the join based on the metadata

Ah right, didn't knw that. I have submitted a PR #20464 which adds the pseudoconstant callback for LineItem and FinancialItem entity_table.

[eileenmcnaughton]

ok - I'm good to merge this then - the other just needs a pair of eyes fresher than mine to take a look - ideally before we fork