Keep schema definition in sync with latest update hook.

CiviMRF / CMRF_Abstract_Core

Abstract implementation for the the CMRF Core. Should be included into the concrete module implementation

GNU Affero General Public License v3.0

1 stars 4 forks source link

Keep schema definition in sync with latest update hook. #10

Closed jensschuppe closed 5 years ago

jensschuppe commented 5 years ago

Fixes #9.

jensschuppe commented 5 years ago

@jaapjansma Could you have a look? I think this removes the inconsistencies between AbstractCore and the Drupal module correctly, but did not test with a new module installation.

jaapjansma commented 5 years ago

Looks good to me. I cannot think why this change could break something.

jensschuppe commented 5 years ago

@jaapjansma Thanks! Could you merge this PR then? I do not have write access to the repository.

P.S. Just a thought: Maybe it's not the best thing having Drupal hook_schema() code within CMRF_Abstract_Core. This could be abstracted and the Drupal module may implement hook_requirements() to check for the correct supported version of CMRF_Abstract_Core, so that there are no inconsistencies between the DB schemas.

jaapjansma commented 5 years ago

I agree on your PS. Drupal core should take care of the schema. In this class we would rather only have the ability to store calls. And the exact implementation should be done in the drupal core, or the wordpress module etc...