CRM-20192 Upgrader: Check for existing columns before trying to add new ones

[mattwire]

No description provided.

[eileenmcnaughton]

Ouch that core function exists in 2 classes CRM_Core_DAO & CRM_Core_BAO_SchemaHandler

It will probably get tidied up at some point, causing this to fail. I think I would duplicate it in the extension & add comments as to why

[mattwire]

@eileenmcnaughton Which core function do you mean? I've been looking at improving some of the SQL stuff to make it more reliable (https://issues.civicrm.org/jira/browse/CRM-20031, https://issues.civicrm.org/jira/browse/CRM-19968) so what functions should we be using?

[eileenmcnaughton]

So there is
CRM_Core_DAO::checkFieldExists
and
CRM_Core_BAO_SchemaHandler::checkIfFieldExists

Which seem to duplicate each other and one should be removed in favour of the other. (I would say SchemaHandler is the better location for the function)

From the POV of an extension the latter function will not be in all releases as it is relatively new. If someone decides the latter location is better & removes the former then the former might not be in all releases, so I would be inclined to copy the function into the extension to protect against running on a Civi version that does not have the chosen function

[colemanw]

Agreed. I'm really tempted to remove one of those functions in core now.

[herbdool]

How about using the latter function but putting this commit in a new release that requires a certain Civi version?

[eileenmcnaughton]

I think that makes sense - except we don't have a methodology for specifying minimum point version for and extension release

[eileenmcnaughton]

I think it might make sense now to change cividiscount to support 5.3 as the minimum version (for new tags of Cividiscount) since

1. 4.6 is now so close to the end of it's life there realistically don't need to be any more releases supporting it - it can stick with the ones that exist and
2. 4.7 -> 5.2 are now officially not secure.

Then we can switch to the preferred version of the function

[colemanw]

@mattwire I've deprecated the one in CRM_Core_DAO. Can you update this PR to change CRM_Core_DAO::checkFieldExists -> CRM_Core_BAO_SchemaHandler::checkIfFieldExists

[mattwire]

Can you update this PR to change CRM_Core_DAO::checkFieldExists -> CRM_Core_BAO_SchemaHandler::checkIfFieldExists

@colemanw That's done.