localgovdrupal / localgov_geo

Entity for storing Geographic information, addresses, and areas; extensible types and plugable backends with defaults to start.
2 stars 1 forks source link

LocalGov Geo upgrade and deployment #90

Closed andybroomfield closed 1 year ago

andybroomfield commented 1 year ago

This test verifies that a custom localgov_geo entity is updated to a geo_entity. Ideally this would be an updateTest, though for simplicitiy this is a functional test that runs the selected update hooks (8002, 8003 and 8004) directly.

andybroomfield commented 1 year ago

Todo:

Need to decide if we would try to include this in an update hook or leave it as a helper class for other modules that created geos to manage the migration themselves.

finnlewis commented 1 year ago

Discussing in Merge Monday.

@andybroomfield is primarily wanting a class to help with the migration to geo_entity.

It would be great to automatically loop through the display modes and migrate those automagically too, in a update hook.

andybroomfield commented 1 year ago

@ekes this is ready for review now and should migrate custom display and form modes from localgov_geo to geo_entity on upgrade.

andybroomfield commented 1 year ago

And the result (testing in localgov install) BHCC Custom parking zones geos can now be migrated.

Screenshot 2023-09-20 at 1 37 27 pm
finnlewis commented 1 year ago

@Adnan-cds this might help with your migration of view modes

ekes commented 1 year ago

Except for the new entities exporting without an UUID it works in a manual test, and automated tests pass.

So fixing UUIDs and then we should merge at the same time as sorting #91 though, as it might also need a by-pass for staging/prod sites with sync config.

andybroomfield commented 1 year ago

@ekes Drupal should add the UUIDs dynamically itself. Though this is less of an issue than #91 as if the display mode config entities are deleted and recreated there is no data loss and the config import will take presedence.

andybroomfield commented 1 year ago

Oh it won't beacuse its wrting the config file directly, but we could change this to create the display mode entity instead and it should get an uuid. and _core hash.

andybroomfield commented 1 year ago

Ready for re-review. I've since changed the migration to create display modes as entities if they don't already exist.

ekes commented 1 year ago

I've got a custom form mode that I made and it's saving with

'postal_address' => type => 'localgov_geo_address'

so on save from line 128 of MigrateDisplayModes I get a

The "localgov_geo_address" plugin does not exist.

error.

andybroomfield commented 1 year ago

@ekes would that be because localgov_geo_address should now be geo_entity_address?

andybroomfield commented 1 year ago

@ekes would that be because localgov_geo_address should now be geo_entity_address?

ekes commented 1 year ago

That's one of the renames yep: https://git.drupalcode.org/project/geo_entity/-/commit/322e03f6562eec6ce43a5e0908d0fea61554fa2d?page=3#45e68ea6c56efe30722dfdff202b02d70e87b8ef_16_15

ekes commented 1 year ago

This is the config (cp config/sync/*localgov_geo* tmp/) I randomly clicked together to test. config.tar.gz

andybroomfield commented 1 year ago

Do you want to try again @ekes. Latest should also catch field widgets and rename them, assuming they are all of the type localgov_geo_ and there replacements are geo_entitiy_.

I had some trouble getting a test case for this as localgov_geo no longer ships with the localgov_geo_address widget, and trying to install geo_entitiy_address gives lots of schema errors.

andybroomfield commented 1 year ago

After the merge of #92 into this branch, i'm not seeing the following

[notice] Update started: localgov_geo_update_update_8001 [warning] Trying to access array offset on value of type bool localgov_geo_update.install:39 [error] array_keys(): Argument #1 ($array) must be of type array, null given [error] Update failed: localgov_geo_update_update_8001

It was because during retesting I deleted all the config sync directory instead of doing a fresh export from Drupal 9. Its an edge case, but changing

$sync_modules = array_keys($sync['module']);

to

$sync_modules = array_keys($sync['module'] ?? []);

Allows the update to proceed unhinded.

andybroomfield commented 1 year ago

Otherwise this update is now precceding as expected.

ekes commented 1 year ago

I say we get this merged now. But I'll make a MR for another release that we can finalize Monday.

More testers welcome!