Regenerate DAO files using civix

civicrm / org.civicrm.volunteer

CiviVolunteer extension.

40 stars 64 forks source link

Regenerate DAO files using civix #548

Closed colemanw closed 3 years ago

colemanw commented 4 years ago

CiviVolunteer originally used a hacked version of setup.sh to generate DAO files and SQL from the xml. That's no longer necessary now, with the command: civix generate:entity-boilerplate Just had to reorganize the files a little, and was able to delete the hacked setup.sh, et al.

ginkgomzd commented 4 years ago

note to self:

rm extension-copy of setup.sh no longer needed.
regenerate DAO using latest civix generate:entity-boilerplate
updates to translation
cleaner un-install and re-install [conjecture]
updated links to reference docs and license
new features in extension framework, e.g. theme hook, entity title...

colemanw commented 4 years ago

I tried running the installer from this branch and hit some problems, which I've now fixed in the second commit.

The old install files were still present and needed to be removed in favor of the newly generated auto_install.sql
The default value for the column volunteer_need.created was causing a sql error, so I updated it to CURRENT_TIMESTAMP which I think was always the original intent but older versions of mysql (which we no longer support) couldn't handle two datetime columns with defaults.
This required a core version compatibility bump to 5.28 which requires the mysql version which supports multiple CURRENT_TIMESTAMP defaults in a single table.

colemanw commented 4 years ago

@ginkgomzd I think this is good to go. It bumps up the core version requirement quite a bit, but that should be fine; people on older versions of civi won't be prompted to upgrade this extension until they upgrade Civi first.

ginkgomzd commented 4 years ago

CC: @MikeyMJCO I think this PR is okay to merge, but I'm feeling probably unjustified reservations.

At first I had reservations about bumping the core minimum version, but since there is no reason for an existing install to upgrade, I think it's fine.

However, once the 2.5 branch is released, I think this could cause a problem for installs lagging behind on core that want the fixes in the 2.5 branch.

Also, given the translation support in the schema, I feel like testing an upgrade path on, let's say English and French, would be a really good idea.

homotechsual commented 4 years ago

I'm happy to test the upgrade path on English, I'll see if I can pull together a multi-language upgrade test site as well.

I have some minor concerns about the version bump, I'll see if there's a workaround, we could require the MySQL version rather than core version it seems? I.e. say "2.5 requires MySQL v5.6 or above".

ginkgomzd commented 3 years ago

I've merged this since my concerns were around an impending release of the 2.5-rc. Since that is less impending, I think it is better to get this out in the wild.