Deprecate CRM_Financial_BAO_EntityFinancialAccount::del()

[aydun]

Deprecate CRM_Financial_BAO_EntityFinancialAccount::del()
Change 'setStatus' to exceptions and create status messages at form layer.

Was originally: 'Deprecate CRM_Financial_BAO_FinancialTypeAccount::del()' but see Coleman's notes below and #25036

[civibot]

(Standard links)

• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[colemanw]

Wow, this BAO had not only a setStatus but also a redirect!!!
Just in case I didn't say before... THAT'S HORRIBLE!
BAOs are supposed to handle business logic ONLY. They can be called from a form, yes, but also from an API, or from an AJAX callback, or from a hook. They should never just ASSUME that they are being called from a form and do form-ey stuff like setting status messages and issuing redirects, that would totally break their use by apis, etc.

[colemanw]

I've dug a bit deeper into this class and the weirdness continues...

1. Because it doesn't follow a logical naming convention, it is effectively "invisible" to APIv3 and APIv4. They just assume that CRM_Financial_DAO_EntityFinancialAccount doesn't even have a BAO class, because none with a matching name could be found.
2. The upshot is that the create and del functions were never called from either API - they were bypassed and the generic fallback (writeRecords and deleteRecords) have always been used.
3. A year ago, @eileenmcnaughton also made that assumption and actually added a BAO class with the correct name (for the purposes of exposing it to SearchKit), I think neither she nor I realized at the time that a BAO already existed, only with the wrong name.
4. Which means that as far as the API is concerned, those create and del functions you are refactoring here have never actually existed. In this case, their assumption that they are only called from Quickform is actually correct.
5. That gives us IMO some extra leeway to not be entirely faithful to them when porting them to use hooks, and discard anything that looks screwy. I haven't closely inspected them yet, but will do so...

[colemanw]

@aydun now that #25036 has been merged, do you want to take a crack at sorting this out?

[eileenmcnaughton]

Test fails will be magically resolved at midnight utc

Test Result (2 failures / ±0)
api_v3_OrderTest.testAddOrderForMembershipWithDates with data set #6
api_v3_OrderTest.testAddOrderForMembershipWithDates with data set #13

[eileenmcnaughton]

@aydun I'm not sure not deleting the related entity was correct. I merged but digging into that part now

[eileenmcnaughton]

Ah - it was the wrong way around - deleting the FinancialTypeAccount should delete the EntityFinancialTypeAccount - putting up a fix for that