dev/core#715 - Fix delete action on RelationshipType form

[colemanw]

Overview

See https://lab.civicrm.org/dev/core/issues/715

Before

Fatal error when deleting a relationship type

After

Form works

Technical Details

This fixes the bug for now but I'm not sure this is really the right fix long-term.

When I dig into the code of CRM_Core_Form_EntityFormTrait I see that every form has to declare a rather cumbersome array in setEntityFields which appears to duplicate metadata that could be retrieved from the api... and then another function setEntityFieldsMetadata which appears to supply that same information that was hard-coded in the previous function.
Couldn't all this be avoided by calling getfields on the entity and passing in the action? Then we'd avoid trying to retrieve inappropriate fields for that action (which was the cause of this bug) and we'd also avoid all the hard-coded (and hard-to-maintain) getfields boilerplate in the setEntityFields fn.

[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).

[colemanw]

@eileenmcnaughton what is the reason for all those hard-coded arrays in setEntityFields functions?

[mattwire]

@colemanw I think the intention was that it would provide a stepping stone, as many of the entities do not have very good metadata (ie. suitable for adding to forms) stored on the XML/DAO. Also not all fields can be added automatically (due to conditionals/logic on the form etc). It would seem to be a good logical next step to optionally build entityFields directly using the getfield API.

[eileenmcnaughton]

This seems OK as an approach - my personal belief is that delete forms should not actually be part of the same form as create/edit - for performance reasons & code complexity reasons

[eileenmcnaughton]

@colemanw can you switch this to the rc- I think this was a recent regression

[colemanw]

@eileenmcnaughton done